From dfe3a75cf5c911a68ccd187c57f7d0d1f30f65f9 Mon Sep 17 00:00:00 2001 Date: Wed, 4 Jan 2023 20:41:20 +0800 Subject: 8253495: CDS generates non-deterministic output --- make/scripts/compare.sh | 8 +- src/hotspot/share/cds/archiveBuilder.cpp | 5 +- src/hotspot/share/cds/archiveUtils.hpp | 5 +- src/hotspot/share/cds/heapShared.cpp | 8 +- src/hotspot/share/gc/shared/collectedHeap.cpp | 18 ++++- src/hotspot/share/gc/shared/collectedHeap.hpp | 1 + src/hotspot/share/prims/jvm.cpp | 20 +++++ src/hotspot/share/runtime/os.cpp | 11 ++- test/hotspot/jtreg/ProblemList.txt | 1 - .../jtreg/runtime/cds/DeterministicDump.java | 10 ++- .../cds/appcds/javaldr/LockDuringDump.java | 4 +- .../appcds/javaldr/LockDuringDumpAgent.java | 16 +++- test/lib/jdk/test/lib/cds/CDSOptions.java | 33 ++++++-- 13 files changed, 103 insertions(+), 33 deletions(-) create mode 100644 make/jdk/src/classes/build/tools/classlist/SortClasslist.java diff --git a/make/scripts/compare.sh b/make/scripts/compare.sh index 76c3a0684..023299771 100644 --- a/make/scripts/compare.sh +++ b/make/scripts/compare.sh @@ -325,7 +325,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) diff --git a/src/hotspot/share/cds/archiveBuilder.cpp b/src/hotspot/share/cds/archiveBuilder.cpp index 699926fcf..8e12cdabb 100644 --- a/src/hotspot/share/cds/archiveBuilder.cpp +++ b/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 @@ -540,7 +540,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) { diff --git a/src/hotspot/share/cds/archiveUtils.hpp b/src/hotspot/share/cds/archiveUtils.hpp index cdb3d99ab..6d8e0b43a 100644 --- a/src/hotspot/share/cds/archiveUtils.hpp +++ b/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; @@ -142,7 +143,7 @@ public: 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; } diff --git a/src/hotspot/share/cds/heapShared.cpp b/src/hotspot/share/cds/heapShared.cpp index 3e50dd750..fd3d199f8 100644 --- a/src/hotspot/share/cds/heapShared.cpp +++ b/src/hotspot/share/cds/heapShared.cpp @@ -454,7 +454,7 @@ 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; @@ -462,8 +462,7 @@ KlassSubGraphInfo* HeapShared::init_subgraph_info(Klass* k, bool is_full_module_ 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; } @@ -624,7 +623,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); } diff --git a/src/hotspot/share/gc/shared/collectedHeap.cpp b/src/hotspot/share/gc/shared/collectedHeap.cpp index 96b3eb946..5a2cb0e51 100644 --- a/src/hotspot/share/gc/shared/collectedHeap.cpp +++ b/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 @@ -405,6 +405,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) { @@ -415,8 +420,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 @@ -433,7 +437,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 diff --git a/src/hotspot/share/gc/shared/collectedHeap.hpp b/src/hotspot/share/gc/shared/collectedHeap.hpp index e4d444b8c..b65a490cb 100644 --- a/src/hotspot/share/gc/shared/collectedHeap.hpp +++ b/src/hotspot/share/gc/shared/collectedHeap.hpp @@ -143,6 +143,7 @@ class CollectedHeap : public CHeapObj { 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);) diff --git a/src/hotspot/share/prims/jvm.cpp b/src/hotspot/share/prims/jvm.cpp index 8baba8c38..9057cc072 100644 --- a/src/hotspot/share/prims/jvm.cpp +++ b/src/hotspot/share/prims/jvm.cpp @@ -2852,6 +2852,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, diff --git a/src/hotspot/share/runtime/os.cpp b/src/hotspot/share/runtime/os.cpp index ef43c3170..2fab965bd 100644 --- a/src/hotspot/share/runtime/os.cpp +++ b/src/hotspot/share/runtime/os.cpp @@ -701,11 +710,14 @@ void* os::malloc(size_t size, MEMFLAGS memflags, const NativeCallStack& stack) { return NULL; } - void* inner_ptr = MemTracker::record_malloc((address)outer_ptr, size, memflags, stack, level); - - DEBUG_ONLY(::memset(inner_ptr, uninitBlockPad, size);) + void* const inner_ptr = MemTracker::record_malloc((address)outer_ptr, size, memflags, stack, level); + 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; } diff --git a/test/hotspot/jtreg/ProblemList.txt b/test/hotspot/jtreg/ProblemList.txt index 8d5ea5105..11bbb5545 100644 --- a/test/hotspot/jtreg/ProblemList.txt +++ b/test/hotspot/jtreg/ProblemList.txt @@ -89,7 +89,6 @@ applications/jcstress/acqrel.java 8277434 linux-aarch64 # :hotspot_runtime runtime/cds/appcds/jigsaw/modulepath/ModulePathAndCP_JFR.java 8253437 windows-x64 -runtime/cds/DeterministicDump.java 8253495 generic-all runtime/InvocationTests/invokevirtualTests.java#current-int 8271125 generic-all runtime/InvocationTests/invokevirtualTests.java#current-comp 8271125 generic-all runtime/InvocationTests/invokevirtualTests.java#old-int 8271125 generic-all diff --git a/test/hotspot/jtreg/runtime/cds/DeterministicDump.java b/test/hotspot/jtreg/runtime/cds/DeterministicDump.java index 9cdba1fa9..6e8ccffce 100644 --- a/test/hotspot/jtreg/runtime/cds/DeterministicDump.java +++ b/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 class DeterministicDump { 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 class DeterministicDump { static String dump(ArrayList 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); diff --git a/test/hotspot/jtreg/runtime/cds/appcds/javaldr/LockDuringDump.java b/test/hotspot/jtreg/runtime/cds/appcds/javaldr/LockDuringDump.java index d60e0d11a..5dc8dbc08 100644 --- a/test/hotspot/jtreg/runtime/cds/appcds/javaldr/LockDuringDump.java +++ b/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 @@ -70,7 +70,7 @@ public class LockDuringDump { TestCommon.testDump(appJar, TestCommon.list(LockDuringDumpApp.class.getName()), "-XX:+UnlockDiagnosticVMOptions", agentArg, agentArg2, biasedLock); - if (i != 0) { + if (i != 0 && !out.getStdout().contains("LockDuringDumpAgent timeout")) { out.shouldContain("Let's hold the lock on the literal string"); } diff --git a/test/hotspot/jtreg/runtime/cds/appcds/javaldr/LockDuringDumpAgent.java b/test/hotspot/jtreg/runtime/cds/appcds/javaldr/LockDuringDumpAgent.java index 5e053d83e..57db61e93 100644 --- a/test/hotspot/jtreg/runtime/cds/appcds/javaldr/LockDuringDumpAgent.java +++ b/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 class LockDuringDumpAgent implements Runnable { 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"); } diff --git a/test/lib/jdk/test/lib/cds/CDSOptions.java b/test/lib/jdk/test/lib/cds/CDSOptions.java index 2fa949dc4..1138b7757 100644 --- a/test/lib/jdk/test/lib/cds/CDSOptions.java +++ b/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 @@ -46,24 +46,43 @@ public class CDSOptions { public CDSOptions addPrefix(String... prefix) { - for (String s : prefix) this.prefix.add(s); + for (String s : prefix) { + this.prefix.add(s); + } return this; } public CDSOptions addPrefix(String prefix[], String... extra) { - for (String s : prefix) this.prefix.add(s); - for (String s : extra) this.prefix.add(s); + for (String s : prefix) { + this.prefix.add(s); + } + for (String s : extra) { + this.prefix.add(s); + } + return this; + } + + public CDSOptions addSuffix(ArrayList suffix) { + for (String s : suffix) { + this.suffix.add(s); + } return this; } public CDSOptions addSuffix(String... suffix) { - for (String s : suffix) this.suffix.add(s); + for (String s : suffix) { + this.suffix.add(s); + } return this; } public CDSOptions addSuffix(String suffix[], String... extra) { - for (String s : suffix) this.suffix.add(s); - for (String s : extra) this.suffix.add(s); + for (String s : suffix) { + this.suffix.add(s); + } + for (String s : extra) { + this.suffix.add(s); + } return this; } -- 2.37.0