8271073: Improve testing with VM option VerifyArchivedFields

Reviewed-by: ccheung, minqi

src/hotspot/share/cds/heapShared.cpp

@@ -674,23 +674,28 @@ void HeapShared::serialize_subgraph_info_table_header(SerializeClosure* soc) {
 }
 
 static void verify_the_heap(Klass* k, const char* which) {
-  if (VerifyArchivedFields) {
+  if (VerifyArchivedFields > 0) {
     ResourceMark rm;
     log_info(cds, heap)("Verify heap %s initializing static field(s) in %s",
                         which, k->external_name());
 
     VM_Verify verify_op;
     VMThread::execute(&verify_op);
 
-    if (!FLAG_IS_DEFAULT(VerifyArchivedFields)) {
-      // If VerifyArchivedFields has a non-default value (e.g., specified on the command-line), do
-      // more expensive checks.
-      if (is_init_completed()) {
-        FlagSetting fs1(VerifyBeforeGC, true);
-        FlagSetting fs2(VerifyDuringGC, true);
-        FlagSetting fs3(VerifyAfterGC,  true);
-        Universe::heap()->collect(GCCause::_java_lang_system_gc);
-      }
+    if (VerifyArchivedFields > 1 && is_init_completed()) {
+      // At this time, the oop->klass() of some archived objects in the heap may not
+      // have been loaded into the system dictionary yet. Nevertheless, oop->klass() should
+      // have enough information (object size, oop maps, etc) so that a GC can be safely
+      // performed.
+      //
+      // -XX:VerifyArchivedFields=2 force a GC to happen in such an early stage
+      // to check for GC safety.
+      log_info(cds, heap)("Trigger GC %s initializing static field(s) in %s",
+                          which, k->external_name());
+      FlagSetting fs1(VerifyBeforeGC, true);
+      FlagSetting fs2(VerifyDuringGC, true);
+      FlagSetting fs3(VerifyAfterGC,  true);
+      Universe::heap()->collect(GCCause::_java_lang_system_gc);
     }
   }
 }
@@ -1707,10 +1712,12 @@ class VerifyLoadedHeapEmbeddedPointers: public BasicOopIterateClosure {
 };
 
 void HeapShared::verify_loaded_heap() {
-  if (!VerifyArchivedFields || !is_loaded()) {
+  if (VerifyArchivedFields <= 0 || !is_loaded()) {
     return;
   }
 
+  log_info(cds, heap)("Verify all oops and pointers in loaded heap");
+
   ResourceMark rm;
   ResourceHashtable<uintptr_t, bool> table;
   VerifyLoadedHeapEmbeddedPointers verifier(&table);

src/hotspot/share/gc/shared/gc_globals.hpp

@@ -523,8 +523,12 @@
   product(bool, VerifyDuringGC, false, DIAGNOSTIC,                          \
           "Verify memory system during GC (between phases)")                \
                                                                             \
-  product(bool, VerifyArchivedFields, trueInDebug, DIAGNOSTIC,              \
-          "Verify memory when archived oop fields are loaded from CDS)")    \
+  product(int, VerifyArchivedFields, 0, DIAGNOSTIC,                         \
+          "Verify memory when archived oop fields are loaded from CDS; "    \
+          "0: No check; "                                                   \
+          "1: Basic verification with VM_Verify (no side effects); "        \
+          "2: Detailed verification by forcing a GC (with side effects)")   \
+          range(0, 2)                                                       \
                                                                             \
   product(ccstrlist, VerifyGCType, "", DIAGNOSTIC,                          \
              "GC type(s) to verify when Verify*GC is enabled."              \

test/hotspot/jtreg/runtime/cds/appcds/TestCommon.java

@@ -409,6 +409,8 @@ public static OutputAnalyzer runWithArchive(AppCDSOptions opts)
             cmd.add(opts.appJar);
         }
 
+        CDSTestUtils.addVerifyArchivedFields(cmd);
+
         for (String s : opts.suffix) cmd.add(s);
 
         if (RUN_WITH_JFR) {

test/hotspot/jtreg/runtime/cds/appcds/cacheObject/VerifyArchivedFields.java

@@ -0,0 +1,53 @@
+/*
+ * Copyright (c) 2021, 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 run with -XX:VerifyArchivedFields=2 for more expensive verification
+ *          of the archived heap objects.
+ * @requires vm.cds.write.archived.java.heap
+ * @library /test/lib /test/hotspot/jtreg/runtime/cds/appcds /test/hotspot/jtreg/runtime/cds/appcds/test-classes
+ * @build Hello
+ * @run driver jdk.test.lib.helpers.ClassFileInstaller -jar Hello.jar Hello
+ * @run driver VerifyArchivedFields
+ */
+
+import jdk.test.lib.helpers.ClassFileInstaller;
+
+public class VerifyArchivedFields {
+    // Note: -XX:VerifyArchivedFields=2 will force a GC every time when
+    // HeapShared::initialize_from_archived_subgraph(Klass* k, ...) is called. This ensures
+    // that it's safe to do a GC even when the oop->klass() of some archived heap objects
+    // are not yet loaded into the system dictionary.
+    public static void main(String[] args) throws Exception {
+        TestCommon.test(ClassFileInstaller.getJarPath("Hello.jar"),
+                        TestCommon.list("Hello"),
+                        "-XX:+UnlockDiagnosticVMOptions",
+                        "-XX:VerifyArchivedFields=2",
+                        "-Xlog:cds=debug",
+                        "-Xlog:cds+heap=debug",
+                        "-Xlog:gc*=debug",
+                        "Hello");
+  }
+}

test/lib/jdk/test/lib/cds/CDSTestUtils.java

@@ -399,6 +399,12 @@ public static OutputAnalyzer runWithArchive(String... cliPrefix)
                                .addPrefix(cliPrefix) );
     }
 
+    // Enable basic verification (VerifyArchivedFields=1, no side effects) for all CDS
+    // tests to make sure the archived heap objects are mapped/loaded properly.
+    public static void addVerifyArchivedFields(ArrayList<String> cmd) {
+        cmd.add("-XX:+UnlockDiagnosticVMOptions");
+        cmd.add("-XX:VerifyArchivedFields=1");
+    }
 
     // Execute JVM with CDS archive, specify CDSOptions
     public static OutputAnalyzer runWithArchive(CDSOptions opts)
@@ -413,6 +419,7 @@ public static OutputAnalyzer runWithArchive(CDSOptions opts)
                 opts.archiveName = getDefaultArchiveName();
             cmd.add("-XX:SharedArchiveFile=" + opts.archiveName);
         }
+        addVerifyArchivedFields(cmd);
 
         if (opts.useVersion)
             cmd.add("-version");