From 2e6f69c704202d41f0ab5ab0aa65583a26184e51 Mon Sep 17 00:00:00 2001 From: David Brazdil Date: Thu, 19 Apr 2018 12:41:04 +0100 Subject: Set hidden API flags of intrinsics Intrinsics overwrite the hidden API access flags of the respective Java method, which is why we need to hardcode their API list membership. VarHandle intrinsics are blacklisted because they could be used to bypass hidden API checks and given they are new in P, should not be used by anybody except tests (that do not enforce API checks). The remaining intrinsics which happen to be @hide are put on light greylist. We used to put them on whitelist implicitly, hence never saw warnings about them. To be safe, we put them on light grey. Note that these are set even for the core image that currently does not have hidden API flags. That is fine because (a) VarHandles can still be tested, and (b) light greylist membership may print warnings but will not change the semantics. Bug: 64382372 Bug: 77733081 Test: make Merged-In: Ia9a7765260acb533560676e7dfcd51065cfb247d Change-Id: Ia9a7765260acb533560676e7dfcd51065cfb247d (cherry picked from commit 49dded0c15dbf3d7c3920ae4744c93c2d6081202) --- runtime/art_method-inl.h | 92 ++++++++++++++++++++++++++++++++++++------------ 1 file changed, 70 insertions(+), 22 deletions(-) (limited to 'runtime/art_method-inl.h') diff --git a/runtime/art_method-inl.h b/runtime/art_method-inl.h index 1565644380..4d54ac1d9f 100644 --- a/runtime/art_method-inl.h +++ b/runtime/art_method-inl.h @@ -384,19 +384,67 @@ inline bool ArtMethod::HasSingleImplementation() { return (GetAccessFlags() & kAccSingleImplementation) != 0; } -inline bool ArtMethod::IsHiddenIntrinsic(uint32_t ordinal) { - switch (static_cast(ordinal)) { - case Intrinsics::kReferenceGetReferent: - case Intrinsics::kSystemArrayCopyChar: - case Intrinsics::kStringGetCharsNoCheck: - case Intrinsics::kVarHandleFullFence: - case Intrinsics::kVarHandleAcquireFence: - case Intrinsics::kVarHandleReleaseFence: - case Intrinsics::kVarHandleLoadLoadFence: - case Intrinsics::kVarHandleStoreStoreFence: - return true; - default: - return false; +inline HiddenApiAccessFlags::ApiList ArtMethod::GetHiddenApiAccessFlags() { + if (UNLIKELY(IsIntrinsic())) { + switch (static_cast(GetIntrinsic())) { + case Intrinsics::kSystemArrayCopyChar: + case Intrinsics::kStringGetCharsNoCheck: + case Intrinsics::kReferenceGetReferent: + // These intrinsics are on the light greylist and will fail a DCHECK in + // SetIntrinsic() if their flags change on the respective dex methods. + // Note that the DCHECK currently won't fail if the dex methods are + // whitelisted, e.g. in the core image (b/77733081). As a result, we + // might print warnings but we won't change the semantics. + return HiddenApiAccessFlags::kLightGreylist; + case Intrinsics::kVarHandleFullFence: + case Intrinsics::kVarHandleAcquireFence: + case Intrinsics::kVarHandleReleaseFence: + case Intrinsics::kVarHandleLoadLoadFence: + case Intrinsics::kVarHandleStoreStoreFence: + case Intrinsics::kVarHandleCompareAndExchange: + case Intrinsics::kVarHandleCompareAndExchangeAcquire: + case Intrinsics::kVarHandleCompareAndExchangeRelease: + case Intrinsics::kVarHandleCompareAndSet: + case Intrinsics::kVarHandleGet: + case Intrinsics::kVarHandleGetAcquire: + case Intrinsics::kVarHandleGetAndAdd: + case Intrinsics::kVarHandleGetAndAddAcquire: + case Intrinsics::kVarHandleGetAndAddRelease: + case Intrinsics::kVarHandleGetAndBitwiseAnd: + case Intrinsics::kVarHandleGetAndBitwiseAndAcquire: + case Intrinsics::kVarHandleGetAndBitwiseAndRelease: + case Intrinsics::kVarHandleGetAndBitwiseOr: + case Intrinsics::kVarHandleGetAndBitwiseOrAcquire: + case Intrinsics::kVarHandleGetAndBitwiseOrRelease: + case Intrinsics::kVarHandleGetAndBitwiseXor: + case Intrinsics::kVarHandleGetAndBitwiseXorAcquire: + case Intrinsics::kVarHandleGetAndBitwiseXorRelease: + case Intrinsics::kVarHandleGetAndSet: + case Intrinsics::kVarHandleGetAndSetAcquire: + case Intrinsics::kVarHandleGetAndSetRelease: + case Intrinsics::kVarHandleGetOpaque: + case Intrinsics::kVarHandleGetVolatile: + case Intrinsics::kVarHandleSet: + case Intrinsics::kVarHandleSetOpaque: + case Intrinsics::kVarHandleSetRelease: + case Intrinsics::kVarHandleSetVolatile: + case Intrinsics::kVarHandleWeakCompareAndSet: + case Intrinsics::kVarHandleWeakCompareAndSetAcquire: + case Intrinsics::kVarHandleWeakCompareAndSetPlain: + case Intrinsics::kVarHandleWeakCompareAndSetRelease: + // These intrinsics are on the blacklist and will fail a DCHECK in + // SetIntrinsic() if their flags change on the respective dex methods. + // Note that the DCHECK currently won't fail if the dex methods are + // whitelisted, e.g. in the core image (b/77733081). Given that they are + // exclusively VarHandle intrinsics, they should not be used outside + // tests that do not enable hidden API checks. + return HiddenApiAccessFlags::kBlacklist; + default: + // Remaining intrinsics are public API. We DCHECK that in SetIntrinsic(). + return HiddenApiAccessFlags::kWhitelist; + } + } else { + return HiddenApiAccessFlags::DecodeFromRuntime(GetAccessFlags()); } } @@ -422,7 +470,7 @@ inline void ArtMethod::SetIntrinsic(uint32_t intrinsic) { bool is_default_conflict = IsDefaultConflicting(); bool is_compilable = IsCompilable(); bool must_count_locks = MustCountLocks(); - HiddenApiAccessFlags::ApiList hidden_api_list = GetHiddenApiAccessFlags(); + HiddenApiAccessFlags::ApiList hidden_api_flags = GetHiddenApiAccessFlags(); SetAccessFlags(new_value); DCHECK_EQ(java_flags, (GetAccessFlags() & kAccJavaFlagsMask)); DCHECK_EQ(is_constructor, IsConstructor()); @@ -436,14 +484,14 @@ inline void ArtMethod::SetIntrinsic(uint32_t intrinsic) { DCHECK_EQ(is_default_conflict, IsDefaultConflicting()); DCHECK_EQ(is_compilable, IsCompilable()); DCHECK_EQ(must_count_locks, MustCountLocks()); - if (kIsDebugBuild) { - if (IsHiddenIntrinsic(intrinsic)) { - // Special case some of our intrinsics because the access flags clash - // with the intrinsics ordinal. - DCHECK_EQ(HiddenApiAccessFlags::kWhitelist, GetHiddenApiAccessFlags()); - } else { - DCHECK_EQ(hidden_api_list, GetHiddenApiAccessFlags()); - } + // Only DCHECK that we have preserved the hidden API access flags if the + // original method was not on the whitelist. This is because the core image + // does not have the access flags set (b/77733081). It is fine to hard-code + // these because (a) warnings on greylist do not change semantics, and + // (b) only VarHandle intrinsics are blacklisted at the moment and they + // should not be used outside tests with disabled API checks. + if (hidden_api_flags != HiddenApiAccessFlags::kWhitelist) { + DCHECK_EQ(hidden_api_flags, GetHiddenApiAccessFlags()); } } else { SetAccessFlags(new_value); -- cgit v1.2.3-59-g8ed1b