8164011: --patch-module support for CDS
authorccheung
Tue, 20 Sep 2016 10:37:19 -0700
changeset 41281 e1dc38ba642f
parent 41279 5a7c83da4a2d
child 41282 474076f73ba1
8164011: --patch-module support for CDS Summary: allows the use of the --patch-module vm option with CDS. However, classes found in --patch-module during dump time will not be archived. Reviewed-by: iklam, dcubed, 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/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	Tue Sep 20 11:41:43 2016 +0200
+++ b/hotspot/src/share/vm/classfile/classLoader.cpp	Tue Sep 20 10:37:19 2016 -0700
@@ -85,6 +85,7 @@
 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;
@@ -95,6 +96,7 @@
 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,6 +152,7 @@
 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
@@ -319,6 +322,20 @@
   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();
@@ -640,7 +657,7 @@
 
   struct stat st;
   if (os::stat(path, &st) == 0) {
-    if ((st.st_mode & S_IFREG) != S_IFREG) { // is directory
+    if ((st.st_mode & S_IFMT) != S_IFREG) { // is not a regular file
       if (!os::dir_is_empty(path)) {
         tty->print_cr("Error: non-empty directory '%s'", path);
         exit_with_path_failure("CDS allows only empty directories in archived classpaths", NULL);
@@ -693,8 +710,6 @@
   GrowableArray<ModulePatchPath*>* patch_mod_args = Arguments::get_patch_mod_prefix();
   int num_of_entries = patch_mod_args->length();
 
-  assert(!DumpSharedSpaces, "DumpSharedSpaces not supported with --patch-module");
-  assert(!UseSharedSpaces, "UseSharedSpaces not supported with --patch-module");
 
   // Set up the boot loader's _patch_mod_entries list
   _patch_mod_entries = new (ResourceObj::C_HEAP, mtModule) GrowableArray<ModuleClassPathList*>(num_of_entries, true);
@@ -851,7 +866,7 @@
                                                      bool is_boot_append, TRAPS) {
   JavaThread* thread = JavaThread::current();
   ClassPathEntry* new_entry = NULL;
-  if ((st->st_mode & S_IFREG) == S_IFREG) {
+  if ((st->st_mode & S_IFMT) == S_IFREG) {
     ResourceMark rm(thread);
     // Regular file, should be a zip or jimage file
     // Canonicalized filename
@@ -914,7 +929,7 @@
   // check for a regular file
   struct stat st;
   if (os::stat(path, &st) == 0) {
-    if ((st.st_mode & S_IFREG) == S_IFREG) {
+    if ((st.st_mode & S_IFMT) == S_IFREG) {
       char canonical_path[JVM_MAXPATHLEN];
       if (get_canonical_path(path, canonical_path, JVM_MAXPATHLEN)) {
         char* error_msg = NULL;
@@ -1068,6 +1083,7 @@
   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 ||
@@ -1395,6 +1411,57 @@
   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");
@@ -1420,8 +1487,8 @@
 
   // If DumpSharedSpaces is true boot loader visibility boundaries are set to:
   //   - [jimage] + [_first_append_entry to _last_append_entry] (all path entries).
-  // No --patch-module entries or exploded module builds are included since CDS
-  // is not supported if --patch-module or exploded module builds are used.
+  // 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:
@@ -1444,8 +1511,17 @@
   // found within its module specification, the search should continue to Load Attempt #2.
   // 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 && !DumpSharedSpaces) {
-    stream = search_module_entries(_patch_mod_entries, class_name, file_name, CHECK_NULL);
+  if (_patch_mod_entries != NULL && !search_append_only) {
+    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
+    }
   }
 
   // Load Attempt #2: [jimage | exploded build]
@@ -1596,8 +1672,57 @@
 }
 
 #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	Tue Sep 20 11:41:43 2016 +0200
+++ b/hotspot/src/share/vm/classfile/classLoader.hpp	Tue Sep 20 10:37:19 2016 -0700
@@ -69,6 +69,7 @@
   // 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;)
 };
@@ -83,6 +84,7 @@
   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);)
 };
@@ -126,6 +128,7 @@
   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);)
 };
@@ -145,6 +148,7 @@
   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);)
