Revert "Disable kBssEntry LoadString sharpening."
Fix .bss GC root walking by registering the oat file
with the class loader's class table.
Also fix potentially outdated ObjPtr<> use in debug build.
This reverts commit b55fdbb30b3bc4e334c241153b98c0a6ea4a4a2b.
Test: m test-art-host
Bug: 32124939
Change-Id: I0b7e3b93cb53c7b22408aa10a04eaf5582c69ee8
diff --git a/compiler/optimizing/sharpening.cc b/compiler/optimizing/sharpening.cc
index df1b351..c1cfe8d 100644
--- a/compiler/optimizing/sharpening.cc
+++ b/compiler/optimizing/sharpening.cc
@@ -312,8 +312,7 @@
desired_load_kind = HLoadString::LoadKind::kBootImageAddress;
address = reinterpret_cast64<uint64_t>(string);
} else {
- // FIXME: Disabled because of BSS root visiting issues. Bug: 32124939
- // desired_load_kind = HLoadString::LoadKind::kBssEntry;
+ desired_load_kind = HLoadString::LoadKind::kBssEntry;
}
}
}
diff --git a/runtime/class_linker.cc b/runtime/class_linker.cc
index cea8377..cb90d2a 100644
--- a/runtime/class_linker.cc
+++ b/runtime/class_linker.cc
@@ -1283,7 +1283,7 @@
}
// Only add the classes to the class loader after the points where we can return false.
for (size_t i = 0; i < num_dex_caches; i++) {
- ObjPtr<mirror::DexCache> const dex_cache = dex_caches->Get(i);
+ ObjPtr<mirror::DexCache> dex_cache = dex_caches->Get(i);
const DexFile* const dex_file = dex_cache->GetDexFile();
const OatFile::OatDexFile* oat_dex_file = dex_file->GetOatDexFile();
if (oat_dex_file != nullptr && oat_dex_file->GetDexCacheArrays() != nullptr) {
@@ -1391,7 +1391,11 @@
/*allow_failure*/true);
CHECK(existing_dex_cache == nullptr);
StackHandleScope<1> hs3(self);
- RegisterDexFileLocked(*dex_file, hs3.NewHandle(dex_cache));
+ Handle<mirror::DexCache> h_dex_cache = hs3.NewHandle(dex_cache);
+ RegisterDexFileLocked(*dex_file, h_dex_cache);
+ if (kIsDebugBuild) {
+ dex_cache.Assign(h_dex_cache.Get()); // Update dex_cache, used below in debug build.
+ }
}
if (kIsDebugBuild) {
CHECK(new_class_set != nullptr);
@@ -1781,6 +1785,12 @@
<< reinterpret_cast<const void*>(section_end);
}
}
+ if (!oat_file->GetBssGcRoots().empty()) {
+ // Insert oat file to class table for visiting .bss GC roots.
+ class_table->InsertOatFile(oat_file);
+ }
+ } else {
+ DCHECK(oat_file->GetBssGcRoots().empty());
}
if (added_class_table) {
WriterMutexLock mu(self, *Locks::classlinker_classes_lock_);
diff --git a/runtime/class_table.cc b/runtime/class_table.cc
index 97c0abd..b44104e 100644
--- a/runtime/class_table.cc
+++ b/runtime/class_table.cc
@@ -170,14 +170,27 @@
const DexFile* dex_file = ObjPtr<mirror::DexCache>::DownCast(obj)->GetDexFile();
if (dex_file != nullptr && dex_file->GetOatDexFile() != nullptr) {
const OatFile* oat_file = dex_file->GetOatDexFile()->GetOatFile();
- if (!oat_file->GetBssGcRoots().empty() && !ContainsElement(oat_files_, oat_file)) {
- oat_files_.push_back(oat_file);
+ if (!oat_file->GetBssGcRoots().empty()) {
+ InsertOatFileLocked(oat_file); // Ignore return value.
}
}
}
return true;
}
+bool ClassTable::InsertOatFile(const OatFile* oat_file) {
+ WriterMutexLock mu(Thread::Current(), lock_);
+ return InsertOatFileLocked(oat_file);
+}
+
+bool ClassTable::InsertOatFileLocked(const OatFile* oat_file) {
+ if (ContainsElement(oat_files_, oat_file)) {
+ return false;
+ }
+ oat_files_.push_back(oat_file);
+ return true;
+}
+
size_t ClassTable::WriteToMemory(uint8_t* ptr) const {
ReaderMutexLock mu(Thread::Current(), lock_);
ClassSet combined;
diff --git a/runtime/class_table.h b/runtime/class_table.h
index 1344990..bc9eaf4 100644
--- a/runtime/class_table.h
+++ b/runtime/class_table.h
@@ -141,6 +141,11 @@
REQUIRES(!lock_)
REQUIRES_SHARED(Locks::mutator_lock_);
+ // Return true if we inserted the oat file, false if it already exists.
+ bool InsertOatFile(const OatFile* oat_file)
+ REQUIRES(!lock_)
+ REQUIRES_SHARED(Locks::mutator_lock_);
+
// Combines all of the tables into one class set.
size_t WriteToMemory(uint8_t* ptr) const
REQUIRES(!lock_)
@@ -168,6 +173,11 @@
private:
void InsertWithoutLocks(ObjPtr<mirror::Class> klass) NO_THREAD_SAFETY_ANALYSIS;
+ // Return true if we inserted the oat file, false if it already exists.
+ bool InsertOatFileLocked(const OatFile* oat_file)
+ REQUIRES(lock_)
+ REQUIRES_SHARED(Locks::mutator_lock_);
+
// Lock to guard inserting and removing.
mutable ReaderWriterMutex lock_;
// We have a vector to help prevent dirty pages after the zygote forks by calling FreezeSnapshot.
diff --git a/runtime/entrypoints/quick/quick_dexcache_entrypoints.cc b/runtime/entrypoints/quick/quick_dexcache_entrypoints.cc
index 4d47b83..d438418 100644
--- a/runtime/entrypoints/quick/quick_dexcache_entrypoints.cc
+++ b/runtime/entrypoints/quick/quick_dexcache_entrypoints.cc
@@ -75,6 +75,10 @@
!dex_file->GetOatDexFile()->GetOatFile()->GetBssGcRoots().empty()) {
mirror::ClassLoader* class_loader = caller->GetDeclaringClass()->GetClassLoader();
DCHECK(class_loader != nullptr); // We do not use .bss GC roots for boot image.
+ DCHECK(
+ !class_loader->GetClassTable()->InsertOatFile(dex_file->GetOatDexFile()->GetOatFile()))
+ << "Oat file with .bss GC roots was not registered in class table: "
+ << dex_file->GetOatDexFile()->GetOatFile()->GetLocation();
// Note that we emit the barrier before the compiled code stores the string as GC root.
// This is OK as there is no suspend point point in between.
Runtime::Current()->GetHeap()->WriteBarrierEveryFieldOf(class_loader);
diff --git a/test/552-checker-sharpening/src/Main.java b/test/552-checker-sharpening/src/Main.java
index ff6ccd4..3c053cf 100644
--- a/test/552-checker-sharpening/src/Main.java
+++ b/test/552-checker-sharpening/src/Main.java
@@ -284,29 +284,28 @@
/// CHECK-START: java.lang.String Main.$noinline$getNonBootImageString() sharpening (before)
/// CHECK: LoadString load_kind:DexCacheViaMethod
- // FIXME: Disabled because of BSS root visiting issues. Bug: 32124939
- // CHECK-START-X86: java.lang.String Main.$noinline$getNonBootImageString() sharpening (after)
- // CHECK: LoadString load_kind:BssEntry
+ /// CHECK-START-X86: java.lang.String Main.$noinline$getNonBootImageString() sharpening (after)
+ /// CHECK: LoadString load_kind:BssEntry
- // CHECK-START-X86: java.lang.String Main.$noinline$getNonBootImageString() pc_relative_fixups_x86 (after)
- // CHECK-DAG: X86ComputeBaseMethodAddress
- // CHECK-DAG: LoadString load_kind:BssEntry
+ /// CHECK-START-X86: java.lang.String Main.$noinline$getNonBootImageString() pc_relative_fixups_x86 (after)
+ /// CHECK-DAG: X86ComputeBaseMethodAddress
+ /// CHECK-DAG: LoadString load_kind:BssEntry
- // CHECK-START-X86_64: java.lang.String Main.$noinline$getNonBootImageString() sharpening (after)
- // CHECK: LoadString load_kind:BssEntry
+ /// CHECK-START-X86_64: java.lang.String Main.$noinline$getNonBootImageString() sharpening (after)
+ /// CHECK: LoadString load_kind:BssEntry
- // CHECK-START-ARM: java.lang.String Main.$noinline$getNonBootImageString() sharpening (after)
- // CHECK: LoadString load_kind:BssEntry
+ /// CHECK-START-ARM: java.lang.String Main.$noinline$getNonBootImageString() sharpening (after)
+ /// CHECK: LoadString load_kind:BssEntry
- // CHECK-START-ARM64: java.lang.String Main.$noinline$getNonBootImageString() sharpening (after)
- // CHECK: LoadString load_kind:BssEntry
+ /// CHECK-START-ARM64: java.lang.String Main.$noinline$getNonBootImageString() sharpening (after)
+ /// CHECK: LoadString load_kind:BssEntry
- // CHECK-START-MIPS: java.lang.String Main.$noinline$getNonBootImageString() sharpening (after)
- // CHECK: LoadString load_kind:BssEntry
+ /// CHECK-START-MIPS: java.lang.String Main.$noinline$getNonBootImageString() sharpening (after)
+ /// CHECK: LoadString load_kind:BssEntry
- // CHECK-START-MIPS: java.lang.String Main.$noinline$getNonBootImageString() pc_relative_fixups_mips (after)
- // CHECK-DAG: MipsComputeBaseMethodAddress
- // CHECK-DAG: LoadString load_kind:BssEntry
+ /// CHECK-START-MIPS: java.lang.String Main.$noinline$getNonBootImageString() pc_relative_fixups_mips (after)
+ /// CHECK-DAG: MipsComputeBaseMethodAddress
+ /// CHECK-DAG: LoadString load_kind:BssEntry
public static String $noinline$getNonBootImageString() {
// Prevent inlining to avoid the string comparison being optimized away.
