-
Notifications
You must be signed in to change notification settings - Fork 170
8382584: [lworld] Mandatory pre-loading still present in systemDictionary.cpp #2409
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: lworld
Are you sure you want to change the base?
Changes from all commits
b24ca87
4c706f8
c6c10df
2d90d4d
6c12ac3
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -1048,29 +1048,29 @@ bool SystemDictionary::is_shared_class_visible_impl(Symbol* class_name, | |
| return visible; | ||
| } | ||
|
|
||
| bool SystemDictionary::check_shared_class_super_type(InstanceKlass* klass, InstanceKlass* super_type, | ||
| bool SystemDictionary::check_shared_class_dependency(InstanceKlass* klass, InstanceKlass* dependency, | ||
| Handle class_loader, bool is_superclass, TRAPS) { | ||
| assert(super_type->in_aot_cache(), "must be"); | ||
| assert(dependency->in_aot_cache(), "must be"); | ||
|
|
||
| // Quick check if the super type has been already loaded. | ||
| // Quick check if the dependency has been already loaded. | ||
| // + Don't do it for unregistered classes -- they can be unloaded so | ||
| // super_type->class_loader_data() could be stale. | ||
| // + Don't check if loader data is null, ie. the super_type isn't fully loaded. | ||
| if (!super_type->defined_by_other_loaders() && super_type->class_loader_data() != nullptr) { | ||
| // Check if the superclass is loaded by the current class_loader | ||
| Symbol* name = super_type->name(); | ||
| // dependency->class_loader_data() could be stale. | ||
| // + Don't check if loader data is null, ie. the dependency isn't fully loaded. | ||
| if (!dependency->defined_by_other_loaders() && dependency->class_loader_data() != nullptr) { | ||
| // Check if the dependency is loaded by the current class_loader. | ||
| Symbol* name = dependency->name(); | ||
| InstanceKlass* check = find_instance_klass(THREAD, name, class_loader); | ||
| if (check == super_type) { | ||
| if (check == dependency) { | ||
| return true; | ||
| } | ||
| } | ||
|
|
||
| Klass *found = resolve_with_circularity_detection(klass->name(), super_type->name(), | ||
| Klass* found = resolve_with_circularity_detection(klass->name(), dependency->name(), | ||
| class_loader, is_superclass, CHECK_false); | ||
| if (found == super_type) { | ||
| if (found == dependency) { | ||
| return true; | ||
| } else { | ||
| // The dynamically resolved super type is not the same as the one we used during dump time, | ||
| // The dynamically resolved dependency is not the same as the one we used during dump time, | ||
| // so we cannot use the class. | ||
| return false; | ||
| } | ||
|
|
@@ -1085,7 +1085,7 @@ bool SystemDictionary::check_shared_class_super_types(InstanceKlass* ik, Handle | |
| // load <ik> from the shared archive. | ||
|
|
||
| if (ik->super() != nullptr) { | ||
| bool check_super = check_shared_class_super_type(ik, ik->super(), | ||
| bool check_super = check_shared_class_dependency(ik, ik->super(), | ||
| class_loader, true, | ||
| CHECK_false); | ||
| if (!check_super) { | ||
|
|
@@ -1096,7 +1096,7 @@ bool SystemDictionary::check_shared_class_super_types(InstanceKlass* ik, Handle | |
| Array<InstanceKlass*>* interfaces = ik->local_interfaces(); | ||
| int num_interfaces = interfaces->length(); | ||
| for (int index = 0; index < num_interfaces; index++) { | ||
| bool check_interface = check_shared_class_super_type(ik, interfaces->at(index), class_loader, false, | ||
| bool check_interface = check_shared_class_dependency(ik, interfaces->at(index), class_loader, false, | ||
| CHECK_false); | ||
| if (!check_interface) { | ||
| return false; | ||
|
|
@@ -1106,43 +1106,51 @@ bool SystemDictionary::check_shared_class_super_types(InstanceKlass* ik, Handle | |
| return true; | ||
| } | ||
|
|
||
| // Pre-load class referred to in non-static null-free instance field. These fields trigger MANDATORY loading. | ||
| // Some pre-loading does not fail fatally | ||
| bool SystemDictionary::preload_from_null_free_field(InstanceKlass* ik, Handle class_loader, Symbol* sig, int field_index, TRAPS) { | ||
| TempNewSymbol name = Signature::strip_envelope(sig); | ||
| log_info(class, preload)("Preloading of class %s during loading of shared class %s. " | ||
| "Cause: a null-free non-static field is declared with this type", | ||
| name->as_C_string(), ik->name()->as_C_string()); | ||
| InstanceKlass* real_k = SystemDictionary::resolve_with_circularity_detection(ik->name(), name, | ||
| class_loader, false, CHECK_false); | ||
| if (HAS_PENDING_EXCEPTION) { | ||
| log_info(class, preload)("Preloading of class %s during loading of class %s " | ||
| "(cause: null-free non-static field) failed: %s", | ||
| name->as_C_string(), ik->name()->as_C_string(), | ||
| PENDING_EXCEPTION->klass()->name()->as_C_string()); | ||
| return false; // Exception is still pending | ||
| // Pre-load class referred to in non-static fields with archived inline field metadata. These fields | ||
| // must be checked against the resolved runtime class before the shared class can be used. | ||
| bool SystemDictionary::preload_from_required_inline_field(InstanceKlass* ik, Handle class_loader, Symbol* sig, int field_index, TRAPS) { | ||
| if (log_is_enabled(Info, class, preload)) { | ||
| TempNewSymbol name = Signature::strip_envelope(sig); | ||
| log_info(class, preload)("Preloading of class %s during loading of shared class %s. " | ||
| "Cause: archived flat/null-restricted field metadata", | ||
| name->as_C_string(), ik->name()->as_C_string()); | ||
| } | ||
|
|
||
| InstanceKlass* k = ik->get_inline_type_field_klass_or_null(field_index); | ||
| if (real_k != k) { | ||
| // oops, the app has substituted a different version of k! Does not fail fatally | ||
| log_info(class, preload)("Preloading of class %s during loading of shared class %s " | ||
| "(cause: null-free non-static field) failed : " | ||
| "app substituted a different version", | ||
| name->as_C_string(), ik->name()->as_C_string()); | ||
| bool check = check_shared_class_dependency(ik, k, class_loader, false, THREAD); | ||
| if (!check) { | ||
| if (HAS_PENDING_EXCEPTION) { | ||
| if (log_is_enabled(Info, class, preload)) { | ||
| TempNewSymbol name = Signature::strip_envelope(sig); | ||
| log_info(class, preload)("Preloading of class %s during loading of shared class %s " | ||
| "(cause: archived flat/null-restricted field metadata) failed : %s", | ||
| name->as_C_string(), ik->name()->as_C_string(), | ||
| PENDING_EXCEPTION->klass()->name()->as_C_string()); | ||
| } | ||
| CLEAR_PENDING_EXCEPTION; | ||
| } else { | ||
| if (log_is_enabled(Info, class, preload)) { | ||
| TempNewSymbol name = Signature::strip_envelope(sig); | ||
| log_info(class, preload)("Preloading of class %s during loading of shared class %s " | ||
| "(cause: archived flat/null-restricted field metadata) failed : " | ||
| "app substituted a different version", | ||
| name->as_C_string(), ik->name()->as_C_string()); | ||
| } | ||
| } | ||
| return false; | ||
| } | ||
| log_info(class, preload)("Preloading of class %s during loading of shared class %s " | ||
| "(cause: null-free non-static field) succeeded", | ||
| name->as_C_string(), ik->name()->as_C_string()); | ||
|
|
||
| assert(real_k != nullptr, "Sanity check"); | ||
| InstanceKlass::check_can_be_annotated_with_NullRestricted(real_k, ik->name(), CHECK_false); | ||
|
|
||
| assert(k != nullptr, "Sanity check"); | ||
| if (log_is_enabled(Info, class, preload)) { | ||
| TempNewSymbol name = Signature::strip_envelope(sig); | ||
| log_info(class, preload)("Preloading of class %s during loading of shared class %s " | ||
| "(cause: archived flat/null-restricted field metadata) succeeded", | ||
| name->as_C_string(), ik->name()->as_C_string()); | ||
| } | ||
| return true; | ||
| } | ||
|
|
||
| // Tries to pre-load classes referred to in non-static nullable instance fields if they are found in the | ||
| // Tries to pre-load classes referred to in non-static instance fields if they are found in the | ||
| // loadable descriptors attribute. If loading fails, we can fail silently. | ||
| void SystemDictionary::try_preload_from_loadable_descriptors(InstanceKlass* ik, Handle class_loader, Symbol* sig, int field_index, TRAPS) { | ||
| TempNewSymbol name = Signature::strip_envelope(sig); | ||
|
|
@@ -1163,7 +1171,6 @@ void SystemDictionary::try_preload_from_loadable_descriptors(InstanceKlass* ik, | |
| "(cause: field type in LoadableDescriptors attribute) failed : " | ||
| "app substituted a different version", | ||
| name->as_C_string(), ik->name()->as_C_string()); | ||
| return; | ||
| } else if (real_k != nullptr) { | ||
| log_info(class, preload)("Preloading of class %s during loading of shared class %s " | ||
| "(cause: field type in LoadableDescriptors attribute) succeeded", | ||
|
|
@@ -1203,15 +1210,18 @@ InstanceKlass* SystemDictionary::load_shared_class(InstanceKlass* ik, | |
| Symbol* sig = fs.signature(); | ||
| int field_index = fs.index(); | ||
|
|
||
| if (fs.is_null_free_inline_type()) { | ||
| // A false return means that the class didn't load for other reasons than an exception. | ||
| bool check = preload_from_null_free_field(ik, class_loader, sig, field_index, CHECK_NULL); | ||
| if (!Signature::has_envelope(sig)) { | ||
| continue; | ||
| } | ||
|
|
||
| if (fs.is_flat() || fs.is_null_free_inline_type()) { | ||
| bool check = preload_from_required_inline_field(ik, class_loader, sig, field_index, CHECK_NULL); | ||
| if (!check) { | ||
| ik->set_shared_loading_failed(); | ||
| return nullptr; | ||
| } | ||
| } else if (Signature::has_envelope(sig)) { | ||
| // Pending exceptions are cleared so we can fail silently | ||
| } else { | ||
| // Pending exceptions are cleared so we can fail silently. | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do we need this branch to attempt to preload? As long as we have the classes for the flattened fields, we should be OK. I would expect the other classes to be optional at this point
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This branch handles the speculative preloading from the
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm fine with leaving this for now but we should think further about this after the merge to mainline. With CDS, these field classes for non-flat fields will be loaded during |
||
| try_preload_from_loadable_descriptors(ik, class_loader, sig, field_index, CHECK_NULL); | ||
| } | ||
| } | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,131 @@ | ||
| /* | ||
| * Copyright (c) 2026, 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 Ensure archived nullable flat field metadata is validated without LoadableDescriptors. | ||
| * @requires vm.cds | ||
| * @library /test/lib | ||
| * @enablePreview | ||
| * @modules java.base/jdk.internal.vm.annotation | ||
| * @compile StrippedLoadableDescriptorsMismatchTest.java | ||
| * @run driver jdk.test.lib.helpers.StrictProcessor StrippedLDHolder | ||
| * @run main/othervm StrippedLoadableDescriptorsMismatchTest | ||
| */ | ||
|
|
||
| import java.io.File; | ||
| import java.lang.classfile.Attributes; | ||
| import java.lang.classfile.ClassFile; | ||
| import java.lang.classfile.ClassTransform; | ||
| import java.lang.classfile.attribute.LoadableDescriptorsAttribute; | ||
| import java.lang.invoke.MethodHandles; | ||
| import java.nio.file.Files; | ||
| import java.nio.file.Path; | ||
| import java.nio.file.StandardCopyOption; | ||
|
|
||
| import jdk.internal.vm.annotation.LooselyConsistentValue; | ||
| import jdk.test.lib.helpers.StrictInit; | ||
|
|
||
| public class StrippedLoadableDescriptorsMismatchTest { | ||
| public static void main(String[] args) throws Exception { | ||
| String appJar = buildJarWithStrippedLoadableDescriptors(); | ||
| String pointClassFile = new File(System.getProperty("test.classes", "."), | ||
| "StrippedLDPoint.class").getPath(); | ||
|
|
||
| TestCommon.dump(appJar, TestCommon.list("StrippedLDPoint", "StrippedLDHolder"), | ||
| "--enable-preview", | ||
| "-XX:+UnlockDiagnosticVMOptions", | ||
| "-XX:+UseNullableNonAtomicValueFlattening"); | ||
|
|
||
| TestCommon.checkExec(TestCommon.exec(appJar, | ||
| "--enable-preview", | ||
| "-XX:+UnlockDiagnosticVMOptions", | ||
| "-XX:+UseNullableNonAtomicValueFlattening", | ||
| "StrippedLDApp", pointClassFile)); | ||
| } | ||
|
|
||
| private static String buildJarWithStrippedLoadableDescriptors() throws Exception { | ||
| Path testClasses = Path.of(System.getProperty("test.classes", ".")); | ||
| Path jarClasses = testClasses.resolve("stripped_loadable_descriptors_mismatch"); | ||
| Files.createDirectories(jarClasses); | ||
|
|
||
| copyClass(testClasses, jarClasses, "StrippedLDApp"); | ||
| copyClass(testClasses, jarClasses, "StrippedLDPoint"); | ||
| stripLoadableDescriptors(testClasses, jarClasses, "StrippedLDHolder"); | ||
|
|
||
| return JarBuilder.build("stripped_loadable_descriptors_mismatch", jarClasses.toFile(), null); | ||
| } | ||
|
|
||
| private static void copyClass(Path fromDir, Path toDir, String className) throws Exception { | ||
| Files.copy(fromDir.resolve(className + ".class"), | ||
| toDir.resolve(className + ".class"), | ||
| StandardCopyOption.REPLACE_EXISTING); | ||
| } | ||
|
|
||
| private static void stripLoadableDescriptors(Path fromDir, Path toDir, String className) throws Exception { | ||
| ClassFile cf = ClassFile.of(); | ||
| byte[] original = Files.readAllBytes(fromDir.resolve(className + ".class")); | ||
| var model = cf.parse(original); | ||
| if (model.findAttribute(Attributes.loadableDescriptors()).isEmpty()) { | ||
| throw new RuntimeException(className + " must have a LoadableDescriptors attribute before transformation"); | ||
| } | ||
|
|
||
| byte[] transformed = cf.transformClass(model, | ||
| ClassTransform.dropping(e -> e instanceof LoadableDescriptorsAttribute)); | ||
| if (cf.parse(transformed).findAttribute(Attributes.loadableDescriptors()).isPresent()) { | ||
| throw new RuntimeException("Failed to strip LoadableDescriptors from " + className); | ||
| } | ||
| Files.write(toDir.resolve(className + ".class"), transformed); | ||
| } | ||
| } | ||
|
|
||
| class StrippedLDApp { | ||
| public static void main(String[] args) throws Throwable { | ||
| Path classFile = Path.of(args[0]); | ||
| byte[] classBytes = Files.readAllBytes(classFile); | ||
| Class<?> fieldClass = MethodHandles.lookup().defineClass(classBytes); | ||
|
|
||
| StrippedLDHolder holder = new StrippedLDHolder(); | ||
|
|
||
| if (holder.p.getClass() != fieldClass) { | ||
| throw new RuntimeException("Mismatched field class"); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| class StrippedLDHolder { | ||
| @StrictInit | ||
| final StrippedLDPoint p; | ||
|
|
||
| StrippedLDHolder() { | ||
| p = new StrippedLDPoint(); | ||
| super(); | ||
| } | ||
| } | ||
|
|
||
| @LooselyConsistentValue | ||
| value class StrippedLDPoint { | ||
| int x = 0; | ||
| int y = 0; | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we safely drop this failure marking?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's used for regular shared class loading so I don't think we can drop this shared_loading_failed marker.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Didn't realise that one was needed. I've re-added it, along with an edit to
try_preload_from_loadable_descriptorsso we can see if we failed or not. It now also has a new parameter that decides if failing means we callshared_loading_failed.