Make classes of LOS objects non-movable.
When using Generational Concurrent Copying (CC) collection and no boot
image, there is an unlikely case where the class of an object
allocated the Large-Object Space (LOS) might be moved by a
sticky-bit (minor) collection, potentially creating a stale reference
in the `klass_` field of this large object. To prevent this situation,
make all the classes of objects eligible to allocations in the
LOS (large strings and primitive arrays) non-movable when running
without a boot image.
This is not an issue when running with a boot image, as these classes
are in an image space, which is immune.
Also disable run-test 669-moveable-string-class-equals, as this test
expects the String class to be movable when running without a boot
image.
Test: art/test/testrunner/testrunner.py --no-image
Bug: 67628039
Bug: 12687968
Bug: 68181300
Change-Id: Iefce8e13b4d6df96c31d77b7e53b4ba79b5e641c
diff --git a/runtime/class_linker.cc b/runtime/class_linker.cc
index e19dedc..511d468 100644
--- a/runtime/class_linker.cc
+++ b/runtime/class_linker.cc
@@ -483,9 +483,17 @@
mirror::ObjectArray<mirror::Object>::ClassSize(image_pointer_size_))));
object_array_class->SetComponentType(java_lang_Object.Get());
- // Setup String.
+ // Setup java.lang.String.
+ //
+ // We make this class non-movable for the unlikely case where it were to be
+ // moved by a sticky-bit (minor) collection when using the Generational
+ // Concurrent Copying (CC) collector, potentially creating a stale reference
+ // in the `klass_` field of one of its instances allocated in the Large-Object
+ // Space (LOS) -- see the comment about the dirty card scanning logic in
+ // art::gc::collector::ConcurrentCopying::MarkingPhase.
Handle<mirror::Class> java_lang_String(hs.NewHandle(
- AllocClass(self, java_lang_Class.Get(), mirror::String::ClassSize(image_pointer_size_))));
+ AllocClass</* kMovable */ false>(
+ self, java_lang_Class.Get(), mirror::String::ClassSize(image_pointer_size_))));
java_lang_String->SetStringClass();
mirror::Class::SetStatus(java_lang_String, ClassStatus::kResolved, self);
@@ -528,13 +536,13 @@
// Create int array type for native pointer arrays (for example vtables) on 32-bit archs.
Handle<mirror::Class> int_array_class(hs.NewHandle(
- AllocClass(self, java_lang_Class.Get(), mirror::Array::ClassSize(image_pointer_size_))));
+ AllocPrimitiveArrayClass(self, java_lang_Class.Get())));
int_array_class->SetComponentType(GetClassRoot(ClassRoot::kPrimitiveInt, this));
SetClassRoot(ClassRoot::kIntArrayClass, int_array_class.Get());
// Create long array type for native pointer arrays (for example vtables) on 64-bit archs.
Handle<mirror::Class> long_array_class(hs.NewHandle(
- AllocClass(self, java_lang_Class.Get(), mirror::Array::ClassSize(image_pointer_size_))));
+ AllocPrimitiveArrayClass(self, java_lang_Class.Get())));
long_array_class->SetComponentType(GetClassRoot(ClassRoot::kPrimitiveLong, this));
SetClassRoot(ClassRoot::kLongArrayClass, long_array_class.Get());
@@ -610,20 +618,29 @@
CHECK_EQ(dalvik_system_ClassExt->GetObjectSize(), mirror::ClassExt::InstanceSize());
// Setup the primitive array type classes - can't be done until Object has a vtable.
- SetClassRoot(ClassRoot::kBooleanArrayClass, FindSystemClass(self, "[Z"));
+ AllocAndSetPrimitiveArrayClassRoot(self,
+ java_lang_Class.Get(),
+ ClassRoot::kBooleanArrayClass,
+ ClassRoot::kPrimitiveBoolean,
+ "[Z");
- SetClassRoot(ClassRoot::kByteArrayClass, FindSystemClass(self, "[B"));
+ AllocAndSetPrimitiveArrayClassRoot(
+ self, java_lang_Class.Get(), ClassRoot::kByteArrayClass, ClassRoot::kPrimitiveByte, "[B");
- SetClassRoot(ClassRoot::kCharArrayClass, FindSystemClass(self, "[C"));
+ AllocAndSetPrimitiveArrayClassRoot(
+ self, java_lang_Class.Get(), ClassRoot::kCharArrayClass, ClassRoot::kPrimitiveChar, "[C");
- SetClassRoot(ClassRoot::kShortArrayClass, FindSystemClass(self, "[S"));
+ AllocAndSetPrimitiveArrayClassRoot(
+ self, java_lang_Class.Get(), ClassRoot::kShortArrayClass, ClassRoot::kPrimitiveShort, "[S");
CheckSystemClass(self, int_array_class, "[I");
CheckSystemClass(self, long_array_class, "[J");
- SetClassRoot(ClassRoot::kFloatArrayClass, FindSystemClass(self, "[F"));
+ AllocAndSetPrimitiveArrayClassRoot(
+ self, java_lang_Class.Get(), ClassRoot::kFloatArrayClass, ClassRoot::kPrimitiveFloat, "[F");
- SetClassRoot(ClassRoot::kDoubleArrayClass, FindSystemClass(self, "[D"));
+ AllocAndSetPrimitiveArrayClassRoot(
+ self, java_lang_Class.Get(), ClassRoot::kDoubleArrayClass, ClassRoot::kPrimitiveDouble, "[D");
// Run Class through FindSystemClass. This initializes the dex_cache_ fields and register it
// in class_table_.
@@ -2165,13 +2182,14 @@
return dex_cache;
}
+template <bool kMovable>
ObjPtr<mirror::Class> ClassLinker::AllocClass(Thread* self,
ObjPtr<mirror::Class> java_lang_Class,
uint32_t class_size) {
DCHECK_GE(class_size, sizeof(mirror::Class));
gc::Heap* heap = Runtime::Current()->GetHeap();
mirror::Class::InitializeClassVisitor visitor(class_size);
- ObjPtr<mirror::Object> k = kMovingClasses ?
+ ObjPtr<mirror::Object> k = (kMovingClasses && kMovable) ?
heap->AllocObject<true>(self, java_lang_Class, class_size, visitor) :
heap->AllocNonMovableObject<true>(self, java_lang_Class, class_size, visitor);
if (UNLIKELY(k == nullptr)) {
@@ -2185,6 +2203,18 @@
return AllocClass(self, GetClassRoot<mirror::Class>(this), class_size);
}
+ObjPtr<mirror::Class> ClassLinker::AllocPrimitiveArrayClass(Thread* self,
+ ObjPtr<mirror::Class> java_lang_Class) {
+ // We make this class non-movable for the unlikely case where it were to be
+ // moved by a sticky-bit (minor) collection when using the Generational
+ // Concurrent Copying (CC) collector, potentially creating a stale reference
+ // in the `klass_` field of one of its instances allocated in the Large-Object
+ // Space (LOS) -- see the comment about the dirty card scanning logic in
+ // art::gc::collector::ConcurrentCopying::MarkingPhase.
+ return AllocClass</* kMovable */ false>(
+ self, java_lang_Class, mirror::Array::ClassSize(image_pointer_size_));
+}
+
ObjPtr<mirror::ObjectArray<mirror::StackTraceElement>> ClassLinker::AllocStackTraceElementArray(
Thread* self,
size_t length) {
@@ -3648,10 +3678,22 @@
new_class.Assign(GetClassRoot<mirror::ObjectArray<mirror::Object>>(this));
} else if (strcmp(descriptor, "[Ljava/lang/String;") == 0) {
new_class.Assign(GetClassRoot<mirror::ObjectArray<mirror::String>>(this));
+ } else if (strcmp(descriptor, "[Z") == 0) {
+ new_class.Assign(GetClassRoot<mirror::BooleanArray>(this));
+ } else if (strcmp(descriptor, "[B") == 0) {
+ new_class.Assign(GetClassRoot<mirror::ByteArray>(this));
+ } else if (strcmp(descriptor, "[C") == 0) {
+ new_class.Assign(GetClassRoot<mirror::CharArray>(this));
+ } else if (strcmp(descriptor, "[S") == 0) {
+ new_class.Assign(GetClassRoot<mirror::ShortArray>(this));
} else if (strcmp(descriptor, "[I") == 0) {
new_class.Assign(GetClassRoot<mirror::IntArray>(this));
} else if (strcmp(descriptor, "[J") == 0) {
new_class.Assign(GetClassRoot<mirror::LongArray>(this));
+ } else if (strcmp(descriptor, "[F") == 0) {
+ new_class.Assign(GetClassRoot<mirror::FloatArray>(this));
+ } else if (strcmp(descriptor, "[D") == 0) {
+ new_class.Assign(GetClassRoot<mirror::DoubleArray>(this));
}
}
if (new_class == nullptr) {
@@ -8607,6 +8649,19 @@
class_roots->Set<false>(index, klass);
}
+void ClassLinker::AllocAndSetPrimitiveArrayClassRoot(Thread* self,
+ ObjPtr<mirror::Class> java_lang_Class,
+ ClassRoot primitive_array_class_root,
+ ClassRoot primitive_class_root,
+ const char* descriptor) {
+ StackHandleScope<1> hs(self);
+ Handle<mirror::Class> primitive_array_class(hs.NewHandle(
+ AllocPrimitiveArrayClass(self, java_lang_Class)));
+ primitive_array_class->SetComponentType(GetClassRoot(primitive_class_root, this));
+ SetClassRoot(primitive_array_class_root, primitive_array_class.Get());
+ CheckSystemClass(self, primitive_array_class, descriptor);
+}
+
jobject ClassLinker::CreateWellKnownClassLoader(Thread* self,
const std::vector<const DexFile*>& dex_files,
jclass loader_class,
@@ -8941,7 +8996,7 @@
ifcount * mirror::IfTable::kMax)));
}
-// Instantiate ResolveMethod.
+// Instantiate ClassLinker::ResolveMethod.
template ArtMethod* ClassLinker::ResolveMethod<ClassLinker::ResolveMode::kCheckICCEAndIAE>(
uint32_t method_idx,
Handle<mirror::DexCache> dex_cache,
@@ -8955,4 +9010,14 @@
ArtMethod* referrer,
InvokeType type);
+// Instantiate ClassLinker::AllocClass.
+template ObjPtr<mirror::Class> ClassLinker::AllocClass</* kMovable */ true>(
+ Thread* self,
+ ObjPtr<mirror::Class> java_lang_Class,
+ uint32_t class_size);
+template ObjPtr<mirror::Class> ClassLinker::AllocClass</* kMovable */ false>(
+ Thread* self,
+ ObjPtr<mirror::Class> java_lang_Class,
+ uint32_t class_size);
+
} // namespace art
diff --git a/runtime/class_linker.h b/runtime/class_linker.h
index e4d9c96..efe29d3 100644
--- a/runtime/class_linker.h
+++ b/runtime/class_linker.h
@@ -775,7 +775,11 @@
REQUIRES_SHARED(Locks::mutator_lock_)
REQUIRES(!Locks::dex_lock_, !Roles::uninterruptible_);
- // For early bootstrapping by Init
+ // For early bootstrapping by Init.
+ // If we do not allow moving classes (`art::kMovingClass` is false) or if
+ // parameter `kMovable` is false (or both), the class object is allocated in
+ // the non-moving space.
+ template <bool kMovable = true>
ObjPtr<mirror::Class> AllocClass(Thread* self,
ObjPtr<mirror::Class> java_lang_Class,
uint32_t class_size)
@@ -789,6 +793,12 @@
REQUIRES_SHARED(Locks::mutator_lock_)
REQUIRES(!Roles::uninterruptible_);
+ // Allocate a primitive array class.
+ ObjPtr<mirror::Class> AllocPrimitiveArrayClass(Thread* self,
+ ObjPtr<mirror::Class> java_lang_Class)
+ REQUIRES_SHARED(Locks::mutator_lock_)
+ REQUIRES(!Roles::uninterruptible_);
+
ObjPtr<mirror::DexCache> AllocDexCache(/*out*/ ObjPtr<mirror::String>* out_location,
Thread* self,
const DexFile& dex_file)
@@ -1206,6 +1216,20 @@
void SetClassRoot(ClassRoot class_root, ObjPtr<mirror::Class> klass)
REQUIRES_SHARED(Locks::mutator_lock_);
+ // Allocate primitive array class for primitive with class root
+ // `primitive_class_root`, and associate it to class root
+ // `primitive_array_class_root`.
+ //
+ // Also check this class returned when searching system classes for
+ // `descriptor` matches the allocated class.
+ void AllocAndSetPrimitiveArrayClassRoot(Thread* self,
+ ObjPtr<mirror::Class> java_lang_Class,
+ ClassRoot primitive_array_class_root,
+ ClassRoot primitive_class_root,
+ const char* descriptor)
+ REQUIRES_SHARED(Locks::mutator_lock_)
+ REQUIRES(!Roles::uninterruptible_);
+
// Return the quick generic JNI stub for testing.
const void* GetRuntimeQuickGenericJniStub() const;
diff --git a/runtime/gc/collector/concurrent_copying.cc b/runtime/gc/collector/concurrent_copying.cc
index 0c18767..eede5a5 100644
--- a/runtime/gc/collector/concurrent_copying.cc
+++ b/runtime/gc/collector/concurrent_copying.cc
@@ -905,13 +905,8 @@
// during a minor (young-generation) collection:
// - In the case where we run with a boot image, these classes are part of the image space,
// which is an immune space.
- // - In the case where we run without a boot image, these classes are allocated in the region
- // space (main space), but they are not expected to move during a minor collection (this
- // would only happen if those classes were allocated between a major and a minor
- // collections, which is unlikely -- we don't expect any GC to happen before these
- // fundamental classes are initialized). Note that these classes could move during a major
- // collection though, but this is fine: in that case, the whole heap is traced and the card
- // table logic below is not used.
+ // - In the case where we run without a boot image, these classes are allocated in the
+ // non-moving space (see art::ClassLinker::InitWithoutImage).
Runtime::Current()->GetHeap()->GetCardTable()->Scan<false>(
space->GetMarkBitmap(),
space->Begin(),
diff --git a/test/knownfailures.json b/test/knownfailures.json
index 6004f25..40b699e 100644
--- a/test/knownfailures.json
+++ b/test/knownfailures.json
@@ -1077,5 +1077,10 @@
"tests": ["566-polymorphic-inlining"],
"variant": "jit & debuggable",
"description": ["We do not inline with debuggable."]
+ },
+ {
+ "tests": ["669-moveable-string-class-equals"],
+ "bug": "b/68181300",
+ "description": ["The String class is now always non-movable."]
}
]
