8197972: Always verify non-system classes during CDS dump time
authorccheung
Tue, 17 Apr 2018 16:19:48 -0700
changeset 49769 b8c9bec06921
parent 49768 54d462a2db54
child 49795 4c77b1453427
8197972: Always verify non-system classes during CDS dump time Summary: use -Verify:remote even if the user specifies -Xverify:none during CDS dump time Reviewed-by: iklam, mseledtsov, jiangli
src/hotspot/share/memory/metaspaceShared.cpp
src/hotspot/share/runtime/arguments.cpp
test/hotspot/jtreg/runtime/appcds/VerifierTest.java
--- a/src/hotspot/share/memory/metaspaceShared.cpp	Tue Apr 17 16:18:22 2018 -0400
+++ b/src/hotspot/share/memory/metaspaceShared.cpp	Tue Apr 17 16:19:48 2018 -0700
@@ -1731,11 +1731,11 @@
   assert(DumpSharedSpaces, "should only be called during dumping");
   if (ik->init_state() < InstanceKlass::linked) {
     bool saved = BytecodeVerificationLocal;
-    if (!(ik->is_shared_boot_class())) {
+    if (ik->loader_type() == 0 && ik->class_loader() == NULL) {
       // The verification decision is based on BytecodeVerificationRemote
       // for non-system classes. Since we are using the NULL classloader
-      // to load non-system classes during dumping, we need to temporarily
-      // change BytecodeVerificationLocal to be the same as
+      // to load non-system classes for customized class loaders during dumping,
+      // we need to temporarily change BytecodeVerificationLocal to be the same as
       // BytecodeVerificationRemote. Note this can cause the parent system
       // classes also being verified. The extra overhead is acceptable during
       // dumping.
--- a/src/hotspot/share/runtime/arguments.cpp	Tue Apr 17 16:18:22 2018 -0400
+++ b/src/hotspot/share/runtime/arguments.cpp	Tue Apr 17 16:19:48 2018 -0700
@@ -3376,6 +3376,12 @@
     // Disable biased locking now as it interferes with the clean up of
     // the archived Klasses and Java string objects (at dump time only).
     UseBiasedLocking = false;
+
+    // Always verify non-system classes during CDS dump
+    if (!BytecodeVerificationRemote) {
+      BytecodeVerificationRemote = true;
+      log_info(cds)("All non-system classes will be verified (-Xverify:remote) during CDS dump time.");
+    }
   }
   if (UseSharedSpaces && patch_mod_javabase) {
     no_shared_spaces("CDS is disabled when " JAVA_BASE_NAME " module is patched.");
--- a/test/hotspot/jtreg/runtime/appcds/VerifierTest.java	Tue Apr 17 16:18:22 2018 -0400
+++ b/test/hotspot/jtreg/runtime/appcds/VerifierTest.java	Tue Apr 17 16:19:48 2018 -0700
@@ -46,6 +46,8 @@
         "shared archive file was created with less restrictive verification setting";
     static final String VFY_ERR = "java.lang.VerifyError";
     static final String PASS_RESULT = "Hi, how are you?";
+    static final String VFY_INFO_MESSAGE =
+        "All non-system classes will be verified (-Xverify:remote) during CDS dump time.";
 
     enum Testset1Part {
         A, B
@@ -137,7 +139,7 @@
             {"app",   VFY_ALL,    VFY_ALL,    VFY_ERR },
             {"app",   VFY_ALL,    VFY_NONE,   ERR },
             // Dump app/ext with -Xverify:none
-            {"app",   VFY_NONE,   VFY_REMOTE, MAP_FAIL},
+            {"app",   VFY_NONE,   VFY_REMOTE, VFY_ERR},
             {"app",   VFY_NONE,   VFY_ALL,    MAP_FAIL},
             {"app",   VFY_NONE,   VFY_NONE,   ERR },
             // Dump sys only with -Xverify:remote
@@ -188,6 +190,10 @@
                                                             // issue - assert failure when dumping archive with the -Xverify:all
                                                             "-Xms256m",
                                                             "-Xmx256m");
+                if (dump_setting.equals(VFY_NONE) &&
+                    runtime_setting.equals(VFY_REMOTE)) {
+                    dumpOutput.shouldContain(VFY_INFO_MESSAGE);
+                }
             }
             TestCommon.run("-cp", jar,
                            runtime_setting,
@@ -221,10 +227,11 @@
             {"app",   VFY_ALL,    VFY_ALL,    PASS_RESULT },
             {"app",   VFY_ALL,    VFY_NONE,   PASS_RESULT },
             // Dump app/ext with -Xverify:none
-            {"app",   VFY_NONE,   VFY_REMOTE, MAP_FAIL},
+            {"app",   VFY_NONE,   VFY_REMOTE, PASS_RESULT},
             {"app",   VFY_NONE,   VFY_ALL,    MAP_FAIL},
             {"app",   VFY_NONE,   VFY_NONE,   PASS_RESULT },
         };
+        String prev_dump_setting = "";
         for (int i = 0; i < config2.length; i ++) {
             // config2[i][0] is always set to "app" in this test
             String dump_setting = config2[i][1];
@@ -233,17 +240,24 @@
             System.out.println("Test case [" + i + "]: dumping " + config2[i][0] +
                                " with " + dump_setting +
                                ", run with " + runtime_setting);
-            OutputAnalyzer dumpOutput = TestCommon.dump(
-                                                        jar, appClasses, dump_setting,
-                                                        "-XX:+UnlockDiagnosticVMOptions",
-                                                        // FIXME: the following options are for working around a GC
-                                                        // issue - assert failure when dumping archive with the -Xverify:all
-                                                        "-Xms256m",
-                                                        "-Xmx256m");
+            if (!dump_setting.equals(prev_dump_setting)) {
+                OutputAnalyzer dumpOutput = TestCommon.dump(
+                                                            jar, appClasses, dump_setting,
+                                                            "-XX:+UnlockDiagnosticVMOptions",
+                                                            // FIXME: the following options are for working around a GC
+                                                            // issue - assert failure when dumping archive with the -Xverify:all
+                                                            "-Xms256m",
+                                                            "-Xmx256m");
+                if (dump_setting.equals(VFY_NONE) &&
+                    runtime_setting.equals(VFY_REMOTE)) {
+                    dumpOutput.shouldContain(VFY_INFO_MESSAGE);
+                }
+            }
             TestCommon.run("-cp", jar,
                            runtime_setting,
                            "Hi")
                 .ifNoMappingFailure(output -> checkRuntimeOutput(output, expected_output_str));
+           prev_dump_setting = dump_setting;
         }
     }