8380291: AOT cache should store only unregistered classes with file: code source

Reviewed-by: kvn, iveresov

src/hotspot/share/classfile/classFileParser.cpp

@@ -34,6 +34,7 @@
 #include "classfile/packageEntry.hpp"
 #include "classfile/symbolTable.hpp"
 #include "classfile/systemDictionary.hpp"
+#include "classfile/systemDictionaryShared.hpp"
 #include "classfile/verificationType.hpp"
 #include "classfile/verifier.hpp"
 #include "classfile/vmClasses.hpp"
@@ -86,9 +87,6 @@
 #include "utilities/macros.hpp"
 #include "utilities/ostream.hpp"
 #include "utilities/utf8.hpp"
-#if INCLUDE_CDS
-#include "classfile/systemDictionaryShared.hpp"
-#endif
 
 // We generally try to create the oops directly when parsing, rather than
 // allocating temporary data structures and copying the bytes twice. A
@@ -5256,6 +5254,9 @@ void ClassFileParser::fill_instance_klass(InstanceKlass* ik,
 
   if (!is_internal()) {
     ik->print_class_load_logging(_loader_data, module_entry, _stream);
+    if (CDSConfig::is_dumping_archive()) {
+      SystemDictionaryShared::check_code_source(ik, _stream);
+    }
 
     if (ik->minor_version() == JAVA_PREVIEW_MINOR_VERSION &&
         ik->major_version() == JVM_CLASSFILE_MAJOR_VERSION &&

src/hotspot/share/classfile/systemDictionaryShared.cpp

@@ -202,6 +202,20 @@ DumpTimeClassInfo* SystemDictionaryShared::get_info_locked(InstanceKlass* k) {
   return info;
 }
 
+void SystemDictionaryShared::check_code_source(InstanceKlass* ik, const ClassFileStream* cfs) {
+  if (CDSConfig::is_dumping_preimage_static_archive() && !is_builtin_loader(ik->class_loader_data())) {
+    if (cfs == nullptr || cfs->source() == nullptr || strncmp(cfs->source(), "file:", 5) != 0) {
+      // AOT cache filtering:
+      // For non-built-in loaders, cache only the classes that have a file: code source, so
+      // we can avoid caching dynamically generated classes that are likely to change from
+      // run to run. This is similar to the filtering in ClassListWriter::write_to_stream()
+      // for the classic CDS static archive.
+      SystemDictionaryShared::log_exclusion(ik, "Not loaded from \"file:\" code source");
+      SystemDictionaryShared::set_excluded(ik);
+    }
+  }
+}
+
 bool SystemDictionaryShared::should_be_excluded_impl(InstanceKlass* k, DumpTimeClassInfo* info) {
   assert_lock_strong(DumpTimeTable_lock);
 

src/hotspot/share/classfile/systemDictionaryShared.hpp

@@ -235,6 +235,7 @@ public:
   static void update_shared_entry(InstanceKlass* klass, int id);
   static void set_shared_class_misc_info(InstanceKlass* k, ClassFileStream* cfs);
 
+  static void check_code_source(InstanceKlass* ik, const ClassFileStream* cfs) NOT_CDS_RETURN;
   static InstanceKlass* lookup_from_stream(Symbol* class_name,
                                            Handle class_loader,
                                            Handle protection_domain,

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

@@ -31,7 +31,10 @@
  * @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 ReturnIntegerAsString
+ * @run driver jdk.test.lib.helpers.ClassFileInstaller -jar cust.jar
+ *                 AppWithCustomLoaders$MyLoadeeA AppWithCustomLoaders$MyLoadeeB
+ *                 AppWithCustomLoaders$MyLoadeeC AppWithCustomLoaders$MyLoadeeD
+ *                 ReturnIntegerAsString
  * @run driver AOTCacheSupportForCustomLoaders AOT
  */
 
@@ -40,6 +43,7 @@ import java.lang.module.ModuleFinder;
 import java.net.URL;
 import java.net.URLClassLoader;
 import java.io.File;
+import java.io.InputStream;
 import java.nio.file.Path;
 import java.nio.file.Paths;
 import java.util.Set;
@@ -62,10 +66,17 @@ public class AOTCacheSupportForCustomLoaders {
                        "--module-path=" + modulePath,
                        "--add-modules=com.test")
             .appCommandLine("AppWithCustomLoaders", modulePath)
+            .setTrainingChecker((OutputAnalyzer out) -> {
+                    out.shouldContain("Skipping AppWithCustomLoaders$MyLoadeeC: Not loaded from \"file:\" code source")
+                       .shouldContain("Skipping AppWithCustomLoaders$MyLoadeeD: super AppWithCustomLoaders$MyLoadeeC is excluded")
+                       .shouldContain("Skipping ReturnIntegerAsString: Failed verification");
+                })
             .setAssemblyChecker((OutputAnalyzer out) -> {
                     out.shouldMatch(",class.*unreg AppWithCustomLoaders[$]MyLoadeeA")
                        .shouldMatch(",class.*unreg com.test.Foo")
                        .shouldMatch(",class.*array \\[LAppWithCustomLoaders[$]MyLoadeeA;")
+                       .shouldNotMatch("class.*unreg.*MyLoadeeC") // not from "file:" code source
+                       .shouldNotMatch("class.*unreg.*MyLoadeeD") // parent is not from "file:" code source
                        .shouldNotMatch(",class.* ReturnIntegerAsString");
                 })
             .setProductionChecker((OutputAnalyzer out) -> {
@@ -76,7 +87,7 @@ public class AOTCacheSupportForCustomLoaders {
 }
 
 class AppWithCustomLoaders {
-    static MyLoader loader; // keep alive so its classes can be cached.
+    static MyLoader loader; // keep alive
 
     public static void main(String args[]) throws Exception {
         File custJar = new File("cust.jar");
@@ -86,6 +97,7 @@ class AppWithCustomLoaders {
         test1(loader);
         test2(loader);
         test3(args[0]);
+        test4(loader);
 
         // TODO: more test cases JDK-8354557
     }
@@ -147,10 +159,37 @@ class AppWithCustomLoaders {
         }
     }
 
+    // Test 4: classes that don't use file: code source should be excluded
+    static void test4(MyLoader loader) throws Exception {
+        Class c = loader.loadLoadeeC();
+        Class d = loader.loadClass("AppWithCustomLoaders$MyLoadeeD");
+
+        URL ccs = c.getProtectionDomain().getCodeSource().getLocation();
+
+        if (ccs != null) {
+            throw new RuntimeException("MyLoadeeC should have null CodeSource but got: " + ccs);
+        }
+
+        if (d.getSuperclass() != c) {
+            throw new RuntimeException("MyLoadeeC should be super class of MyLoadeeD");
+        }
+    }
+
     public static class MyLoader extends URLClassLoader {
         public MyLoader(URL[] urls, ClassLoader parent) {
             super(urls, parent);
         }
+
+        public Class<?> loadLoadeeC() throws Exception {
+            try (InputStream in = getResourceAsStream("AppWithCustomLoaders$MyLoadeeC.class")) {
+                byte[] b = in.readAllBytes();
+                // Define MyLoadeeC without specifying a ProtectionDomain. As a result, this
+                // class will get an empty ProtectionDomain whose CodeSource location will be null.
+                //
+                // This class should be excluded from the AOT cache. See JDK-8380291
+                return defineClass(b, 0, b.length);
+            }
+        }
     }
 
     public static class MyLoadeeA {
@@ -180,4 +219,9 @@ class AppWithCustomLoaders {
     }
 
     public static class MyLoadeeB extends MyLoadeeA {}
+
+
+    public static class MyLoadeeC {}
+
+    public static class MyLoadeeD extends MyLoadeeC {}
 }