From 85865697ff9fabede3d64ff64cde72727c3fc4c1 Mon Sep 17 00:00:00 2001 From: David Brazdil Date: Tue, 30 Oct 2018 17:26:20 +0000 Subject: Runtime flags only for fast/slow hiddenapi path With more flags being supported in the dex file, stop copying all of them into ArtField/ArtMethod access flags. Instead, store the information needed to figure out whether to enter the slow path and retrieve full access flags from dex or not. At the moment, the only runtime flag is kAccPublicApi assigned to all class members on the whitelist. The CL also moves hardcoded API membership of intrinsics out of ArtMethod and into hidden_api.h, and moves ArtMethod::SetIntrinsic into the .cc file. Test: m test-art Change-Id: Ia1cc05060dbc22341768161dfd8697c6158e803a --- libdexfile/dex/hidden_api_access_flags.h | 21 ---- libdexfile/dex/modifiers.h | 7 +- openjdkjvmti/ti_redefine.cc | 10 ++ runtime/art_field.h | 4 - runtime/art_method-inl.h | 154 ------------------------ runtime/art_method.cc | 60 ++++++++- runtime/art_method.h | 2 - runtime/class_linker.cc | 19 +-- runtime/hidden_api.cc | 75 +++++++++++- runtime/hidden_api.h | 147 ++++++++++++++++++++-- runtime/mirror/class.h | 4 +- test/674-hiddenapi/hiddenapi.cc | 2 +- test/999-redefine-hiddenapi/src-redefine/gen.sh | 12 +- test/999-redefine-hiddenapi/src/Main.java | 84 +++++++++---- 14 files changed, 350 insertions(+), 251 deletions(-) diff --git a/libdexfile/dex/hidden_api_access_flags.h b/libdexfile/dex/hidden_api_access_flags.h index fd5c8654c3..77bfbc99b3 100644 --- a/libdexfile/dex/hidden_api_access_flags.h +++ b/libdexfile/dex/hidden_api_access_flags.h @@ -48,27 +48,6 @@ enum class ApiList { kNoList, }; -static const int kAccFlagsShift = CTZ(kAccHiddenApiBits); -static_assert(IsPowerOfTwo((kAccHiddenApiBits >> kAccFlagsShift) + 1), - "kAccHiddenApiBits are not continuous"); - -inline ApiList DecodeFromRuntime(uint32_t runtime_access_flags) { - // This is used in the fast path, only DCHECK here. - DCHECK_EQ(runtime_access_flags & kAccIntrinsic, 0u); - uint32_t int_value = (runtime_access_flags & kAccHiddenApiBits) >> kAccFlagsShift; - return static_cast(int_value); -} - -inline uint32_t EncodeForRuntime(uint32_t runtime_access_flags, ApiList value) { - CHECK_EQ(runtime_access_flags & kAccIntrinsic, 0u); - - uint32_t hidden_api_flags = static_cast(value) << kAccFlagsShift; - CHECK_EQ(hidden_api_flags & ~kAccHiddenApiBits, 0u); - - runtime_access_flags &= ~kAccHiddenApiBits; - return runtime_access_flags | hidden_api_flags; -} - inline bool AreValidFlags(uint32_t flags) { return flags <= static_cast(ApiList::kBlacklist); } diff --git a/libdexfile/dex/modifiers.h b/libdexfile/dex/modifiers.h index c4ea2d39b4..114c8e63e3 100644 --- a/libdexfile/dex/modifiers.h +++ b/libdexfile/dex/modifiers.h @@ -52,7 +52,7 @@ static constexpr uint32_t kAccObsoleteMethod = 0x00040000; // method (ru static constexpr uint32_t kAccSkipAccessChecks = 0x00080000; // method (runtime, not native) // Used by a class to denote that the verifier has attempted to check it at least once. static constexpr uint32_t kAccVerificationAttempted = 0x00080000; // class (runtime) -static constexpr uint32_t kAccSkipHiddenApiChecks = 0x00100000; // class (runtime) +static constexpr uint32_t kAccSkipHiddenapiChecks = 0x00100000; // class (runtime) // This is set by the class linker during LinkInterfaceMethods. It is used by a method to represent // that it was copied from its declaring class into another class. All methods marked kAccMiranda // and kAccDefaultConflict will have this bit set. Any kAccDefault method contained in the methods_ @@ -84,7 +84,8 @@ static constexpr uint32_t kAccMustCountLocks = 0x04000000; // method (ru // virtual call. static constexpr uint32_t kAccSingleImplementation = 0x08000000; // method (runtime) -static constexpr uint32_t kAccHiddenApiBits = 0x30000000; // field, method +static constexpr uint32_t kAccPublicApi = 0x10000000; // field, method +static constexpr uint32_t kAccHiddenapiBits = 0x30000000; // field, method // Non-intrinsics: Caches whether we can use fast-path in the interpreter invokes. // Intrinsics: These bits are part of the intrinsic ordinal. @@ -103,7 +104,7 @@ static constexpr uint32_t kAccClassIsFinalizable = 0x80000000; // Continuous sequence of bits used to hold the ordinal of an intrinsic method. Flags // which overlap are not valid when kAccIntrinsic is set. -static constexpr uint32_t kAccIntrinsicBits = kAccHiddenApiBits | +static constexpr uint32_t kAccIntrinsicBits = kAccHiddenapiBits | kAccSingleImplementation | kAccMustCountLocks | kAccCompileDontBother | kAccDefaultConflict | kAccPreviouslyWarm | kAccFastInterpreterToInterpreterInvoke; diff --git a/openjdkjvmti/ti_redefine.cc b/openjdkjvmti/ti_redefine.cc index 7cd10394d5..cabb758912 100644 --- a/openjdkjvmti/ti_redefine.cc +++ b/openjdkjvmti/ti_redefine.cc @@ -1427,6 +1427,11 @@ void Redefiner::ClassRedefinition::UpdateMethods(art::ObjPtr method.SetCodeItemOffset(dex_file_->FindCodeItemOffset(class_def, dex_method_idx)); // Clear all the intrinsics related flags. method.SetNotIntrinsic(); + // Disable hiddenapi checks when accessing this method. + // Redefining hiddenapi flags is unsupported for the same reasons as redefining + // access flags. Moreover, ArtMethod loses pointer to the old dex file, so just + // disable the checks completely for consistency. + method.SetAccessFlags(method.GetAccessFlags() | art::kAccPublicApi); } } @@ -1445,6 +1450,11 @@ void Redefiner::ClassRedefinition::UpdateFields(art::ObjPtr CHECK(new_field_id != nullptr); // We only need to update the index since the other data in the ArtField cannot be updated. field.SetDexFieldIndex(dex_file_->GetIndexForFieldId(*new_field_id)); + // Disable hiddenapi checks when accessing this method. + // Redefining hiddenapi flags is unsupported for the same reasons as redefining + // access flags. Moreover, ArtField loses pointer to the old dex file, so just + // disable the checks completely for consistency. + field.SetAccessFlags(field.GetAccessFlags() | art::kAccPublicApi); } } } diff --git a/runtime/art_field.h b/runtime/art_field.h index dc7f985b91..1cf7afa022 100644 --- a/runtime/art_field.h +++ b/runtime/art_field.h @@ -180,10 +180,6 @@ class ArtField final { return (GetAccessFlags() & kAccVolatile) != 0; } - hiddenapi::ApiList GetHiddenApiAccessFlags() REQUIRES_SHARED(Locks::mutator_lock_) { - return hiddenapi::DecodeFromRuntime(GetAccessFlags()); - } - // Returns an instance field with this offset in the given class or null if not found. // If kExactOffset is true then we only find the matching offset, not the field containing the // offset. diff --git a/runtime/art_method-inl.h b/runtime/art_method-inl.h index d8da9129ed..f2541160ff 100644 --- a/runtime/art_method-inl.h +++ b/runtime/art_method-inl.h @@ -367,160 +367,6 @@ inline bool ArtMethod::HasSingleImplementation() { return (GetAccessFlags() & kAccSingleImplementation) != 0; } -inline hiddenapi::ApiList ArtMethod::GetHiddenApiAccessFlags() - REQUIRES_SHARED(Locks::mutator_lock_) { - if (UNLIKELY(IsIntrinsic())) { - switch (static_cast(GetIntrinsic())) { - case Intrinsics::kSystemArrayCopyChar: - case Intrinsics::kStringGetCharsNoCheck: - case Intrinsics::kReferenceGetReferent: - case Intrinsics::kMemoryPeekByte: - case Intrinsics::kMemoryPokeByte: - case Intrinsics::kUnsafeCASInt: - case Intrinsics::kUnsafeCASLong: - case Intrinsics::kUnsafeCASObject: - case Intrinsics::kUnsafeGet: - case Intrinsics::kUnsafeGetAndAddInt: - case Intrinsics::kUnsafeGetAndAddLong: - case Intrinsics::kUnsafeGetAndSetInt: - case Intrinsics::kUnsafeGetAndSetLong: - case Intrinsics::kUnsafeGetAndSetObject: - case Intrinsics::kUnsafeGetLong: - case Intrinsics::kUnsafeGetLongVolatile: - case Intrinsics::kUnsafeGetObject: - case Intrinsics::kUnsafeGetObjectVolatile: - case Intrinsics::kUnsafeGetVolatile: - case Intrinsics::kUnsafePut: - case Intrinsics::kUnsafePutLong: - case Intrinsics::kUnsafePutLongOrdered: - case Intrinsics::kUnsafePutLongVolatile: - case Intrinsics::kUnsafePutObject: - case Intrinsics::kUnsafePutObjectOrdered: - case Intrinsics::kUnsafePutObjectVolatile: - case Intrinsics::kUnsafePutOrdered: - case Intrinsics::kUnsafePutVolatile: - case Intrinsics::kUnsafeLoadFence: - case Intrinsics::kUnsafeStoreFence: - case Intrinsics::kUnsafeFullFence: - case Intrinsics::kCRC32Update: - // 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 hiddenapi::ApiList::kLightGreylist; - case Intrinsics::kStringNewStringFromBytes: - case Intrinsics::kStringNewStringFromChars: - case Intrinsics::kStringNewStringFromString: - case Intrinsics::kMemoryPeekIntNative: - case Intrinsics::kMemoryPeekLongNative: - case Intrinsics::kMemoryPeekShortNative: - case Intrinsics::kMemoryPokeIntNative: - case Intrinsics::kMemoryPokeLongNative: - case Intrinsics::kMemoryPokeShortNative: - return hiddenapi::ApiList::kDarkGreylist; - 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 hiddenapi::ApiList::kBlacklist; - default: - // Remaining intrinsics are public API. We DCHECK that in SetIntrinsic(). - return hiddenapi::ApiList::kWhitelist; - } - } else { - return hiddenapi::DecodeFromRuntime(GetAccessFlags()); - } -} - -inline void ArtMethod::SetIntrinsic(uint32_t intrinsic) { - // Currently we only do intrinsics for static/final methods or methods of final - // classes. We don't set kHasSingleImplementation for those methods. - DCHECK(IsStatic() || IsFinal() || GetDeclaringClass()->IsFinal()) << - "Potential conflict with kAccSingleImplementation"; - static const int kAccFlagsShift = CTZ(kAccIntrinsicBits); - DCHECK_LE(intrinsic, kAccIntrinsicBits >> kAccFlagsShift); - uint32_t intrinsic_bits = intrinsic << kAccFlagsShift; - uint32_t new_value = (GetAccessFlags() & ~kAccIntrinsicBits) | kAccIntrinsic | intrinsic_bits; - if (kIsDebugBuild) { - uint32_t java_flags = (GetAccessFlags() & kAccJavaFlagsMask); - bool is_constructor = IsConstructor(); - bool is_synchronized = IsSynchronized(); - bool skip_access_checks = SkipAccessChecks(); - bool is_fast_native = IsFastNative(); - bool is_critical_native = IsCriticalNative(); - bool is_copied = IsCopied(); - bool is_miranda = IsMiranda(); - bool is_default = IsDefault(); - bool is_default_conflict = IsDefaultConflicting(); - bool is_compilable = IsCompilable(); - bool must_count_locks = MustCountLocks(); - hiddenapi::ApiList hidden_api_flags = GetHiddenApiAccessFlags(); - SetAccessFlags(new_value); - DCHECK_EQ(java_flags, (GetAccessFlags() & kAccJavaFlagsMask)); - DCHECK_EQ(is_constructor, IsConstructor()); - DCHECK_EQ(is_synchronized, IsSynchronized()); - DCHECK_EQ(skip_access_checks, SkipAccessChecks()); - DCHECK_EQ(is_fast_native, IsFastNative()); - DCHECK_EQ(is_critical_native, IsCriticalNative()); - DCHECK_EQ(is_copied, IsCopied()); - DCHECK_EQ(is_miranda, IsMiranda()); - DCHECK_EQ(is_default, IsDefault()); - DCHECK_EQ(is_default_conflict, IsDefaultConflicting()); - DCHECK_EQ(is_compilable, IsCompilable()); - DCHECK_EQ(must_count_locks, MustCountLocks()); - // 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 != hiddenapi::ApiList::kWhitelist) { - DCHECK_EQ(hidden_api_flags, GetHiddenApiAccessFlags()) << PrettyMethod(); - } - } else { - SetAccessFlags(new_value); - } -} - template void ArtMethod::VisitRoots(RootVisitorType& visitor, PointerSize pointer_size) { if (LIKELY(!declaring_class_.IsNull())) { diff --git a/runtime/art_method.cc b/runtime/art_method.cc index 9bf31edd52..abfdd5547d 100644 --- a/runtime/art_method.cc +++ b/runtime/art_method.cc @@ -33,6 +33,7 @@ #include "dex/dex_instruction.h" #include "entrypoints/runtime_asm_entrypoints.h" #include "gc/accounting/card_table-inl.h" +#include "hidden_api.h" #include "interpreter/interpreter.h" #include "jit/jit.h" #include "jit/jit_code_cache.h" @@ -682,23 +683,72 @@ bool ArtMethod::HasAnyCompiledCode() { return GetOatMethodQuickCode(runtime->GetClassLinker()->GetImagePointerSize()) != nullptr; } +void ArtMethod::SetIntrinsic(uint32_t intrinsic) { + // Currently we only do intrinsics for static/final methods or methods of final + // classes. We don't set kHasSingleImplementation for those methods. + DCHECK(IsStatic() || IsFinal() || GetDeclaringClass()->IsFinal()) << + "Potential conflict with kAccSingleImplementation"; + static const int kAccFlagsShift = CTZ(kAccIntrinsicBits); + DCHECK_LE(intrinsic, kAccIntrinsicBits >> kAccFlagsShift); + uint32_t intrinsic_bits = intrinsic << kAccFlagsShift; + uint32_t new_value = (GetAccessFlags() & ~kAccIntrinsicBits) | kAccIntrinsic | intrinsic_bits; + if (kIsDebugBuild) { + uint32_t java_flags = (GetAccessFlags() & kAccJavaFlagsMask); + bool is_constructor = IsConstructor(); + bool is_synchronized = IsSynchronized(); + bool skip_access_checks = SkipAccessChecks(); + bool is_fast_native = IsFastNative(); + bool is_critical_native = IsCriticalNative(); + bool is_copied = IsCopied(); + bool is_miranda = IsMiranda(); + bool is_default = IsDefault(); + bool is_default_conflict = IsDefaultConflicting(); + bool is_compilable = IsCompilable(); + bool must_count_locks = MustCountLocks(); + uint32_t hiddenapi_flags = hiddenapi::GetRuntimeFlags(this); + SetAccessFlags(new_value); + DCHECK_EQ(java_flags, (GetAccessFlags() & kAccJavaFlagsMask)); + DCHECK_EQ(is_constructor, IsConstructor()); + DCHECK_EQ(is_synchronized, IsSynchronized()); + DCHECK_EQ(skip_access_checks, SkipAccessChecks()); + DCHECK_EQ(is_fast_native, IsFastNative()); + DCHECK_EQ(is_critical_native, IsCriticalNative()); + DCHECK_EQ(is_copied, IsCopied()); + DCHECK_EQ(is_miranda, IsMiranda()); + DCHECK_EQ(is_default, IsDefault()); + DCHECK_EQ(is_default_conflict, IsDefaultConflicting()); + DCHECK_EQ(is_compilable, IsCompilable()); + DCHECK_EQ(must_count_locks, MustCountLocks()); + // 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 ((hiddenapi_flags & kAccHiddenapiBits) == 0) { + DCHECK_EQ(hiddenapi_flags, hiddenapi::GetRuntimeFlags(this)) << PrettyMethod(); + } + } else { + SetAccessFlags(new_value); + } +} + void ArtMethod::SetNotIntrinsic() { if (!IsIntrinsic()) { return; } - // Query the hidden API access flags of the intrinsic. - hiddenapi::ApiList intrinsic_api_list = GetHiddenApiAccessFlags(); + // Read the existing hiddenapi flags. + uint32_t hiddenapi_runtime_flags = hiddenapi::GetRuntimeFlags(this); // Clear intrinsic-related access flags. ClearAccessFlags(kAccIntrinsic | kAccIntrinsicBits); // Re-apply hidden API access flags now that the method is not an intrinsic. - SetAccessFlags(hiddenapi::EncodeForRuntime(GetAccessFlags(), intrinsic_api_list)); - DCHECK_EQ(GetHiddenApiAccessFlags(), intrinsic_api_list); + SetAccessFlags(GetAccessFlags() | hiddenapi_runtime_flags); + DCHECK_EQ(hiddenapi_runtime_flags, hiddenapi::GetRuntimeFlags(this)); } - void ArtMethod::CopyFrom(ArtMethod* src, PointerSize image_pointer_size) { memcpy(reinterpret_cast(this), reinterpret_cast(src), Size(image_pointer_size)); diff --git a/runtime/art_method.h b/runtime/art_method.h index 4e3ef5366a..5bbee92c14 100644 --- a/runtime/art_method.h +++ b/runtime/art_method.h @@ -343,8 +343,6 @@ class ArtMethod final { AddAccessFlags(kAccMustCountLocks); } - hiddenapi::ApiList GetHiddenApiAccessFlags() REQUIRES_SHARED(Locks::mutator_lock_); - // Returns true if this method could be overridden by a default method. bool IsOverridableByDefaultMethod() REQUIRES_SHARED(Locks::mutator_lock_); diff --git a/runtime/class_linker.cc b/runtime/class_linker.cc index 35379cc251..ce7dfaf8af 100644 --- a/runtime/class_linker.cc +++ b/runtime/class_linker.cc @@ -3468,14 +3468,8 @@ void ClassLinker::LoadField(const ClassAccessor::Field& field, dst->SetDexFieldIndex(field_idx); dst->SetDeclaringClass(klass.Get()); - // Get access flags from the DexFile. If this is a boot class path class, - // also set its runtime hidden API access flags. - uint32_t access_flags = field.GetAccessFlags(); - if (klass->IsBootStrapClassLoaded()) { - access_flags = hiddenapi::EncodeForRuntime( - access_flags, static_cast(field.GetHiddenapiFlags())); - } - dst->SetAccessFlags(access_flags); + // Get access flags from the DexFile and set hiddenapi runtime access flags. + dst->SetAccessFlags(field.GetAccessFlags() | hiddenapi::CreateRuntimeFlags(field)); } void ClassLinker::LoadMethod(const DexFile& dex_file, @@ -3491,13 +3485,8 @@ void ClassLinker::LoadMethod(const DexFile& dex_file, dst->SetDeclaringClass(klass.Get()); dst->SetCodeItemOffset(method.GetCodeItemOffset()); - // Get access flags from the DexFile. If this is a boot class path class, - // also set its runtime hidden API access flags. - uint32_t access_flags = method.GetAccessFlags(); - if (klass->IsBootStrapClassLoaded()) { - access_flags = hiddenapi::EncodeForRuntime( - access_flags, static_cast(method.GetHiddenapiFlags())); - } + // Get access flags from the DexFile and set hiddenapi runtime access flags. + uint32_t access_flags = method.GetAccessFlags() | hiddenapi::CreateRuntimeFlags(method); if (UNLIKELY(strcmp("finalize", method_name) == 0)) { // Set finalizable flag on declaring class. diff --git a/runtime/hidden_api.cc b/runtime/hidden_api.cc index 3b7b938d50..ab9e65c458 100644 --- a/runtime/hidden_api.cc +++ b/runtime/hidden_api.cc @@ -18,8 +18,12 @@ #include +#include "art_field-inl.h" +#include "art_method-inl.h" #include "base/dumpable.h" -#include "thread-current-inl.h" +#include "dex/class_accessor-inl.h" +#include "scoped_thread_state_change.h" +#include "thread-inl.h" #include "well_known_classes.h" #ifdef ART_TARGET_ANDROID @@ -235,23 +239,82 @@ void MemberSignature::NotifyHiddenApiListener(AccessMethod access_method) { } } -static ALWAYS_INLINE bool CanUpdateMemberAccessFlags(ArtField*) { +static ALWAYS_INLINE bool CanUpdateRuntimeFlags(ArtField*) { return true; } -static ALWAYS_INLINE bool CanUpdateMemberAccessFlags(ArtMethod* method) { +static ALWAYS_INLINE bool CanUpdateRuntimeFlags(ArtMethod* method) { return !method->IsIntrinsic(); } template static ALWAYS_INLINE void MaybeWhitelistMember(Runtime* runtime, T* member) REQUIRES_SHARED(Locks::mutator_lock_) { - if (CanUpdateMemberAccessFlags(member) && runtime->ShouldDedupeHiddenApiWarnings()) { - member->SetAccessFlags(hiddenapi::EncodeForRuntime( - member->GetAccessFlags(), hiddenapi::ApiList::kWhitelist)); + if (CanUpdateRuntimeFlags(member) && runtime->ShouldDedupeHiddenApiWarnings()) { + member->SetAccessFlags(member->GetAccessFlags() | kAccPublicApi); } } +static constexpr uint32_t kNoDexFlags = 0u; +static constexpr uint32_t kInvalidDexFlags = static_cast(-1); + +uint32_t GetDexFlags(ArtField* field) REQUIRES_SHARED(Locks::mutator_lock_) { + ObjPtr declaring_class = field->GetDeclaringClass(); + DCHECK(declaring_class != nullptr) << "Fields always have a declaring class"; + + const DexFile::ClassDef* class_def = declaring_class->GetClassDef(); + if (class_def == nullptr) { + return kNoDexFlags; + } + + uint32_t flags = kInvalidDexFlags; + DCHECK(!AreValidFlags(flags)); + + ClassAccessor accessor(declaring_class->GetDexFile(), + *class_def, + /* parse_hiddenapi_class_data= */ true); + auto fn_visit = [&](const ClassAccessor::Field& dex_field) { + if (dex_field.GetIndex() == field->GetDexFieldIndex()) { + flags = dex_field.GetHiddenapiFlags(); + } + }; + accessor.VisitFields(fn_visit, fn_visit); + + CHECK_NE(flags, kInvalidDexFlags) << "Could not find flags for field " << field->PrettyField(); + DCHECK(AreValidFlags(flags)); + return flags; +} + +uint32_t GetDexFlags(ArtMethod* method) REQUIRES_SHARED(Locks::mutator_lock_) { + ObjPtr declaring_class = method->GetDeclaringClass(); + if (declaring_class.IsNull()) { + DCHECK(method->IsRuntimeMethod()); + return kNoDexFlags; + } + + const DexFile::ClassDef* class_def = declaring_class->GetClassDef(); + if (class_def == nullptr) { + return kNoDexFlags; + } + + uint32_t flags = kInvalidDexFlags; + DCHECK(!AreValidFlags(flags)); + + ClassAccessor accessor(declaring_class->GetDexFile(), + *class_def, + /* parse_hiddenapi_class_data= */ true); + auto fn_visit = [&](const ClassAccessor::Method& dex_method) { + if (dex_method.GetIndex() == method->GetDexMethodIndex()) { + flags = dex_method.GetHiddenapiFlags(); + } + }; + accessor.VisitMethods(fn_visit, fn_visit); + + CHECK_NE(flags, kInvalidDexFlags) << "Could not find flags for method " << method->PrettyMethod(); + DCHECK(AreValidFlags(flags)); + return flags; +} + template bool ShouldDenyAccessToMemberImpl(T* member, hiddenapi::ApiList api_list, diff --git a/runtime/hidden_api.h b/runtime/hidden_api.h index ed00e2a892..98414f7a87 100644 --- a/runtime/hidden_api.h +++ b/runtime/hidden_api.h @@ -17,10 +17,11 @@ #ifndef ART_RUNTIME_HIDDEN_API_H_ #define ART_RUNTIME_HIDDEN_API_H_ -#include "art_field-inl.h" -#include "art_method-inl.h" +#include "art_field.h" +#include "art_method.h" #include "base/mutex.h" #include "dex/hidden_api_access_flags.h" +#include "intrinsics_enum.h" #include "mirror/class-inl.h" #include "reflection.h" #include "runtime.h" @@ -164,12 +165,136 @@ class MemberSignature { void NotifyHiddenApiListener(AccessMethod access_method); }; +// Locates hiddenapi flags for `field` in the corresponding dex file. +// NB: This is an O(N) operation, linear with the number of members in the class def. +uint32_t GetDexFlags(ArtField* field) REQUIRES_SHARED(Locks::mutator_lock_); + +// Locates hiddenapi flags for `method` in the corresponding dex file. +// NB: This is an O(N) operation, linear with the number of members in the class def. +uint32_t GetDexFlags(ArtMethod* method) REQUIRES_SHARED(Locks::mutator_lock_); + template bool ShouldDenyAccessToMemberImpl(T* member, ApiList api_list, AccessMethod access_method) REQUIRES_SHARED(Locks::mutator_lock_); } // namespace detail +// Returns access flags for the runtime representation of a class member (ArtField/ArtMember). +ALWAYS_INLINE inline uint32_t CreateRuntimeFlags(const ClassAccessor::BaseItem& member) { + uint32_t runtime_flags = 0u; + + uint32_t dex_flags = member.GetHiddenapiFlags(); + DCHECK(AreValidFlags(dex_flags)); + + ApiList api_list = static_cast(dex_flags); + if (api_list == ApiList::kWhitelist) { + runtime_flags |= kAccPublicApi; + } + + DCHECK_EQ(runtime_flags & kAccHiddenapiBits, runtime_flags) + << "Runtime flags not in reserved access flags bits"; + return runtime_flags; +} + +// Extracts hiddenapi runtime flags from access flags of ArtField. +ALWAYS_INLINE inline uint32_t GetRuntimeFlags(ArtField* field) + REQUIRES_SHARED(Locks::mutator_lock_) { + return field->GetAccessFlags() & kAccHiddenapiBits; +} + +// Extracts hiddenapi runtime flags from access flags of ArtMethod. +// Uses hardcoded values for intrinsics. +ALWAYS_INLINE inline uint32_t GetRuntimeFlags(ArtMethod* method) + REQUIRES_SHARED(Locks::mutator_lock_) { + if (UNLIKELY(method->IsIntrinsic())) { + switch (static_cast(method->GetIntrinsic())) { + case Intrinsics::kSystemArrayCopyChar: + case Intrinsics::kStringGetCharsNoCheck: + case Intrinsics::kReferenceGetReferent: + case Intrinsics::kMemoryPeekByte: + case Intrinsics::kMemoryPokeByte: + case Intrinsics::kUnsafeCASInt: + case Intrinsics::kUnsafeCASLong: + case Intrinsics::kUnsafeCASObject: + case Intrinsics::kUnsafeGet: + case Intrinsics::kUnsafeGetAndAddInt: + case Intrinsics::kUnsafeGetAndAddLong: + case Intrinsics::kUnsafeGetAndSetInt: + case Intrinsics::kUnsafeGetAndSetLong: + case Intrinsics::kUnsafeGetAndSetObject: + case Intrinsics::kUnsafeGetLong: + case Intrinsics::kUnsafeGetLongVolatile: + case Intrinsics::kUnsafeGetObject: + case Intrinsics::kUnsafeGetObjectVolatile: + case Intrinsics::kUnsafeGetVolatile: + case Intrinsics::kUnsafePut: + case Intrinsics::kUnsafePutLong: + case Intrinsics::kUnsafePutLongOrdered: + case Intrinsics::kUnsafePutLongVolatile: + case Intrinsics::kUnsafePutObject: + case Intrinsics::kUnsafePutObjectOrdered: + case Intrinsics::kUnsafePutObjectVolatile: + case Intrinsics::kUnsafePutOrdered: + case Intrinsics::kUnsafePutVolatile: + case Intrinsics::kUnsafeLoadFence: + case Intrinsics::kUnsafeStoreFence: + case Intrinsics::kUnsafeFullFence: + case Intrinsics::kCRC32Update: + case Intrinsics::kStringNewStringFromBytes: + case Intrinsics::kStringNewStringFromChars: + case Intrinsics::kStringNewStringFromString: + case Intrinsics::kMemoryPeekIntNative: + case Intrinsics::kMemoryPeekLongNative: + case Intrinsics::kMemoryPeekShortNative: + case Intrinsics::kMemoryPokeIntNative: + case Intrinsics::kMemoryPokeLongNative: + case Intrinsics::kMemoryPokeShortNative: + 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: + return 0u; + default: + // Remaining intrinsics are public API. We DCHECK that in SetIntrinsic(). + return kAccPublicApi; + } + } else { + return method->GetAccessFlags() & kAccHiddenapiBits; + } +} + // Returns true if access to `member` should be denied in the given context. // The decision is based on whether the caller is in a trusted context or not. // Because determining the access context can be expensive, a lambda function @@ -183,16 +308,9 @@ inline bool ShouldDenyAccessToMember(T* member, REQUIRES_SHARED(Locks::mutator_lock_) { DCHECK(member != nullptr); - // Decode hidden API access flags. - // NB Multiple threads might try to access (and overwrite) these simultaneously, - // causing a race. We only do that if access has not been denied, so the race - // cannot change Java semantics. We should, however, decode the access flags - // once and use it throughout this function, otherwise we may get inconsistent - // results, e.g. print whitelist warnings (b/78327881). - ApiList api_list = member->GetHiddenApiAccessFlags(); - - // Exit early if member is on the whitelist. - if (api_list == ApiList::kWhitelist) { + // Exit early if member is public API. This flag is also set for non-boot class + // path fields/methods. + if ((GetRuntimeFlags(member) & kAccPublicApi) != 0) { return false; } @@ -202,6 +320,11 @@ inline bool ShouldDenyAccessToMember(T* member, return false; } + // Decode hidden API access flags from the dex file. + // This is an O(N) operation scaling with the number of fields/methods + // in the class. Only do this on slow path and only do it once. + ApiList api_list = static_cast(detail::GetDexFlags(member)); + // Member is hidden and caller is not exempted. Enter slow path. return detail::ShouldDenyAccessToMemberImpl(member, api_list, access_method); } diff --git a/runtime/mirror/class.h b/runtime/mirror/class.h index c38cc86c59..4e551adfde 100644 --- a/runtime/mirror/class.h +++ b/runtime/mirror/class.h @@ -211,12 +211,12 @@ class MANAGED Class final : public Object { } ALWAYS_INLINE bool ShouldSkipHiddenApiChecks() REQUIRES_SHARED(Locks::mutator_lock_) { - return (GetAccessFlags() & kAccSkipHiddenApiChecks) != 0; + return (GetAccessFlags() & kAccSkipHiddenapiChecks) != 0; } ALWAYS_INLINE void SetSkipHiddenApiChecks() REQUIRES_SHARED(Locks::mutator_lock_) { uint32_t flags = GetAccessFlags(); - SetAccessFlags(flags | kAccSkipHiddenApiChecks); + SetAccessFlags(flags | kAccSkipHiddenapiChecks); } ALWAYS_INLINE void SetRecursivelyInitialized() REQUIRES_SHARED(Locks::mutator_lock_) { diff --git a/test/674-hiddenapi/hiddenapi.cc b/test/674-hiddenapi/hiddenapi.cc index 8e3e4eb5ce..c4439de550 100644 --- a/test/674-hiddenapi/hiddenapi.cc +++ b/test/674-hiddenapi/hiddenapi.cc @@ -284,7 +284,7 @@ extern "C" JNIEXPORT jboolean JNICALL Java_JNI_canInvokeConstructorV( } extern "C" JNIEXPORT jint JNICALL Java_Reflection_getHiddenApiAccessFlags(JNIEnv*, jclass) { - return static_cast(kAccHiddenApiBits); + return static_cast(kAccHiddenapiBits); } } // namespace Test674HiddenApi diff --git a/test/999-redefine-hiddenapi/src-redefine/gen.sh b/test/999-redefine-hiddenapi/src-redefine/gen.sh index 6948cbbfc3..f78a025265 100755 --- a/test/999-redefine-hiddenapi/src-redefine/gen.sh +++ b/test/999-redefine-hiddenapi/src-redefine/gen.sh @@ -18,13 +18,21 @@ set -e DIR="$( cd "$( dirname "${BASH_SOURCE[0]}" )" && pwd )" TMP=`mktemp -d` -CLASS "art/Test999" +CLASS="art/Test999" -(cd "$TMP" && javac -d "${TMP}" "$DIR/${CLASS}.java" && d8 --output . "$TMP/${CLASS}.class") +(cd "$TMP" && \ + javac -d "${TMP}" "$DIR/${CLASS}.java" && \ + d8 --output . "$TMP/${CLASS}.class" && + hiddenapi encode --input-dex="$TMP/classes.dex" \ + --output-dex="$TMP/classes-hiddenapi.dex" \ + --flags="$DIR/../hiddenapi-flags.csv" \ + --no-force-assign-all) echo ' private static final byte[] CLASS_BYTES = Base64.getDecoder().decode(' base64 "${TMP}/${CLASS}.class" | sed -E 's/^/ "/' | sed ':a;N;$!ba;s/\n/" +\n/g' | sed -E '$ s/$/");/' echo ' private static final byte[] DEX_BYTES = Base64.getDecoder().decode(' base64 "${TMP}/classes.dex" | sed -E 's/^/ "/' | sed ':a;N;$!ba;s/\n/" +\n/g' | sed -E '$ s/$/");/' +echo ' private static final byte[] DEX_BYTES_HIDDEN = Base64.getDecoder().decode(' +base64 "${TMP}/classes-hiddenapi.dex" | sed -E 's/^/ "/' | sed ':a;N;$!ba;s/\n/" +\n/g' | sed -E '$ s/$/");/' rm -rf "$TMP" diff --git a/test/999-redefine-hiddenapi/src/Main.java b/test/999-redefine-hiddenapi/src/Main.java index c6365ac234..4627b4fd22 100644 --- a/test/999-redefine-hiddenapi/src/Main.java +++ b/test/999-redefine-hiddenapi/src/Main.java @@ -19,7 +19,7 @@ import java.lang.reflect.Method; import java.util.Base64; public class Main { - public static void main(String[] args) throws Exception { + public static void main(String[] args) throws ClassNotFoundException { System.loadLibrary(args[0]); // Run the initialization routine. This will enable hidden API checks in @@ -31,35 +31,53 @@ public class Main { // Find the test class in boot class loader and verify that its members are hidden. Class klass = Class.forName("art.Test999", true, BOOT_CLASS_LOADER); - assertMethodIsHidden(klass, "before redefinition"); - assertFieldIsHidden(klass, "before redefinition"); + assertMethodIsHidden(true, klass, "before redefinition"); + assertFieldIsHidden(true, klass, "before redefinition"); - // Redefine the class using JVMTI. + // Redefine the class using JVMTI. Use dex file without hiddenapi flags. art.Redefinition.setTestConfiguration(art.Redefinition.Config.COMMON_REDEFINE); art.Redefinition.doCommonClassRedefinition(klass, CLASS_BYTES, DEX_BYTES); - // Verify that the class members are still hidden. - assertMethodIsHidden(klass, "after redefinition"); - assertFieldIsHidden(klass, "after redefinition"); + // Verify that the class members are not hidden anymore. + assertMethodIsHidden(false, klass, "after first redefinition"); + assertFieldIsHidden(false, klass, "after first redefinition"); + + // Redefine the class using JVMTI, this time with a dex file with hiddenapi flags. + art.Redefinition.setTestConfiguration(art.Redefinition.Config.COMMON_REDEFINE); + art.Redefinition.doCommonClassRedefinition(klass, CLASS_BYTES, DEX_BYTES_HIDDEN); + + // Verify that the class members are still accessible. + assertMethodIsHidden(false, klass, "after second redefinition"); + assertFieldIsHidden(false, klass, "after second redefinition"); } - private static void assertMethodIsHidden(Class klass, String msg) throws Exception { + private static void assertMethodIsHidden(boolean expectedHidden, Class klass, String msg) { try { klass.getDeclaredMethod("foo"); - // Unexpected. Should have thrown NoSuchMethodException. - throw new Exception("Method should not be accessible " + msg); + if (expectedHidden) { + // Unexpected. Should have thrown NoSuchMethodException. + throw new RuntimeException("Method should not be accessible " + msg); + } } catch (NoSuchMethodException ex) { - // Expected. + if (!expectedHidden) { + // Unexpected. Should not have thrown NoSuchMethodException. + throw new RuntimeException("Method should be accessible " + msg); + } } } - private static void assertFieldIsHidden(Class klass, String msg) throws Exception { + private static void assertFieldIsHidden(boolean expectedHidden, Class klass, String msg) { try { klass.getDeclaredField("bar"); - // Unexpected. Should have thrown NoSuchFieldException. - throw new Exception("Field should not be accessible " + msg); + if (expectedHidden) { + // Unexpected. Should have thrown NoSuchFieldException. + throw new RuntimeException("Field should not be accessible " + msg); + } } catch (NoSuchFieldException ex) { - // Expected. + if (!expectedHidden) { + // Unexpected. Should not have thrown NoSuchFieldException. + throw new RuntimeException("Field should be accessible " + msg); + } } } @@ -93,19 +111,37 @@ public class Main { "ASoQQLUAArEAAAABAA0AAAAKAAIAAAATAAQAGAABAA4ACwABAAwAAAAlAAIAAQAAAAmyAAMSBLYA" + "BbEAAAABAA0AAAAKAAIAAAAVAAgAFgABAA8AAAACABA="); private static final byte[] DEX_BYTES = Base64.getDecoder().decode( - "ZGV4CjAzNQD0dZ+IWxOi+cJDSWjfTnUerlZj1Lll3ONIAwAAcAAAAHhWNBIAAAAAAAAAAJwCAAAQ" + - "AAAAcAAAAAcAAACwAAAAAgAAAMwAAAACAAAA5AAAAAQAAAD0AAAAAQAAABQBAAAUAgAANAEAAIYB" + + "ZGV4CjAzNQDlfmgFfKulToQpDF+P4dsgeOkgfzzH+5lgAwAAcAAAAHhWNBIAAAAAAAAAALQCAAAQ" + + "AAAAcAAAAAcAAACwAAAAAgAAAMwAAAACAAAA5AAAAAQAAAD0AAAAAQAAABQBAAAsAgAANAEAAIYB" + + "AACOAQAAlwEAAJoBAACpAQAAwAEAANQBAADoAQAA/AEAAAoCAAANAgAAEQIAABYCAAAbAgAAIAIA" + + "ACkCAAACAAAAAwAAAAQAAAAFAAAABgAAAAcAAAAJAAAACQAAAAYAAAAAAAAACgAAAAYAAACAAQAA" + + "AQAAAAsAAAAFAAIADQAAAAEAAAAAAAAAAQAAAAwAAAACAAEADgAAAAMAAAAAAAAAAQAAAAEAAAAD" + + "AAAAAAAAAAgAAAAAAAAAoAIAAAAAAAACAAEAAQAAAHQBAAAIAAAAcBADAAEAEwBAAFkQAAAOAAMA" + + "AQACAAAAeQEAAAgAAABiAAEAGgEBAG4gAgAQAA4AEwAOQAAVAA54AAAAAQAAAAQABjxpbml0PgAH" + + "R29vZGJ5ZQABSQANTGFydC9UZXN0OTk5OwAVTGphdmEvaW8vUHJpbnRTdHJlYW07ABJMamF2YS9s" + + "YW5nL09iamVjdDsAEkxqYXZhL2xhbmcvU3RyaW5nOwASTGphdmEvbGFuZy9TeXN0ZW07AAxUZXN0" + + "OTk5LmphdmEAAVYAAlZMAANiYXIAA2ZvbwADb3V0AAdwcmludGxuAHV+fkQ4eyJjb21waWxhdGlv" + + "bi1tb2RlIjoiZGVidWciLCJtaW4tYXBpIjoxLCJzaGEtMSI6ImQyMmFiNGYxOWI3NTYxNDQ3NTI4" + + "NTdjYTg2YjJjZWU0ZGQ5Y2ExNjYiLCJ2ZXJzaW9uIjoiMS40LjktZGV2In0AAAEBAQABAIGABLQC" + + "AQHUAgAAAAAOAAAAAAAAAAEAAAAAAAAAAQAAABAAAABwAAAAAgAAAAcAAACwAAAAAwAAAAIAAADM" + + "AAAABAAAAAIAAADkAAAABQAAAAQAAAD0AAAABgAAAAEAAAAUAQAAASAAAAIAAAA0AQAAAyAAAAIA" + + "AAB0AQAAARAAAAEAAACAAQAAAiAAABAAAACGAQAAACAAAAEAAACgAgAAAxAAAAEAAACwAgAAABAA" + + "AAEAAAC0AgAA"); + private static final byte[] DEX_BYTES_HIDDEN = Base64.getDecoder().decode( + "ZGV4CjAzNQDsgG5ufKulToQpDF+P4dsgeOkgfzzH+5l4AwAAcAAAAHhWNBIAAAAAAAAAAMACAAAQ" + + "AAAAcAAAAAcAAACwAAAAAgAAAMwAAAACAAAA5AAAAAQAAAD0AAAAAQAAABQBAABEAgAANAEAAIYB" + "AACOAQAAlwEAAJoBAACpAQAAwAEAANQBAADoAQAA/AEAAAoCAAANAgAAEQIAABYCAAAbAgAAIAIA" + "ACkCAAACAAAAAwAAAAQAAAAFAAAABgAAAAcAAAAJAAAACQAAAAYAAAAAAAAACgAAAAYAAACAAQAA" + "AQAAAAsAAAAFAAIADQAAAAEAAAAAAAAAAQAAAAwAAAACAAEADgAAAAMAAAAAAAAAAQAAAAEAAAAD" + - "AAAAAAAAAAgAAAAAAAAAhwIAAAAAAAACAAEAAQAAAHQBAAAIAAAAcBADAAEAEwBAAFkQAAAOAAMA" + + "AAAAAAAAAAgAAAAAAAAAoAIAAAAAAAACAAEAAQAAAHQBAAAIAAAAcBADAAEAEwBAAFkQAAAOAAMA" + "AQACAAAAeQEAAAgAAABiAAEAGgEBAG4gAgAQAA4AEwAOQAAVAA54AAAAAQAAAAQABjxpbml0PgAH" + "R29vZGJ5ZQABSQANTGFydC9UZXN0OTk5OwAVTGphdmEvaW8vUHJpbnRTdHJlYW07ABJMamF2YS9s" + "YW5nL09iamVjdDsAEkxqYXZhL2xhbmcvU3RyaW5nOwASTGphdmEvbGFuZy9TeXN0ZW07AAxUZXN0" + - "OTk5LmphdmEAAVYAAlZMAANiYXIAA2ZvbwADb3V0AAdwcmludGxuAFx+fkQ4eyJtaW4tYXBpIjox" + - "LCJzaGEtMSI6IjU2YzJlMzBmNTIzM2I4NDRmZjZkZGQ4N2ZiNTNkMzRmYjE3MjM3ZGYiLCJ2ZXJz" + - "aW9uIjoidjEuMi4xNS1kZXYifQAAAQEBAAEAgYAEtAIBAdQCAAAAAAAOAAAAAAAAAAEAAAAAAAAA" + - "AQAAABAAAABwAAAAAgAAAAcAAACwAAAAAwAAAAIAAADMAAAABAAAAAIAAADkAAAABQAAAAQAAAD0" + - "AAAABgAAAAEAAAAUAQAAASAAAAIAAAA0AQAAAyAAAAIAAAB0AQAAARAAAAEAAACAAQAAAiAAABAA" + - "AACGAQAAACAAAAEAAACHAgAAAxAAAAEAAACYAgAAABAAAAEAAACcAgAA"); + "OTk5LmphdmEAAVYAAlZMAANiYXIAA2ZvbwADb3V0AAdwcmludGxuAHV+fkQ4eyJjb21waWxhdGlv" + + "bi1tb2RlIjoiZGVidWciLCJtaW4tYXBpIjoxLCJzaGEtMSI6ImQyMmFiNGYxOWI3NTYxNDQ3NTI4" + + "NTdjYTg2YjJjZWU0ZGQ5Y2ExNjYiLCJ2ZXJzaW9uIjoiMS40LjktZGV2In0AAAEBAQABAIGABLQC" + + "AQHUAgAAAAALAAAACAAAAAIAAgAPAAAAAAAAAAEAAAAAAAAAAQAAABAAAABwAAAAAgAAAAcAAACw" + + "AAAAAwAAAAIAAADMAAAABAAAAAIAAADkAAAABQAAAAQAAAD0AAAABgAAAAEAAAAUAQAAASAAAAIA" + + "AAA0AQAAAyAAAAIAAAB0AQAAARAAAAEAAACAAQAAAiAAABAAAACGAQAAACAAAAEAAACgAgAAAxAA" + + "AAEAAACwAgAAAPAAAAEAAAC0AgAAABAAAAEAAADAAgAA"); } -- cgit v1.2.3-59-g8ed1b From 2bb2fbd2879d0a6d9ebf7acff817079dde89b417 Mon Sep 17 00:00:00 2001 From: David Brazdil Date: Tue, 13 Nov 2018 18:24:26 +0000 Subject: Create SdkVersion enum, migrate users to it Creates a new SdkVersion enum with integer codes of known Android SDK versions, together with helper functions for common predicates. Also converts target_sdk_version_ in Runtime to uint32_t and cleans up its uses. Test: m test-art Change-Id: Idc6e518c8675068bf952d0256686c88bb0eb833e --- libartbase/base/sdk_version.h | 58 ++++++++++++++++++++++++++++++ runtime/dex/dex_file_annotations.cc | 4 +-- runtime/entrypoints/entrypoint_utils-inl.h | 6 ++-- runtime/entrypoints/entrypoint_utils.cc | 5 +-- runtime/hidden_api.cc | 17 ++++----- runtime/hidden_api.h | 7 ---- runtime/hidden_api_test.cc | 5 +-- runtime/jni/java_vm_ext.cc | 3 +- runtime/native/dalvik_system_VMRuntime.cc | 8 +++-- runtime/parsed_options.cc | 2 +- runtime/runtime.cc | 3 +- runtime/runtime.h | 8 ++--- runtime/runtime_options.cc | 1 + runtime/runtime_options.def | 3 +- runtime/verifier/method_verifier.cc | 16 ++++++--- test/674-hiddenapi/hiddenapi.cc | 3 +- 16 files changed, 107 insertions(+), 42 deletions(-) create mode 100644 libartbase/base/sdk_version.h diff --git a/libartbase/base/sdk_version.h b/libartbase/base/sdk_version.h new file mode 100644 index 0000000000..4372e5a02f --- /dev/null +++ b/libartbase/base/sdk_version.h @@ -0,0 +1,58 @@ +/* + * Copyright (C) 2018 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#ifndef ART_LIBARTBASE_BASE_SDK_VERSION_H_ +#define ART_LIBARTBASE_BASE_SDK_VERSION_H_ + +#include +#include + +namespace art { + +enum class SdkVersion : uint32_t { + kMin = 0u, + kUnset = 0u, + kL = 21u, + kL_MR1 = 22u, + kM = 23u, + kN = 24u, + kN_MR1 = 25u, + kO = 26u, + kO_MR1 = 27u, + kP = 28u, + kP_MR1 = 29u, + kMax = std::numeric_limits::max(), +}; + +inline bool IsSdkVersionSetAndMoreThan(uint32_t lhs, SdkVersion rhs) { + return lhs != static_cast(SdkVersion::kUnset) && lhs > static_cast(rhs); +} + +inline bool IsSdkVersionSetAndAtLeast(uint32_t lhs, SdkVersion rhs) { + return lhs != static_cast(SdkVersion::kUnset) && lhs >= static_cast(rhs); +} + +inline bool IsSdkVersionSetAndAtMost(uint32_t lhs, SdkVersion rhs) { + return lhs != static_cast(SdkVersion::kUnset) && lhs <= static_cast(rhs); +} + +inline bool IsSdkVersionSetAndLessThan(uint32_t lhs, SdkVersion rhs) { + return lhs != static_cast(SdkVersion::kUnset) && lhs < static_cast(rhs); +} + +} // namespace art + +#endif // ART_LIBARTBASE_BASE_SDK_VERSION_H_ diff --git a/runtime/dex/dex_file_annotations.cc b/runtime/dex/dex_file_annotations.cc index 15398672b2..6434828298 100644 --- a/runtime/dex/dex_file_annotations.cc +++ b/runtime/dex/dex_file_annotations.cc @@ -22,6 +22,7 @@ #include "art_field-inl.h" #include "art_method-inl.h" +#include "base/sdk_version.h" #include "class_linker-inl.h" #include "class_root.h" #include "dex/dex_file-inl.h" @@ -129,8 +130,7 @@ ObjPtr CreateAnnotationMember(const ClassData& klass, bool IsVisibilityCompatible(uint32_t actual, uint32_t expected) { if (expected == DexFile::kDexVisibilityRuntime) { - int32_t sdk_version = Runtime::Current()->GetTargetSdkVersion(); - if (sdk_version > 0 && sdk_version <= 23) { + if (IsSdkVersionSetAndAtMost(Runtime::Current()->GetTargetSdkVersion(), SdkVersion::kM)) { return actual == DexFile::kDexVisibilityRuntime || actual == DexFile::kDexVisibilityBuild; } } diff --git a/runtime/entrypoints/entrypoint_utils-inl.h b/runtime/entrypoints/entrypoint_utils-inl.h index 0b005e0851..2236e61d75 100644 --- a/runtime/entrypoints/entrypoint_utils-inl.h +++ b/runtime/entrypoints/entrypoint_utils-inl.h @@ -22,6 +22,7 @@ #include "art_field-inl.h" #include "art_method-inl.h" #include "base/enums.h" +#include "base/sdk_version.h" #include "class_linker-inl.h" #include "common_throws.h" #include "dex/dex_file.h" @@ -94,8 +95,9 @@ inline ArtMethod* GetResolvedMethod(ArtMethod* outer_method, // even going back from boot image methods to the same oat file. However, this is // not currently implemented in the compiler. Therefore crossing dex file boundary // indicates that the inlined definition is not the same as the one used at runtime. - bool target_sdk_pre_p = Runtime::Current()->GetTargetSdkVersion() < 28; - LOG(target_sdk_pre_p ? WARNING : FATAL) + bool target_sdk_at_least_p = + IsSdkVersionSetAndAtLeast(Runtime::Current()->GetTargetSdkVersion(), SdkVersion::kP); + LOG(target_sdk_at_least_p ? FATAL : WARNING) << "Inlined method resolution crossed dex file boundary: from " << method->PrettyMethod() << " in " << method->GetDexFile()->GetLocation() << "/" diff --git a/runtime/entrypoints/entrypoint_utils.cc b/runtime/entrypoints/entrypoint_utils.cc index 12136bf476..19498f386c 100644 --- a/runtime/entrypoints/entrypoint_utils.cc +++ b/runtime/entrypoints/entrypoint_utils.cc @@ -20,6 +20,7 @@ #include "art_method-inl.h" #include "base/enums.h" #include "base/mutex.h" +#include "base/sdk_version.h" #include "class_linker-inl.h" #include "dex/dex_file-inl.h" #include "entrypoints/entrypoint_utils-inl.h" @@ -64,9 +65,9 @@ JValue InvokeProxyInvocationHandler(ScopedObjectAccessAlreadyRunnable& soa, cons soa.Self()->AssertThreadSuspensionIsAllowable(); jobjectArray args_jobj = nullptr; const JValue zero; - int32_t target_sdk_version = Runtime::Current()->GetTargetSdkVersion(); + uint32_t target_sdk_version = Runtime::Current()->GetTargetSdkVersion(); // Do not create empty arrays unless needed to maintain Dalvik bug compatibility. - if (args.size() > 0 || (target_sdk_version > 0 && target_sdk_version <= 21)) { + if (args.size() > 0 || IsSdkVersionSetAndAtMost(target_sdk_version, SdkVersion::kL)) { args_jobj = soa.Env()->NewObjectArray(args.size(), WellKnownClasses::java_lang_Object, nullptr); if (args_jobj == nullptr) { CHECK(soa.Self()->IsExceptionPending()); diff --git a/runtime/hidden_api.cc b/runtime/hidden_api.cc index ab9e65c458..188c5f353b 100644 --- a/runtime/hidden_api.cc +++ b/runtime/hidden_api.cc @@ -21,6 +21,7 @@ #include "art_field-inl.h" #include "art_method-inl.h" #include "base/dumpable.h" +#include "base/sdk_version.h" #include "dex/class_accessor-inl.h" #include "scoped_thread_state_change.h" #include "thread-inl.h" @@ -75,24 +76,19 @@ enum AccessContextFlags { kAccessDenied = 1 << 1, }; -static int32_t GetMaxAllowedSdkVersionForApiList(ApiList api_list) { - SdkCodes sdk = SdkCodes::kVersionNone; +static SdkVersion GetMaxAllowedSdkVersionForApiList(ApiList api_list) { switch (api_list) { case ApiList::kWhitelist: case ApiList::kLightGreylist: - sdk = SdkCodes::kVersionUnlimited; - break; + return SdkVersion::kMax; case ApiList::kDarkGreylist: - sdk = SdkCodes::kVersionO_MR1; - break; + return SdkVersion::kO_MR1; case ApiList::kBlacklist: - sdk = SdkCodes::kVersionNone; - break; + return SdkVersion::kMin; case ApiList::kNoList: LOG(FATAL) << "Unexpected value"; UNREACHABLE(); } - return static_cast(sdk); } MemberSignature::MemberSignature(ArtField* field) { @@ -326,7 +322,8 @@ bool ShouldDenyAccessToMemberImpl(T* member, const bool deny_access = (policy == EnforcementPolicy::kEnabled) && - (runtime->GetTargetSdkVersion() > GetMaxAllowedSdkVersionForApiList(api_list)); + IsSdkVersionSetAndMoreThan(runtime->GetTargetSdkVersion(), + GetMaxAllowedSdkVersionForApiList(api_list)); MemberSignature member_signature(member); diff --git a/runtime/hidden_api.h b/runtime/hidden_api.h index 98414f7a87..32bae1127b 100644 --- a/runtime/hidden_api.h +++ b/runtime/hidden_api.h @@ -117,13 +117,6 @@ class ScopedHiddenApiEnforcementPolicySetting { // Implementation details. DO NOT ACCESS DIRECTLY. namespace detail { -enum class SdkCodes { - kVersionNone = std::numeric_limits::min(), - kVersionUnlimited = std::numeric_limits::max(), - kVersionO_MR1 = 27, - kVersionP = 28, -}; - // Class to encapsulate the signature of a member (ArtField or ArtMethod). This // is used as a helper when matching prefixes, and when logging the signature. class MemberSignature { diff --git a/runtime/hidden_api_test.cc b/runtime/hidden_api_test.cc index 627d9a7e1c..314d878c66 100644 --- a/runtime/hidden_api_test.cc +++ b/runtime/hidden_api_test.cc @@ -16,6 +16,7 @@ #include "hidden_api.h" +#include "base/sdk_version.h" #include "common_runtime_test.h" #include "jni/jni_internal.h" #include "proxy_test.h" @@ -112,14 +113,14 @@ TEST_F(HiddenApiTest, CheckGetActionFromRuntimeFlags) { ASSERT_EQ(ShouldDenyAccess(hiddenapi::ApiList::kBlacklist), false); runtime_->SetHiddenApiEnforcementPolicy(hiddenapi::EnforcementPolicy::kEnabled); - runtime_->SetTargetSdkVersion(static_cast(hiddenapi::detail::SdkCodes::kVersionO_MR1)); + runtime_->SetTargetSdkVersion(static_cast(SdkVersion::kO_MR1)); ASSERT_EQ(ShouldDenyAccess(hiddenapi::ApiList::kWhitelist), false); ASSERT_EQ(ShouldDenyAccess(hiddenapi::ApiList::kLightGreylist), false); ASSERT_EQ(ShouldDenyAccess(hiddenapi::ApiList::kDarkGreylist), false); ASSERT_EQ(ShouldDenyAccess(hiddenapi::ApiList::kBlacklist), true); runtime_->SetHiddenApiEnforcementPolicy(hiddenapi::EnforcementPolicy::kEnabled); - runtime_->SetTargetSdkVersion(static_cast(hiddenapi::detail::SdkCodes::kVersionP)); + runtime_->SetTargetSdkVersion(static_cast(SdkVersion::kP)); ASSERT_EQ(ShouldDenyAccess(hiddenapi::ApiList::kWhitelist), false); ASSERT_EQ(ShouldDenyAccess(hiddenapi::ApiList::kLightGreylist), false); ASSERT_EQ(ShouldDenyAccess(hiddenapi::ApiList::kDarkGreylist), true); diff --git a/runtime/jni/java_vm_ext.cc b/runtime/jni/java_vm_ext.cc index 6769368ee4..a61a48a29b 100644 --- a/runtime/jni/java_vm_ext.cc +++ b/runtime/jni/java_vm_ext.cc @@ -23,6 +23,7 @@ #include "art_method-inl.h" #include "base/dumpable.h" #include "base/mutex-inl.h" +#include "base/sdk_version.h" #include "base/stl_util.h" #include "base/systrace.h" #include "check_jni.h" @@ -1030,7 +1031,7 @@ bool JavaVMExt::LoadNativeLibrary(JNIEnv* env, JNI_OnLoadFn jni_on_load = reinterpret_cast(sym); int version = (*jni_on_load)(this, nullptr); - if (runtime_->GetTargetSdkVersion() != 0 && runtime_->GetTargetSdkVersion() <= 21) { + if (IsSdkVersionSetAndAtMost(runtime_->GetTargetSdkVersion(), SdkVersion::kL)) { // Make sure that sigchain owns SIGSEGV. EnsureFrontOfChain(SIGSEGV); } diff --git a/runtime/native/dalvik_system_VMRuntime.cc b/runtime/native/dalvik_system_VMRuntime.cc index 49b71cd801..e213dc79b8 100644 --- a/runtime/native/dalvik_system_VMRuntime.cc +++ b/runtime/native/dalvik_system_VMRuntime.cc @@ -29,6 +29,7 @@ extern "C" void android_set_application_target_sdk_version(uint32_t version); #include "arch/instruction_set.h" #include "art_method-inl.h" #include "base/enums.h" +#include "base/sdk_version.h" #include "class_linker-inl.h" #include "common_throws.h" #include "debugger.h" @@ -256,12 +257,15 @@ static void VMRuntime_setTargetSdkVersionNative(JNIEnv*, jobject, jint target_sd // where workarounds can be enabled. // Note that targetSdkVersion may be CUR_DEVELOPMENT (10000). // Note that targetSdkVersion may be 0, meaning "current". - Runtime::Current()->SetTargetSdkVersion(target_sdk_version); + uint32_t uint_target_sdk_version = + target_sdk_version <= 0 ? static_cast(SdkVersion::kUnset) + : static_cast(target_sdk_version); + Runtime::Current()->SetTargetSdkVersion(uint_target_sdk_version); #ifdef ART_TARGET_ANDROID // This part is letting libc/dynamic linker know about current app's // target sdk version to enable compatibility workarounds. - android_set_application_target_sdk_version(static_cast(target_sdk_version)); + android_set_application_target_sdk_version(uint_target_sdk_version); #endif } diff --git a/runtime/parsed_options.cc b/runtime/parsed_options.cc index 715792876d..33c85973b3 100644 --- a/runtime/parsed_options.cc +++ b/runtime/parsed_options.cc @@ -323,7 +323,7 @@ std::unique_ptr ParsedOptions::MakeParser(bool ignore_unrecognize .WithValueMap({{"false", false}, {"true", true}}) .IntoKey(M::SlowDebug) .Define("-Xtarget-sdk-version:_") - .WithType() + .WithType() .IntoKey(M::TargetSdkVersion) .Define("-Xhidden-api-checks") .IntoKey(M::HiddenApiChecks) diff --git a/runtime/runtime.cc b/runtime/runtime.cc index ccbc2d92e1..19c1623d1f 100644 --- a/runtime/runtime.cc +++ b/runtime/runtime.cc @@ -61,6 +61,7 @@ #include "base/mutex.h" #include "base/os.h" #include "base/quasi_atomic.h" +#include "base/sdk_version.h" #include "base/stl_util.h" #include "base/systrace.h" #include "base/unix_file/fd_file.h" @@ -253,7 +254,7 @@ Runtime::Runtime() preinitialization_transactions_(), verify_(verifier::VerifyMode::kNone), allow_dex_file_fallback_(true), - target_sdk_version_(kUnsetSdkVersion), + target_sdk_version_(static_cast(SdkVersion::kUnset)), implicit_null_checks_(false), implicit_so_checks_(false), implicit_suspend_checks_(false), diff --git a/runtime/runtime.h b/runtime/runtime.h index f6a5634379..a696c2845e 100644 --- a/runtime/runtime.h +++ b/runtime/runtime.h @@ -583,11 +583,11 @@ class Runtime { return is_running_on_memory_tool_; } - void SetTargetSdkVersion(int32_t version) { + void SetTargetSdkVersion(uint32_t version) { target_sdk_version_ = version; } - int32_t GetTargetSdkVersion() const { + uint32_t GetTargetSdkVersion() const { return target_sdk_version_; } @@ -793,8 +793,6 @@ class Runtime { return jdwp_provider_; } - static constexpr int32_t kUnsetSdkVersion = 0u; - uint32_t GetVerifierLoggingThresholdMs() const { return verifier_logging_threshold_ms_; } @@ -975,7 +973,7 @@ class Runtime { std::vector cpu_abilist_; // Specifies target SDK version to allow workarounds for certain API levels. - int32_t target_sdk_version_; + uint32_t target_sdk_version_; // Implicit checks flags. bool implicit_null_checks_; // NullPointer checks are implicit. diff --git a/runtime/runtime_options.cc b/runtime/runtime_options.cc index f8c680db44..12dab158e5 100644 --- a/runtime/runtime_options.cc +++ b/runtime/runtime_options.cc @@ -18,6 +18,7 @@ #include +#include "base/sdk_version.h" #include "base/utils.h" #include "debugger.h" #include "gc/heap.h" diff --git a/runtime/runtime_options.def b/runtime/runtime_options.def index ae1e08f10b..5cec309453 100644 --- a/runtime/runtime_options.def +++ b/runtime/runtime_options.def @@ -116,7 +116,8 @@ RUNTIME_OPTIONS_KEY (std::vector, \ ImageCompilerOptions) // -Ximage-compiler-option ... RUNTIME_OPTIONS_KEY (verifier::VerifyMode, \ Verify, verifier::VerifyMode::kEnable) -RUNTIME_OPTIONS_KEY (int, TargetSdkVersion, Runtime::kUnsetSdkVersion) +RUNTIME_OPTIONS_KEY (unsigned int, TargetSdkVersion, \ + static_cast(SdkVersion::kUnset)) RUNTIME_OPTIONS_KEY (Unit, HiddenApiChecks) RUNTIME_OPTIONS_KEY (std::string, NativeBridge) RUNTIME_OPTIONS_KEY (unsigned int, ZygoteMaxFailedBoots, 10) diff --git a/runtime/verifier/method_verifier.cc b/runtime/verifier/method_verifier.cc index 7b07389057..0b33a0b3c0 100644 --- a/runtime/verifier/method_verifier.cc +++ b/runtime/verifier/method_verifier.cc @@ -28,6 +28,7 @@ #include "base/indenter.h" #include "base/logging.h" // For VLOG. #include "base/mutex-inl.h" +#include "base/sdk_version.h" #include "base/stl_util.h" #include "base/systrace.h" #include "base/time_utils.h" @@ -3677,9 +3678,10 @@ const RegType& MethodVerifier::ResolveClass(dex::TypeIndex class_idx) { // Note: we do this for unresolved classes to trigger re-verification at runtime. if (C == CheckAccess::kYes && result->IsNonZeroReferenceTypes() && - (api_level_ >= 28u || !result->IsUnresolvedTypes())) { + (IsSdkVersionSetAndAtLeast(api_level_, SdkVersion::kP) || !result->IsUnresolvedTypes())) { const RegType& referrer = GetDeclaringClass(); - if ((api_level_ >= 28u || !referrer.IsUnresolvedTypes()) && !referrer.CanAccess(*result)) { + if ((IsSdkVersionSetAndAtLeast(api_level_, SdkVersion::kP) || !referrer.IsUnresolvedTypes()) && + !referrer.CanAccess(*result)) { Fail(VERIFY_ERROR_ACCESS_CLASS) << "(possibly) illegal class access: '" << referrer << "' -> '" << *result << "'"; } @@ -4562,7 +4564,9 @@ ArtField* MethodVerifier::GetStaticField(int field_idx) { } if (klass_type.IsUnresolvedTypes()) { // Accessibility checks depend on resolved fields. - DCHECK(klass_type.Equals(GetDeclaringClass()) || !failures_.empty() || api_level_ < 28u); + DCHECK(klass_type.Equals(GetDeclaringClass()) || + !failures_.empty() || + IsSdkVersionSetAndLessThan(api_level_, SdkVersion::kP)); return nullptr; // Can't resolve Class so no more to do here, will do checking at runtime. } @@ -4603,7 +4607,9 @@ ArtField* MethodVerifier::GetInstanceField(const RegType& obj_type, int field_id } if (klass_type.IsUnresolvedTypes()) { // Accessibility checks depend on resolved fields. - DCHECK(klass_type.Equals(GetDeclaringClass()) || !failures_.empty() || api_level_ < 28u); + DCHECK(klass_type.Equals(GetDeclaringClass()) || + !failures_.empty() || + IsSdkVersionSetAndLessThan(api_level_, SdkVersion::kP)); return nullptr; // Can't resolve Class so no more to do here } @@ -4739,7 +4745,7 @@ void MethodVerifier::VerifyISFieldAccess(const Instruction* inst, const RegType& DCHECK(!can_load_classes_ || self_->IsExceptionPending()); self_->ClearException(); } - } else if (api_level_ >= 28u) { + } else if (IsSdkVersionSetAndAtLeast(api_level_, SdkVersion::kP)) { // If we don't have the field (it seems we failed resolution) and this is a PUT, we need to // redo verification at runtime as the field may be final, unless the field id shows it's in // the same class. diff --git a/test/674-hiddenapi/hiddenapi.cc b/test/674-hiddenapi/hiddenapi.cc index c4439de550..d11aa579e5 100644 --- a/test/674-hiddenapi/hiddenapi.cc +++ b/test/674-hiddenapi/hiddenapi.cc @@ -14,6 +14,7 @@ * limitations under the License. */ +#include "base/sdk_version.h" #include "class_linker.h" #include "dex/art_dex_file_loader.h" #include "hidden_api.h" @@ -29,7 +30,7 @@ namespace Test674HiddenApi { extern "C" JNIEXPORT void JNICALL Java_Main_init(JNIEnv*, jclass) { Runtime* runtime = Runtime::Current(); runtime->SetHiddenApiEnforcementPolicy(hiddenapi::EnforcementPolicy::kEnabled); - runtime->SetTargetSdkVersion(static_cast(hiddenapi::detail::SdkCodes::kVersionO_MR1)); + runtime->SetTargetSdkVersion(static_cast(SdkVersion::kO_MR1)); runtime->SetDedupeHiddenApiWarnings(false); } -- cgit v1.2.3-59-g8ed1b