diff --git a/src/hotspot/share/classfile/systemDictionaryShared.cpp b/src/hotspot/share/classfile/systemDictionaryShared.cpp index 5c4ee3f9452..1f4bf80fbd6 100644 --- a/src/hotspot/share/classfile/systemDictionaryShared.cpp +++ b/src/hotspot/share/classfile/systemDictionaryShared.cpp @@ -752,29 +752,43 @@ void SystemDictionaryShared::dumptime_classes_do(MetaspaceClosure* it) { } } -bool SystemDictionaryShared::add_verification_constraint(InstanceKlass* k, Symbol* name, - Symbol* from_name, bool from_field_is_protected, bool from_is_array, bool from_is_object) { +// Called from VerificationType::is_reference_assignable_from() before performing the assignability check of +// T1 must be assignable from T2 +// Where: +// L is the class loader of +// T1 is the type resolved by L using the name +// T2 is the type resolved by L using the name +// +// The meaning of (*skip_assignability_check): +// true: is_reference_assignable_from() should SKIP the assignability check +// false: is_reference_assignable_from() should COMPLETE the assignability check +void SystemDictionaryShared::add_verification_constraint(InstanceKlass* k, Symbol* name, + Symbol* from_name, bool from_field_is_protected, bool from_is_array, bool from_is_object, + bool* skip_assignability_check) { assert(CDSConfig::is_dumping_archive(), "sanity"); DumpTimeClassInfo* info = get_info(k); info->add_verification_constraint(k, name, from_name, from_field_is_protected, from_is_array, from_is_object); - if (CDSConfig::is_dumping_dynamic_archive()) { - // For dynamic dumping, we can resolve all the constraint classes for all class loaders during - // the initial run prior to creating the archive before vm exit. We will also perform verification - // check when running with the archive. - return false; + if (CDSConfig::is_dumping_classic_static_archive() && !is_builtin(k)) { + // This applies ONLY to the "classic" CDS static dump, which reads the list of + // unregistered classes (those intended for custom class loaders) from the classlist + // and loads them using jdk.internal.misc.CDS$UnregisteredClassLoader. + // + // When the classlist contains an unregistered class k, the supertypes of k are also + // recorded in the classlist. However, the classlist does not contain information about + // any class X that's not a supertype of k but is needed in the verification of k. + // As a result, CDS$UnregisteredClassLoader will not know how to resolve X. + // + // Therefore, we tell the verifier to refrain from resolving X. Instead, X is recorded + // (symbolically) in the verification constraints of k. In the production run, + // when k is loaded, we will go through its verification constraints and resolve X to complete + // the is_reference_assignable_from() checks. + *skip_assignability_check = true; } else { - if (is_builtin(k)) { - // For builtin class loaders, we can try to complete the verification check at dump time, - // because we can resolve all the constraint classes. We will also perform verification check - // when running with the archive. - return false; - } else { - // For non-builtin class loaders, we cannot complete the verification check at dump time, - // because at dump time we don't know how to resolve classes for such loaders. - return true; - } + // In all other cases, we are using an *actual* class loader to load k, so it should be able + // to resolve any types that are needed for the verification of k. + *skip_assignability_check = false; } } diff --git a/src/hotspot/share/classfile/systemDictionaryShared.hpp b/src/hotspot/share/classfile/systemDictionaryShared.hpp index 22fc3fd825d..a0019ac0bd8 100644 --- a/src/hotspot/share/classfile/systemDictionaryShared.hpp +++ b/src/hotspot/share/classfile/systemDictionaryShared.hpp @@ -233,9 +233,10 @@ public: // ensures that you cannot load a shared class if its super type(s) are changed. However, // we need an additional check to ensure that the verification_constraints did not change // between dump time and runtime. - static bool add_verification_constraint(InstanceKlass* k, Symbol* name, + static void add_verification_constraint(InstanceKlass* k, Symbol* name, Symbol* from_name, bool from_field_is_protected, - bool from_is_array, bool from_is_object) NOT_CDS_RETURN_(false); + bool from_is_array, bool from_is_object, + bool* skip_assignability_check); static void check_verification_constraints(InstanceKlass* klass, TRAPS) NOT_CDS_RETURN; static void add_enum_klass_static_field(InstanceKlass* ik, int root_index); diff --git a/src/hotspot/share/classfile/verificationType.cpp b/src/hotspot/share/classfile/verificationType.cpp index d0541a0d20f..e6115dfce9b 100644 --- a/src/hotspot/share/classfile/verificationType.cpp +++ b/src/hotspot/share/classfile/verificationType.cpp @@ -106,16 +106,19 @@ bool VerificationType::is_reference_assignable_from( return true; } +#if INCLUDE_CDS if (CDSConfig::is_dumping_archive()) { - if (SystemDictionaryShared::add_verification_constraint(klass, + bool skip_assignability_check = false; + SystemDictionaryShared::add_verification_constraint(klass, name(), from.name(), from_field_is_protected, from.is_array(), - from.is_object())) { - // If add_verification_constraint() returns true, the resolution/check should be - // delayed until runtime. + from.is_object(), &skip_assignability_check); + if (skip_assignability_check) { + // We are not able to resolve name() or from.name(). The actual assignability check + // will be delayed until runtime. return true; } } - +#endif return resolve_and_check_assignability(klass, name(), from.name(), from_field_is_protected, from.is_array(), from.is_object(), THREAD); } else if (is_array() && from.is_array()) { diff --git a/test/hotspot/jtreg/runtime/cds/appcds/aotCache/AOTCacheSupportForCustomLoaders.java b/test/hotspot/jtreg/runtime/cds/appcds/aotCache/AOTCacheSupportForCustomLoaders.java index dabc35a4604..50505502217 100644 --- a/test/hotspot/jtreg/runtime/cds/appcds/aotCache/AOTCacheSupportForCustomLoaders.java +++ b/test/hotspot/jtreg/runtime/cds/appcds/aotCache/AOTCacheSupportForCustomLoaders.java @@ -29,10 +29,11 @@ * @requires vm.cds.supports.aot.class.linking * @comment work around JDK-8345635 * @requires !vm.jvmci.enabled - * @library /test/lib + * @library /test/lib /test/hotspot/jtreg/runtime/cds/appcds/test-classes + * @build ReturnIntegerAsString * @build AOTCacheSupportForCustomLoaders * @run driver jdk.test.lib.helpers.ClassFileInstaller -jar app.jar AppWithCustomLoaders AppWithCustomLoaders$MyLoader - * @run driver jdk.test.lib.helpers.ClassFileInstaller -jar cust.jar AppWithCustomLoaders$MyLoadeeA AppWithCustomLoaders$MyLoadeeB + * @run driver jdk.test.lib.helpers.ClassFileInstaller -jar cust.jar AppWithCustomLoaders$MyLoadeeA AppWithCustomLoaders$MyLoadeeB ReturnIntegerAsString * @run driver AOTCacheSupportForCustomLoaders AOT */ @@ -50,7 +51,8 @@ public class AOTCacheSupportForCustomLoaders { .appCommandLine("AppWithCustomLoaders") .setAssemblyChecker((OutputAnalyzer out) -> { out.shouldMatch("cds,class.*unreg AppWithCustomLoaders[$]MyLoadeeA") - .shouldMatch("cds,class.*array \\[LAppWithCustomLoaders[$]MyLoadeeA;"); + .shouldMatch("cds,class.*array \\[LAppWithCustomLoaders[$]MyLoadeeA;") + .shouldNotMatch("cds,class.* ReturnIntegerAsString"); }) .setProductionChecker((OutputAnalyzer out) -> { out.shouldContain("Using AOT-linked classes: true"); @@ -69,6 +71,16 @@ class AppWithCustomLoaders { Class klass = loader.loadClass("AppWithCustomLoaders$MyLoadeeA"); klass.newInstance(); + // Test 2: VerificationType::is_reference_assignable_from() cannot be skipped (JDK-8356407) + try { + Class bad = loader.loadClass("ReturnIntegerAsString"); + Object o = bad.newInstance(); // force verification + System.out.println("Expected String but got: " + o.toString().getClass()); + throw new RuntimeException("VerifyError expected but not thrown"); + } catch (VerifyError ve) { + System.out.println("Expected: " + ve); + } + // TODO: more test cases JDK-8354557 } diff --git a/test/hotspot/jtreg/runtime/cds/appcds/test-classes/ReturnIntegerAsString.jasm b/test/hotspot/jtreg/runtime/cds/appcds/test-classes/ReturnIntegerAsString.jasm new file mode 100644 index 00000000000..31b67b90e6e --- /dev/null +++ b/test/hotspot/jtreg/runtime/cds/appcds/test-classes/ReturnIntegerAsString.jasm @@ -0,0 +1,56 @@ +/* + * 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. + * + */ + + +/* + +class ReturnIntegerAsString { + String doit() { + return new Integer(1); + } +} + +*/ + +public super class ReturnIntegerAsString + version 60:0 +{ + public Method "":"()V" + stack 1 locals 1 + { + aload_0; + invokespecial Method java/lang/Object."":"()V"; + return; + } + public Method toString:"()Ljava/lang/String;" + stack 3 locals 2 + { + new class java/lang/Integer; + dup; + iconst_1; + invokespecial Method java/lang/Integer."":"(I)V"; + areturn; + } + +} // end Class ReturnIntegerAsString