Fix inserting classes to initiating loader's class table.
Fix two cases that were missed in
https://android-review.googlesource.com/312285
First, copy all class references to app image class table,
including boot image classes where the class loader used for
AOT is only the initiating loader, not the defining loader.
Second, add array classes to the initiating loader's class
table.
Without these fixes, ClassLinker::LookupResolvedType() was
actually relying on the type being in the dex cache because
in some cases the slow path would not be able to find it.
Add a test for ClassLinker::LookupResolvedType() with an
array type and fix that function to avoid null pointer
dereference.
Test: m test-art-host
Bug: 30627598
Change-Id: I7cb14788700e6a22d16c364f8a35e2b6b3d954e4
diff --git a/runtime/class_linker.cc b/runtime/class_linker.cc
index 49cffed..976c2bb 100644
--- a/runtime/class_linker.cc
+++ b/runtime/class_linker.cc
@@ -1397,7 +1397,11 @@
class_loader_(class_loader) {}
bool operator()(ObjPtr<mirror::Class> klass) const REQUIRES_SHARED(Locks::mutator_lock_) {
- klass->SetClassLoader(class_loader_);
+ // Do not update class loader for boot image classes where the app image
+ // class loader is only the initiating loader but not the defining loader.
+ if (klass->GetClassLoader() != nullptr) {
+ klass->SetClassLoader(class_loader_);
+ }
return true;
}
@@ -2457,10 +2461,8 @@
return EnsureResolved(self, descriptor, klass);
}
// Class is not yet loaded.
- if (descriptor[0] == '[') {
- return CreateArrayClass(self, descriptor, hash, class_loader);
- } else if (class_loader.Get() == nullptr) {
- // The boot class loader, search the boot class path.
+ if (descriptor[0] != '[' && class_loader.Get() == nullptr) {
+ // Non-array class and the boot class loader, search the boot class path.
ClassPathEntry pair = FindInClassPath(descriptor, hash, boot_class_path_);
if (pair.second != nullptr) {
return DefineClass(self,
@@ -2473,14 +2475,21 @@
// The boot class loader is searched ahead of the application class loader, failures are
// expected and will be wrapped in a ClassNotFoundException. Use the pre-allocated error to
// trigger the chaining with a proper stack trace.
- ObjPtr<mirror::Throwable> pre_allocated = Runtime::Current()->GetPreAllocatedNoClassDefFoundError();
+ ObjPtr<mirror::Throwable> pre_allocated =
+ Runtime::Current()->GetPreAllocatedNoClassDefFoundError();
self->SetException(pre_allocated);
return nullptr;
}
+ }
+ ObjPtr<mirror::Class> result_ptr;
+ bool descriptor_equals;
+ if (descriptor[0] == '[') {
+ result_ptr = CreateArrayClass(self, descriptor, hash, class_loader);
+ DCHECK_EQ(result_ptr == nullptr, self->IsExceptionPending());
+ DCHECK(result_ptr == nullptr || result_ptr->DescriptorEquals(descriptor));
+ descriptor_equals = true;
} else {
ScopedObjectAccessUnchecked soa(self);
- ObjPtr<mirror::Class> result_ptr;
- bool descriptor_equals;
bool known_hierarchy =
FindClassInBaseDexClassLoader(soa, self, descriptor, hash, class_loader, &result_ptr);
if (result_ptr != nullptr) {
@@ -2524,16 +2533,7 @@
WellKnownClasses::java_lang_ClassLoader_loadClass,
class_name_object.get()));
}
- if (self->IsExceptionPending()) {
- // If the ClassLoader threw, pass that exception up.
- // However, to comply with the RI behavior, first check if another thread succeeded.
- result_ptr = LookupClass(self, descriptor, hash, class_loader.Get());
- if (result_ptr != nullptr && !result_ptr->IsErroneous()) {
- self->ClearException();
- return EnsureResolved(self, descriptor, result_ptr);
- }
- return nullptr;
- } else if (result.get() == nullptr) {
+ if (result.get() == nullptr && !self->IsExceptionPending()) {
// broken loader - throw NPE to be compatible with Dalvik
ThrowNullPointerException(StringPrintf("ClassLoader.loadClass returned null for %s",
class_name_string.c_str()).c_str());
@@ -2541,50 +2541,60 @@
}
result_ptr = soa.Decode<mirror::Class>(result.get());
// Check the name of the returned class.
- descriptor_equals = result_ptr->DescriptorEquals(descriptor);
+ descriptor_equals = (result_ptr != nullptr) && result_ptr->DescriptorEquals(descriptor);
}
-
- // Try to insert the class to the class table, checking for mismatch.
- ObjPtr<mirror::Class> old;
- {
- WriterMutexLock mu(self, *Locks::classlinker_classes_lock_);
- ClassTable* const class_table = InsertClassTableForClassLoader(class_loader.Get());
- old = class_table->Lookup(descriptor, hash);
- if (old == nullptr) {
- old = result_ptr; // For the comparison below, after releasing the lock.
- if (descriptor_equals) {
- class_table->InsertWithHash(result_ptr.Ptr(), hash);
- Runtime::Current()->GetHeap()->WriteBarrierEveryFieldOf(class_loader.Get());
- } // else throw below, after releasing the lock.
- }
- }
- if (UNLIKELY(old != result_ptr)) {
- // Return `old` (even if `!descriptor_equals`) to mimic the RI behavior for parallel
- // capable class loaders. (All class loaders are considered parallel capable on Android.)
- mirror::Class* loader_class = class_loader->GetClass();
- const char* loader_class_name =
- loader_class->GetDexFile().StringByTypeIdx(loader_class->GetDexTypeIndex());
- LOG(WARNING) << "Initiating class loader of type " << DescriptorToDot(loader_class_name)
- << " is not well-behaved; it returned a different Class for racing loadClass(\""
- << DescriptorToDot(descriptor) << "\").";
- return EnsureResolved(self, descriptor, old);
- }
- if (UNLIKELY(!descriptor_equals)) {
- std::string result_storage;
- const char* result_name = result_ptr->GetDescriptor(&result_storage);
- std::string loader_storage;
- const char* loader_class_name = class_loader->GetClass()->GetDescriptor(&loader_storage);
- ThrowNoClassDefFoundError(
- "Initiating class loader of type %s returned class %s instead of %s.",
- DescriptorToDot(loader_class_name).c_str(),
- DescriptorToDot(result_name).c_str(),
- DescriptorToDot(descriptor).c_str());
- return nullptr;
- }
- // success, return mirror::Class*
- return result_ptr.Ptr();
}
- UNREACHABLE();
+
+ if (self->IsExceptionPending()) {
+ // If the ClassLoader threw or array class allocation failed, pass that exception up.
+ // However, to comply with the RI behavior, first check if another thread succeeded.
+ result_ptr = LookupClass(self, descriptor, hash, class_loader.Get());
+ if (result_ptr != nullptr && !result_ptr->IsErroneous()) {
+ self->ClearException();
+ return EnsureResolved(self, descriptor, result_ptr);
+ }
+ return nullptr;
+ }
+
+ // Try to insert the class to the class table, checking for mismatch.
+ ObjPtr<mirror::Class> old;
+ {
+ WriterMutexLock mu(self, *Locks::classlinker_classes_lock_);
+ ClassTable* const class_table = InsertClassTableForClassLoader(class_loader.Get());
+ old = class_table->Lookup(descriptor, hash);
+ if (old == nullptr) {
+ old = result_ptr; // For the comparison below, after releasing the lock.
+ if (descriptor_equals) {
+ class_table->InsertWithHash(result_ptr.Ptr(), hash);
+ Runtime::Current()->GetHeap()->WriteBarrierEveryFieldOf(class_loader.Get());
+ } // else throw below, after releasing the lock.
+ }
+ }
+ if (UNLIKELY(old != result_ptr)) {
+ // Return `old` (even if `!descriptor_equals`) to mimic the RI behavior for parallel
+ // capable class loaders. (All class loaders are considered parallel capable on Android.)
+ mirror::Class* loader_class = class_loader->GetClass();
+ const char* loader_class_name =
+ loader_class->GetDexFile().StringByTypeIdx(loader_class->GetDexTypeIndex());
+ LOG(WARNING) << "Initiating class loader of type " << DescriptorToDot(loader_class_name)
+ << " is not well-behaved; it returned a different Class for racing loadClass(\""
+ << DescriptorToDot(descriptor) << "\").";
+ return EnsureResolved(self, descriptor, old);
+ }
+ if (UNLIKELY(!descriptor_equals)) {
+ std::string result_storage;
+ const char* result_name = result_ptr->GetDescriptor(&result_storage);
+ std::string loader_storage;
+ const char* loader_class_name = class_loader->GetClass()->GetDescriptor(&loader_storage);
+ ThrowNoClassDefFoundError(
+ "Initiating class loader of type %s returned class %s instead of %s.",
+ DescriptorToDot(loader_class_name).c_str(),
+ DescriptorToDot(result_name).c_str(),
+ DescriptorToDot(descriptor).c_str());
+ return nullptr;
+ }
+ // success, return mirror::Class*
+ return result_ptr.Ptr();
}
mirror::Class* ClassLinker::DefineClass(Thread* self,
@@ -3488,7 +3498,8 @@
// class to the hash table --- necessary because of possible races with
// other threads.)
if (class_loader.Get() != component_type->GetClassLoader()) {
- ObjPtr<mirror::Class> new_class = LookupClass(self, descriptor, hash, component_type->GetClassLoader());
+ ObjPtr<mirror::Class> new_class =
+ LookupClass(self, descriptor, hash, component_type->GetClassLoader());
if (new_class != nullptr) {
return new_class.Ptr();
}
@@ -7706,7 +7717,7 @@
type = LookupClass(self, descriptor, hash, class_loader.Ptr());
}
}
- if (type != nullptr || type->IsResolved()) {
+ if (type != nullptr && type->IsResolved()) {
return type.Ptr();
}
return nullptr;
diff --git a/runtime/class_linker_test.cc b/runtime/class_linker_test.cc
index 0341c64..795d5db 100644
--- a/runtime/class_linker_test.cc
+++ b/runtime/class_linker_test.cc
@@ -906,6 +906,41 @@
klass);
}
+TEST_F(ClassLinkerTest, LookupResolvedTypeArray) {
+ ScopedObjectAccess soa(Thread::Current());
+ StackHandleScope<2> hs(soa.Self());
+ Handle<mirror::ClassLoader> class_loader(
+ hs.NewHandle(soa.Decode<mirror::ClassLoader>(LoadDex("AllFields"))));
+ // Get the AllFields class for the dex cache and dex file.
+ ObjPtr<mirror::Class> all_fields_klass
+ = class_linker_->FindClass(soa.Self(), "LAllFields;", class_loader);
+ ASSERT_OBJ_PTR_NE(all_fields_klass, ObjPtr<mirror::Class>(nullptr));
+ Handle<mirror::DexCache> dex_cache = hs.NewHandle(all_fields_klass->GetDexCache());
+ const DexFile& dex_file = *dex_cache->GetDexFile();
+ // Get the index of the array class we want to test.
+ const DexFile::TypeId* array_id = dex_file.FindTypeId("[Ljava/lang/Object;");
+ ASSERT_TRUE(array_id != nullptr);
+ dex::TypeIndex array_idx = dex_file.GetIndexForTypeId(*array_id);
+ // Check that the array class wasn't resolved yet.
+ EXPECT_OBJ_PTR_EQ(
+ class_linker_->LookupResolvedType(dex_file, array_idx, dex_cache.Get(), class_loader.Get()),
+ ObjPtr<mirror::Class>(nullptr));
+ // Resolve the array class we want to test.
+ ObjPtr<mirror::Class> array_klass
+ = class_linker_->FindClass(soa.Self(), "[Ljava/lang/Object;", class_loader);
+ ASSERT_OBJ_PTR_NE(array_klass, ObjPtr<mirror::Class>(nullptr));
+ // Test that LookupResolvedType() finds the array class.
+ EXPECT_OBJ_PTR_EQ(
+ class_linker_->LookupResolvedType(dex_file, array_idx, dex_cache.Get(), class_loader.Get()),
+ array_klass);
+ // Zero out the resolved type and make sure LookupResolvedType() still finds it.
+ dex_cache->SetResolvedType(array_idx, nullptr);
+ EXPECT_TRUE(dex_cache->GetResolvedType(array_idx) == nullptr);
+ EXPECT_OBJ_PTR_EQ(
+ class_linker_->LookupResolvedType(dex_file, array_idx, dex_cache.Get(), class_loader.Get()),
+ array_klass);
+}
+
TEST_F(ClassLinkerTest, LibCore) {
ScopedObjectAccess soa(Thread::Current());
ASSERT_TRUE(java_lang_dex_file_ != nullptr);
diff --git a/runtime/class_table.cc b/runtime/class_table.cc
index 0f985c6..ff846a7 100644
--- a/runtime/class_table.cc
+++ b/runtime/class_table.cc
@@ -129,6 +129,19 @@
classes_.back().InsertWithHash(TableSlot(klass, hash), hash);
}
+void ClassTable::CopyWithoutLocks(const ClassTable& source_table) {
+ if (kIsDebugBuild) {
+ for (ClassSet& class_set : classes_) {
+ CHECK(class_set.Empty());
+ }
+ }
+ for (const ClassSet& class_set : source_table.classes_) {
+ for (const TableSlot& slot : class_set) {
+ classes_.back().Insert(slot);
+ }
+ }
+}
+
void ClassTable::InsertWithoutLocks(ObjPtr<mirror::Class> klass) {
const uint32_t hash = TableSlot::HashDescriptor(klass);
classes_.back().InsertWithHash(TableSlot(klass, hash), hash);
diff --git a/runtime/class_table.h b/runtime/class_table.h
index f27d809..c8ec28e 100644
--- a/runtime/class_table.h
+++ b/runtime/class_table.h
@@ -240,6 +240,7 @@
}
private:
+ void CopyWithoutLocks(const ClassTable& source_table) NO_THREAD_SAFETY_ANALYSIS;
void InsertWithoutLocks(ObjPtr<mirror::Class> klass) NO_THREAD_SAFETY_ANALYSIS;
size_t CountDefiningLoaderClasses(ObjPtr<mirror::ClassLoader> defining_loader,