# HG changeset patch # User iklam # Date 1553874152 25200 # Node ID 2221f042556d9ba4640273ff5a17db2185cca029 # Parent f69a2f675f195427a4e717b20dbc571b003f4a96 8221351: Crash in KlassFactory::check_shared_class_file_load_hook Reviewed-by: dholmes, ccheung diff -r f69a2f675f19 -r 2221f042556d src/hotspot/share/classfile/classLoader.cpp --- 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: // /// // @@ -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 diff -r f69a2f675f19 -r 2221f042556d src/hotspot/share/classfile/classLoader.hpp --- 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 diff -r f69a2f675f19 -r 2221f042556d src/hotspot/share/classfile/klassFactory.cpp --- 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(); diff -r f69a2f675f19 -r 2221f042556d src/hotspot/share/memory/filemap.cpp --- 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 diff -r f69a2f675f19 -r 2221f042556d src/hotspot/share/memory/filemap.hpp --- 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) { diff -r f69a2f675f19 -r 2221f042556d test/hotspot/jtreg/TEST.groups --- 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 \ diff -r f69a2f675f19 -r 2221f042556d test/hotspot/jtreg/runtime/appcds/TestCommon.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"); diff -r f69a2f675f19 -r 2221f042556d test/hotspot/jtreg/runtime/appcds/customLoader/HelloCustom.java --- 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); } } diff -r f69a2f675f19 -r 2221f042556d test/hotspot/jtreg/runtime/appcds/customLoader/HelloCustom_JFR.java --- /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"); + } +} + diff -r f69a2f675f19 -r 2221f042556d test/hotspot/jtreg/runtime/appcds/jigsaw/modulepath/ModulePathAndCP.java --- 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. diff -r f69a2f675f19 -r 2221f042556d test/hotspot/jtreg/runtime/appcds/jigsaw/modulepath/ModulePathAndCP_JFR.java --- /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"); + } +} + diff -r f69a2f675f19 -r 2221f042556d test/hotspot/jtreg/runtime/appcds/jvmti/ClassFileLoadHook.java --- 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); diff -r f69a2f675f19 -r 2221f042556d test/hotspot/jtreg/runtime/appcds/jvmti/ClassFileLoadHookTest.java --- 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 {