8344904: Interned strings in old classes are not stored in CDS archive

Reviewed-by: dholmes, ccheung

Author: iklam

src/hotspot/share/cds/metaspaceShared.cpp

@@ -1014,9 +1014,7 @@ void VM_PopulateDumpSharedSpace::dump_java_heap_objects(GrowableArray<Klass*>* k
     Klass* k = klasses->at(i);
     if (k->is_instance_klass()) {
       InstanceKlass* ik = InstanceKlass::cast(k);
-      if (ik->is_linked()) {
-        ik->constants()->add_dumped_interned_strings();
-      }
+      ik->constants()->add_dumped_interned_strings();
     }
   }
   if (_extra_interned_strings != nullptr) {

src/hotspot/share/oops/constantPool.cpp

@@ -53,6 +53,7 @@
 #include "oops/array.hpp"
 #include "oops/constantPool.inline.hpp"
 #include "oops/cpCache.inline.hpp"
+#include "oops/fieldStreams.inline.hpp"
 #include "oops/instanceKlass.hpp"
 #include "oops/klass.inline.hpp"
 #include "oops/objArrayKlass.hpp"
@@ -61,6 +62,7 @@
 #include "oops/typeArrayOop.inline.hpp"
 #include "prims/jvmtiExport.hpp"
 #include "runtime/atomic.hpp"
+#include "runtime/fieldDescriptor.inline.hpp"
 #include "runtime/handles.inline.hpp"
 #include "runtime/init.hpp"
 #include "runtime/javaCalls.hpp"
@@ -403,18 +405,38 @@ void ConstantPool::find_required_hidden_classes() {
 }
 
 void ConstantPool::add_dumped_interned_strings() {
-  objArrayOop rr = resolved_references();
-  if (rr != nullptr) {
-    int rr_len = rr->length();
-    for (int i = 0; i < rr_len; i++) {
-      oop p = rr->obj_at(i);
-      if (java_lang_String::is_instance(p) &&
-          !ArchiveHeapWriter::is_string_too_large_to_archive(p)) {
-        HeapShared::add_to_dumped_interned_strings(p);
+  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,6 +162,7 @@ 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();

test/hotspot/jtreg/runtime/cds/appcds/sharedStrings/OldClassWithStaticString.jasm

@@ -0,0 +1,49 @@
+/*
+ * Copyright (c) 2024, 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.
+ *
+ */
+
+/*
+
+// Compiled from this source, but we want the version to be 49.0 to use the old verifier.
+
+class OldClassWithStaticString {
+    static final String s = "xxxx123yyyy456";
+    static final String t = "OldClassWithStaticString";
+}
+
+*/
+
+public super class OldClassWithStaticString
+    version 49:0
+{
+    public static final Field s:"Ljava/lang/String;" = String "xxxx123yyyy456";
+    public static final Field t:"Ljava/lang/String;" = String "OldClassWithStaticString";
+
+    Method "<init>":"()V"
+        stack 1 locals 1
+    {
+        aload_0;
+        invokespecial Method java/lang/Object."<init>":"()V";
+        return;
+    }
+}

test/hotspot/jtreg/runtime/cds/appcds/sharedStrings/StaticStringInOldClass.java

@@ -0,0 +1,72 @@
+/*
+ * Copyright (c) 2024, 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
+ * @bug 8344904
+ * @summary make sure all interned strings in old classes are archived.
+ * @requires vm.cds.write.archived.java.heap
+ * @library /test/lib /test/hotspot/jtreg/runtime/cds/appcds
+ * @build OldClassWithStaticString
+ * @build StaticStringInOldClass
+ * @run driver jdk.test.lib.helpers.ClassFileInstaller
+ *                 -jar StaticStringInOldClass.jar StaticStringInOldClass StaticStringInOldClassApp OldClassWithStaticString
+ * @run driver StaticStringInOldClass
+ */
+
+import java.lang.reflect.Field;
+import jdk.test.lib.process.OutputAnalyzer;
+import jdk.test.lib.helpers.ClassFileInstaller;
+
+public class StaticStringInOldClass {
+    static final String appClass = StaticStringInOldClassApp.class.getName();
+    static String[] classes = {
+        appClass,
+        OldClassWithStaticString.class.getName(),
+    };
+
+    public static void main(String[] args) throws Exception {
+        String appJar = ClassFileInstaller.getJarPath("StaticStringInOldClass.jar");
+        OutputAnalyzer output;
+        output = TestCommon.testDump(appJar, TestCommon.list(classes));
+        output = TestCommon.exec(appJar, appClass);
+        TestCommon.checkExec(output, "Hello");
+    }
+}
+
+class StaticStringInOldClassApp {
+    static String a = "xxxx123";
+    public static void main(String args[]) throws Exception {
+        System.out.println("Hello");
+        String x = (a + "yyyy456").intern();
+        Class c = OldClassWithStaticString.class;
+        Field f = c.getField("s");
+        String y = (String)(f.get(null));
+        if (x != y) {
+            throw new RuntimeException("Interned strings not equal: " +
+                                       "\"" + x + "\" @ " + System.identityHashCode(x) + " vs " +
+                                       "\"" + y + "\" @ " + System.identityHashCode(y));
+        }
+    }
+}