8168797: do not load any archived classes from a patched module
authorjiangli
Mon, 19 Dec 2016 13:54:33 -0500
changeset 42876 ff8ff9dcccec
parent 42875 bac62054c0b6
child 42877 6cbcb55d5232
child 42878 b9ba1d1ec877
8168797: do not load any archived classes from a patched module Summary: Add new runtime shared class visibility check to ensure shared classes from patched module are not loaded at runtime. Reviewed-by: acorn, ccheung, hseigel, iklam, lfoltan
hotspot/src/share/vm/classfile/classLoader.cpp
hotspot/src/share/vm/classfile/classLoader.hpp
hotspot/src/share/vm/classfile/sharedPathsMiscInfo.cpp
hotspot/src/share/vm/classfile/sharedPathsMiscInfo.hpp
hotspot/src/share/vm/classfile/systemDictionary.cpp
hotspot/src/share/vm/memory/filemap.cpp
hotspot/src/share/vm/memory/filemap.hpp
hotspot/src/share/vm/runtime/arguments.cpp
hotspot/test/runtime/modules/PatchModule/PatchModuleCDS.java
--- a/hotspot/src/share/vm/classfile/classLoader.cpp	Mon Dec 19 15:21:11 2016 +0000
+++ b/hotspot/src/share/vm/classfile/classLoader.cpp	Mon Dec 19 13:54:33 2016 -0500
@@ -84,7 +84,6 @@
 typedef jzentry* (JNICALL *GetNextEntry_t)(jzfile *zip, jint n);
 typedef jboolean (JNICALL *ZipInflateFully_t)(void *inBuf, jlong inLen, void *outBuf, jlong outLen, char **pmsg);
 typedef jint     (JNICALL *Crc32_t)(jint crc, const jbyte *buf, jint len);
-typedef void     (JNICALL *FreeEntry_t)(jzfile *zip, jzentry *entry);
 
 static ZipOpen_t         ZipOpen            = NULL;
 static ZipClose_t        ZipClose           = NULL;
@@ -94,7 +93,6 @@
 static canonicalize_fn_t CanonicalizeEntry  = NULL;
 static ZipInflateFully_t ZipInflateFully    = NULL;
 static Crc32_t           Crc32              = NULL;
-static FreeEntry_t       FreeEntry          = NULL;
 
 // Entry points for jimage.dll for loading jimage file entries
 
@@ -150,7 +148,6 @@
 GrowableArray<char*>* ClassLoader::_boot_modules_array = NULL;
 GrowableArray<char*>* ClassLoader::_platform_modules_array = NULL;
 SharedPathsMiscInfo* ClassLoader::_shared_paths_misc_info = NULL;
-int  ClassLoader::_num_patch_mod_prefixes = 0;
 #endif
 
 // helper routines
@@ -320,20 +317,6 @@
   FREE_C_HEAP_ARRAY(char, _zip_name);
 }
 
