8166931: Do not include classes which are unusable during run time in the classlist file
authorccheung
Tue, 18 Oct 2016 16:15:03 -0700
changeset 41741 2f5b8bbcb18c
parent 41740 5ce9b71d69d4
child 41743 39d7567a68d4
8166931: Do not include classes which are unusable during run time in the classlist file Summary: added check to exclude classes found in the --patch-module list Reviewed-by: jiangli, lfoltan, iklam
hotspot/src/share/vm/classfile/classFileParser.cpp
hotspot/src/share/vm/classfile/classLoader.cpp
hotspot/src/share/vm/classfile/classLoader.hpp
hotspot/test/runtime/modules/PatchModule/PatchModuleClassList.java
--- a/hotspot/src/share/vm/classfile/classFileParser.cpp	Tue Oct 18 17:38:17 2016 -0400
+++ b/hotspot/src/share/vm/classfile/classFileParser.cpp	Tue Oct 18 16:15:03 2016 -0700
@@ -5775,9 +5775,22 @@
       // Anonymous classes such as generated LambdaForm classes are also not included.
       if (SystemDictionaryShared::is_sharing_possible(_loader_data) &&
           _host_klass == NULL) {
+        oop class_loader = _loader_data->class_loader();
         ResourceMark rm(THREAD);
-        classlist_file->print_cr("%s", _class_name->as_C_string());
-        classlist_file->flush();
+        // For the boot and platform class loaders, check if the class is not found in the
+        // java runtime image. Additional check for the boot class loader is if the class
+        // is not found in the boot loader's appended entries. This indicates that the class
+        // is not useable during run time, such as the ones found in the --patch-module entries,
+        // so it should not be included in the classlist file.
+        if (((class_loader == NULL && !ClassLoader::contains_append_entry(stream->source())) ||
+             SystemDictionary::is_platform_class_loader(class_loader)) &&
+            !ClassLoader::is_jrt(stream->source())) {
+          tty->print_cr("skip writing class %s from source %s to classlist file",
+            _class_name->as_C_string(), stream->source());
+        } else {
+          classlist_file->print_cr("%s", _class_name->as_C_string());
+          classlist_file->flush();
+        }
       }
     }
 #endif
--- a/hotspot/src/share/vm/classfile/classLoader.cpp	Tue Oct 18 17:38:17 2016 -0400
+++ b/hotspot/src/share/vm/classfile/classLoader.cpp	Tue Oct 18 16:15:03 2016 -0700
@@ -945,11 +945,11 @@
 }
 
 // returns true if entry already on class path
