8221351: Crash in KlassFactory::check_shared_class_file_load_hook
authoriklam
Fri, 29 Mar 2019 08:42:32 -0700
changeset 54340 2221f042556d
parent 54339 f69a2f675f19
child 54341 0b47455de59b
8221351: Crash in KlassFactory::check_shared_class_file_load_hook Reviewed-by: dholmes, ccheung
src/hotspot/share/classfile/classLoader.cpp
src/hotspot/share/classfile/classLoader.hpp
src/hotspot/share/classfile/klassFactory.cpp
src/hotspot/share/memory/filemap.cpp
src/hotspot/share/memory/filemap.hpp
test/hotspot/jtreg/TEST.groups
test/hotspot/jtreg/runtime/appcds/TestCommon.java
test/hotspot/jtreg/runtime/appcds/customLoader/HelloCustom.java
test/hotspot/jtreg/runtime/appcds/customLoader/HelloCustom_JFR.java
test/hotspot/jtreg/runtime/appcds/jigsaw/modulepath/ModulePathAndCP.java
test/hotspot/jtreg/runtime/appcds/jigsaw/modulepath/ModulePathAndCP_JFR.java
test/hotspot/jtreg/runtime/appcds/jvmti/ClassFileLoadHook.java
test/hotspot/jtreg/runtime/appcds/jvmti/ClassFileLoadHookTest.java
--- a/src/hotspot/share/classfile/classLoader.cpp	Fri Mar 29 07:38:34 2019 -0700
+++ b/src/hotspot/share/classfile/classLoader.cpp	Fri Mar 29 08:42:32 2019 -0700
@@ -396,6 +396,10 @@
   }
 }
 
+ClassFileStream* ClassPathImageEntry::open_stream(const char* name, TRAPS) {
+  return open_stream_for_loader(name, ClassLoaderData::the_null_class_loader_data(), THREAD);
+}
+
 // For a class in a named module, look it up in the jimage file using this syntax:
 //    /<module-name>/<package-name>/<base-class>
 //
@@ -403,7 +407,7 @@
 //     1. There are no unnamed modules in the jimage file.
 //     2. A package is in at most one module in the jimage file.
 //
