8220095: Assertion failure when symlink (with different name) is used for lib/modules file
authoriklam
Sat, 23 Mar 2019 21:51:07 -0700
changeset 54257 21702e87efdf
parent 54256 aa937fac07f3
child 54258 0db90e99f13b
8220095: Assertion failure when symlink (with different name) is used for lib/modules file Summary: Removed confusing function ClassLoader::is_modules_image(char*) Reviewed-by: lfoltan, ccheung Contributed-by: Jiangli Zhou <jianglizhou@google.com>
src/hotspot/share/classfile/classFileParser.cpp
src/hotspot/share/classfile/classFileStream.cpp
src/hotspot/share/classfile/classFileStream.hpp
src/hotspot/share/classfile/classLoader.cpp
src/hotspot/share/classfile/classLoader.hpp
src/hotspot/share/oops/instanceKlass.cpp
test/hotspot/jtreg/runtime/modules/ModulesSymLink.java
--- a/src/hotspot/share/classfile/classFileParser.cpp	Sat Mar 23 17:18:49 2019 +0100
+++ b/src/hotspot/share/classfile/classFileParser.cpp	Sat Mar 23 21:51:07 2019 -0700
@@ -6113,7 +6113,7 @@
           // For the boot and platform class loaders, skip classes that are not found in the
           // java runtime image, such as those found in the --patch-module entries.
           // These classes can't be loaded from the archive during runtime.
-          if (!ClassLoader::is_modules_image(stream->source()) && strncmp(stream->source(), "jrt:", 4) != 0) {
+          if (!stream->from_boot_loader_modules_image() && strncmp(stream->source(), "jrt:", 4) != 0) {
             skip = true;
           }
 
--- a/src/hotspot/share/classfile/classFileStream.cpp	Sat Mar 23 17:18:49 2019 +0100
+++ b/src/hotspot/share/classfile/classFileStream.cpp	Sat Mar 23 21:51:07 2019 -0700
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 1997, 2016, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 1997, 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
@@ -38,12 +38,14 @@
 ClassFileStream::ClassFileStream(const u1* buffer,
                                  int length,
                                  const char* source,
-                                 bool verify_stream) :
+                                 bool verify_stream,
+                                 bool from_boot_loader_modules_image) :
   _buffer_start(buffer),
   _buffer_end(buffer + length),
   _current(buffer),
   _source(source),
-  _need_verify(verify_stream) {}
+  _need_verify(verify_stream),
+  _from_boot_loader_modules_image(from_boot_loader_modules_image) {}
 
 const u1* ClassFileStream::clone_buffer() const {
   u1* const new_buffer_start = NEW_RESOURCE_ARRAY(u1, length());
@@ -69,7 +71,8 @@
   return new ClassFileStream(new_buffer_start,
                              length(),
                              clone_source(),
-                             need_verify());
+                             need_verify(),
+                             from_boot_loader_modules_image());
 }
 
 uint64_t ClassFileStream::compute_fingerprint() const {
--- a/src/hotspot/share/classfile/classFileStream.hpp	Sat Mar 23 17:18:49 2019 +0100
+++ b/src/hotspot/share/classfile/classFileStream.hpp	Sat Mar 23 21:51:07 2019 -0700
@@ -44,7 +44,7 @@
   mutable const u1* _current;    // Current buffer position
   const char* const _source;     // Source of stream (directory name, ZIP/JAR archive name)
   bool _need_verify;             // True if verification is on for the class file
-
+  bool _from_boot_loader_modules_image;  // True if this was created by ClassPathImageEntry.
   void truncated_file_error(TRAPS) const ;
 
  protected:
@@ -57,7 +57,8 @@
   ClassFileStream(const u1* buffer,
                   int length,
                   const char* source,
-                  bool verify_stream = verify); // to be verified by default
+                  bool verify_stream = verify,  // to be verified by default
+                  bool from_boot_loader_modules_image = false);
 
   virtual const ClassFileStream* clone() const;
 