-bool ClassPathZipEntry::stream_exists(const char* name) {
-  // enable call to C land
-  JavaThread* thread = JavaThread::current();
-  ThreadToNativeFromVM ttn(thread);
-  // check whether zip archive contains name
-  jint name_len, filesize;
-  jzentry* entry = (*FindEntry)(_zip, name, &filesize, &name_len);
-  if (entry != NULL) {
-    (*FreeEntry)(_zip, entry);
-    return true;
-  }
-  return false;
-}
-
 u1* ClassPathZipEntry::open_entry(const char* name, jint* filesize, bool nul_terminate, TRAPS) {
     // enable call to C land
   JavaThread* thread = JavaThread::current();
@@ -1090,7 +1073,6 @@
   GetNextEntry = CAST_TO_FN_PTR(GetNextEntry_t, os::dll_lookup(handle, "ZIP_GetNextEntry"));
   ZipInflateFully = CAST_TO_FN_PTR(ZipInflateFully_t, os::dll_lookup(handle, "ZIP_InflateFully"));
   Crc32        = CAST_TO_FN_PTR(Crc32_t, os::dll_lookup(handle, "ZIP_CRC32"));
-  FreeEntry    = CAST_TO_FN_PTR(FreeEntry_t, os::dll_lookup(handle, "ZIP_FreeEntry"));
 
   // ZIP_Close is not exported on Windows in JDK5.0 so don't abort if ZIP_Close is NULL
   if (ZipOpen == NULL || FindEntry == NULL || ReadEntry == NULL ||
@@ -1418,57 +1400,6 @@
   return NULL;
 }
 
-#if INCLUDE_CDS
-// The following function is only used during CDS dump time.
-// It checks if a class can be found in the jar entries of the _patch_mod_entries.
-// It does not support non-jar entries.
-bool ClassLoader::is_in_patch_module(const char* const file_name) {
-  assert(DumpSharedSpaces, "dump time only");
-  if (_patch_mod_entries == NULL) {
-    return false;
-  }
-
-  int num_of_entries = _patch_mod_entries->length();
-  char* class_module_name = NULL;
-  ResourceMark rm;
-  const char *pkg_name = package_from_name(file_name);
-  // Using the jimage to obtain the class' module name.
-  // The ModuleEntryTable cannot be used at this point during dump time
-  // because the module system hasn't been initialized yet.
-  if (pkg_name != NULL) {
-    JImageFile *jimage = _jrt_entry->jimage();
-    class_module_name = (char*)(*JImagePackageToModule)(jimage, pkg_name);
-  }
-
-  if (class_module_name == NULL) {
-    return false;
-  }
-
-  // Loop through all the patch module entries looking for module
-  for (int i = 0; i < num_of_entries; i++) {
-    ModuleClassPathList* module_cpl = _patch_mod_entries->at(i);
-    Symbol* module_cpl_name = module_cpl->module_name();
-
-    if (strcmp(module_cpl_name->as_C_string(), class_module_name) == 0) {
-      // Class' module has been located, attempt to locate
-      // the class from the module's ClassPathEntry list.
-      ClassPathEntry* e = module_cpl->module_first_entry();
-      while (e != NULL) {
-        if (e->is_jar_file()) {
-          if (e->stream_exists(file_name)) {
-            return true;
-          } else {
-            e = e->next();
-          }
-        }
-      }
-    }
-  }
-
-  return false;
-}
-#endif // INCLUDE_CDS
-
 instanceKlassHandle ClassLoader::load_class(Symbol* name, bool search_append_only, TRAPS) {
   assert(name != NULL, "invariant");
   assert(THREAD->is_Java_thread(), "must be a JavaThread");
@@ -1494,8 +1425,6 @@
 
   // If DumpSharedSpaces is true boot loader visibility boundaries are set to:
   //   - [jimage] + [_first_append_entry to _last_append_entry] (all path entries).
-  // If a class is found in the --patch-module entries, the class will not be included in the
-  // CDS archive. Also, CDS is not supported if exploded module builds are used.
   //
   // If search_append_only is true, boot loader visibility boundaries are
   // set to be _first_append_entry to the end. This includes:
@@ -1519,15 +1448,13 @@
   // Note: The --patch-module entries are never searched if the boot loader's
   //       visibility boundary is limited to only searching the append entries.
   if (_patch_mod_entries != NULL && !search_append_only) {
+    // At CDS dump time, the --patch-module entries are ignored. That means a
+    // class is still loaded from the runtime image even if it might
+    // appear in the _patch_mod_entries. The runtime shared class visibility
+    // check will determine if a shared class is visible based on the runtime
+    // environemnt, including the runtime --patch-module setting.
     if (!DumpSharedSpaces) {
       stream = search_module_entries(_patch_mod_entries, class_name, file_name, CHECK_NULL);
-    } else {
-#if INCLUDE_CDS
-      if (is_in_patch_module(file_name)) {
-        tty->print_cr("Preload Warning: Skip archiving class %s found in --patch-module entry", class_name);
-        return NULL;
-      }
-#endif
     }
   }
 
@@ -1679,57 +1606,8 @@
 }
 
 #if INCLUDE_CDS
-// Capture all the --patch-module entries specified during CDS dump time.
-// It also captures the non-existing path(s) and the required file(s) during inspecting
-// the entries.
-void ClassLoader::setup_patch_mod_path() {
-  assert(DumpSharedSpaces, "only used with -Xshare:dump");
-  ResourceMark rm;
-  GrowableArray<ModulePatchPath*>* patch_mod_args = Arguments::get_patch_mod_prefix();
-  if (patch_mod_args != NULL) {
-    int num_of_entries = patch_mod_args->length();
-    for (int i = 0; i < num_of_entries; i++) {
-      const char* module_name = (patch_mod_args->at(i))->module_name();
-      const char* module_path = (patch_mod_args->at(i))->path_string();
-      int path_len = (int)strlen(module_path);
-      int name_len = (int)strlen(module_name);
-      int buf_len = name_len + path_len + 2; // add 2 for the '=' and NULL terminator
-      int end = 0;
-      char* buf = NEW_C_HEAP_ARRAY(char, buf_len, mtInternal);
-      // Iterate over the module's class path entries
-      for (int start = 0; start < path_len; start = end) {
-        while (module_path[end] && module_path[end] != os::path_separator()[0]) {
-          end++;
-        }
-        strncpy(buf, &module_path[start], end - start);
-        buf[end - start] = '\0';
-        struct stat st;
-        if (os::stat(buf, &st) != 0) {
-          // File not found
-          _shared_paths_misc_info->add_nonexist_path(buf);
-        } else {
-          if ((st.st_mode & S_IFMT) != S_IFREG) { // is not a regular file
-            vm_exit_during_initialization(
-              "--patch-module requires a regular file during dumping", buf);
-          } else {
-            _shared_paths_misc_info->add_required_file(buf);
-          }
-        }
-        while (module_path[end] == os::path_separator()[0]) {
-          end++;
-        }
-      };
-      jio_snprintf(buf, buf_len, "%s=%s", module_name, module_path);
-      _shared_paths_misc_info->add_patch_mod_classpath((const char*)buf);
-      _num_patch_mod_prefixes++;
-      FREE_C_HEAP_ARRAY(char, buf);
-    }
-  }
-}
-
 void ClassLoader::initialize_shared_path() {
   if (DumpSharedSpaces) {
-    setup_patch_mod_path();
     ClassLoaderExt::setup_search_paths();
     _shared_paths_misc_info->write_jint(0); // see comments in SharedPathsMiscInfo::check()
   }
--- a/hotspot/src/share/vm/classfile/classLoader.hpp	Mon Dec 19 15:21:11 2016 +0000
+++ b/hotspot/src/share/vm/classfile/classLoader.hpp	Mon Dec 19 13:54:33 2016 -0500
@@ -69,7 +69,6 @@
   // 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;
-  virtual bool stream_exists(const char* name) = 0;
   // Debugging
   NOT_PRODUCT(virtual void compile_the_world(Handle loader, TRAPS) = 0;)
 };
@@ -84,7 +83,6 @@
   JImageFile* jimage() const { return NULL; }
   ClassPathDirEntry(const char* dir);
   ClassFileStream* open_stream(const char* name, TRAPS);
-  bool stream_exists(const char* name) { return false; }
   // Debugging
   NOT_PRODUCT(void compile_the_world(Handle loader, TRAPS);)
 };