-ClassFileStream* ClassPathImageEntry::open_stream(const char* name, TRAPS) {
+ClassFileStream* ClassPathImageEntry::open_stream_for_loader(const char* name, ClassLoaderData* loader_data, TRAPS) {
   jlong size;
   JImageLocationRef location = (*JImageFindResource)(_jimage, "", get_jimage_version_string(), name, &size);
 
@@ -414,20 +418,8 @@
     if (pkg_name != NULL) {
       if (!Universe::is_module_initialized()) {
         location = (*JImageFindResource)(_jimage, JAVA_BASE_NAME, get_jimage_version_string(), name, &size);
-#if INCLUDE_CDS
-        // CDS uses the boot class loader to load classes whose packages are in
-        // modules defined for other class loaders.  So, for now, get their module
-        // names from the "modules" jimage file.
-        if (DumpSharedSpaces && location == 0) {
-          const char* module_name = (*JImagePackageToModule)(_jimage, pkg_name);
-          if (module_name != NULL) {
-            location = (*JImageFindResource)(_jimage, module_name, get_jimage_version_string(), name, &size);
-          }
-        }
-#endif
-
       } else {
-        PackageEntry* package_entry = ClassLoader::get_package_entry(name, ClassLoaderData::the_null_class_loader_data(), CHECK_NULL);
+        PackageEntry* package_entry = ClassLoader::get_package_entry(name, loader_data, CHECK_NULL);
         if (package_entry != NULL) {
           ResourceMark rm;
           // Get the module name
--- a/src/hotspot/share/classfile/classLoader.hpp	Fri Mar 29 07:38:34 2019 -0700
+++ b/src/hotspot/share/classfile/classLoader.hpp	Fri Mar 29 08:42:32 2019 -0700
@@ -61,6 +61,10 @@
   // Attempt to locate file_name through this class path entry.
   // Returns a class file parsing stream if successfull.
   virtual ClassFileStream* open_stream(const char* name, TRAPS) = 0;
+  // Open the stream for a specific class loader
+  virtual ClassFileStream* open_stream_for_loader(const char* name, ClassLoaderData* loader_data, TRAPS) {
+    return open_stream(name, THREAD);
+  }
 };
 
 class ClassPathDirEntry: public ClassPathEntry {
@@ -125,6 +129,7 @@
   ClassPathImageEntry(JImageFile* jimage, const char* name);
   virtual ~ClassPathImageEntry();
   ClassFileStream* open_stream(const char* name, TRAPS);
+  ClassFileStream* open_stream_for_loader(const char* name, ClassLoaderData* loader_data, TRAPS);
 };
 
 // ModuleClassPathList contains a linked list of ClassPathEntry's
--- a/src/hotspot/share/classfile/klassFactory.cpp	Fri Mar 29 07:38:34 2019 -0700
+++ b/src/hotspot/share/classfile/klassFactory.cpp	Fri Mar 29 08:42:32 2019 -0700
@@ -58,7 +58,7 @@
     // Post the CFLH
     JvmtiCachedClassFileData* cached_class_file = NULL;
     if (cfs == NULL) {
-      cfs = FileMapInfo::open_stream_for_jvmti(ik, CHECK_NULL);
+      cfs = FileMapInfo::open_stream_for_jvmti(ik, class_loader, CHECK_NULL);
     }
     unsigned char* ptr = (unsigned char*)cfs->buffer();
     unsigned char* end_ptr = ptr + cfs->length();
--- a/src/hotspot/share/memory/filemap.cpp	Fri Mar 29 07:38:34 2019 -0700
+++ b/src/hotspot/share/memory/filemap.cpp	Fri Mar 29 08:42:32 2019 -0700
@@ -24,7 +24,9 @@
 
 #include "precompiled.hpp"
 #include "jvm.h"
+#include "classfile/classFileStream.hpp"
 #include "classfile/classLoader.inline.hpp"
+#include "classfile/classLoaderData.inline.hpp"
 #include "classfile/classLoaderExt.hpp"
 #include "classfile/symbolTable.hpp"
 #include "classfile/systemDictionaryShared.hpp"
@@ -1489,7 +1491,7 @@
   return ent;
 }
 
-ClassFileStream* FileMapInfo::open_stream_for_jvmti(InstanceKlass* ik, TRAPS) {
+ClassFileStream* FileMapInfo::open_stream_for_jvmti(InstanceKlass* ik, Handle class_loader, TRAPS) {
   int path_index = ik->shared_classpath_index();
   assert(path_index >= 0, "should be called for shared built-in classes only");
   assert(path_index < (int)_shared_path_table_size, "sanity");
@@ -1501,7 +1503,12 @@
   const char* const class_name = name->as_C_string();
   const char* const file_name = ClassLoader::file_name_for_class_name(class_name,
                                                                       name->utf8_length());
-  return cpe->open_stream(file_name, THREAD);
+  ClassLoaderData* loader_data = ClassLoaderData::class_loader_data(class_loader());
+  ClassFileStream* cfs = cpe->open_stream_for_loader(file_name, loader_data, THREAD);
+  assert(cfs != NULL, "must be able to read the classfile data of shared classes for built-in loaders.");
+  log_debug(cds, jvmti)("classfile data for %s [%d: %s] = %d bytes", class_name, path_index,
+                        cfs->source(), cfs->length());
+  return cfs;
 }
 
 #endif
--- a/src/hotspot/share/memory/filemap.hpp	Fri Mar 29 07:38:34 2019 -0700
+++ b/src/hotspot/share/memory/filemap.hpp	Fri Mar 29 08:42:32 2019 -0700
@@ -303,7 +303,7 @@
   static void update_shared_classpath(ClassPathEntry *cpe, SharedClassPathEntry* ent, TRAPS);
 
 #if INCLUDE_JVMTI
-  static ClassFileStream* open_stream_for_jvmti(InstanceKlass* ik, TRAPS);
+  static ClassFileStream* open_stream_for_jvmti(InstanceKlass* ik, Handle class_loader, TRAPS);
 #endif
 
   static SharedClassPathEntry* shared_path(int index) {
--- a/test/hotspot/jtreg/TEST.groups	Fri Mar 29 07:38:34 2019 -0700
+++ b/test/hotspot/jtreg/TEST.groups	Fri Mar 29 08:42:32 2019 -0700
@@ -321,6 +321,21 @@
   runtime/appcds/ \
   -:tier1_runtime_appcds
 
+# This group should be executed with "jtreg -Dtest.cds.run.with.jfr=true ..."
+# to test interaction between AppCDS and JFR. It also has the side effect of
+# testing JVMTI ClassFileLoadHook.
+#
+# The excluded tests disallow the jdk.jfr module, which is required to
+# run with JFR.
+hotspot_appcds_with_jfr = \
+  runtime/appcds/ \
+ -runtime/appcds/cacheObject/ArchivedModuleCompareTest.java \
+ -runtime/appcds/jigsaw/classpathtests/BootAppendTests.java \
+ -runtime/appcds/jigsaw/classpathtests/ClassPathTests.java \
+ -runtime/appcds/jigsaw/classpathtests/EmptyClassInBootClassPath.java \
+ -runtime/appcds/jigsaw/JigsawOptionsCombo.java \
+ -runtime/appcds/jigsaw/modulepath/MainModuleOnly.java
+
 tier1_serviceability = \
   serviceability/dcmd/compiler \
   -serviceability/dcmd/compiler/CompilerQueueTest.java \
--- a/test/hotspot/jtreg/runtime/appcds/TestCommon.java	Fri Mar 29 07:38:34 2019 -0700
+++ b/test/hotspot/jtreg/runtime/appcds/TestCommon.java	Fri Mar 29 08:42:32 2019 -0700
@@ -145,6 +145,15 @@
         return executeAndLog(pb, "dump");
     }
 
+    // This allows you to run the AppCDS tests with JFR enabled at runtime (though not at
+    // dump time, as that's uncommon for typical AppCDS users).
+    //
+    // To run in this special mode, add the following to your jtreg command-line
+    //    -Dtest.cds.run.with.jfr=true
+    //
+    // Some AppCDS tests are not compatible with this mode. See the group
+    // hotspot_appcds_with_jfr in ../../TEST.ROOT for details.
+    private static final boolean RUN_WITH_JFR = Boolean.getBoolean("test.cds.run.with.jfr");
 
     // Execute JVM using AppCDS archive with specified AppCDSOptions
     public static OutputAnalyzer runWithArchive(AppCDSOptions opts)
@@ -166,6 +175,22 @@
 
         for (String s : opts.suffix) cmd.add(s);
 
+        if (RUN_WITH_JFR) {
+            boolean usesJFR = false;
+            for (String s : cmd) {
+                if (s.startsWith("-XX:StartFlightRecording=") || s.startsWith("-XX:FlightRecorderOptions")) {
+                    System.out.println("JFR option might have been specified. Don't interfere: " + s);
+                    usesJFR = true;
+                    break;
+                }
+            }
+            if (!usesJFR) {
+                System.out.println("JFR option not specified. Enabling JFR ...");
+                cmd.add(0, "-XX:StartFlightRecording=dumponexit=true");
+                System.out.println(cmd);
+            }
+        }
+
         String[] cmdLine = cmd.toArray(new String[cmd.size()]);
         ProcessBuilder pb = ProcessTools.createJavaProcessBuilder(true, cmdLine);
         return executeAndLog(pb, "exec");
--- a/test/hotspot/jtreg/runtime/appcds/customLoader/HelloCustom.java	Fri Mar 29 07:38:34 2019 -0700
+++ b/test/hotspot/jtreg/runtime/appcds/customLoader/HelloCustom.java	Fri Mar 29 08:42:32 2019 -0700
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2015, 2018, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2015, 2019, 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
@@ -28,8 +28,6 @@
  * @requires vm.cds
  * @requires vm.cds.custom.loaders
  * @library /test/lib /test/hotspot/jtreg/runtime/appcds
- * @modules java.base/jdk.internal.misc
- *          java.management
  * @compile test-classes/Hello.java test-classes/CustomLoadee.java
  * @build sun.hotspot.WhiteBox
  * @run driver ClassFileInstaller -jar hello.jar Hello
@@ -43,6 +41,9 @@
 
 public class HelloCustom {
     public static void main(String[] args) throws Exception {
+        run();
+    }
+    public static void run(String... extra_runtime_args) throws Exception {
         String wbJar = ClassFileInstaller.getJarPath("WhiteBox.jar");
         String use_whitebox_jar = "-Xbootclasspath/a:" + wbJar;
 
@@ -62,11 +63,12 @@
                             use_whitebox_jar);
 
         output = TestCommon.exec(appJar,
-                                 // command-line arguments ...
-                                 use_whitebox_jar,
-                                 "-XX:+UnlockDiagnosticVMOptions",
-                                 "-XX:+WhiteBoxAPI",
-                                 "Hello", customJarPath);
+                                 TestCommon.concat(extra_runtime_args,
+                                     // command-line arguments ...
+                                     use_whitebox_jar,
+                                     "-XX:+UnlockDiagnosticVMOptions",
+                                     "-XX:+WhiteBoxAPI",
+                                     "Hello", customJarPath));
         TestCommon.checkExec(output);
     }
 }
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/test/hotspot/jtreg/runtime/appcds/customLoader/HelloCustom_JFR.java	Fri Mar 29 08:42:32 2019 -0700
@@ -0,0 +1,50 @@
+/*
+ * Copyright (c) 2019, 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 Same as HelloCustom, but add -XX:StartFlightRecording=dumponexit=true to the runtime
+ *          options. This makes sure that the shared classes are compatible with both
+ *          JFR and JVMTI ClassFileLoadHook.
+ * @requires vm.hasJFR
+ * @requires vm.cds
+ * @requires vm.cds.custom.loaders
+ * @library /test/lib /test/hotspot/jtreg/runtime/appcds
+ * @compile test-classes/Hello.java test-classes/CustomLoadee.java
+ * @build sun.hotspot.WhiteBox
+ * @run driver ClassFileInstaller -jar hello.jar Hello
+ * @run driver ClassFileInstaller -jar hello_custom.jar CustomLoadee
+ * @run driver ClassFileInstaller -jar WhiteBox.jar sun.hotspot.WhiteBox
+ * @run driver HelloCustom_JFR
+ */
+
+import jdk.test.lib.process.OutputAnalyzer;
+import sun.hotspot.WhiteBox;
+
+public class HelloCustom_JFR {
+    public static void main(String[] args) throws Exception {
+        HelloCustom.run("-XX:StartFlightRecording=dumponexit=true", "-Xlog:cds+jvmti=debug");
+    }
+}
+
--- a/test/hotspot/jtreg/runtime/appcds/jigsaw/modulepath/ModulePathAndCP.java	Fri Mar 29 07:38:34 2019 -0700
+++ b/test/hotspot/jtreg/runtime/appcds/jigsaw/modulepath/ModulePathAndCP.java	Fri Mar 29 08:42:32 2019 -0700
@@ -93,6 +93,10 @@
     }
 
     public static void main(String... args) throws Exception {
+        run();
+    }
+
+    public static void run(String... extra_runtime_args) throws Exception {
         // compile the modules and create the modular jar files
         buildTestModule();
         String appClasses[] = {MAIN_CLASS, APP_CLASS};
@@ -104,6 +108,7 @@
                                         "-m", MAIN_MODULE);
         TestCommon.checkDump(output);
         String prefix[] = {"-Djava.class.path=", "-Xlog:class+load=trace"};
+        prefix = TestCommon.concat(prefix, extra_runtime_args);
 
         // run with the archive with the --module-path the same as the one during
         // dump time. The classes should be loaded from the archive.
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/test/hotspot/jtreg/runtime/appcds/jigsaw/modulepath/ModulePathAndCP_JFR.java	Fri Mar 29 08:42:32 2019 -0700
@@ -0,0 +1,43 @@
+/*
+ * Copyright (c) 2019, 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
+ * @requires vm.hasJFR & vm.cds
+ * @library /test/lib /test/hotspot/jtreg/runtime/appcds
+ * @modules jdk.compiler
+ *          jdk.jartool/sun.tools.jar
+ *          jdk.jlink
+ * @run driver ModulePathAndCP_JFR
+ * @summary Same as ModulePathAndCP, but add -XX:StartFlightRecording=dumponexit=true to the runtime
+ *          options. This makes sure that the shared classes are compatible with both
+ *          JFR and JVMTI ClassFileLoadHook.
+ */
+
+public class ModulePathAndCP_JFR {
+    public static void main(String... args) throws Exception {
+        ModulePathAndCP.run("-XX:StartFlightRecording=dumponexit=true", "-Xlog:cds+jvmti=debug");
+    }
+}
+
--- a/test/hotspot/jtreg/runtime/appcds/jvmti/ClassFileLoadHook.java	Fri Mar 29 07:38:34 2019 -0700
+++ b/test/hotspot/jtreg/runtime/appcds/jvmti/ClassFileLoadHook.java	Fri Mar 29 08:42:32 2019 -0700
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2016, 2017, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2016, 2019, 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
@@ -41,8 +41,14 @@
         SHARING_ON_CFLH_ON
     }
 