-bool ClassLoader::contains_entry(ClassPathEntry *entry) {
+bool ClassLoader::contains_append_entry(const char* name) {
   ClassPathEntry* e = _first_append_entry;
   while (e != NULL) {
     // assume zip entries have been canonicalized
-    if (strcmp(entry->name(), e->name()) == 0) {
+    if (strcmp(name, e->name()) == 0) {
       return true;
     }
     e = e->next();
@@ -991,7 +991,7 @@
 
     // Do not reorder the bootclasspath which would break get_system_package().
     // Add new entry to linked list
-    if (!check_for_duplicates || !contains_entry(new_entry)) {
+    if (!check_for_duplicates || !contains_append_entry(new_entry->name())) {
       ClassLoaderExt::add_class_path_entry(path, check_for_duplicates, new_entry);
     }
     return true;
--- a/hotspot/src/share/vm/classfile/classLoader.hpp	Tue Oct 18 17:38:17 2016 -0400
+++ b/hotspot/src/share/vm/classfile/classLoader.hpp	Tue Oct 18 16:15:03 2016 -0700
@@ -451,7 +451,7 @@
   static void set_first_append_entry(ClassPathEntry* entry);
 
   // indicates if class path already contains a entry (exact match by name)
-  static bool contains_entry(ClassPathEntry* entry);
+  static bool contains_append_entry(const char* name);
 
   // adds a class path list
   static void add_to_list(ClassPathEntry* new_entry);
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/hotspot/test/runtime/modules/PatchModule/PatchModuleClassList.java	Tue Oct 18 16:15:03 2016 -0700
@@ -0,0 +1,142 @@
+/*
+ * Copyright (c) 2016, 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 classes which are not useable during run time should not be included in the classlist
+ * @library /test/lib
+ * @modules java.base/jdk.internal.misc
+ *          jdk.jartool/sun.tools.jar
+ * @build PatchModuleMain
+ * @run main PatchModuleClassList
+ */
+
+import java.nio.file.Files;
+import java.nio.file.Paths;
+import jdk.test.lib.InMemoryJavaCompiler;
+import jdk.test.lib.process.OutputAnalyzer;
+import jdk.test.lib.process.ProcessTools;
+
+public class PatchModuleClassList {
+    private static final String BOOT_CLASS = "javax/naming/spi/NamingManager";
+    private static final String PLATFORM_CLASS = "javax/transaction/InvalidTransactionException";
+
+    public static void main(String args[]) throws Throwable {
+        // Case 1. A class to be loaded by the boot class loader
+
+        // Create a class file in the module java.naming. This class file
+        // will be put in the javanaming.jar file.
+        String source = "package javax.naming.spi; "                +
+                        "public class NamingManager { "             +
+                        "    static { "                             +
+                        "        System.out.println(\"I pass!\"); " +
+                        "    } "                                    +
+                        "}";
+
+        ClassFileInstaller.writeClassToDisk(BOOT_CLASS,
+             InMemoryJavaCompiler.compile(BOOT_CLASS.replace('/', '.'), source, "-Xmodule:java.naming"),
+             System.getProperty("test.classes"));
+
+        // Build the jar file that will be used for the module "java.naming".
+        BasicJarBuilder.build("javanaming", BOOT_CLASS);
+        String moduleJar = BasicJarBuilder.getTestJar("javanaming.jar");
+
+        String classList = "javanaming.list";
+        ProcessBuilder pb = ProcessTools.createJavaProcessBuilder(
+            true,
+            "-XX:DumpLoadedClassList=" + classList,
+            "--patch-module=java.naming=" + moduleJar,
+            "PatchModuleMain", BOOT_CLASS.replace('/', '.'));
+        new OutputAnalyzer(pb.start()).shouldHaveExitValue(0);
+
+        // check the generated classlist file
+        String content = new String(Files.readAllBytes(Paths.get(classList)));
+        if (content.indexOf(BOOT_CLASS) >= 0) {
+            throw new RuntimeException(BOOT_CLASS + " should not be in the classlist");
+        }
+
+        // Case 2. A class to be loaded by the platform class loader
+
+        // Create a class file in the module java.transaction. This class file
+        // will be put in the javatransaction.jar file.
+        source = "package javax.transaction; "                 +
+                 "public class InvalidTransactionException { " +
+                 "    static { "                               +
+                 "        System.out.println(\"I pass!\"); "   +
+                 "    } "                                      +
+                 "}";
+
+        ClassFileInstaller.writeClassToDisk(PLATFORM_CLASS,
+             InMemoryJavaCompiler.compile(PLATFORM_CLASS.replace('/', '.'), source, "-Xmodule:java.transaction"),
+             System.getProperty("test.classes"));
+
+        // Build the jar file that will be used for the module "java.transaction".
+        BasicJarBuilder.build("javatransaction", PLATFORM_CLASS);
+        moduleJar = BasicJarBuilder.getTestJar("javatransaction.jar");
+
+        classList = "javatransaction.list";
+        pb = ProcessTools.createJavaProcessBuilder(
+            true,
+            "-XX:DumpLoadedClassList=" + classList,
+            "--patch-module=java.naming=" + moduleJar,
+            "PatchModuleMain", PLATFORM_CLASS.replace('/', '.'));
+        new OutputAnalyzer(pb.start()).shouldHaveExitValue(0);
+
+        // check the generated classlist file
+        content = new String(Files.readAllBytes(Paths.get(classList)));
+        if (content.indexOf(PLATFORM_CLASS) >= 0) {
+            throw new RuntimeException(PLATFORM_CLASS + " should not be in the classlist");
+        }
+
+        // Case 3. A class to be loaded from the bootclasspath/a
+
+        // Create a simple class file
+        source = "public class Hello { "                         +
+                 "    public static void main(String args[]) { " +
+                 "        System.out.println(\"Hello\"); "       +
+                 "    } "                                        +
+                 "}";
+
+        ClassFileInstaller.writeClassToDisk("Hello",
+             InMemoryJavaCompiler.compile("Hello", source),
+             System.getProperty("test.classes"));
+
+        // Build hello.jar
+        BasicJarBuilder.build("hello", "Hello");
+        moduleJar = BasicJarBuilder.getTestJar("hello.jar");
+
+        classList = "hello.list";
+        pb = ProcessTools.createJavaProcessBuilder(
+            true,
+            "-XX:DumpLoadedClassList=" + classList,
+            "-Xbootclasspath/a:" + moduleJar,
+            "Hello");
+        new OutputAnalyzer(pb.start()).shouldHaveExitValue(0);
+
+        // check the generated classlist file
+        content = new String(Files.readAllBytes(Paths.get(classList)));
+        if (content.indexOf("Hello") < 0) {
+            throw new RuntimeException("Hello should be in the classlist");
+        }
+    }
+}