@@ -77,6 +78,7 @@
   const char* source() const { return _source; }
   bool need_verify() const { return _need_verify; }
   void set_verify(bool flag) { _need_verify = flag; }
+  bool from_boot_loader_modules_image() const { return _from_boot_loader_modules_image; }
 
   void check_truncated_file(bool b, TRAPS) const {
     if (b) {
--- a/src/hotspot/share/classfile/classLoader.cpp	Sat Mar 23 17:18:49 2019 +0100
+++ b/src/hotspot/share/classfile/classLoader.cpp	Sat Mar 23 21:51:07 2019 -0700
@@ -361,6 +361,8 @@
   }
 }
 
+DEBUG_ONLY(ClassPathImageEntry* ClassPathImageEntry::_singleton = NULL;)
+
 void ClassPathImageEntry::close_jimage() {
   if (_jimage != NULL) {
     (*JImageClose)(_jimage);
@@ -373,12 +375,17 @@
   _jimage(jimage) {
   guarantee(jimage != NULL, "jimage file is null");
   guarantee(name != NULL, "jimage file name is null");
+  assert(_singleton == NULL, "VM supports only one jimage");
+  DEBUG_ONLY(_singleton = this);
   size_t len = strlen(name) + 1;
   _name = NEW_C_HEAP_ARRAY(const char, len, mtClass);
   strncpy((char *)_name, name, len);
 }
 
 ClassPathImageEntry::~ClassPathImageEntry() {
+  assert(_singleton == this, "must be");
+  DEBUG_ONLY(_singleton = NULL);
+
   if (_name != NULL) {
     FREE_C_HEAP_ARRAY(const char, _name);
     _name = NULL;
@@ -442,10 +449,12 @@
     char* data = NEW_RESOURCE_ARRAY(char, size);
     (*JImageGetResource)(_jimage, location, data, size);
     // Resource allocated
+    assert(this == (ClassPathImageEntry*)ClassLoader::get_jrt_entry(), "must be");
     return new ClassFileStream((u1*)data,
                                (int)size,
                                _name,
-                               ClassFileStream::verify);
+                               ClassFileStream::verify,
+                               true); // from_boot_loader_modules_image
   }
 
   return NULL;
@@ -459,7 +468,9 @@
 }
 
 bool ClassPathImageEntry::is_modules_image() const {
-  return ClassLoader::is_modules_image(name());
+  assert(this == _singleton, "VM supports a single jimage");
+  assert(this == (ClassPathImageEntry*)ClassLoader::get_jrt_entry(), "must be used for jrt entry");
+  return true;
 }
 
 #if INCLUDE_CDS
@@ -737,8 +748,8 @@
         // Check for a jimage
         if (Arguments::has_jimage()) {
           assert(_jrt_entry == NULL, "should not setup bootstrap class search path twice");
+          _jrt_entry = new_entry;
           assert(new_entry != NULL && new_entry->is_modules_image(), "No java runtime image present");
-          _jrt_entry = new_entry;
           assert(_jrt_entry->jimage() != NULL, "No java runtime image");
         }
       } else {
@@ -1499,7 +1510,7 @@
       }
       // for index 0 and the stream->source() is the modules image or has the jrt: protocol.
       // The class must be from the runtime modules image.
-      if (i == 0 && (is_modules_image(src) || string_starts_with(src, "jrt:"))) {
+      if (i == 0 && (stream->from_boot_loader_modules_image() || string_starts_with(src, "jrt:"))) {
         classpath_index = i;
         break;
       }
@@ -1515,7 +1526,7 @@
     // The shared path table is set up after module system initialization.
     // The path table contains no entry before that. Any classes loaded prior
     // to the setup of the shared path table must be from the modules image.
-    assert(is_modules_image(src), "stream must be from modules image");
+    assert(stream->from_boot_loader_modules_image(), "stream must be loaded by boot loader from modules image");
     assert(FileMapInfo::get_number_of_shared_paths() == 0, "shared path table must not have been setup");
     classpath_index = 0;
   }