-    public static void main(String args[]) {
+    public static void main(String args[]) throws Exception {
         TestCaseId testCase = TestCaseId.valueOf(args[0]);
+        test1(testCase);
+        test2(testCase);
+    }
+
+    // Test rewriting the classfile data using CFLH
+    static void test1(TestCaseId testCase) {
         WhiteBox wb = WhiteBox.getWhiteBox();
 
         System.out.println("====== ClassFileLoadHook.main():testCase = " + testCase);
@@ -81,6 +87,20 @@
         }
     }
 
+    // Test the loading of classfile data for non-boot shared classes from jrt:/xxx.
+    // See JDK-8221351.
+    static void test2(TestCaseId testCase) throws Exception {
+        WhiteBox wb = WhiteBox.getWhiteBox();
+        Class c = Class.forName("java.sql.SQLException"); // defined by platform class loader.
+
+        switch (testCase) {
+        case SHARING_ON_CFLH_OFF:
+        case SHARING_AUTO_CFLH_ON:
+        case SHARING_ON_CFLH_ON:
+            assertTrue(wb.isSharedClass(c), "must be shared");
+        }
+    }
+
     private static void assertTrue(boolean expr, String msg) {
         if (!expr)
             throw new RuntimeException(msg);
--- a/test/hotspot/jtreg/runtime/appcds/jvmti/ClassFileLoadHookTest.java	Fri Mar 29 07:38:34 2019 -0700
+++ b/test/hotspot/jtreg/runtime/appcds/jvmti/ClassFileLoadHookTest.java	Fri Mar 29 08:42:32 2019 -0700
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2016, 2017, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2016, 2019, 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
@@ -27,9 +27,6 @@
  * @summary Test jvmti class file loader hook interaction with AppCDS
  * @library /test/lib /test/hotspot/jtreg/runtime/appcds
  * @requires vm.cds
- * @modules java.base/jdk.internal.misc
- *          jdk.jartool/sun.tools.jar
- *          java.management
  * @build ClassFileLoadHook
  * @run main/othervm/native ClassFileLoadHookTest
  */
@@ -46,7 +43,8 @@
         "ClassFileLoadHook",
         "ClassFileLoadHook$TestCaseId",
         "ClassFileLoadHook$1",
-        "LoadMe"
+        "LoadMe",
+        "java/sql/SQLException"
     };
 
     public static void main(String[] args) throws Exception {