@@ -128,7 +126,6 @@
   ClassFileStream* open_stream(const char* name, TRAPS);
   void contents_do(void f(const char* name, void* context), void* context);
   bool is_multiple_versioned(TRAPS) NOT_CDS_RETURN_(false);
-  bool stream_exists(const char* name);
   // Debugging
   NOT_PRODUCT(void compile_the_world(Handle loader, TRAPS);)
 };
@@ -148,7 +145,6 @@
   ClassPathImageEntry(JImageFile* jimage, const char* name);
   ~ClassPathImageEntry();
   ClassFileStream* open_stream(const char* name, TRAPS);
-  bool stream_exists(const char* name) { return false; }
 
   // Debugging
   NOT_PRODUCT(void compile_the_world(Handle loader, TRAPS);)
@@ -259,7 +255,6 @@
 
   // Info used by CDS
   CDS_ONLY(static SharedPathsMiscInfo * _shared_paths_misc_info;)
-  CDS_ONLY(static int _num_patch_mod_prefixes;)
 
   // Initialization:
   //   - setup the boot loader's system class path
@@ -434,9 +429,6 @@
   static void initialize_module_loader_map(JImageFile* jimage);
   static s2 classloader_type(Symbol* class_name, ClassPathEntry* e,
                              int classpath_index, TRAPS);
