8356838: AOT incorrectly sets a cached class's loader type to boot

Reviewed-by: ccheung, iveresov

Author: iklam

src/hotspot/share/cds/aotClassLocation.cpp

@@ -1035,3 +1035,38 @@ bool AOTClassLocationConfig::validate(bool has_aot_linked_classes, bool* has_ext
   }
   return success;
 }
+
+void AOTClassLocationConfig::print() {
+  if (CDSConfig::is_dumping_archive()) {
+    tty->print_cr("AOTClassLocationConfig::_dumptime_instance = %p", _dumptime_instance);
+    if (_dumptime_instance != nullptr) {
+      _dumptime_instance->print_on(tty);
+    }
+  }
+  if (CDSConfig::is_using_archive()) {
+    tty->print_cr("AOTClassLocationConfig::_runtime_instance = %p", _runtime_instance);
+    if (_runtime_instance != nullptr) {
+      _runtime_instance->print_on(tty);
+    }
+  }
+}
+
+void AOTClassLocationConfig::print_on(outputStream* st) const {
+  int n = class_locations()->length();
+  for (int i = 0; i < n; i++) {
+    const AOTClassLocation* cs = class_location_at(i);
+    const char* path;
+    if (i == 0) {
+      path = ClassLoader::get_jrt_entry()->name();
+    } else {
+      path = cs->path();
+    }
+    st->print_cr("[%d] = %s", i, path);
+    if (i == boot_cp_end_index() && i < n) {
+      st->print_cr("--- end of boot");
+    }
+    if (i == app_cp_end_index() && i < n) {
+      st->print_cr("--- end of app");
+    }
+  }
+}

src/hotspot/share/cds/aotClassLocation.hpp

@@ -31,6 +31,7 @@
 #include "utilities/growableArray.hpp"
 #include "utilities/globalDefinitions.hpp"
 #include "utilities/macros.hpp"
+#include "utilities/ostream.hpp"
 
 class AllClassLocationStreams;
 class ClassLocationStream;
