8209385: CDS runtime classpath checking is too strict when only classes from the system modules are archived

Skip checking the path entries which are not being referenced during CDS dump time

Reviewed-by: jiangli, iklam

src/hotspot/share/classfile/classLoaderExt.cpp

@@ -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 @@ void ClassLoaderExt::record_result(const s2 classpath_index,
     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);
 }

src/hotspot/share/classfile/classLoaderExt.hpp

@@ -49,6 +49,8 @@ private:
   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 @@ public:
 
   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;
   }

src/hotspot/share/classfile/sharedPathsMiscInfo.cpp

@@ -115,10 +115,15 @@ bool SharedPathsMiscInfo::check() {
     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 @@ bool SharedPathsMiscInfo::check() {
       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;

src/hotspot/share/memory/filemap.cpp

@@ -199,6 +199,7 @@ void FileMapInfo::FileMapHeader::populate(FileMapInfo* mapinfo, size_t alignment
   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 @@ void FileMapInfo::check_nonempty_dir_in_shared_path_table() {
 
   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 @@ bool FileMapInfo::validate_shared_path_table() {
 
   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");

src/hotspot/share/memory/filemap.hpp

@@ -192,6 +192,7 @@ public:
 
     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 @@ public:
       _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);
 

test/hotspot/jtreg/runtime/appcds/DirClasspathTest.java

@@ -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 @@ public class DirClasspathTest {
         /////////////////////////////////////////////////////////////////
         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");
     }

test/hotspot/jtreg/runtime/appcds/PrintSharedArchiveAndExit.java

@@ -65,7 +65,7 @@ public class PrintSharedArchiveAndExit {
     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(

test/hotspot/jtreg/runtime/appcds/UnusedCPDuringDump.java

@@ -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");
+  }
+}