8356125: Interned strings are omitted from AOT cache

Reviewed-by: shade, ccheung

src/hotspot/share/cds/aotClassLinker.cpp

@@ -64,8 +64,6 @@ void AOTClassLinker::initialize() {
   }
 
   assert(is_initialized(), "sanity");
-
-  AOTConstantPoolResolver::initialize();
 }
 
 void AOTClassLinker::dispose() {
@@ -79,8 +77,6 @@ void AOTClassLinker::dispose() {
   _sorted_candidates = nullptr;
 
   assert(!is_initialized(), "sanity");
-
-  AOTConstantPoolResolver::dispose();
 }
 
 bool AOTClassLinker::is_vm_class(InstanceKlass* ik) {

src/hotspot/share/cds/aotConstantPoolResolver.cpp

@@ -37,19 +37,6 @@
 #include "oops/klass.inline.hpp"
 #include "runtime/handles.inline.hpp"
 
-AOTConstantPoolResolver::ClassesTable* AOTConstantPoolResolver::_processed_classes = nullptr;
-
-void AOTConstantPoolResolver::initialize() {
-  assert(_processed_classes == nullptr, "must be");
-  _processed_classes = new (mtClass)ClassesTable();
-}
-
-void AOTConstantPoolResolver::dispose() {
-  assert(_processed_classes != nullptr, "must be");
-  delete _processed_classes;
-  _processed_classes = nullptr;
-}
-
 // Returns true if we CAN PROVE that cp_index will always resolve to
 // the same information at both dump time and run time. This is a
 // necessary (but not sufficient) condition for pre-resolving cp_index
@@ -144,17 +131,11 @@ bool AOTConstantPoolResolver::is_class_resolution_deterministic(InstanceKlass* c
   }
 }
 
-void AOTConstantPoolResolver::dumptime_resolve_constants(InstanceKlass* ik, TRAPS) {
+void AOTConstantPoolResolver::preresolve_string_cp_entries(InstanceKlass* ik, TRAPS) {
   if (!ik->is_linked()) {
+    // The cp->resolved_referenced() array is not ready yet, so we can't call resolve_string().
     return;
   }
-  bool first_time;
-  _processed_classes->put_if_absent(ik, &first_time);
-  if (!first_time) {
-    // We have already resolved the constants in class, so no need to do it again.
-    return;
-  }
-
   constantPoolHandle cp(THREAD, ik->constants());
   for (int cp_index = 1; cp_index < cp->length(); cp_index++) { // Index 0 is unused
     switch (cp->tag_at(cp_index).value()) {

src/hotspot/share/cds/aotConstantPoolResolver.hpp

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2022, 2024, 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
@@ -80,17 +80,10 @@ class AOTConstantPoolResolver :  AllStatic {
   static bool check_lambda_metafactory_methodhandle_arg(ConstantPool* cp, int bsms_attribute_index, int arg_i);
 
 public:
-  static void initialize();
-  static void dispose();
-
   static void preresolve_class_cp_entries(JavaThread* current, InstanceKlass* ik, GrowableArray<bool>* preresolve_list);
   static void preresolve_field_and_method_cp_entries(JavaThread* current, InstanceKlass* ik, GrowableArray<bool>* preresolve_list);
   static void preresolve_indy_cp_entries(JavaThread* current, InstanceKlass* ik, GrowableArray<bool>* preresolve_list);
-
-
-  // Resolve all constant pool entries that are safe to be stored in the
-  // CDS archive.
-  static void dumptime_resolve_constants(InstanceKlass* ik, TRAPS);
+  static void preresolve_string_cp_entries(InstanceKlass* ik, TRAPS);
 
   static bool is_resolution_deterministic(ConstantPool* cp, int cp_index);
 };

src/hotspot/share/cds/heapShared.cpp

@@ -606,11 +606,8 @@ static objArrayOop get_archived_resolved_references(InstanceKlass* src_ik) {
 }
 
 void HeapShared::archive_strings() {
-  oop shared_strings_array = StringTable::init_shared_strings_array(_dumped_interned_strings);
+  oop shared_strings_array = StringTable::init_shared_strings_array();
   bool success = archive_reachable_objects_from(1, _dump_time_special_subgraph, shared_strings_array);
-  // We must succeed because:
-  // - _dumped_interned_strings do not contain any large strings.
-  // - StringTable::init_shared_table() doesn't create any large arrays.
   assert(success, "shared strings array must not point to arrays or strings that are too large to archive");
   StringTable::set_shared_strings_array_index(append_root(shared_strings_array));
 }
@@ -683,7 +680,7 @@ void HeapShared::write_heap(ArchiveHeapInfo *heap_info) {
     check_special_subgraph_classes();
   }
 
-  StringTable::write_shared_table(_dumped_interned_strings);
+  StringTable::write_shared_table();
   ArchiveHeapWriter::write(_pending_roots, heap_info);
 
   ArchiveBuilder::OtherROAllocMark mark;
@@ -710,8 +707,6 @@ void HeapShared::scan_java_class(Klass* orig_k) {
       bool success = HeapShared::archive_reachable_objects_from(1, _dump_time_special_subgraph, rr);
       assert(success, "must be");
     }
-
-    orig_ik->constants()->add_dumped_interned_strings();
   }
 }
 
@@ -2067,11 +2062,8 @@ void HeapShared::archive_object_subgraphs(ArchivableStaticFieldInfo fields[],
 #endif
 }
 
-// Not all the strings in the global StringTable are dumped into the archive, because
-// some of those strings may be only referenced by classes that are excluded from
-// the archive. We need to explicitly mark the strings that are:
-//   [1] used by classes that WILL be archived;
-//   [2] included in the SharedArchiveConfigFile.
+// Keep track of the contents of the archived interned string table. This table
+// is used only by CDSHeapVerifier.
 void HeapShared::add_to_dumped_interned_strings(oop string) {
   assert_at_safepoint(); // DumpedInternedStrings uses raw oops
   assert(!ArchiveHeapWriter::is_string_too_large_to_archive(string), "must be");

src/hotspot/share/cds/metaspaceShared.cpp

@@ -646,15 +646,6 @@ void VM_PopulateDumpSharedSpace::doit() {
   // Block concurrent class unloading from changing the _dumptime_table
   MutexLocker ml(DumpTimeTable_lock, Mutex::_no_safepoint_check_flag);
 
-#if INCLUDE_CDS_JAVA_HEAP
-  if (CDSConfig::is_dumping_heap() && _extra_interned_strings != nullptr) {
-    for (int i = 0; i < _extra_interned_strings->length(); i ++) {
-      OopHandle string = _extra_interned_strings->at(i);
-      HeapShared::add_to_dumped_interned_strings(string.resolve());
-    }
-  }
-#endif
-
   _builder.gather_source_objs();
   _builder.reserve_buffer();
 
@@ -777,7 +768,7 @@ void MetaspaceShared::link_shared_classes(TRAPS) {
     // Keep scanning until we have linked no more classes.
   }
 
-  // Resolve constant pool entries -- we don't load any new classes during this stage
+  // Eargerly resolve all string constants in constant pools
   {
     ResourceMark rm(THREAD);
     CollectClassesForLinking collect_classes;
@@ -785,7 +776,7 @@ void MetaspaceShared::link_shared_classes(TRAPS) {
     for (int i = 0; i < mirrors->length(); i++) {
       OopHandle mirror = mirrors->at(i);
       InstanceKlass* ik = InstanceKlass::cast(java_lang_Class::as_Klass(mirror.resolve()));
-      AOTConstantPoolResolver::dumptime_resolve_constants(ik, CHECK);
+      AOTConstantPoolResolver::preresolve_string_cp_entries(ik, CHECK);
     }
   }
 

src/hotspot/share/classfile/stringTable.cpp

@@ -946,6 +946,8 @@ void StringTable::allocate_shared_strings_array(TRAPS) {
   if (!CDSConfig::is_dumping_heap()) {
     return;
   }
+  assert(CDSConfig::allow_only_single_java_thread(), "No more interned strings can be added");
+
   if (_items_count > (size_t)max_jint) {
     fatal("Too many strings to be archived: %zu", _items_count);
   }
@@ -1026,48 +1028,61 @@ void StringTable::verify_secondary_array_index_bits() {
 // For each shared string:
 // [1] Store it into _shared_strings_array. Encode its position as a 32-bit index.
 // [2] Store the index and hashcode into _shared_table.
-oop StringTable::init_shared_strings_array(const DumpedInternedStrings* dumped_interned_strings) {
+oop StringTable::init_shared_strings_array() {
   assert(CDSConfig::is_dumping_heap(), "must be");
   objArrayOop array = (objArrayOop)(_shared_strings_array.resolve());
 
   verify_secondary_array_index_bits();
 
   int index = 0;
-  auto copy_into_array = [&] (oop string, bool value_ignored) {
-    if (!_is_two_dimensional_shared_strings_array) {
-      assert(index < array->length(), "no strings should have been added");
-      array->obj_at_put(index, string);
-    } else {
-      int primary_index = index >> _secondary_array_index_bits;
-      int secondary_index = index & _secondary_array_index_mask;
+  auto copy_into_array = [&] (WeakHandle* val) {
+    oop string = val->peek();
+    if (string != nullptr && !ArchiveHeapWriter::is_string_too_large_to_archive(string)) {
+      // If string is too large, don't put it into the string table.
+      // - If there are no other refernences to it, it won't be stored into the archive,
+      //   so we are all good.
+      // - If there's a referece to it, we will report an error inside HeapShared.cpp and
+      //   dumping will fail.
+      HeapShared::add_to_dumped_interned_strings(string);
+      if (!_is_two_dimensional_shared_strings_array) {
+        assert(index < array->length(), "no strings should have been added");
+        array->obj_at_put(index, string);
+      } else {
+        int primary_index = index >> _secondary_array_index_bits;
+        int secondary_index = index & _secondary_array_index_mask;
 
-      assert(primary_index < array->length(), "no strings should have been added");
-      objArrayOop secondary = (objArrayOop)array->obj_at(primary_index);
+        assert(primary_index < array->length(), "no strings should have been added");
+        objArrayOop secondary = (objArrayOop)array->obj_at(primary_index);
 
-      assert(secondary != nullptr && secondary->is_objArray(), "must be");
-      assert(secondary_index < secondary->length(), "no strings should have been added");
-      secondary->obj_at_put(secondary_index, string);
+        assert(secondary != nullptr && secondary->is_objArray(), "must be");
+        assert(secondary_index < secondary->length(), "no strings should have been added");
+        secondary->obj_at_put(secondary_index, string);
+      }
+      index ++;
     }
-
-    index ++;
+    return true;
   };
-  dumped_interned_strings->iterate_all(copy_into_array);
 
+  _local_table->do_safepoint_scan(copy_into_array);
+  log_info(cds)("Archived %d interned strings", index);
   return array;
-}
+};
 
-void StringTable::write_shared_table(const DumpedInternedStrings* dumped_interned_strings) {
+void StringTable::write_shared_table() {
   _shared_table.reset();
   CompactHashtableWriter writer((int)_items_count, ArchiveBuilder::string_stats());
 
   int index = 0;
-  auto copy_into_shared_table = [&] (oop string, bool value_ignored) {
-    unsigned int hash = java_lang_String::hash_code(string);
-    writer.add(hash, index);
-    index ++;
+  auto copy_into_shared_table = [&] (WeakHandle* val) {
+    oop string = val->peek();
+    if (string != nullptr && !ArchiveHeapWriter::is_string_too_large_to_archive(string)) {
+      unsigned int hash = java_lang_String::hash_code(string);
+      writer.add(hash, index);
+      index ++;
+    }
+    return true;
   };
-  dumped_interned_strings->iterate_all(copy_into_shared_table);
-
+  _local_table->do_safepoint_scan(copy_into_shared_table);
   writer.dump(&_shared_table, "string");
 }
 

src/hotspot/share/classfile/stringTable.hpp

@@ -33,7 +33,6 @@
 #include "utilities/tableStatistics.hpp"
 
 class CompactHashtableWriter;
-class DumpedInternedStrings;
 class JavaThread;
 class SerializeClosure;
 
@@ -147,8 +146,8 @@ private:
   static oop lookup_shared(const jchar* name, int len) NOT_CDS_JAVA_HEAP_RETURN_(nullptr);
   static size_t shared_entry_count() NOT_CDS_JAVA_HEAP_RETURN_(0);
   static void allocate_shared_strings_array(TRAPS) NOT_CDS_JAVA_HEAP_RETURN;
-  static oop init_shared_strings_array(const DumpedInternedStrings* dumped_interned_strings) NOT_CDS_JAVA_HEAP_RETURN_(nullptr);
-  static void write_shared_table(const DumpedInternedStrings* dumped_interned_strings) NOT_CDS_JAVA_HEAP_RETURN;
+  static oop init_shared_strings_array() NOT_CDS_JAVA_HEAP_RETURN_(nullptr);
+  static void write_shared_table() NOT_CDS_JAVA_HEAP_RETURN;
   static void set_shared_strings_array_index(int root_index) NOT_CDS_JAVA_HEAP_RETURN;
   static void serialize_shared_table_header(SerializeClosure* soc) NOT_CDS_JAVA_HEAP_RETURN;
 

src/hotspot/share/oops/constantPool.cpp

@@ -374,40 +374,6 @@ objArrayOop ConstantPool::prepare_resolved_references_for_archiving() {
   }
   return rr;
 }
-
-void ConstantPool::add_dumped_interned_strings() {
-  InstanceKlass* ik = pool_holder();
-  if (!ik->is_linked()) {
-    // resolved_references() doesn't exist yet, so we have no resolved CONSTANT_String entries. However,
-    // some static final fields may have default values that were initialized when the class was parsed.
-    // We need to enter those into the CDS archive strings table.
-    for (JavaFieldStream fs(ik); !fs.done(); fs.next()) {
-      if (fs.access_flags().is_static()) {
-        fieldDescriptor& fd = fs.field_descriptor();
-        if (fd.field_type() == T_OBJECT) {
-          int offset = fd.offset();
-          check_and_add_dumped_interned_string(ik->java_mirror()->obj_field(offset));
-        }
-      }
-    }
-  } else {
-    objArrayOop rr = resolved_references();
-    if (rr != nullptr) {
-      int rr_len = rr->length();
-      for (int i = 0; i < rr_len; i++) {
-        check_and_add_dumped_interned_string(rr->obj_at(i));
-      }
-    }
-  }
-}
-
-void ConstantPool::check_and_add_dumped_interned_string(oop obj) {
-  if (obj != nullptr && java_lang_String::is_instance(obj) &&
-      !ArchiveHeapWriter::is_string_too_large_to_archive(obj)) {
-    HeapShared::add_to_dumped_interned_strings(obj);
-  }
-}
-
 #endif
 
 #if INCLUDE_CDS

src/hotspot/share/oops/constantPool.hpp

@@ -162,7 +162,6 @@ class ConstantPool : public Metadata {
     assert(is_within_bounds(cp_index), "index out of bounds");
     return (jdouble*) &base()[cp_index];
   }
-  static void check_and_add_dumped_interned_string(oop obj);
 
   ConstantPool(Array<u1>* tags);
   ConstantPool();
@@ -684,7 +683,6 @@ class ConstantPool : public Metadata {
 #if INCLUDE_CDS
   // CDS support
   objArrayOop prepare_resolved_references_for_archiving() NOT_CDS_JAVA_HEAP_RETURN_(nullptr);
-  void add_dumped_interned_strings() NOT_CDS_JAVA_HEAP_RETURN;
   void remove_unshareable_info();
   void restore_unshareable_info(TRAPS);
 private:

test/hotspot/jtreg/runtime/cds/appcds/aotClassLinking/GeneratedInternedString.java

@@ -0,0 +1,72 @@
+/*
+ * Copyright (c) 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
+ * under the terms of the GNU General Public License version 2 only, as
+ * published by the Free Software Foundation.
+ *
+ * This code is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
+ * version 2 for more details (a copy is included in the LICENSE file that
+ * accompanied this code).
+ *
+ * You should have received a copy of the GNU General Public License version
+ * 2 along with this work; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
+ * or visit www.oracle.com if you need additional information or have any
+ * questions.
+ *
+ */
+
+/*
+ * @test storing a dynamically generated interned string in the AOT cache
+ * @bug 8356125
+ * @requires vm.cds.write.archived.java.heap
+ * @requires vm.cds.supports.aot.class.linking
+ * @requires vm.debug
+ * @comment work around JDK-8345635
+ * @requires !vm.jvmci.enabled
+ * @library /test/jdk/lib/testlibrary /test/lib /test/hotspot/jtreg/runtime/cds/appcds
+ * @build GeneratedInternedString
+ * @run driver jdk.test.lib.helpers.ClassFileInstaller -jar app.jar GeneratedInternedStringApp
+ * @run driver GeneratedInternedString
+ */
+
+import jdk.test.lib.cds.SimpleCDSAppTester;
+
+public class GeneratedInternedString {
+    public static void main(String... args) throws Exception {
+        SimpleCDSAppTester.of("GeneratedInternedString")
+            .addVmArgs("-XX:AOTInitTestClass=GeneratedInternedStringApp")
+            .classpath("app.jar")
+            .appCommandLine("GeneratedInternedStringApp")
+            .runAOTWorkflow();
+    }
+}
+
+// This class is cached in the AOT-initialized state. At the beginning of the production
+// run, all of the static fields in GeneratedInternedStringApp will retain their values
+// at the end of the assembly phase. GeneratedInternedStringApp::<clinit> is NOT executed in the
+// production run.
+class GeneratedInternedStringApp {
+    static volatile int n = 0;
+    static final String generatedInternedString = generate();
+
+    public static void main(String args[]) {
+        n = args.length;
+        String b = generate();
+        if (generatedInternedString != b) {
+            throw new RuntimeException("generatedInternedString: " + System.identityHashCode(generatedInternedString)
+                                       + " vs b:" + System.identityHashCode(b));
+        }
+    }
+
+    static String generate() {
+        System.out.println("generate() is called");
+        return ("GeneratedInternedStringApp_String" + n).intern();
+    }
+}

test/hotspot/jtreg/runtime/cds/appcds/aotClassLinking/NonFinalStaticWithInitVal.java

@@ -0,0 +1,79 @@
+/*
+ * Copyright (c) 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
+ * under the terms of the GNU General Public License version 2 only, as
+ * published by the Free Software Foundation.
+ *
+ * This code is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
+ * version 2 for more details (a copy is included in the LICENSE file that
+ * accompanied this code).
+ *
+ * You should have received a copy of the GNU General Public License version
+ * 2 along with this work; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
+ * or visit www.oracle.com if you need additional information or have any
+ * questions.
+ *
+ */
+
+/*
+ * @test
+ * @summary Handling of non-final static string that has an initial value
+ * @bug 8356125
+ * @requires vm.cds.supports.aot.class.linking
+ * @comment work around JDK-8345635
+ * @requires !vm.jvmci.enabled
+ * @library /test/lib
+ * @build NonFinalStaticWithInitVal_Helper
+ * @build NonFinalStaticWithInitVal
+ * @run driver jdk.test.lib.helpers.ClassFileInstaller -jar app.jar MyTestApp NonFinalStaticWithInitVal_Helper
+ * @run driver NonFinalStaticWithInitVal AOT
+ */
+
+import jdk.test.lib.cds.SimpleCDSAppTester;
+import jdk.test.lib.helpers.ClassFileInstaller;
+import jdk.test.lib.process.OutputAnalyzer;
+
+public class NonFinalStaticWithInitVal {
+    static final String appJar = ClassFileInstaller.getJarPath("app.jar");
+    static final String mainClass = "MyTestApp";
+
+    public static void main(String[] args) throws Exception {
+        for (int i = 0; i < 2; i++) {
+            SimpleCDSAppTester.of("NonFinalStaticWithInitVal")
+                .addVmArgs("-XX:" + (i == 0 ? "-" : "+") + "AOTClassLinking",
+                           "-Xlog:cds")
+            .classpath("app.jar")
+            .appCommandLine("MyTestApp")
+            .setProductionChecker((OutputAnalyzer out) -> {
+                    out.shouldContain("field = Dummy 12345678");
+                })
+            .runStaticWorkflow()
+            .runAOTWorkflow();
+        }
+    }
+}
+
+class MyTestApp {
+    volatile static int x = 0;
+
+    public static void main(String args[]) throws Exception {
+        StringBuilder sb = new StringBuilder();
+        sb.append("Dummy ");
+        sb.append("1234567");
+        sb.append(8 + x);
+        String myValue = sb.toString().intern();
+        String theirValue = NonFinalStaticWithInitVal_Helper.foo;
+        System.out.println("field = " + theirValue);
+        if (myValue != theirValue) {
+            // String literals from different class files must be interned.
+            throw new RuntimeException("Interned strings do not match");
+        }
+    }
+}

test/hotspot/jtreg/runtime/cds/appcds/aotClassLinking/NonFinalStaticWithInitVal_Helper.jasm

@@ -0,0 +1,38 @@
+/*
+ * Copyright (c) 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
+ * under the terms of the GNU General Public License version 2 only, as
+ * published by the Free Software Foundation.
+ *
+ * This code is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
+ * version 2 for more details (a copy is included in the LICENSE file that
+ * accompanied this code).
+ *
+ * You should have received a copy of the GNU General Public License version
+ * 2 along with this work; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
+ * or visit www.oracle.com if you need additional information or have any
+ * questions.
+ *
+ */
+
+/*
+
+public class NonFinalStaticWithInitVal_Helper {
+    static String foo = String "Dummy 12345678";
+}
+
+*/
+
+public class NonFinalStaticWithInitVal_Helper
+    version 51:0
+{
+    static Field foo:"Ljava/lang/String;" = String "Dummy 12345678";
+}
+