@@ -201,6 +202,9 @@ class AOTClassLocationConfig : public CHeapObj<mtClassShared> {
   void print_dumptime_classpath(LogStream& ls, int index_start, int index_limit,
                                 bool do_substitute, size_t remove_prefix_len,
                                 const char* prepend, size_t prepend_len) const;
+
+  void print_on(outputStream* st) const;
+
 public:
   static AOTClassLocationConfig* dumptime() {
     assert(_dumptime_instance != nullptr, "can only be called when dumping an AOT cache");
@@ -268,6 +272,8 @@ class AOTClassLocationConfig : public CHeapObj<mtClassShared> {
   bool validate(bool has_aot_linked_classes, bool* has_extra_module_paths) const;
 
   bool is_valid_classpath_index(int classpath_index, InstanceKlass* ik);
+
+  static void print();
 };
 
 

src/hotspot/share/classfile/classLoader.cpp

@@ -1200,10 +1200,20 @@ void ClassLoader::record_result(JavaThread* current, InstanceKlass* ik,
     return;
   }
 
+  if (!SystemDictionaryShared::is_builtin_loader(ik->class_loader_data())) {
+    // A class loaded by a user-defined classloader.
+    assert(ik->shared_classpath_index() < 0, "not assigned yet");
+    ik->set_shared_classpath_index(UNREGISTERED_INDEX);
+    SystemDictionaryShared::set_shared_class_misc_info(ik, (ClassFileStream*)stream);
+    return;
+  }
+
   assert(has_jrt_entry(), "CDS dumping does not support exploded JDK build");
 
   ResourceMark rm(current);
   int classpath_index = -1;
+  bool found_invalid = false;
+
   PackageEntry* pkg_entry = ik->package();
 
   if (!AOTClassLocationConfig::dumptime_is_ready()) {
@@ -1218,7 +1228,6 @@ void ClassLoader::record_result(JavaThread* current, InstanceKlass* ik,
     // must be valid since the class has been successfully parsed.
     const char* path = ClassLoader::uri_to_path(src);
     assert(path != nullptr, "sanity");
-    bool found_invalid = false;
     AOTClassLocationConfig::dumptime_iterate([&] (AOTClassLocation* cl) {
       int i = cl->index();
       // for index 0 and the stream->source() is the modules image or has the jrt: protocol.
@@ -1245,8 +1254,6 @@ void ClassLoader::record_result(JavaThread* current, InstanceKlass* ik,
               if (loader != nullptr) {
                 // Probably loaded by jdk/internal/loader/ClassLoaders$BootClassLoader. Don't archive
                 // such classes.
-                ik->set_shared_classpath_index(-1);
-                ik->set_shared_class_loader_type(ClassLoader::BOOT_LOADER);
                 found_invalid = true;
               } else {
                 classpath_index = i;
@@ -1262,31 +1269,22 @@ void ClassLoader::record_result(JavaThread* current, InstanceKlass* ik,
         }
       }
       if (classpath_index >= 0 || found_invalid) {
-        return false; // quit iterating
+        return false; // Break the AOTClassLocationConfig::dumptime_iterate() loop.
       } else {
         return true; // Keep iterating
       }
     });
+  }
 
-    if (found_invalid) {
-      return;
-    }
-
-    // No path entry found for this class: most likely a shared class loaded by the
-    // user defined classloader.
-    if (classpath_index < 0 && !SystemDictionaryShared::is_builtin_loader(ik->class_loader_data())) {
-      assert(ik->shared_classpath_index() < 0, "not assigned yet");
-      ik->set_shared_classpath_index(UNREGISTERED_INDEX);
-      SystemDictionaryShared::set_shared_class_misc_info(ik, (ClassFileStream*)stream);
-      return;
-    }
+  if (found_invalid) {
+    assert(classpath_index == -1, "sanity");
   }
 
   const char* const class_name = ik->name()->as_C_string();
   const char* const file_name = file_name_for_class_name(class_name,
                                                          ik->name()->utf8_length());
   assert(file_name != nullptr, "invariant");
-  ClassLoaderExt::record_result(checked_cast<s2>(classpath_index), ik, redefined);
+  ClassLoaderExt::record_result_for_builtin_loader(checked_cast<s2>(classpath_index), ik, redefined);
 }
 
 void ClassLoader::record_hidden_class(InstanceKlass* ik) {

src/hotspot/share/classfile/classLoaderExt.cpp

@@ -67,18 +67,20 @@ int ClassLoaderExt::compare_module_names(const char** p1, const char** p2) {
   return strcmp(*p1, *p2);
 }
 
-void ClassLoaderExt::record_result(s2 classpath_index, InstanceKlass* result, bool redefined) {
+void ClassLoaderExt::record_result_for_builtin_loader(s2 classpath_index, InstanceKlass* result, bool redefined) {
   assert(CDSConfig::is_dumping_archive(), "sanity");
 
-  // We need to remember where the class comes from during dumping.
   oop loader = result->class_loader();
-  s2 classloader_type = ClassLoader::BOOT_LOADER;
+  s2 classloader_type;
   if (SystemDictionary::is_system_class_loader(loader)) {
     classloader_type = ClassLoader::APP_LOADER;
     AOTClassLocationConfig::dumptime_set_has_app_classes();
   } else if (SystemDictionary::is_platform_class_loader(loader)) {
     classloader_type = ClassLoader::PLATFORM_LOADER;
     AOTClassLocationConfig::dumptime_set_has_platform_classes();
+  } else {
+    precond(loader == nullptr);
+    classloader_type = ClassLoader::BOOT_LOADER;
   }
 
   if (CDSConfig::is_dumping_preimage_static_archive() || CDSConfig::is_dumping_dynamic_archive()) {

src/hotspot/share/classfile/classLoaderExt.hpp

@@ -40,7 +40,7 @@ class ClassLoaderExt: public ClassLoader { // AllStatic
   static void append_boot_classpath(ClassPathEntry* new_entry);
 
   static int compare_module_names(const char** p1, const char** p2);
-  static void record_result(s2 classpath_index, InstanceKlass* result, bool redefined);
+  static void record_result_for_builtin_loader(s2 classpath_index, InstanceKlass* result, bool redefined);
 #endif // INCLUDE_CDS
 };
 

test/hotspot/jtreg/runtime/cds/appcds/aotCache/AOTCacheSupportForCustomLoaders.java

@@ -25,7 +25,7 @@
 /*
  * @test
  * @summary Test AOT cache support for array classes in custom class loaders.
- * @bug 8353298
+ * @bug 8353298 8356838
  * @requires vm.cds.supports.aot.class.linking
  * @comment work around JDK-8345635
  * @requires !vm.jvmci.enabled
@@ -37,20 +37,36 @@
  * @run driver AOTCacheSupportForCustomLoaders AOT
  */
 
+import java.lang.module.Configuration;
+import java.lang.module.ModuleFinder;
 import java.net.URL;
 import java.net.URLClassLoader;
 import java.io.File;
+import java.nio.file.Path;
+import java.nio.file.Paths;
+import java.util.Set;
+import jdk.test.lib.cds.CDSJarUtils;
+import jdk.test.lib.cds.CDSModulePackager;
 import jdk.test.lib.cds.SimpleCDSAppTester;
 import jdk.test.lib.process.OutputAnalyzer;
 
 public class AOTCacheSupportForCustomLoaders {
+    static final Path SRC = Paths.get(System.getProperty("test.src")).resolve("modules");
+
     public static void main(String... args) throws Exception {
+        CDSModulePackager modulePackager = new CDSModulePackager(SRC);
+        String modulePath = modulePackager.getOutputDir().toString();
+        modulePackager.createModularJar("com.test");
+
         SimpleCDSAppTester.of("AOTCacheSupportForCustomLoaders")
             .classpath("app.jar")
-            .addVmArgs("-Xlog:cds+class=debug", "-Xlog:cds")
-            .appCommandLine("AppWithCustomLoaders")
+            .addVmArgs("-Xlog:cds+class=debug", "-Xlog:cds",
+                       "--module-path=" + modulePath,
+                       "--add-modules=com.test")
+            .appCommandLine("AppWithCustomLoaders", modulePath)
             .setAssemblyChecker((OutputAnalyzer out) -> {
                     out.shouldMatch("cds,class.*unreg AppWithCustomLoaders[$]MyLoadeeA")
+                       .shouldMatch("cds,class.*unreg com.test.Foo")
                        .shouldMatch("cds,class.*array \\[LAppWithCustomLoaders[$]MyLoadeeA;")
                        .shouldNotMatch("cds,class.* ReturnIntegerAsString");
                 })
@@ -67,11 +83,21 @@ public static void main(String args[]) throws Exception {
         URL[] urls = new URL[] {custJar.toURI().toURL()};
         MyLoader loader = new MyLoader(urls, AppWithCustomLoaders.class.getClassLoader());
 
-        // Test 1: array class of MyLoadeeA (JDK-8353298)
+        test1(loader);
+        test2(loader);
+        test3(args[0]);
+
+        // TODO: more test cases JDK-8354557
+    }
+
+    // Test 1: array class of MyLoadeeA (JDK-8353298)
+    static void test1(MyLoader loader) throws Exception {
         Class klass = loader.loadClass("AppWithCustomLoaders$MyLoadeeA");
         klass.newInstance();
+    }
 
-        // Test 2: VerificationType::is_reference_assignable_from() cannot be skipped (JDK-8356407)
+    // Test 2: VerificationType::is_reference_assignable_from() cannot be skipped (JDK-8356407)
+    static void test2(MyLoader loader) throws Exception {
         try {
             Class bad = loader.loadClass("ReturnIntegerAsString");
             Object o = bad.newInstance(); // force verification
@@ -80,8 +106,41 @@ public static void main(String args[]) throws Exception {
         } catch (VerifyError ve) {
             System.out.println("Expected: " + ve);
         }
+    }
 
-        // TODO: more test cases JDK-8354557
+    // Test 3: custom loader defines a class from the exact location as a class defined in the boot layer.
+    static void test3(String modulePath) throws Exception {
+        Class<?> c0 = Class.forName("com.test.Foo");
+        System.out.println(c0);
+        System.out.println(System.identityHashCode(c0.getModule()));
+        System.out.println(c0.getModule().getName());
+        System.out.println(c0.getClassLoader());
+
+        // Regression test for JDK-8356838
+        //
+        // We create a new layer that loads the com.test module from the modulePath into
+        // a different class loader.
+        ModuleFinder finder = ModuleFinder.of(Paths.get(modulePath));
+        ModuleLayer parent = ModuleLayer.boot();
+        Configuration cf = parent.configuration().resolve(finder, ModuleFinder.of(), Set.of("com.test"));
+        ClassLoader scl = ClassLoader.getSystemClassLoader();
+        ModuleLayer layer = parent.defineModulesWithOneLoader(cf, scl);
+        Class<?> c1 = layer.findLoader("com.test").loadClass("com.test.Foo");
+
+        System.out.println(c1);
+        System.out.println(System.identityHashCode(c1.getModule()));
+        System.out.println(c1.getModule().getName());
+        System.out.println(c1.getClassLoader());
+
+        if (!c1.getModule().getName().equals("com.test")) {
+            throw new RuntimeException("Unexpected module: " + c1.getModule());
+        }
+        if (c1.getModule() == c0.getModule()) {
+            throw new RuntimeException("Unexpected module: " + c1.getModule());
+        }
+        if (c1.getClassLoader() == c0.getClassLoader()) {
+            throw new RuntimeException("Unexpected class loader: " + c1.getClassLoader());
+        }
     }
 
     public static class MyLoader extends URLClassLoader {

test/hotspot/jtreg/runtime/cds/appcds/aotCache/modules/com.test/com/test/Foo.java

@@ -0,0 +1,27 @@
+/*
+ * 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.
+ *
+ */
+
+package com.test;
+
+public class Foo {}

test/hotspot/jtreg/runtime/cds/appcds/aotCache/modules/com.test/module-info.java

@@ -0,0 +1,27 @@
+/*
+ * 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.
+ *
+ */
+
+module com.test {
+  exports com.test;
+}