From b890130a66e167404a9a60cf0893a015538778ca Mon Sep 17 00:00:00 2001 From: Mathieu Chartier Date: Fri, 30 Sep 2016 10:27:43 -0700 Subject: [PATCH] Use ObjPtr for ResolvedFieldAccessTest and ResolvedMethodAccessTest Also added LookupResolvedType that is guaranteed to not do thread suspension but deals with multidex since GetResolvedType will return null if the type was resolved in another dex file. Added test. Bug: 31113334 Test: test-art-host CC baker Change-Id: I50493bca7d8ce9760546c3116b717484c62c47a4 --- runtime/class_linker.cc | 26 +++++++++++++++++ runtime/class_linker.h | 8 ++++++ runtime/class_linker_test.cc | 22 +++++++++++++++ runtime/mirror/class-inl.h | 66 +++++++++++++++++--------------------------- runtime/mirror/class.h | 11 +++++--- 5 files changed, 89 insertions(+), 44 deletions(-) diff --git a/runtime/class_linker.cc b/runtime/class_linker.cc index 06a824336..0d3c0127d 100644 --- a/runtime/class_linker.cc +++ b/runtime/class_linker.cc @@ -7638,6 +7638,32 @@ mirror::String* ClassLinker::LookupString(const DexFile& dex_file, return string; } +ObjPtr ClassLinker::LookupResolvedType(const DexFile& dex_file, + uint16_t type_idx, + ObjPtr dex_cache, + ObjPtr class_loader) { + ObjPtr type = dex_cache->GetResolvedType(type_idx); + if (type == nullptr) { + const char* descriptor = dex_file.StringByTypeIdx(type_idx); + DCHECK_NE(*descriptor, '\0') << "descriptor is empty string"; + if (descriptor[1] == '\0') { + // only the descriptors of primitive types should be 1 character long, also avoid class lookup + // for primitive classes that aren't backed by dex files. + type = FindPrimitiveClass(descriptor[0]); + } else { + Thread* const self = Thread::Current(); + DCHECK(self != nullptr); + const size_t hash = ComputeModifiedUtf8Hash(descriptor); + // Find the class in the loaded classes table. + type = LookupClass(self, descriptor, hash, class_loader.Decode()); + } + } + if (type != nullptr || type->IsResolved()) { + return type; + } + return nullptr; +} + mirror::Class* ClassLinker::ResolveType(const DexFile& dex_file, uint16_t type_idx, mirror::Class* referrer) { diff --git a/runtime/class_linker.h b/runtime/class_linker.h index df7fb6115..f69a5767e 100644 --- a/runtime/class_linker.h +++ b/runtime/class_linker.h @@ -278,6 +278,14 @@ class ClassLinker { REQUIRES_SHARED(Locks::mutator_lock_) REQUIRES(!dex_lock_, !Roles::uninterruptible_); + // Look up a resolved type with the given ID from the DexFile. The ClassLoader is used to search + // for the type, since it may be referenced from but not contained within the given DexFile. + ObjPtr LookupResolvedType(const DexFile& dex_file, + uint16_t type_idx, + ObjPtr dex_cache, + ObjPtr class_loader) + REQUIRES_SHARED(Locks::mutator_lock_); + // Resolve a type with the given ID from the DexFile, storing the // result in DexCache. The ClassLoader is used to search for the // type, since it may be referenced from but not contained within diff --git a/runtime/class_linker_test.cc b/runtime/class_linker_test.cc index 3c12b18f9..4a926e761 100644 --- a/runtime/class_linker_test.cc +++ b/runtime/class_linker_test.cc @@ -865,6 +865,28 @@ TEST_F(ClassLinkerTest, FindClass) { AssertNonExistentClass("[[[[LNonExistentClass;"); } +TEST_F(ClassLinkerTest, LookupResolvedType) { + ScopedObjectAccess soa(Thread::Current()); + StackHandleScope<1> hs(soa.Self()); + Handle class_loader( + hs.NewHandle(soa.Decode(LoadDex("MyClass")))); + AssertNonExistentClass("LMyClass;"); + ObjPtr klass = class_linker_->FindClass(soa.Self(), "LMyClass;", class_loader); + uint32_t type_idx = klass->GetClassDef()->class_idx_; + ObjPtr dex_cache = klass->GetDexCache(); + const DexFile& dex_file = klass->GetDexFile(); + EXPECT_EQ(dex_cache->GetResolvedType(type_idx), klass.Decode()); + EXPECT_OBJ_PTR_EQ( + class_linker_->LookupResolvedType(dex_file, type_idx, dex_cache, class_loader.Get()), + klass); + // Zero out the resolved type and make sure LookupResolvedType still finds it. + dex_cache->SetResolvedType(type_idx, nullptr); + EXPECT_TRUE(dex_cache->GetResolvedType(type_idx) == nullptr); + EXPECT_OBJ_PTR_EQ( + class_linker_->LookupResolvedType(dex_file, type_idx, dex_cache, class_loader.Get()), + klass); +} + TEST_F(ClassLinkerTest, LibCore) { ScopedObjectAccess soa(Thread::Current()); ASSERT_TRUE(java_lang_dex_file_ != nullptr); diff --git a/runtime/mirror/class-inl.h b/runtime/mirror/class-inl.h index 8aebd6ed9..3cbd58b77 100644 --- a/runtime/mirror/class-inl.h +++ b/runtime/mirror/class-inl.h @@ -362,36 +362,29 @@ inline bool Class::IsAssignableFromArray(ObjPtr src) { } template -inline bool Class::ResolvedFieldAccessTest(Class* access_to, +inline bool Class::ResolvedFieldAccessTest(ObjPtr access_to, ArtField* field, uint32_t field_idx, - DexCache* dex_cache) { + ObjPtr dex_cache) { DCHECK_EQ(use_referrers_cache, dex_cache == nullptr); if (UNLIKELY(!this->CanAccess(access_to))) { // The referrer class can't access the field's declaring class but may still be able // to access the field if the FieldId specifies an accessible subclass of the declaring // class rather than the declaring class itself. - DexCache* referrer_dex_cache = use_referrers_cache ? this->GetDexCache() : dex_cache; + ObjPtr referrer_dex_cache = use_referrers_cache ? this->GetDexCache() : dex_cache; uint32_t class_idx = referrer_dex_cache->GetDexFile()->GetFieldId(field_idx).class_idx_; // The referenced class has already been resolved with the field, but may not be in the dex - // cache. Using ResolveType here without handles in the caller should be safe since there + // cache. Use LookupResolveType here to search the class table if it is not in the dex cache. // should be no thread suspension due to the class being resolved. - // TODO: Clean this up to use handles in the caller. - Class* dex_access_to; - { - StackHandleScope<2> hs(Thread::Current()); - Handle h_dex_cache(hs.NewHandle(referrer_dex_cache)); - Handle h_class_loader(hs.NewHandle(access_to->GetClassLoader())); - dex_access_to = Runtime::Current()->GetClassLinker()->ResolveType( - *referrer_dex_cache->GetDexFile(), - class_idx, - h_dex_cache, - h_class_loader); - } + ObjPtr dex_access_to = Runtime::Current()->GetClassLinker()->LookupResolvedType( + *referrer_dex_cache->GetDexFile(), + class_idx, + referrer_dex_cache, + access_to->GetClassLoader()); DCHECK(dex_access_to != nullptr); if (UNLIKELY(!this->CanAccess(dex_access_to))) { if (throw_on_failure) { - ThrowIllegalAccessErrorClass(this, dex_access_to); + ThrowIllegalAccessErrorClass(this, dex_access_to.Decode()); } return false; } @@ -406,36 +399,32 @@ inline bool Class::ResolvedFieldAccessTest(Class* access_to, } template -inline bool Class::ResolvedMethodAccessTest(Class* access_to, ArtMethod* method, - uint32_t method_idx, DexCache* dex_cache) { +inline bool Class::ResolvedMethodAccessTest(ObjPtr access_to, + ArtMethod* method, + uint32_t method_idx, + ObjPtr dex_cache) { static_assert(throw_on_failure || throw_invoke_type == kStatic, "Non-default throw invoke type"); DCHECK_EQ(use_referrers_cache, dex_cache == nullptr); if (UNLIKELY(!this->CanAccess(access_to))) { // The referrer class can't access the method's declaring class but may still be able // to access the method if the MethodId specifies an accessible subclass of the declaring // class rather than the declaring class itself. - DexCache* referrer_dex_cache = use_referrers_cache ? this->GetDexCache() : dex_cache; + ObjPtr referrer_dex_cache = use_referrers_cache ? this->GetDexCache() : dex_cache; uint32_t class_idx = referrer_dex_cache->GetDexFile()->GetMethodId(method_idx).class_idx_; // The referenced class has already been resolved with the method, but may not be in the dex - // cache. Using ResolveType here without handles in the caller should be safe since there - // should be no thread suspension due to the class being resolved. - // TODO: Clean this up to use handles in the caller. - Class* dex_access_to; - { - StackHandleScope<2> hs(Thread::Current()); - Handle h_dex_cache(hs.NewHandle(referrer_dex_cache)); - Handle h_class_loader(hs.NewHandle(access_to->GetClassLoader())); - dex_access_to = Runtime::Current()->GetClassLinker()->ResolveType( - *referrer_dex_cache->GetDexFile(), - class_idx, - h_dex_cache, - h_class_loader); - } + // cache. + ObjPtr dex_access_to = Runtime::Current()->GetClassLinker()->LookupResolvedType( + *referrer_dex_cache->GetDexFile(), + class_idx, + referrer_dex_cache, + access_to->GetClassLoader()); DCHECK(dex_access_to != nullptr); if (UNLIKELY(!this->CanAccess(dex_access_to))) { if (throw_on_failure) { - ThrowIllegalAccessErrorClassForMethodDispatch(this, dex_access_to, - method, throw_invoke_type); + ThrowIllegalAccessErrorClassForMethodDispatch(this, + dex_access_to.Decode(), + method, + throw_invoke_type); } return false; } @@ -453,10 +442,7 @@ inline bool Class::CanAccessResolvedField(ObjPtr access_to, ArtField* field, ObjPtr dex_cache, uint32_t field_idx) { - return ResolvedFieldAccessTest(access_to.Decode(), - field, - field_idx, - dex_cache.Decode()); + return ResolvedFieldAccessTest(access_to, field, field_idx, dex_cache); } inline bool Class::CheckResolvedFieldAccess(ObjPtr access_to, diff --git a/runtime/mirror/class.h b/runtime/mirror/class.h index d38f235b8..a0d6f3767 100644 --- a/runtime/mirror/class.h +++ b/runtime/mirror/class.h @@ -1287,14 +1287,17 @@ class MANAGED Class FINAL : public Object { void SetVerifyError(Object* klass) REQUIRES_SHARED(Locks::mutator_lock_); template - bool ResolvedFieldAccessTest(Class* access_to, + bool ResolvedFieldAccessTest(ObjPtr access_to, ArtField* field, uint32_t field_idx, - DexCache* dex_cache) + ObjPtr dex_cache) REQUIRES_SHARED(Locks::mutator_lock_); + template - bool ResolvedMethodAccessTest(Class* access_to, ArtMethod* resolved_method, - uint32_t method_idx, DexCache* dex_cache) + bool ResolvedMethodAccessTest(ObjPtr access_to, + ArtMethod* resolved_method, + uint32_t method_idx, + ObjPtr dex_cache) REQUIRES_SHARED(Locks::mutator_lock_); bool Implements(Class* klass) REQUIRES_SHARED(Locks::mutator_lock_); -- 2.11.0