--- a/src/hotspot/share/classfile/classLoader.hpp	Sat Mar 23 17:18:49 2019 +0100
+++ b/src/hotspot/share/classfile/classLoader.hpp	Sat Mar 23 21:51:07 2019 -0700
@@ -114,6 +114,7 @@
 private:
   JImageFile* _jimage;
   const char* _name;
+  DEBUG_ONLY(static ClassPathImageEntry* _singleton;)
 public:
   bool is_modules_image() const;
   bool is_jar_file() const { return false; }
@@ -439,8 +440,6 @@
   // distinguish from a class_name with no package name, as both cases have a NULL return value
   static const char* package_from_name(const char* const class_name, bool* bad_class_name = NULL);
 
-  static bool is_modules_image(const char* name) { return string_ends_with(name, MODULES_IMAGE_NAME); }
-
   // Debugging
   static void verify()              PRODUCT_RETURN;
 };
--- a/src/hotspot/share/oops/instanceKlass.cpp	Sat Mar 23 17:18:49 2019 +0100
+++ b/src/hotspot/share/oops/instanceKlass.cpp	Sat Mar 23 21:51:07 2019 -0700
@@ -3367,7 +3367,9 @@
   if (cfs != NULL) {
     if (cfs->source() != NULL) {
       if (module_name != NULL) {
-        if (ClassLoader::is_modules_image(cfs->source())) {
+        // When the boot loader created the stream, it didn't know the module name
+        // yet. Let's format it now.
+        if (cfs->from_boot_loader_modules_image()) {
           info_stream.print(" source: jrt:/%s", module_name);
         } else {
           info_stream.print(" source: %s", cfs->source());
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/test/hotspot/jtreg/runtime/modules/ModulesSymLink.java	Sat Mar 23 21:51:07 2019 -0700
@@ -0,0 +1,79 @@
+/*
+ * Copyright (c) 2019, Google Inc. 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.  Oracle designates this
+ * particular file as subject to the "Classpath" exception as provided
+ * by Oracle in the LICENSE file that accompanied this code.
+ *
+ * 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 Test with symbolic linked lib/modules
+ * @bug 8220095
+ * @requires (os.family == "solaris" | os.family == "linux" | os.family == "mac")
+ * @library /test/lib
+ * @modules java.management
+ *          jdk.jlink
+ * @run driver ModulesSymLink
+ */
+
+import java.io.File;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.nio.file.Paths;
+import jdk.test.lib.process.ProcessTools;
+import jdk.test.lib.process.OutputAnalyzer;
+
+public class ModulesSymLink {
+    static String java_home;
+    static String test_jdk;
+
+    public static void main(String[] args) throws Throwable {
+        java_home = System.getProperty("java.home");
+        test_jdk = System.getProperty("user.dir") + File.separator +
+                   "modulessymlink_jdk" + Long.toString(System.currentTimeMillis());
+
+        constructTestJDK();
+
+        ProcessBuilder pb = new ProcessBuilder(
+            test_jdk + File.separator + "bin" + File.separator + "java",
+            "-version");
+        OutputAnalyzer out = new OutputAnalyzer(pb.start());
+        out.shouldHaveExitValue(0);
+    }
+
+    // 1) Create a test JDK binary (jlink is used to help simplify the process,
+    //    alternatively a test JDK could be copied from JAVA_HOME.)
+    // 2) Rename the test JDK's lib/modules to lib/0.
+    // 3) Then create a link to lib/0 as lib/modules.
+    static void constructTestJDK() throws Throwable {
+        Path jlink = Paths.get(java_home, "bin", "jlink");
+        System.out.println("Jlink = " + jlink);
+        OutputAnalyzer out = ProcessTools.executeProcess(jlink.toString(),
+                  "--output", test_jdk,
+                  "--add-modules", "java.base");
+        out.shouldHaveExitValue(0);
+
+        Path modules = Paths.get(test_jdk, "lib", "modules");
+        Path renamed_modules = Paths.get(test_jdk, "lib", "0");
+        Files.move(modules, renamed_modules);
+        Files.createSymbolicLink(modules, renamed_modules);
+    }
+}