@@ -255,6 +259,7 @@
 
   // 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
@@ -427,6 +432,9 @@
   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	Tue Sep 20 11:41:43 2016 +0200
+++ b/hotspot/src/share/vm/classfile/sharedPathsMiscInfo.cpp	Tue Sep 20 10:37:19 2016 -0700
@@ -86,6 +86,9 @@
   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();
   }
@@ -146,6 +149,9 @@
           // But we want it to not exist -> fail
           return fail("File must not exist");
         }
+        if ((st.st_mode & S_IFMT) != S_IFREG) {
+          return fail("Did not get a regular file as expected.");
+        }
         time_t    timestamp;
         long      filesize;
 
@@ -161,7 +167,26 @@
       }
     }
     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	Tue Sep 20 11:41:43 2016 +0200
+++ b/hotspot/src/share/vm/classfile/sharedPathsMiscInfo.hpp	Tue Sep 20 10:37:19 2016 -0700
@@ -104,10 +104,28 @@
     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;
@@ -129,7 +147,8 @@
   enum {
     BOOT      = 1,
     NON_EXIST = 2,
-    REQUIRED  = 3
+    REQUIRED  = 3,
+    PATCH_MOD = 4
   };
 
   virtual const char* type_name(int type) {
@@ -137,6 +156,7 @@
     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/memory/filemap.cpp	Tue Sep 20 11:41:43 2016 +0200
+++ b/hotspot/src/share/vm/memory/filemap.cpp	Tue Sep 20 10:37:19 2016 -0700
@@ -179,6 +179,7 @@
   _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
@@ -911,11 +912,6 @@
     return false;
   }
 
