8209385: CDS runtime classpath checking is too strict when only classes from the system modules are archived
Summary: skip checking the path entries which are not being referenced during CDS dump time
Reviewed-by: jiangli, iklam
--- a/src/hotspot/share/classfile/classLoaderExt.cpp Fri Aug 17 22:20:47 2018 +0100
+++ b/src/hotspot/share/classfile/classLoaderExt.cpp Fri Aug 17 14:50:59 2018 -0700
@@ -50,6 +50,7 @@
jshort ClassLoaderExt::_app_class_paths_start_index = ClassLoaderExt::max_classpath_index;
jshort ClassLoaderExt::_app_module_paths_start_index = ClassLoaderExt::max_classpath_index;
+jshort ClassLoaderExt::_max_used_path_index = 0;
bool ClassLoaderExt::_has_app_classes = false;
bool ClassLoaderExt::_has_platform_classes = false;
@@ -242,6 +243,9 @@
classloader_type = ClassLoader::PLATFORM_LOADER;
ClassLoaderExt::set_has_platform_classes();
}
+ if (classpath_index > ClassLoaderExt::max_used_path_index()) {
+ ClassLoaderExt::set_max_used_path_index(classpath_index);
+ }
result->set_shared_classpath_index(classpath_index);
result->set_class_loader_type(classloader_type);
}
--- a/src/hotspot/share/classfile/classLoaderExt.hpp Fri Aug 17 22:20:47 2018 +0100
+++ b/src/hotspot/share/classfile/classLoaderExt.hpp Fri Aug 17 14:50:59 2018 -0700
@@ -49,6 +49,8 @@
static jshort _app_class_paths_start_index;
// index of first modular JAR in shared modulepath entry table
static jshort _app_module_paths_start_index;
+ // the largest path index being used during CDS dump time
+ static jshort _max_used_path_index;
static bool _has_app_classes;
static bool _has_platform_classes;
@@ -91,6 +93,12 @@
static jshort app_module_paths_start_index() { return _app_module_paths_start_index; }
+ static jshort max_used_path_index() { return _max_used_path_index; }
+
+ static void set_max_used_path_index(jshort used_index) {
+ _max_used_path_index = used_index;
+ }
+
static void init_paths_start_index(jshort app_start) {
_app_class_paths_start_index = app_start;
}
--- a/src/hotspot/share/classfile/sharedPathsMiscInfo.cpp Fri Aug 17 22:20:47 2018 +0100
+++ b/src/hotspot/share/classfile/sharedPathsMiscInfo.cpp Fri Aug 17 14:50:59 2018 -0700
@@ -115,10 +115,15 @@
return fail("Corrupted archive file header");
}
+ jshort cur_index = 0;
+ jshort max_cp_index = FileMapInfo::current_info()->header()->max_used_path_index();
+ jshort module_paths_start_index =
+ FileMapInfo::current_info()->header()->app_module_paths_start_index();
while (_cur_ptr < _end_ptr) {
jint type;
const char* path = _cur_ptr;
_cur_ptr += strlen(path) + 1;
+
if (!read_jint(&type)) {
return fail("Corrupted archive file header");
}
@@ -129,13 +134,19 @@
print_path(&ls, type, path);
ls.cr();
}
- if (!check(type, path)) {
- if (!PrintSharedArchiveAndExit) {
- return false;
+ // skip checking the class path(s) which was not referenced during CDS dump
+ if ((cur_index <= max_cp_index) || (cur_index >= module_paths_start_index)) {
+ if (!check(type, path)) {
+ if (!PrintSharedArchiveAndExit) {
+ return false;
+ }
+ } else {
+ ClassLoader::trace_class_path("ok");
}
} else {
- ClassLoader::trace_class_path("ok");
+ ClassLoader::trace_class_path("skipped check");
}
+ cur_index++;
}
return true;
--- a/src/hotspot/share/memory/filemap.cpp Fri Aug 17 22:20:47 2018 +0100
+++ b/src/hotspot/share/memory/filemap.cpp Fri Aug 17 14:50:59 2018 -0700
@@ -199,6 +199,7 @@
ClassLoaderExt::finalize_shared_paths_misc_info();
_app_class_paths_start_index = ClassLoaderExt::app_class_paths_start_index();
_app_module_paths_start_index = ClassLoaderExt::app_module_paths_start_index();
+ _max_used_path_index = ClassLoaderExt::max_used_path_index();
_verify_local = BytecodeVerificationLocal;
_verify_remote = BytecodeVerificationRemote;
@@ -359,13 +360,13 @@
bool has_nonempty_dir = false;
- int end = _shared_path_table_size;
- if (!ClassLoaderExt::has_platform_or_app_classes()) {
- // only check the boot path if no app class is loaded
- end = ClassLoaderExt::app_class_paths_start_index();
+ int last = _shared_path_table_size - 1;
+ if (last > ClassLoaderExt::max_used_path_index()) {
+ // no need to check any path beyond max_used_path_index
+ last = ClassLoaderExt::max_used_path_index();
}
- for (int i = 0; i < end; i++) {
+ for (int i = 0; i <= last; i++) {
SharedClassPathEntry *e = shared_path(i);
if (e->is_dir()) {
const char* path = e->name();
@@ -467,13 +468,8 @@
int module_paths_start_index = _header->_app_module_paths_start_index;
- // If the shared archive contain app or platform classes, validate all entries
- // in the shared path table. Otherwise, only validate the boot path entries (with
- // entry index < _app_class_paths_start_index).
- int count = _header->has_platform_or_app_classes() ?
- _shared_path_table_size : _header->_app_class_paths_start_index;
-
- for (int i=0; i<count; i++) {
+ // validate the path entries up to the _max_used_path_index
+ for (int i=0; i < _header->_max_used_path_index + 1; i++) {
if (i < module_paths_start_index) {
if (shared_path(i)->validate()) {
log_info(class, path)("ok");
--- a/src/hotspot/share/memory/filemap.hpp Fri Aug 17 22:20:47 2018 +0100
+++ b/src/hotspot/share/memory/filemap.hpp Fri Aug 17 14:50:59 2018 -0700
@@ -192,6 +192,7 @@
jshort _app_class_paths_start_index; // Index of first app classpath entry
jshort _app_module_paths_start_index; // Index of first module path entry
+ jshort _max_used_path_index; // max path index referenced during CDS dump
bool _verify_local; // BytecodeVerificationLocal setting
bool _verify_remote; // BytecodeVerificationRemote setting
bool _has_platform_or_app_classes; // Archive contains app classes
@@ -200,6 +201,8 @@
_has_platform_or_app_classes = v;
}
bool has_platform_or_app_classes() { return _has_platform_or_app_classes; }
+ jshort max_used_path_index() { return _max_used_path_index; }
+ jshort app_module_paths_start_index() { return _app_module_paths_start_index; }
char* region_addr(int idx);
--- a/test/hotspot/jtreg/runtime/appcds/DirClasspathTest.java Fri Aug 17 22:20:47 2018 +0100
+++ b/test/hotspot/jtreg/runtime/appcds/DirClasspathTest.java Fri Aug 17 14:50:59 2018 -0700
@@ -27,12 +27,14 @@
* @summary Handling of directories in -cp is based on the classlist
* @requires vm.cds
* @library /test/lib
+ * @compile test-classes/Hello.java
* @run main DirClasspathTest
*/
import jdk.test.lib.Platform;
import jdk.test.lib.process.OutputAnalyzer;
import java.io.File;
+import java.nio.file.Files;
import java.nio.file.Path;
import java.nio.file.Paths;
import java.util.Arrays;
@@ -87,13 +89,28 @@
/////////////////////////////////////////////////////////////////
String appClassList[] = {"java/lang/Object", "com/sun/tools/javac/Main"};
- // Non-empty dir in -cp: should report error
+ // Non-empty dir in -cp: should be OK (as long as no classes were loaded from there)
output = TestCommon.dump(dir.getPath(), appClassList, "-Xlog:class+path=info");
+ TestCommon.checkDump(output);
+
+ // Long path to non-empty dir in -cp: should be OK (as long as no classes were loaded from there)
+ output = TestCommon.dump(longDir.getPath(), appClassList, "-Xlog:class+path=info");
+ TestCommon.checkDump(output);
+
+ /////////////////////////////////////////////////////////////////
+ // Loading an app class from a directory
+ /////////////////////////////////////////////////////////////////
+ String appClassList2[] = {"Hello", "java/lang/Object", "com/sun/tools/javac/Main"};
+ // Non-empty dir in -cp: should report error if a class is loaded from it
+ output = TestCommon.dump(classDir.toString(), appClassList2, "-Xlog:class+path=info");
output.shouldNotHaveExitValue(0);
output.shouldContain("Cannot have non-empty directory in paths");
- // Long path to non-empty dir in -cp: should report error
- output = TestCommon.dump(longDir.getPath(), appClassList, "-Xlog:class+path=info");
+ // Long path to non-empty dir in -cp: should report error if a class is loaded from it
+ File srcClass = new File(classDir.toFile(), "Hello.class");
+ File destClass = new File(longDir, "Hello.class");
+ Files.copy(srcClass.toPath(), destClass.toPath());
+ output = TestCommon.dump(longDir.getPath(), appClassList2, "-Xlog:class+path=info");
output.shouldNotHaveExitValue(0);
output.shouldContain("Cannot have non-empty directory in paths");
}
--- a/test/hotspot/jtreg/runtime/appcds/PrintSharedArchiveAndExit.java Fri Aug 17 22:20:47 2018 +0100
+++ b/test/hotspot/jtreg/runtime/appcds/PrintSharedArchiveAndExit.java Fri Aug 17 14:50:59 2018 -0700
@@ -65,7 +65,7 @@
String cp = appJar + File.pathSeparator + appJar2;
String lastCheckMsg = "checking shared classpath entry: " + appJar2; // the last JAR to check
- TestCommon.testDump(cp, TestCommon.list("Hello"));
+ TestCommon.testDump(cp, TestCommon.list("Hello", "HelloMore"));
log("Normal execution -- all the JAR paths should be checked");
TestCommon.run(
--- /dev/null Thu Jan 01 00:00:00 1970 +0000
+++ b/test/hotspot/jtreg/runtime/appcds/UnusedCPDuringDump.java Fri Aug 17 14:50:59 2018 -0700
@@ -0,0 +1,63 @@
+/*
+ * Copyright (c) 2018, 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
+ * @bug 8209385
+ * @summary non-empty dir in -cp should be fine during dump time if only classes
+ * from the system modules are being loaded even though some are
+ * defined to the PlatformClassLoader and AppClassLoader.
+ * @requires vm.cds
+ * @library /test/lib
+ * @modules jdk.jartool/sun.tools.jar
+ * @compile test-classes/Hello.java
+ * @run main/othervm -Dtest.cds.copy.child.stdout=false UnusedCPDuringDump
+ */
+
+import java.io.File;
+import jdk.test.lib.process.OutputAnalyzer;
+
+public class UnusedCPDuringDump {
+
+ public static void main(String[] args) throws Exception {
+ File dir = new File(System.getProperty("user.dir"));
+ File emptydir = new File(dir, "emptydir");
+ emptydir.mkdir();
+ String appJar = JarBuilder.getOrCreateHelloJar();
+
+ OutputAnalyzer output = TestCommon.dump(dir.getPath(),
+ TestCommon.list("sun/util/resources/cldr/provider/CLDRLocaleDataMetaInfo",
+ "com/sun/tools/sjavac/client/ClientMain"),
+ "-Xlog:class+path=info,class+load=debug");
+ TestCommon.checkDump(output,
+ "[class,load] sun.util.resources.cldr.provider.CLDRLocaleDataMetaInfo",
+ "for instance a 'jdk/internal/loader/ClassLoaders$PlatformClassLoader'",
+ "[class,load] com.sun.tools.sjavac.client.ClientMain",
+ "for instance a 'jdk/internal/loader/ClassLoaders$AppClassLoader'");
+
+ String jsaOpt = "-XX:SharedArchiveFile=" + TestCommon.getCurrentArchiveName();
+ TestCommon.run("-cp", appJar, jsaOpt, "-Xlog:class+path=info,class+load=debug", "Hello")
+ .assertNormalExit("Hello World");
+ }
+}