diff --git a/src/hotspot/share/cds/archiveBuilder.hpp b/src/hotspot/share/cds/archiveBuilder.hpp index 39fffc26c37..e42b2f478ec 100644 --- a/src/hotspot/share/cds/archiveBuilder.hpp +++ b/src/hotspot/share/cds/archiveBuilder.hpp @@ -343,10 +343,20 @@ public: template 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 + u4 any_or_null_to_offset_u4(T p) const { + if (p == nullptr) { + return 0; + } else { + return any_to_offset_u4(p); + } + } + template T offset_to_buffered(u4 offset) const { return (T)offset_to_buffered_address(offset); diff --git a/src/hotspot/share/cds/archiveUtils.hpp b/src/hotspot/share/cds/archiveUtils.hpp index ee1135b70fa..723332c40e5 100644 --- a/src/hotspot/share/cds/archiveUtils.hpp +++ b/src/hotspot/share/cds/archiveUtils.hpp @@ -259,16 +259,33 @@ public: static bool has_aot_initialized_mirror(InstanceKlass* src_ik); template static Array* archive_array(GrowableArray* 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 T static from_offset(u4 offset) { + template 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 T static offset_to_archived_address_or_null(u4 offset) { + if (offset == 0) { + return nullptr; + } else { + return offset_to_archived_address(offset); + } + } + // p must be an archived object. Get its offset from SharedBaseAddress - template static u4 to_offset(T p) { + template 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(offset); } + + template static u4 archived_address_or_null_to_offset(T p) { + if (p == nullptr) { + return 0; + } else { + return archived_address_to_offset(p); + } + } }; class HeapRootSegments { diff --git a/src/hotspot/share/cds/lambdaFormInvokers.cpp b/src/hotspot/share/cds/lambdaFormInvokers.cpp index 5455a5e66f2..7765bc26aa3 100644 --- a/src/hotspot/share/cds/lambdaFormInvokers.cpp +++ b/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* line = ArchiveUtils::from_offset*>(offset); + Array* line = ArchiveUtils::offset_to_archived_address*>(offset); char* str = line->adr_at(0); append(str); } diff --git a/src/hotspot/share/cds/lambdaProxyClassDictionary.hpp b/src/hotspot/share/cds/lambdaProxyClassDictionary.hpp index 2a9f87681db..bd163fc551d 100644 --- a/src/hotspot/share/cds/lambdaProxyClassDictionary.hpp +++ b/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), - ArchiveUtils::to_offset(invoked_name), - ArchiveUtils::to_offset(invoked_type), - ArchiveUtils::to_offset(method_type), - ArchiveUtils::to_offset(member_method), - ArchiveUtils::to_offset(instantiated_method_type)); + return RunTimeLambdaProxyClassKey(ArchiveUtils::archived_address_to_offset(caller_ik), + ArchiveUtils::archived_address_to_offset(invoked_name), + ArchiveUtils::archived_address_to_offset(invoked_type), + ArchiveUtils::archived_address_to_offset(method_type), + ArchiveUtils::archived_address_or_null_to_offset(member_method), // could be null + ArchiveUtils::archived_address_to_offset(instantiated_method_type)); } unsigned int hash() const; diff --git a/src/hotspot/share/cds/runTimeClassInfo.cpp b/src/hotspot/share/cds/runTimeClassInfo.cpp index 5ad9c14d13e..0acd89b5bce 100644 --- a/src/hotspot/share/cds/runTimeClassInfo.cpp +++ b/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(_klass_offset); } else { - return ArchiveUtils::from_offset(_klass_offset); + return ArchiveUtils::offset_to_archived_address(_klass_offset); } } diff --git a/src/hotspot/share/cds/runTimeClassInfo.hpp b/src/hotspot/share/cds/runTimeClassInfo.hpp index e3c1ad4f8fe..ca60e11736d 100644 --- a/src/hotspot/share/cds/runTimeClassInfo.hpp +++ b/src/hotspot/share/cds/runTimeClassInfo.hpp @@ -52,15 +52,15 @@ public: struct RTVerifierConstraint { u4 _name; u4 _from_name; - Symbol* name() { return ArchiveUtils::from_offset(_name); } - Symbol* from_name() { return ArchiveUtils::from_offset(_from_name); } + Symbol* name() { return ArchiveUtils::offset_to_archived_address(_name); } + Symbol* from_name() { return ArchiveUtils::offset_to_archived_address(_from_name); } }; struct RTLoaderConstraint { u4 _name; char _loader_type1; char _loader_type2; - Symbol* constraint_name() { return ArchiveUtils::from_offset(_name); } + Symbol* constraint_name() { return ArchiveUtils::offset_to_archived_address(_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 nullptr; - } else { - return ArchiveUtils::from_offset(_nest_host_offset); - } + return ArchiveUtils::offset_to_archived_address_or_null(_nest_host_offset); } RTLoaderConstraint* loader_constraints() { diff --git a/test/hotspot/jtreg/TEST.groups b/test/hotspot/jtreg/TEST.groups index 718ecf610f1..d04ed90ae07 100644 --- a/test/hotspot/jtreg/TEST.groups +++ b/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 \ diff --git a/test/hotspot/jtreg/runtime/cds/appcds/LambdaInvokeVirtual.java b/test/hotspot/jtreg/runtime/cds/appcds/LambdaInvokeVirtual.java new file mode 100644 index 00000000000..8d59d202bbd --- /dev/null +++ b/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 + "\""); + } + } +}