-  if (Arguments::get_patch_mod_prefix() != NULL) {
-    FileMapInfo::fail_continue("The shared archive file cannot be used with --patch-module.");
-    return false;
-  }
-
   if (!Arguments::has_jimage()) {
     FileMapInfo::fail_continue("The shared archive file cannot be used with an exploded module build.");
     return false;
@@ -952,6 +948,23 @@
     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	Tue Sep 20 11:41:43 2016 +0200
+++ b/hotspot/src/share/vm/memory/filemap.hpp	Tue Sep 20 10:37:19 2016 -0700
@@ -155,6 +155,7 @@
     // 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	Tue Sep 20 11:41:43 2016 +0200
+++ b/hotspot/src/share/vm/runtime/arguments.cpp	Tue Sep 20 10:37:19 2016 -0700
@@ -3897,10 +3897,6 @@
 
 void Arguments::set_shared_spaces_flags() {
   if (DumpSharedSpaces) {
-    if (Arguments::get_patch_mod_prefix() != NULL) {
-      vm_exit_during_initialization(
-        "Cannot use the following option when dumping the shared archive: --patch-module");
-    }
 
     if (RequireSharedSpaces) {
       warning("Cannot dump shared archive while using shared archive");
--- a/hotspot/test/runtime/modules/PatchModule/PatchModuleCDS.java	Tue Sep 20 11:41:43 2016 +0200
+++ b/hotspot/test/runtime/modules/PatchModule/PatchModuleCDS.java	Tue Sep 20 10:37:19 2016 -0700
@@ -23,41 +23,83 @@
 
 /*
  * @test
+ * @summary test that --patch-module works with CDS
  * @library /test/lib
  * @modules java.base/jdk.internal.misc
+ *          jdk.jartool/sun.tools.jar
+ * @build PatchModuleMain
  * @run main PatchModuleCDS
  */
 
 import java.io.File;
+import jdk.test.lib.InMemoryJavaCompiler;
 import jdk.test.lib.process.OutputAnalyzer;
 import jdk.test.lib.process.ProcessTools;
 
 public class PatchModuleCDS {
 
     public static void main(String args[]) throws Throwable {
-        System.out.println("Test that --patch-module and -Xshare:dump are incompatibable");
-        ProcessBuilder pb = ProcessTools.createJavaProcessBuilder("--patch-module=java.naming=mods/java.naming", "-Xshare:dump");
-        OutputAnalyzer output = new OutputAnalyzer(pb.start());
-        output.shouldContain("Cannot use the following option when dumping the shared archive: --patch-module");
 
-        System.out.println("Test that --patch-module and -Xshare:on are incompatibable");
+        // Case 1: Test that --patch-module and -Xshare:dump are compatible
         String filename = "patch_module.jsa";
-        pb = ProcessTools.createJavaProcessBuilder(
+        ProcessBuilder pb = ProcessTools.createJavaProcessBuilder(
             "-XX:+UnlockDiagnosticVMOptions",
             "-XX:SharedArchiveFile=" + filename,
-            "-Xshare:dump");
-        output = new OutputAnalyzer(pb.start());
-        output.shouldContain("ro space:"); // Make sure archive got created.
+            "-Xshare:dump",
+            "--patch-module=java.naming=no/such/directory",
+            "-Xlog:class+path=info",
+            "-version");
+        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
+        // Create a class file in the module java.base.
+        String source = "package javax.naming.spi; "                +
+                        "public class NamingManager { "             +
+                        "    static { "                             +
+                        "        System.out.println(\"I pass!\"); " +
+                        "    } "                                    +
+                        "}";
+
+        ClassFileInstaller.writeClassToDisk("javax/naming/spi/NamingManager",
+             InMemoryJavaCompiler.compile("javax.naming.spi.NamingManager", source, "-Xmodule:java.naming"),
+             System.getProperty("test.classes"));
 
         pb = ProcessTools.createJavaProcessBuilder(
             "-XX:+UnlockDiagnosticVMOptions",
             "-XX:SharedArchiveFile=" + filename,
-            "-Xshare:on",
-            "--patch-module=java.naming=mods/java.naming",
+            "-Xshare:dump",
+            "--patch-module=java.base=" + System.getProperty("test.classes"),
+            "-Xlog:class+path=info",
             "-version");
-        output = new OutputAnalyzer(pb.start());
-        output.shouldContain("The shared archive file cannot be used with --patch-module");
+        new OutputAnalyzer(pb.start())
+            .shouldContain("--patch-module requires a regular file during dumping");
 
-        output.shouldHaveExitValue(1);
+        // Case 3a: Test CDS dumping with jar file in --patch-module
+        BasicJarBuilder.build("javanaming", "javax/naming/spi/NamingManager");
+        String moduleJar = BasicJarBuilder.getTestJar("javanaming.jar");
+        pb = ProcessTools.createJavaProcessBuilder(
+            "-XX:+UnlockDiagnosticVMOptions",
+            "-XX:SharedArchiveFile=" + filename,
+            "-Xshare:dump",
+            "--patch-module=java.naming=" + moduleJar,
+            "-Xlog:class+load",
+            "-Xlog:class+path=info",
+            "PatchModuleMain", "javax.naming.spi.NamingManager");
+        new OutputAnalyzer(pb.start())
+            .shouldContain("ro space:"); // Make sure archive got created.
+
+        // Case 3b: Test CDS run with jar file in --patch-module
+        pb = ProcessTools.createJavaProcessBuilder(
+            "-XX:+UnlockDiagnosticVMOptions",
+            "-XX:SharedArchiveFile=" + filename,
+            "-Xshare:auto",
+            "--patch-module=java.naming=" + moduleJar,
+            "-Xlog:class+load",
+            "-Xlog:class+path=info",
+            "PatchModuleMain", "javax.naming.spi.NamingManager");
+        new OutputAnalyzer(pb.start())
+            .shouldContain("I pass!")
+            .shouldHaveExitValue(0);
     }
 }