-  static bool is_in_patch_module(const char* const file_name);
-  static void setup_patch_mod_path(); // Only when -Xshare:dump
-  static int num_patch_mod_prefixes() { return _num_patch_mod_prefixes; }
 #endif
 
   static void  trace_class_path(const char* msg, const char* name = NULL);
--- a/hotspot/src/share/vm/classfile/sharedPathsMiscInfo.cpp	Mon Dec 19 15:21:11 2016 +0000
+++ b/hotspot/src/share/vm/classfile/sharedPathsMiscInfo.cpp	Mon Dec 19 13:54:33 2016 -0500
@@ -86,9 +86,6 @@
   case REQUIRED:
     out->print("Expecting that file %s must exist and is not altered", path);
     break;
-  case PATCH_MOD:
-    out->print("Expecting --patch-module=%s", path);
-    break;
   default:
     ShouldNotReachHere();
   }
@@ -167,26 +164,6 @@
       }
     }
     break;
-  case PATCH_MOD:
-    {
-      GrowableArray<ModulePatchPath*>* patch_mod_args = Arguments::get_patch_mod_prefix();
-      if (patch_mod_args != NULL) {
-        int num_of_entries = patch_mod_args->length();
-        for (int i = 0; i < num_of_entries; i++) {
-          const char* module_name = (patch_mod_args->at(i))->module_name();
-          const char* path_string = (patch_mod_args->at(i))->path_string();
-          size_t n = strlen(module_name);
-          // path contains the module name, followed by '=', and one or more entries.
-          // E.g.: "java.base=foo" or "java.naming=dir1:dir2:dir3"
-          if ((strncmp(module_name, path, n) != 0) ||
-              (path[n] != '=') ||
-              (strcmp(path + n + 1, path_string) != 0)) {
-            return fail("--patch-module mismatch, path not found in run time: ", path);
-          }
-        }
-      }
-    }
-    break;
   default:
     return fail("Corrupted archive file header");
   }
--- a/hotspot/src/share/vm/classfile/sharedPathsMiscInfo.hpp	Mon Dec 19 15:21:11 2016 +0000
+++ b/hotspot/src/share/vm/classfile/sharedPathsMiscInfo.hpp	Mon Dec 19 13:54:33 2016 -0500
@@ -104,28 +104,10 @@
     add_path(path, NON_EXIST);
   }
 
-  // The path must exist and have required size and modification time
-  void add_required_file(const char* path) {
-    add_path(path, REQUIRED);
-
-    struct stat st;
-    if (os::stat(path, &st) != 0) {
-      assert(0, "sanity");
-#if INCLUDE_CDS
-      ClassLoader::exit_with_path_failure("failed to os::stat(%s)", path); // should not happen
-#endif
-    }
-    write_time(st.st_mtime);
-    write_long(st.st_size);
-  }
-
   // The path must exist, and must contain exactly <num_entries> files/dirs
   void add_boot_classpath(const char* path) {
     add_path(path, BOOT);
   }
