8362657: Make tables used in AOT assembly phase GC-safe

Reviewed-by: shade, dholmes

Author: iklam

src/hotspot/share/cds/aotMetaspace.cpp

@@ -720,6 +720,7 @@ void VM_PopulateDumpSharedSpace::doit() {
   _map_info->set_cloned_vtables(CppVtables::vtables_serialized_base());
   _map_info->header()->set_class_location_config(cl_config);
 
+  HeapShared::delete_tables_with_raw_oops();
   CDSConfig::set_is_at_aot_safepoint(false);
 }
 

src/hotspot/share/cds/archiveHeapWriter.cpp

@@ -95,6 +95,11 @@ void ArchiveHeapWriter::init() {
   }
 }
 
+void ArchiveHeapWriter::delete_tables_with_raw_oops() {
+  delete _source_objs;
+  _source_objs = nullptr;
+}
+
 void ArchiveHeapWriter::add_source_obj(oop src_obj) {
   _source_objs->append(src_obj);
 }
@@ -145,7 +150,7 @@ oop ArchiveHeapWriter::requested_obj_from_buffer_offset(size_t offset) {
 
 oop ArchiveHeapWriter::source_obj_to_requested_obj(oop src_obj) {
   assert(CDSConfig::is_dumping_heap(), "dump-time only");
-  HeapShared::CachedOopInfo* p = HeapShared::archived_object_cache()->get(src_obj);
+  HeapShared::CachedOopInfo* p = HeapShared::get_cached_oop_info(src_obj);
   if (p != nullptr) {
     return requested_obj_from_buffer_offset(p->buffer_offset());
   } else {
@@ -154,9 +159,9 @@ oop ArchiveHeapWriter::source_obj_to_requested_obj(oop src_obj) {
 }
 
 oop ArchiveHeapWriter::buffered_addr_to_source_obj(address buffered_addr) {
-  oop* p = _buffer_offset_to_source_obj_table->get(buffered_address_to_offset(buffered_addr));
-  if (p != nullptr) {
-    return *p;
+  OopHandle* oh = _buffer_offset_to_source_obj_table->get(buffered_address_to_offset(buffered_addr));
+  if (oh != nullptr) {
+    return oh->resolve();
   } else {
     return nullptr;
   }
@@ -356,12 +361,13 @@ void ArchiveHeapWriter::copy_source_objs_to_buffer(GrowableArrayCHeap<oop, mtCla
   for (int i = 0; i < _source_objs_order->length(); i++) {
     int src_obj_index = _source_objs_order->at(i)._index;
     oop src_obj = _source_objs->at(src_obj_index);
-    HeapShared::CachedOopInfo* info = HeapShared::archived_object_cache()->get(src_obj);
+    HeapShared::CachedOopInfo* info = HeapShared::get_cached_oop_info(src_obj);
     assert(info != nullptr, "must be");
     size_t buffer_offset = copy_one_source_obj_to_buffer(src_obj);
     info->set_buffer_offset(buffer_offset);
 
-    _buffer_offset_to_source_obj_table->put_when_absent(buffer_offset, src_obj);
+    OopHandle handle(Universe::vm_global(), src_obj);
+    _buffer_offset_to_source_obj_table->put_when_absent(buffer_offset, handle);
     _buffer_offset_to_source_obj_table->maybe_grow();
 
     if (java_lang_Module::is_instance(src_obj)) {
@@ -696,7 +702,7 @@ void ArchiveHeapWriter::relocate_embedded_oops(GrowableArrayCHeap<oop, mtClassSh
   for (int i = 0; i < _source_objs_order->length(); i++) {
     int src_obj_index = _source_objs_order->at(i)._index;
     oop src_obj = _source_objs->at(src_obj_index);
-    HeapShared::CachedOopInfo* info = HeapShared::archived_object_cache()->get(src_obj);
+    HeapShared::CachedOopInfo* info = HeapShared::get_cached_oop_info(src_obj);
     assert(info != nullptr, "must be");
     oop requested_obj = requested_obj_from_buffer_offset(info->buffer_offset());
     update_header_for_requested_obj(requested_obj, src_obj, src_obj->klass());
@@ -758,7 +764,7 @@ void ArchiveHeapWriter::compute_ptrmap(ArchiveHeapInfo* heap_info) {
     NativePointerInfo info = _native_pointers->at(i);
     oop src_obj = info._src_obj;
     int field_offset = info._field_offset;
-    HeapShared::CachedOopInfo* p = HeapShared::archived_object_cache()->get(src_obj);
+    HeapShared::CachedOopInfo* p = HeapShared::get_cached_oop_info(src_obj);
     // requested_field_addr = the address of this field in the requested space
     oop requested_obj = requested_obj_from_buffer_offset(p->buffer_offset());
     Metadata** requested_field_addr = (Metadata**)(cast_from_oop<address>(requested_obj) + field_offset);

src/hotspot/share/cds/archiveHeapWriter.hpp

@@ -152,7 +152,7 @@ class ArchiveHeapWriter : AllStatic {
   };
   static GrowableArrayCHeap<HeapObjOrder, mtClassShared>* _source_objs_order;
 
-  typedef ResizeableHashTable<size_t, oop,
+  typedef ResizeableHashTable<size_t, OopHandle,
       AnyObj::C_HEAP,
       mtClassShared> BufferOffsetToSourceObjectTable;
   static BufferOffsetToSourceObjectTable* _buffer_offset_to_source_obj_table;
@@ -227,6 +227,7 @@ class ArchiveHeapWriter : AllStatic {
 
 public:
   static void init() NOT_CDS_JAVA_HEAP_RETURN;
+  static void delete_tables_with_raw_oops();
   static void add_source_obj(oop src_obj);
   static bool is_too_large_to_archive(size_t size);
   static bool is_too_large_to_archive(oop obj);

src/hotspot/share/cds/cdsHeapVerifier.cpp

@@ -36,6 +36,7 @@
 #include "oops/fieldStreams.inline.hpp"
 #include "oops/klass.inline.hpp"
 #include "oops/oop.inline.hpp"
+#include "oops/oopHandle.inline.hpp"
 #include "runtime/fieldDescriptor.inline.hpp"
 
 #if INCLUDE_CDS_JAVA_HEAP
@@ -273,7 +274,8 @@ void CDSHeapVerifier::add_static_obj_field(InstanceKlass* ik, oop field, Symbol*
 
 // This function is called once for every archived heap object. Warn if this object is referenced by
 // a static field of a class that's not aot-initialized.
-inline bool CDSHeapVerifier::do_entry(oop& orig_obj, HeapShared::CachedOopInfo& value) {
+inline bool CDSHeapVerifier::do_entry(OopHandle& orig_obj_handle, HeapShared::CachedOopInfo& value) {
+  oop orig_obj = orig_obj_handle.resolve();
   _archived_objs++;
 
   if (java_lang_String::is_instance(orig_obj) && HeapShared::is_dumped_interned_string(orig_obj)) {
@@ -323,7 +325,7 @@ class CDSHeapVerifier::TraceFields : public FieldClosure {
 
 // Call this function (from gdb, etc) if you want to know why an object is archived.
 void CDSHeapVerifier::trace_to_root(outputStream* st, oop orig_obj) {
-  HeapShared::CachedOopInfo* info = HeapShared::archived_object_cache()->get(orig_obj);
+  HeapShared::CachedOopInfo* info = HeapShared::get_cached_oop_info(orig_obj);
   if (info != nullptr) {
     trace_to_root(st, orig_obj, nullptr, info);
   } else {
@@ -357,7 +359,7 @@ const char* static_field_name(oop mirror, oop field) {
 int CDSHeapVerifier::trace_to_root(outputStream* st, oop orig_obj, oop orig_field, HeapShared::CachedOopInfo* info) {
   int level = 0;
   if (info->orig_referrer() != nullptr) {
-    HeapShared::CachedOopInfo* ref = HeapShared::archived_object_cache()->get(info->orig_referrer());
+    HeapShared::CachedOopInfo* ref = HeapShared::get_cached_oop_info(info->orig_referrer());
     assert(ref != nullptr, "sanity");
     level = trace_to_root(st, info->orig_referrer(), orig_obj, ref) + 1;
   } else if (java_lang_String::is_instance(orig_obj)) {

src/hotspot/share/cds/cdsHeapVerifier.hpp

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2022, 2023, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2022, 2025, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -27,6 +27,7 @@
 
 #include "cds/heapShared.hpp"
 #include "memory/iterator.hpp"
+#include "oops/oopHandle.hpp"
 #include "utilities/growableArray.hpp"
 #include "utilities/hashTable.hpp"
 
@@ -80,7 +81,7 @@ class CDSHeapVerifier : public KlassClosure {
   virtual void do_klass(Klass* k);
 
   // For HashTable::iterate()
-  inline bool do_entry(oop& orig_obj, HeapShared::CachedOopInfo& value);
+  inline bool do_entry(OopHandle& orig_obj, HeapShared::CachedOopInfo& value);
 
   static void verify();
 

src/hotspot/share/cds/heapShared.cpp

@@ -58,6 +58,7 @@
 #include "oops/fieldStreams.inline.hpp"
 #include "oops/objArrayOop.inline.hpp"
 #include "oops/oop.inline.hpp"
+#include "oops/oopHandle.inline.hpp"
 #include "oops/typeArrayOop.inline.hpp"
 #include "prims/jvmtiExport.hpp"
 #include "runtime/arguments.hpp"
@@ -159,12 +160,35 @@ bool HeapShared::is_subgraph_root_class(InstanceKlass* ik) {
          is_subgraph_root_class_of(fmg_archive_subgraph_entry_fields, ik);
 }
 
+oop HeapShared::CachedOopInfo::orig_referrer() const {
+  return _orig_referrer.resolve();
+}
+
 unsigned HeapShared::oop_hash(oop const& p) {
+  assert(SafepointSynchronize::is_at_safepoint() ||
+         JavaThread::current()->is_in_no_safepoint_scope(), "sanity");
   // Do not call p->identity_hash() as that will update the
   // object header.
   return primitive_hash(cast_from_oop<intptr_t>(p));
 }
 
+unsigned int HeapShared::oop_handle_hash_raw(const OopHandle& oh) {
+  return oop_hash(oh.resolve());
+}
+
+unsigned int HeapShared::oop_handle_hash(const OopHandle& oh) {
+  oop o = oh.resolve();
+  if (o == nullptr) {
+    return 0;
+  } else {
+    return o->identity_hash();
+  }
+}
+
+bool HeapShared::oop_handle_equals(const OopHandle& a, const OopHandle& b) {
+  return a.resolve() == b.resolve();
+}
+
 static void reset_states(oop obj, TRAPS) {
   Handle h_obj(THREAD, obj);
   InstanceKlass* klass = InstanceKlass::cast(obj->klass());
@@ -216,7 +240,8 @@ HeapShared::ArchivedObjectCache* HeapShared::_archived_object_cache = nullptr;
 
 bool HeapShared::has_been_archived(oop obj) {
   assert(CDSConfig::is_dumping_heap(), "dump-time only");
-  return archived_object_cache()->get(obj) != nullptr;
+  OopHandle oh(&obj);
+  return archived_object_cache()->get(oh) != nullptr;
 }
 
 int HeapShared::append_root(oop obj) {
@@ -303,7 +328,9 @@ bool HeapShared::archive_object(oop obj, oop referrer, KlassSubGraphInfo* subgra
     count_allocation(obj->size());
     ArchiveHeapWriter::add_source_obj(obj);
     CachedOopInfo info = make_cached_oop_info(obj, referrer);
-    archived_object_cache()->put_when_absent(obj, info);
+
+    OopHandle oh(Universe::vm_global(), obj);
+    archived_object_cache()->put_when_absent(oh, info);
     archived_object_cache()->maybe_grow();
     mark_native_pointers(obj);
 
@@ -636,14 +663,16 @@ void HeapShared::mark_native_pointers(oop orig_obj) {
 }
 
 void HeapShared::get_pointer_info(oop src_obj, bool& has_oop_pointers, bool& has_native_pointers) {
-  CachedOopInfo* info = archived_object_cache()->get(src_obj);
+  OopHandle oh(&src_obj);
+  CachedOopInfo* info = archived_object_cache()->get(oh);
   assert(info != nullptr, "must be");
   has_oop_pointers = info->has_oop_pointers();
   has_native_pointers = info->has_native_pointers();
 }
 
 void HeapShared::set_has_native_pointers(oop src_obj) {
-  CachedOopInfo* info = archived_object_cache()->get(src_obj);
+  OopHandle oh(&src_obj);
+  CachedOopInfo* info = archived_object_cache()->get(oh);
   assert(info != nullptr, "must be");
   info->set_has_native_pointers();
 }
@@ -1453,7 +1482,7 @@ class PointsToOopsChecker : public BasicOopIterateClosure {
 HeapShared::CachedOopInfo HeapShared::make_cached_oop_info(oop obj, oop referrer) {
   PointsToOopsChecker points_to_oops_checker;
   obj->oop_iterate(&points_to_oops_checker);
-  return CachedOopInfo(referrer, points_to_oops_checker.result());
+  return CachedOopInfo(OopHandle(Universe::vm_global(), referrer), points_to_oops_checker.result());
 }
 
 void HeapShared::init_box_classes(TRAPS) {
@@ -2096,6 +2125,18 @@ bool HeapShared::is_dumped_interned_string(oop o) {
   return _dumped_interned_strings->get(o) != nullptr;
 }
 
+// These tables should be used only within the CDS safepoint, so
+// delete them before we exit the safepoint. Otherwise the table will
+// contain bad oops after a GC.
+void HeapShared::delete_tables_with_raw_oops() {
+  assert(_seen_objects_table == nullptr, "should have been deleted");
+
+  delete _dumped_interned_strings;
+  _dumped_interned_strings = nullptr;
+
+  ArchiveHeapWriter::delete_tables_with_raw_oops();
+}
+
 void HeapShared::debug_trace() {
   ResourceMark rm;
   oop referrer = _object_being_archived.referrer();

src/hotspot/share/cds/heapShared.hpp

@@ -167,6 +167,9 @@ class HeapShared: AllStatic {
 public:
   static void debug_trace();
   static unsigned oop_hash(oop const& p);
+  static unsigned oop_handle_hash(OopHandle const& oh);
+  static unsigned oop_handle_hash_raw(OopHandle const& oh);
+  static bool oop_handle_equals(const OopHandle& a, const OopHandle& b);
   static unsigned string_oop_hash(oop const& string) {
     return java_lang_String::hash_code(string);
   }
@@ -175,7 +178,7 @@ class HeapShared: AllStatic {
 
   class CachedOopInfo {
     // Used by CDSHeapVerifier.
-    oop _orig_referrer;
+    OopHandle _orig_referrer;
 
     // The location of this object inside ArchiveHeapWriter::_buffer
     size_t _buffer_offset;
@@ -186,12 +189,12 @@ class HeapShared: AllStatic {
     // One or more fields in this object are pointing to MetaspaceObj
     bool _has_native_pointers;
   public:
-    CachedOopInfo(oop orig_referrer, bool has_oop_pointers)
+    CachedOopInfo(OopHandle orig_referrer, bool has_oop_pointers)
       : _orig_referrer(orig_referrer),
         _buffer_offset(0),
         _has_oop_pointers(has_oop_pointers),
         _has_native_pointers(false) {}
-    oop orig_referrer()             const { return _orig_referrer;   }
+    oop orig_referrer() const;
     void set_buffer_offset(size_t offset) { _buffer_offset = offset; }
     size_t buffer_offset()          const { return _buffer_offset;   }
     bool has_oop_pointers()         const { return _has_oop_pointers; }
@@ -202,10 +205,11 @@ class HeapShared: AllStatic {
 private:
   static const int INITIAL_TABLE_SIZE = 15889; // prime number
   static const int MAX_TABLE_SIZE     = 1000000;
-  typedef ResizeableHashTable<oop, CachedOopInfo,
+  typedef ResizeableHashTable<OopHandle, CachedOopInfo,
       AnyObj::C_HEAP,
       mtClassShared,
-      HeapShared::oop_hash> ArchivedObjectCache;
+      HeapShared::oop_handle_hash_raw,
+      HeapShared::oop_handle_equals> ArchivedObjectCache;
   static ArchivedObjectCache* _archived_object_cache;
 
   class DumpTimeKlassSubGraphInfoTable
@@ -378,6 +382,11 @@ class HeapShared: AllStatic {
     return _archived_object_cache;
   }
 
+  static CachedOopInfo* get_cached_oop_info(oop orig_obj) {
+    OopHandle oh(&orig_obj);
+    return _archived_object_cache->get(oh);
+  }
+
   static int archive_exception_instance(oop exception);
 
   static bool archive_reachable_objects_from(int level,
@@ -435,6 +444,7 @@ class HeapShared: AllStatic {
     CDS_JAVA_HEAP_ONLY(return (idx == AOTMetaspace::hp);)
     NOT_CDS_JAVA_HEAP_RETURN_(false);
   }
+  static void delete_tables_with_raw_oops() NOT_CDS_JAVA_HEAP_RETURN;
 
   static void resolve_classes(JavaThread* current) NOT_CDS_JAVA_HEAP_RETURN;
   static void initialize_from_archived_subgraph(JavaThread* current, Klass* k) NOT_CDS_JAVA_HEAP_RETURN;