8344824: CDS dump crashes when member_method of a lambda proxy is null

Reviewed-by: ccheung, matsaave

src/hotspot/share/cds/archiveBuilder.hpp

@@ -343,10 +343,20 @@ public:
 
   template <typename T>
   u4 any_to_offset_u4(T p) const {
+    assert(p != nullptr, "must not be null");
     uintx offset = any_to_offset((address)p);
     return to_offset_u4(offset);
   }
 
+  template <typename T>
+  u4 any_or_null_to_offset_u4(T p) const {
+    if (p == nullptr) {
+      return 0;
+    } else {
+      return any_to_offset_u4<T>(p);
+    }
+  }
+
   template <typename T>
   T offset_to_buffered(u4 offset) const {
     return (T)offset_to_buffered_address(offset);

src/hotspot/share/cds/archiveUtils.hpp

@@ -259,16 +259,33 @@ public:
   static bool has_aot_initialized_mirror(InstanceKlass* src_ik);
   template <typename T> static Array<T>* archive_array(GrowableArray<T>* tmp_array);
 
+  // The following functions translate between a u4 offset and an address in the
+  // the range of the mapped CDS archive (e.g., Metaspace::is_in_shared_metaspace()).
+  // Since the first 16 bytes in this range are dummy data (see ArchiveBuilder::reserve_buffer()),
+  // we know that offset 0 never represents a valid object. As a result, an offset of 0
+  // is used to encode a nullptr.
+  //
+  // Use the "archived_address_or_null" variants if a nullptr may be encoded.
+
   // offset must represent an object of type T in the mapped shared space. Return
   // a direct pointer to this object.
-  template <typename T> T static from_offset(u4 offset) {
+  template <typename T> T static offset_to_archived_address(u4 offset) {
+    assert(offset != 0, "sanity");
     T p = (T)(SharedBaseAddress + offset);
     assert(Metaspace::is_in_shared_metaspace(p), "must be");
     return p;
   }
 
+  template <typename T> T static offset_to_archived_address_or_null(u4 offset) {
+    if (offset == 0) {
+      return nullptr;
+    } else {
+      return offset_to_archived_address<T>(offset);
+    }
+  }
+
   // p must be an archived object. Get its offset from SharedBaseAddress
-  template <typename T> static u4 to_offset(T p) {
+  template <typename T> static u4 archived_address_to_offset(T p) {
     uintx pn = (uintx)p;
     uintx base = (uintx)SharedBaseAddress;
     assert(Metaspace::is_in_shared_metaspace(p), "must be");
@@ -277,6 +294,14 @@ public:
     assert(offset <= MAX_SHARED_DELTA, "range check");
     return static_cast<u4>(offset);
   }
+
+  template <typename T> static u4 archived_address_or_null_to_offset(T p) {
+    if (p == nullptr) {
+      return 0;
+    } else {
+      return archived_address_to_offset<T>(p);
+    }
+  }
 };
 
 class HeapRootSegments {

src/hotspot/share/cds/lambdaFormInvokers.cpp

@@ -254,7 +254,7 @@ void LambdaFormInvokers::read_static_archive_invokers() {
   if (_static_archive_invokers != nullptr) {
     for (int i = 0; i < _static_archive_invokers->length(); i++) {
       u4 offset = _static_archive_invokers->at(i);
-      Array<char>* line = ArchiveUtils::from_offset<Array<char>*>(offset);
+      Array<char>* line = ArchiveUtils::offset_to_archived_address<Array<char>*>(offset);
       char* str = line->adr_at(0);
       append(str);
     }

src/hotspot/share/cds/lambdaProxyClassDictionary.hpp

@@ -143,7 +143,7 @@ public:
     u4 invoked_name             = b->any_to_offset_u4(key.invoked_name());
     u4 invoked_type             = b->any_to_offset_u4(key.invoked_type());
     u4 method_type              = b->any_to_offset_u4(key.method_type());
-    u4 member_method            = b->any_to_offset_u4(key.member_method());
+    u4 member_method            = b->any_or_null_to_offset_u4(key.member_method()); // could be null
     u4 instantiated_method_type = b->any_to_offset_u4(key.instantiated_method_type());
 
     return RunTimeLambdaProxyClassKey(caller_ik, invoked_name, invoked_type, method_type,
@@ -158,12 +158,12 @@ public:
                                                      Symbol*        instantiated_method_type) {
     // All parameters must be in shared space, or else you'd get an assert in
     // ArchiveUtils::to_offset().
-    return RunTimeLambdaProxyClassKey(ArchiveUtils::to_offset(caller_ik),
+    return RunTimeLambdaProxyClassKey(ArchiveUtils::archived_address_to_offset(caller_ik),
-                                      ArchiveUtils::to_offset(invoked_name),
+                                      ArchiveUtils::archived_address_to_offset(invoked_name),
-                                      ArchiveUtils::to_offset(invoked_type),
+                                      ArchiveUtils::archived_address_to_offset(invoked_type),
-                                      ArchiveUtils::to_offset(method_type),
+                                      ArchiveUtils::archived_address_to_offset(method_type),
-                                      ArchiveUtils::to_offset(member_method),
+                                      ArchiveUtils::archived_address_or_null_to_offset(member_method), // could be null
-                                      ArchiveUtils::to_offset(instantiated_method_type));
+                                      ArchiveUtils::archived_address_to_offset(instantiated_method_type));
   }
 
   unsigned int hash() const;

src/hotspot/share/cds/runTimeClassInfo.cpp

@@ -79,7 +79,7 @@ InstanceKlass* RunTimeClassInfo::klass() const {
   if (ArchiveBuilder::is_active() && ArchiveBuilder::current()->is_in_buffer_space((address)this)) {
     return ArchiveBuilder::current()->offset_to_buffered<InstanceKlass*>(_klass_offset);
   } else {
-    return ArchiveUtils::from_offset<InstanceKlass*>(_klass_offset);
+    return ArchiveUtils::offset_to_archived_address<InstanceKlass*>(_klass_offset);
   }
 }
 

src/hotspot/share/cds/runTimeClassInfo.hpp

@@ -52,15 +52,15 @@ public:
   struct RTVerifierConstraint {
     u4 _name;
     u4 _from_name;
-    Symbol* name() { return ArchiveUtils::from_offset<Symbol*>(_name); }
+    Symbol* name() { return ArchiveUtils::offset_to_archived_address<Symbol*>(_name); }
-    Symbol* from_name() { return ArchiveUtils::from_offset<Symbol*>(_from_name); }
+    Symbol* from_name() { return ArchiveUtils::offset_to_archived_address<Symbol*>(_from_name); }
   };
 
   struct RTLoaderConstraint {
     u4   _name;
     char _loader_type1;
     char _loader_type2;
-    Symbol* constraint_name() { return ArchiveUtils::from_offset<Symbol*>(_name); }
+    Symbol* constraint_name() { return ArchiveUtils::offset_to_archived_address<Symbol*>(_name); }
   };
   struct RTEnumKlassStaticFields {
     int _num;
@@ -177,11 +177,7 @@ public:
 
   InstanceKlass* nest_host() {
     assert(!ArchiveBuilder::is_active(), "not called when dumping archive");
-    if (_nest_host_offset == 0) {
+    return ArchiveUtils::offset_to_archived_address_or_null<InstanceKlass*>(_nest_host_offset);
-      return nullptr;
-    } else {
-      return ArchiveUtils::from_offset<InstanceKlass*>(_nest_host_offset);
-    }
   }
 
   RTLoaderConstraint* loader_constraints() {

test/hotspot/jtreg/TEST.groups

@@ -462,6 +462,7 @@ hotspot_appcds_dynamic = \
  -runtime/cds/appcds/ExtraSymbols.java \
  -runtime/cds/appcds/LambdaContainsOldInf.java \
  -runtime/cds/appcds/LambdaEagerInit.java \
+ -runtime/cds/appcds/LambdaInvokeVirtual.java \
  -runtime/cds/appcds/LambdaProxyClasslist.java \
  -runtime/cds/appcds/LambdaVerificationFailedDuringDump.java \
  -runtime/cds/appcds/LambdaWithJavaAgent.java \

test/hotspot/jtreg/runtime/cds/appcds/LambdaInvokeVirtual.java

@@ -0,0 +1,108 @@
+/*
+ * Copyright (c) 2021, 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 8344824
+ * @summary CDS dump crashes when member_method of a lambda proxy is null
+ * @requires vm.cds
+ * @library /test/lib
+ * @build LambdaInvokeVirtual
+ * @run driver jdk.test.lib.helpers.ClassFileInstaller -jar LambdaInvokeVirtualApp.jar LambdaInvokeVirtualApp MyFunctionalInterface
+ * @run driver LambdaInvokeVirtual
+ */
+
+import java.lang.invoke.MethodHandle;
+import java.lang.invoke.MethodHandles;
+import java.lang.invoke.MethodType;
+import jdk.test.lib.cds.CDSOptions;
+import jdk.test.lib.cds.CDSTestUtils;
+import jdk.test.lib.helpers.ClassFileInstaller;
+import jdk.test.lib.process.OutputAnalyzer;
+
+public class LambdaInvokeVirtual {
+
+    public static void main(String[] args) throws Exception {
+        String appJar =  ClassFileInstaller.getJarPath("LambdaInvokeVirtualApp.jar");
+        String mainClass = LambdaInvokeVirtualApp.class.getName();
+        String namePrefix = "LambdaInvokeVirtualApp";
+        String classList = namePrefix + ".list";
+        String archiveName = namePrefix + ".jsa";
+
+        // dump class list
+        CDSTestUtils.dumpClassList(classList, "-cp", appJar, mainClass);
+
+        // create archive with the class list
+        CDSOptions opts = (new CDSOptions())
+            .addPrefix("-XX:ExtraSharedClassListFile=" + classList,
+                       "-cp", appJar,
+                       "-Xlog:cds,cds+class=debug")
+            .setArchiveName(archiveName);
+        CDSTestUtils.createArchiveAndCheck(opts);
+
+        // run with archive
+        CDSOptions runOpts = (new CDSOptions())
+            .addPrefix("-cp", appJar)
+            .setArchiveName(archiveName)
+            .setUseVersion(false)
+            .addSuffix(mainClass);
+        OutputAnalyzer output = CDSTestUtils.runWithArchive(runOpts);
+        output.shouldHaveExitValue(0);
+    }
+}
+
+interface MyFunctionalInterface {
+    Object invokeMethodReference(String s, char c1, char c2) throws Throwable;
+}
+
+class LambdaInvokeVirtualApp {
+    private static MethodHandle createMethodHandle() throws NoSuchMethodException, IllegalAccessException {
+        MethodHandles.Lookup lookup = MethodHandles.lookup();
+        MethodType mt = MethodType.methodType(String.class, char.class, char.class);
+        return lookup.findVirtual(String.class, "replace", mt);
+    }
+
+    public static void main(String argv[]) throws Throwable {
+        MethodHandle ms = createMethodHandle();
+        MyFunctionalInterface instance = ms::invoke;
+
+/*
+        The above is compiled into this bytecode. Note that the MethodHandle is of type REF_invokeVirtual
+
+        invokedynamic    InvokeDynamic REF_invokeStatic:Method java/lang/invoke/LambdaMetafactory.metafactory:"(Ljava/lang/invoke/MethodHandles$Lookup;Ljava/lang/String;Ljava/lang/invoke/MethodType;Ljava/lang/invoke/MethodType;Ljava/lang/invoke/MethodHandle;Ljava/lang/invoke/MethodType;)Ljava/lang/invoke/CallSite;":invokeMethodReference:"(Ljava/lang/invoke/MethodHandle;)LMyFunctionalInterface;" {
+            MethodType "(Ljava/lang/String;CC)Ljava/lang/Object;",
+            MethodHandle REF_invokeVirtual:Method java/lang/invoke/MethodHandle.invoke:"(Ljava/lang/String;CC)Ljava/lang/Object;",
+            MethodType "(Ljava/lang/String;CC)Ljava/lang/Object;"
+        };
+
+*/
+
+        Object result = instance.invokeMethodReference("some string to search", 's', 'o');
+        String expected = "oome otring to oearch";
+        if (!result.equals(expected)) {
+            throw new RuntimeException("Expected \"" + expected + "\" but got \"" +
+                                       result + "\"");
+        }
+    }
+}