-  void add_patch_mod_classpath(const char* path) {
-    add_path(path, PATCH_MOD);
-  }
   int write_jint(jint num) {
     write(&num, sizeof(num));
     return 0;
@@ -147,8 +129,7 @@
   enum {
     BOOT      = 1,
     NON_EXIST = 2,
-    REQUIRED  = 3,
-    PATCH_MOD = 4
+    REQUIRED  = 3
   };
 
   virtual const char* type_name(int type) {
@@ -156,7 +137,6 @@
     case BOOT:      return "BOOT";
     case NON_EXIST: return "NON_EXIST";
     case REQUIRED:  return "REQUIRED";
-    case PATCH_MOD: return "PATCH_MOD";
     default:        ShouldNotReachHere(); return "?";
     }
   }
--- a/hotspot/src/share/vm/classfile/systemDictionary.cpp	Mon Dec 19 15:21:11 2016 +0000
+++ b/hotspot/src/share/vm/classfile/systemDictionary.cpp	Mon Dec 19 13:54:33 2016 -0500
@@ -1231,6 +1231,8 @@
 bool SystemDictionary::is_shared_class_visible(Symbol* class_name,
                                                instanceKlassHandle ik,
                                                Handle class_loader, TRAPS) {
+  assert(!ModuleEntryTable::javabase_moduleEntry()->is_patched(),
+         "Cannot use sharing if java.base is patched");
   ResourceMark rm;
   int path_index = ik->shared_classpath_index();
   SharedClassPathEntry* ent =
@@ -1258,6 +1260,12 @@
     }
   }
 
+  // If the archived class is from a module that has been patched at runtime,
+  // the class cannot be loaded from the archive.
+  if (mod_entry != NULL && mod_entry->is_patched()) {
+    return false;
+  }
+
   if (class_loader.is_null()) {
     assert(ent != NULL, "Shared class for NULL classloader must have valid SharedClassPathEntry");
     // The NULL classloader can load archived class originated from the
--- a/hotspot/src/share/vm/memory/filemap.cpp	Mon Dec 19 15:21:11 2016 +0000
+++ b/hotspot/src/share/vm/memory/filemap.cpp	Mon Dec 19 13:54:33 2016 -0500
@@ -179,7 +179,6 @@
   _classpath_entry_table_size = mapinfo->_classpath_entry_table_size;
   _classpath_entry_table = mapinfo->_classpath_entry_table;
   _classpath_entry_size = mapinfo->_classpath_entry_size;
-  _num_patch_mod_prefixes = ClassLoader::num_patch_mod_prefixes();
 
   // The following fields are for sanity checks for whether this archive
   // will function correctly with this JVM and the bootclasspath it's
@@ -948,23 +947,6 @@
     return false;
   }
 
-  // Check if there is a mismatch in --patch-module entry counts between dump time and run time.
-  // More checks will be performed on individual --patch-module entry in the
-  // SharedPathsMiscInfo::check() function.
-  GrowableArray<ModulePatchPath*>* patch_mod_args = Arguments::get_patch_mod_prefix();
-  if (patch_mod_args != NULL) {
-    if (_num_patch_mod_prefixes == 0) {
-      FileMapInfo::fail_stop("--patch-module found in run time but none was specified in dump time");
-    }
-    if (patch_mod_args->length() != _num_patch_mod_prefixes) {
-      FileMapInfo::fail_stop("mismatched --patch-module entry counts between dump time and run time");
-    }
-  } else {
-    if (_num_patch_mod_prefixes > 0) {
-      FileMapInfo::fail_stop("--patch-module specified in dump time but none was specified in run time");
-    }
-  }
-
   return true;
 }
 
--- a/hotspot/src/share/vm/memory/filemap.hpp	Mon Dec 19 15:21:11 2016 +0000
+++ b/hotspot/src/share/vm/memory/filemap.hpp	Mon Dec 19 13:54:33 2016 -0500
@@ -155,7 +155,6 @@
     // loading failures during runtime.
     int _classpath_entry_table_size;
     size_t _classpath_entry_size;
