8253495: CDS generates non-deterministic output

Reviewed-by: erikj, kbarrett, ccheung, ihse

make/scripts/compare.sh

@@ -1,6 +1,6 @@
 #!/bin/bash
 #
-# Copyright (c) 2012, 2021, Oracle and/or its affiliates. All rights reserved.
+# Copyright (c) 2012, 2022, 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
@@ -324,7 +324,7 @@ compare_general_files() {
         ! -name "*.cpl" ! -name "*.pdb" ! -name "*.exp" ! -name "*.ilk" \
         ! -name "*.lib" ! -name "*.jmod" ! -name "*.exe" \
         ! -name "*.obj" ! -name "*.o" ! -name "jspawnhelper" ! -name "*.a" \
-        ! -name "*.tar.gz" ! -name "*.jsa" ! -name "gtestLauncher" \
+        ! -name "*.tar.gz" ! -name "classes_nocoops.jsa" ! -name "gtestLauncher" \
         ! -name "*.map" \
         | $GREP -v "./bin/"  | $SORT | $FILTER)
 

src/hotspot/share/cds/archiveBuilder.cpp

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2020, 2021, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2020, 2022, 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
@@ -522,7 +522,8 @@ ArchiveBuilder::FollowMode ArchiveBuilder::get_follow_mode(MetaspaceClosure::Ref
   if (MetaspaceShared::is_in_shared_metaspace(obj)) {
     // Don't dump existing shared metadata again.
     return point_to_it;
-  } else if (ref->msotype() == MetaspaceObj::MethodDataType) {
+  } else if (ref->msotype() == MetaspaceObj::MethodDataType ||
+             ref->msotype() == MetaspaceObj::MethodCountersType) {
     return set_to_null;
   } else {
     if (ref->msotype() == MetaspaceObj::ClassType) {

src/hotspot/share/cds/archiveUtils.hpp

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2019, 2021, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2019, 2022, 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
@@ -30,6 +30,7 @@
 #include "memory/virtualspace.hpp"
 #include "utilities/bitMap.hpp"
 #include "utilities/exceptions.hpp"
+#include "utilities/macros.hpp"
 
 class BootstrapInfo;
 class ReservedSpace;
@@ -147,7 +148,7 @@ class DumpRegion {
   char* expand_top_to(char* newtop);
   char* allocate(size_t num_bytes);
 
-  void append_intptr_t(intptr_t n, bool need_to_mark = false);
+  void append_intptr_t(intptr_t n, bool need_to_mark = false) NOT_CDS_RETURN;
 
   char* base()      const { return _base;        }
   char* top()       const { return _top;         }

src/hotspot/share/cds/dumpTimeClassInfo.hpp

@@ -167,15 +167,16 @@ class DumpTimeClassInfo: public CHeapObj<mtClass> {
   size_t runtime_info_bytesize() const;
 };
 
-inline unsigned DumpTimeSharedClassTable_hash(InstanceKlass* const& k) {
+template <typename T>
+inline unsigned DumpTimeSharedClassTable_hash(T* const& k) {
   if (DumpSharedSpaces) {
     // Deterministic archive contents
     uintx delta = k->name() - MetaspaceShared::symbol_rs_base();
     return primitive_hash<uintx>(delta);
   } else {
     // Deterministic archive is not possible because classes can be loaded
     // in multiple threads.
-    return primitive_hash<InstanceKlass*>(k);
+    return primitive_hash<T*>(k);
   }
 }
 

src/hotspot/share/cds/heapShared.cpp

@@ -574,16 +574,15 @@ KlassSubGraphInfo* HeapShared::init_subgraph_info(Klass* k, bool is_full_module_
   bool created;
   Klass* relocated_k = ArchiveBuilder::get_relocated_klass(k);
   KlassSubGraphInfo* info =
-    _dump_time_subgraph_info_table->put_if_absent(relocated_k, KlassSubGraphInfo(relocated_k, is_full_module_graph),
+    _dump_time_subgraph_info_table->put_if_absent(k, KlassSubGraphInfo(relocated_k, is_full_module_graph),
                                                   &created);
   assert(created, "must not initialize twice");
   return info;
 }
 
 KlassSubGraphInfo* HeapShared::get_subgraph_info(Klass* k) {
   assert(DumpSharedSpaces, "dump time only");
-  Klass* relocated_k = ArchiveBuilder::get_relocated_klass(k);
-  KlassSubGraphInfo* info = _dump_time_subgraph_info_table->get(relocated_k);
+  KlassSubGraphInfo* info = _dump_time_subgraph_info_table->get(k);
   assert(info != NULL, "must have been initialized");
   return info;
 }
@@ -744,7 +743,8 @@ struct CopyKlassSubGraphInfoToArchive : StackObj {
         (ArchivedKlassSubGraphInfoRecord*)ArchiveBuilder::ro_region_alloc(sizeof(ArchivedKlassSubGraphInfoRecord));
       record->init(&info);
 
-      unsigned int hash = SystemDictionaryShared::hash_for_shared_dictionary((address)klass);
+      Klass* relocated_k = ArchiveBuilder::get_relocated_klass(klass);
+      unsigned int hash = SystemDictionaryShared::hash_for_shared_dictionary((address)relocated_k);
       u4 delta = ArchiveBuilder::current()->any_to_offset_u4(record);
       _writer->add(hash, delta);
     }

src/hotspot/share/cds/heapShared.hpp

@@ -25,6 +25,7 @@
 #ifndef SHARE_CDS_HEAPSHARED_HPP
 #define SHARE_CDS_HEAPSHARED_HPP
 
+#include "cds/dumpTimeClassInfo.hpp"
 #include "cds/metaspaceShared.hpp"
 #include "classfile/compactHashtable.hpp"
 #include "classfile/javaClasses.hpp"
@@ -252,17 +253,12 @@ class HeapShared: AllStatic {
       HeapShared::oop_hash> ArchivedObjectCache;
   static ArchivedObjectCache* _archived_object_cache;
 
-  static unsigned klass_hash(Klass* const& klass) {
-    // Generate deterministic hashcode even if SharedBaseAddress is changed due to ASLR.
-    return primitive_hash<address>(address(klass) - SharedBaseAddress);
-  }
-
   class DumpTimeKlassSubGraphInfoTable
     : public ResourceHashtable<Klass*, KlassSubGraphInfo,
                                137, // prime number
                                ResourceObj::C_HEAP,
                                mtClassShared,
-                               HeapShared::klass_hash> {
+                               DumpTimeSharedClassTable_hash> {
   public:
     int _count;
   };

src/hotspot/share/gc/shared/collectedHeap.cpp

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2001, 2021, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2001, 2022, 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
@@ -417,6 +417,11 @@ size_t CollectedHeap::filler_array_min_size() {
   return align_object_size(filler_array_hdr_size()); // align to MinObjAlignment
 }
 
+void CollectedHeap::zap_filler_array_with(HeapWord* start, size_t words, juint value) {
+  Copy::fill_to_words(start + filler_array_hdr_size(),
+                      words - filler_array_hdr_size(), value);
+}
+
 #ifdef ASSERT
 void CollectedHeap::fill_args_check(HeapWord* start, size_t words)
 {
@@ -427,8 +432,7 @@ void CollectedHeap::fill_args_check(HeapWord* start, size_t words)
 void CollectedHeap::zap_filler_array(HeapWord* start, size_t words, bool zap)
 {
   if (ZapFillerObjects && zap) {
-    Copy::fill_to_words(start + filler_array_hdr_size(),
-                        words - filler_array_hdr_size(), 0XDEAFBABE);
+    zap_filler_array_with(start, words, 0XDEAFBABE);
   }
 }
 #endif // ASSERT
@@ -445,7 +449,13 @@ CollectedHeap::fill_with_array(HeapWord* start, size_t words, bool zap)
 
   ObjArrayAllocator allocator(Universe::intArrayKlassObj(), words, (int)len, /* do_zero */ false);
   allocator.initialize(start);
-  DEBUG_ONLY(zap_filler_array(start, words, zap);)
+  if (DumpSharedSpaces) {
+    // This array is written into the CDS archive. Make sure it
+    // has deterministic contents.
+    zap_filler_array_with(start, words, 0);
+  } else {
+    DEBUG_ONLY(zap_filler_array(start, words, zap);)
+  }
 }
 
 void

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

@@ -157,6 +157,7 @@ class CollectedHeap : public CHeapObj<mtGC> {
   static inline size_t filler_array_hdr_size();
   static inline size_t filler_array_min_size();
 
+  static inline void zap_filler_array_with(HeapWord* start, size_t words, juint value);
   DEBUG_ONLY(static void fill_args_check(HeapWord* start, size_t words);)
   DEBUG_ONLY(static void zap_filler_array(HeapWord* start, size_t words, bool zap = true);)
 

src/hotspot/share/prims/jvm.cpp

@@ -2865,6 +2865,26 @@ static void thread_entry(JavaThread* thread, TRAPS) {
 
 
 JVM_ENTRY(void, JVM_StartThread(JNIEnv* env, jobject jthread))
+#if INCLUDE_CDS
+  if (DumpSharedSpaces) {
+    // During java -Xshare:dump, if we allow multiple Java threads to
+    // execute in parallel, symbols and classes may be loaded in
+    // random orders which will make the resulting CDS archive
+    // non-deterministic.
+    //
+    // Lucikly, during java -Xshare:dump, it's important to run only
+    // the code in the main Java thread (which is NOT started here) that
+    // creates the module graph, etc. It's safe to not start the other
+    // threads which are launched by class static initializers
+    // (ReferenceHandler, FinalizerThread and CleanerImpl).
+    if (log_is_enabled(Info, cds)) {
+      ResourceMark rm;
+      oop t = JNIHandles::resolve_non_null(jthread);
+      log_info(cds)("JVM_StartThread() ignored: %s", t->klass()->external_name());
+    }
+    return;
+  }
+#endif
   JavaThread *native_thread = NULL;
 
   // We cannot hold the Threads_lock when we throw an exception,

src/hotspot/share/runtime/os.cpp

@@ -634,6 +634,8 @@ void* os::malloc(size_t size, MEMFLAGS memflags, const NativeCallStack& stack) {
   // Special handling for NMT preinit phase before arguments are parsed
   void* rc = NULL;
   if (NMTPreInit::handle_malloc(&rc, size)) {
+    // No need to fill with 0 because DumpSharedSpaces doesn't use these
+    // early allocations.
     return rc;
   }
 
@@ -658,9 +660,13 @@ void* os::malloc(size_t size, MEMFLAGS memflags, const NativeCallStack& stack) {
 
   void* const inner_ptr = MemTracker::record_malloc((address)outer_ptr, size, memflags, stack);
 
-  DEBUG_ONLY(::memset(inner_ptr, uninitBlockPad, size);)
+  if (DumpSharedSpaces) {
+    // Need to deterministically fill all the alignment gaps in C++ structures.
+    ::memset(inner_ptr, 0, size);
+  } else {
+    DEBUG_ONLY(::memset(inner_ptr, uninitBlockPad, size);)
+  }
   DEBUG_ONLY(break_if_ptr_caught(inner_ptr);)
-
   return inner_ptr;
 }
 

test/hotspot/jtreg/ProblemList.txt

@@ -94,7 +94,6 @@ gc/metaspace/CompressedClassSpaceSizeInJmapHeap.java 8241293 macosx-x64
 # :hotspot_runtime
 
 runtime/cds/appcds/jigsaw/modulepath/ModulePathAndCP_JFR.java 8253437 windows-x64
-runtime/cds/DeterministicDump.java 8253495 generic-all
 runtime/jni/terminatedThread/TestTerminatedThread.java 8219652 aix-ppc64
 runtime/os/TestTracePageSizes.java#no-options 8267460 linux-aarch64
 runtime/os/TestTracePageSizes.java#explicit-large-page-size 8267460 linux-aarch64

test/hotspot/jtreg/runtime/cds/DeterministicDump.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2020, 2021, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2020, 2022, 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
@@ -54,6 +54,11 @@ public static void doTest(boolean compressed) throws Exception {
         baseArgs.add("-Xmx128M");
 
         if (Platform.is64bit()) {
+            if (!compressed) {
+                System.out.println("CDS archives with uncompressed oops are still non-deterministic");
+                System.out.println("See https://bugs.openjdk.java.net/browse/JDK-8282828");
+                return;
+            }
             // These options are available only on 64-bit.
             String sign = (compressed) ?  "+" : "-";
             baseArgs.add("-XX:" + sign + "UseCompressedOops");
@@ -78,9 +83,12 @@ public static void doTest(boolean compressed) throws Exception {
     static String dump(ArrayList<String> args, String... more) throws Exception {
         String logName = "SharedArchiveFile" + (id++);
         String archiveName = logName + ".jsa";
+        String mapName = logName + ".map";
         CDSOptions opts = (new CDSOptions())
             .addPrefix("-Xlog:cds=debug")
+            .addPrefix("-Xlog:cds+map=trace:file=" + mapName + ":none:filesize=0")
             .setArchiveName(archiveName)
+            .addSuffix(args)
             .addSuffix(more);
         CDSTestUtils.createArchiveAndCheck(opts);
 

test/hotspot/jtreg/runtime/cds/appcds/javaldr/LockDuringDump.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2020, 2021, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2020, 2022, 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
@@ -68,7 +68,7 @@ public static void main(String[] args) throws Throwable {
                 TestCommon.testDump(appJar, TestCommon.list(LockDuringDumpApp.class.getName()),
                                     "-XX:+UnlockDiagnosticVMOptions",
                                     agentArg, agentArg2);
-            if (i != 0) {
+            if (i != 0 && !out.getStdout().contains("LockDuringDumpAgent timeout")) {
                 out.shouldContain("Let's hold the lock on the literal string");
             }
 

test/hotspot/jtreg/runtime/cds/appcds/javaldr/LockDuringDumpAgent.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2020, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2020, 2022, 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
@@ -51,9 +51,21 @@ public static void premain(String agentArg, Instrumentation instrumentation) {
 
     static void waitForThreadStart() {
         try {
+            long started = System.currentTimeMillis();
+            long timeout = 10000;
+            synchronized (LITERAL) {
+                Thread.sleep(1);
+            }
             synchronized (lock) {
                 while (!threadStarted) {
-                    lock.wait();
+                    lock.wait(timeout);
+                    long elapsed = System.currentTimeMillis() - started;
+                    if (elapsed >= timeout) {
+                        System.out.println("This JVM may decide to not launch any Java threads during -Xshare:dump.");
+                        System.out.println("This is OK because no string objects could be in a locked state during heap dump.");
+                        System.out.println("LockDuringDumpAgent timeout after " + elapsed + " ms");
+                        return;
+                    }
                 }
                 System.out.println("Thread has started");
             }

test/lib/jdk/test/lib/cds/CDSOptions.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2017, 2021, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2017, 2022, 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
@@ -56,6 +56,11 @@ public CDSOptions addPrefix(String prefix[], String... extra) {
         return this;
     }
 
+    public CDSOptions addSuffix(ArrayList<String> suffix) {
+        for (String s : suffix) this.suffix.add(s);
+        return this;
+    }
+
     public CDSOptions addSuffix(String... suffix) {
         for (String s : suffix) this.suffix.add(s);
         return this;