-    int    _num_patch_mod_prefixes;   // number of --patch-module entries
     SharedClassPathEntry* _classpath_entry_table;
 
     char* region_addr(int idx);
--- a/hotspot/src/share/vm/runtime/arguments.cpp	Mon Dec 19 15:21:11 2016 +0000
+++ b/hotspot/src/share/vm/runtime/arguments.cpp	Mon Dec 19 13:54:33 2016 -0500
@@ -1324,12 +1324,15 @@
                                            "jdk.module.limitmods",
                                            "jdk.module.path",
                                            "jdk.module.upgrade.path",
-                                           "jdk.module.addmods.0" };
-  const char* unsupported_options[] = { "-m",
-                                        "--limit-modules",
-                                        "--module-path",
-                                        "--upgrade-module-path",
-                                        "--add-modules" };
+                                           "jdk.module.addmods.0",
+                                           "jdk.module.patch.0" };
+  const char* unsupported_options[] = { "-m", // cannot use at dump time
+                                        "--limit-modules", // cannot use at dump time
+                                        "--module-path", // ignored at dump time
+                                        "--upgrade-module-path", // ignored at dump time
+                                        "--add-modules", // ignored at dump time
+                                        "--patch-module" // ignored at dump time
+                                      };
   assert(ARRAY_SIZE(unsupported_properties) == ARRAY_SIZE(unsupported_options), "must be");
   // If a vm option is found in the unsupported_options array with index less than the warning_idx,
   // vm will exit with an error message. Otherwise, it will result in a warning message.
@@ -1420,10 +1423,8 @@
   }
 }
 
-#if defined(COMPILER2) || INCLUDE_JVMCI || defined(_LP64) || !INCLUDE_CDS
 // Conflict: required to use shared spaces (-Xshare:on), but
 // incompatible command line options were chosen.
-
 static void no_shared_spaces(const char* message) {
   if (RequireSharedSpaces) {
     jio_fprintf(defaultStream::error_stream(),
@@ -1433,7 +1434,6 @@
     FLAG_SET_DEFAULT(UseSharedSpaces, false);
   }
 }
-#endif
 
 // Returns threshold scaled with the value of scale.
 // If scale < 0.0, threshold is returned without scaling.
@@ -2682,6 +2682,12 @@
     return result;
   }
 
+#if INCLUDE_CDS
+  if (UseSharedSpaces && patch_mod_javabase) {
+    no_shared_spaces("CDS is disabled when " JAVA_BASE_NAME " module is patched.");
+  }
+#endif
+
   return JNI_OK;
 }
 
@@ -4410,7 +4416,6 @@
 }
 
 jint Arguments::apply_ergo() {
-
   // Set flags based on ergonomics.
   set_ergonomics_flags();
 
--- a/hotspot/test/runtime/modules/PatchModule/PatchModuleCDS.java	Mon Dec 19 15:21:11 2016 +0000
+++ b/hotspot/test/runtime/modules/PatchModule/PatchModuleCDS.java	Mon Dec 19 13:54:33 2016 -0500
@@ -52,7 +52,7 @@
         new OutputAnalyzer(pb.start())
             .shouldContain("ro space:"); // Make sure archive got created.
 
-       // Case 2: Test that only jar file in --patch-module is supported for CDS dumping
+        // Case 2: Test that directory in --patch-module is supported for CDS dumping
         // Create a class file in the module java.base.
         String source = "package javax.naming.spi; "                +
                         "public class NamingManager { "             +
@@ -73,7 +73,7 @@
             "-Xlog:class+path=info",
             "-version");
         new OutputAnalyzer(pb.start())
-            .shouldContain("--patch-module requires a regular file during dumping");
+            .shouldContain("ro space:"); // Make sure archive got created.
 
         // Case 3a: Test CDS dumping with jar file in --patch-module
         BasicJarBuilder.build("javanaming", "javax/naming/spi/NamingManager");