8341371: CDS cannot load archived heap objects with -XX:+UseSerialGC -XX:-UseCompressedOops

Reviewed-by: ccheung, iklam

Author: Matias Saavedra Silva

src/hotspot/share/cds/archiveHeapLoader.cpp

@@ -142,15 +142,22 @@ class PatchCompressedEmbeddedPointersQuick: public BitMapClosure {
 
 class PatchUncompressedEmbeddedPointers: public BitMapClosure {
   oop* _start;
+  intptr_t _delta;
 
  public:
-  PatchUncompressedEmbeddedPointers(oop* start) : _start(start) {}
+  PatchUncompressedEmbeddedPointers(oop* start, intx runtime_offset) :
+    _start(start),
+    _delta(runtime_offset) {}
+
+  PatchUncompressedEmbeddedPointers(oop* start) :
+    _start(start),
+    _delta(ArchiveHeapLoader::mapped_heap_delta()) {}
 
   bool do_bit(size_t offset) {
     oop* p = _start + offset;
     intptr_t dumptime_oop = (intptr_t)((void*)*p);
     assert(dumptime_oop != 0, "null oops should have been filtered out at dump time");
-    intptr_t runtime_oop = dumptime_oop + ArchiveHeapLoader::mapped_heap_delta();
+    intptr_t runtime_oop = dumptime_oop + _delta;
     RawAccess<IS_NOT_NULL>::oop_store(p, cast_to_oop(runtime_oop));
     return true;
   }
@@ -221,10 +228,6 @@ void ArchiveHeapLoader::init_loaded_heap_relocation(LoadedArchiveHeapRegion* loa
 }
 
 bool ArchiveHeapLoader::can_load() {
-  if (!UseCompressedOops) {
-    // Pointer relocation for uncompressed oops is unimplemented.
-    return false;
-  }
   return Universe::heap()->can_load_archived_objects();
 }
 
@@ -312,13 +315,18 @@ bool ArchiveHeapLoader::load_heap_region_impl(FileMapInfo* mapinfo, LoadedArchiv
   uintptr_t oopmap = bitmap_base + r->oopmap_offset();
   BitMapView bm((BitMap::bm_word_t*)oopmap, r->oopmap_size_in_bits());
 
-  PatchLoadedRegionPointers patcher((narrowOop*)load_address + FileMapInfo::current_info()->heap_oopmap_start_pos(), loaded_region);
-  bm.iterate(&patcher);
+  if (UseCompressedOops) {
+    PatchLoadedRegionPointers patcher((narrowOop*)load_address + FileMapInfo::current_info()->heap_oopmap_start_pos(), loaded_region);
+    bm.iterate(&patcher);
+  } else {
+    PatchUncompressedEmbeddedPointers patcher((oop*)load_address + FileMapInfo::current_info()->heap_oopmap_start_pos(), loaded_region->_runtime_offset);
+    bm.iterate(&patcher);
+  }
   return true;
 }
 
 bool ArchiveHeapLoader::load_heap_region(FileMapInfo* mapinfo) {
-  assert(UseCompressedOops, "loaded heap for !UseCompressedOops is unimplemented");
+  assert(can_load(), "loaded heap for must be supported");
   init_narrow_oop_decoding(mapinfo->narrow_oop_base(), mapinfo->narrow_oop_shift());
 
   LoadedArchiveHeapRegion loaded_region;
@@ -358,8 +366,12 @@ class VerifyLoadedHeapEmbeddedPointers: public BasicOopIterateClosure {
     }
   }
   virtual void do_oop(oop* p) {
-    // Uncompressed oops are not supported by loaded heaps.
-    Unimplemented();
+    oop v = *p;
+    if(v != nullptr) {
+      uintptr_t u = cast_from_oop<uintptr_t>(v);
+      ArchiveHeapLoader::assert_in_loaded_heap(u);
+      guarantee(_table->contains(u), "must point to beginning of object in loaded archived region");
+    }
   }
 };
 

src/hotspot/share/cds/archiveHeapLoader.hpp

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2018, 2023, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2018, 2024, 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
@@ -146,6 +146,7 @@ class ArchiveHeapLoader : AllStatic {
   inline static oop decode_from_archive_impl(narrowOop v) NOT_CDS_JAVA_HEAP_RETURN_(nullptr);
 
   class PatchLoadedRegionPointers;
+  class PatchUncompressedLoadedRegionPointers;
 
 public:
 

src/hotspot/share/cds/filemap.cpp

@@ -2054,8 +2054,7 @@ void FileMapInfo::map_or_load_heap_region() {
       success = ArchiveHeapLoader::load_heap_region(this);
     } else {
       if (!UseCompressedOops && !ArchiveHeapLoader::can_map()) {
-        // TODO - remove implicit knowledge of G1
-        log_info(cds)("Cannot use CDS heap data. UseG1GC is required for -XX:-UseCompressedOops");
+        log_info(cds)("Cannot use CDS heap data. Selected GC not compatible -XX:-UseCompressedOops");
       } else {
         log_info(cds)("Cannot use CDS heap data. UseEpsilonGC, UseG1GC, UseSerialGC, UseParallelGC, or UseShenandoahGC are required.");
       }
@@ -2135,7 +2134,7 @@ address FileMapInfo::heap_region_requested_address() {
   assert(CDSConfig::is_using_archive(), "runtime only");
   FileMapRegion* r = region_at(MetaspaceShared::hp);
   assert(is_aligned(r->mapping_offset(), sizeof(HeapWord)), "must be");
-  assert(ArchiveHeapLoader::can_map(), "cannot be used by ArchiveHeapLoader::can_load() mode");
+  assert(ArchiveHeapLoader::can_use(), "GC must support mapping or loading");
   if (UseCompressedOops) {
     // We can avoid relocation if each region's offset from the runtime CompressedOops::base()
     // is the same as its offset from the CompressedOops::base() during dumptime.

src/hotspot/share/gc/epsilon/epsilonHeap.hpp

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2023, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2023, 2024, Oracle and/or its affiliates. All rights reserved.
  * Copyright (c) 2017, 2022, Red Hat, Inc. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
@@ -128,7 +128,7 @@ class EpsilonHeap : public CollectedHeap {
   bool is_in_reserved(const void* addr) const { return _reserved.contains(addr); }
 
   // Support for loading objects from CDS archive into the heap
-  bool can_load_archived_objects() const override { return UseCompressedOops; }
+  bool can_load_archived_objects() const override { return true; }
   HeapWord* allocate_loaded_archive_space(size_t size) override;
 
   void print_on(outputStream* st) const override;

src/hotspot/share/gc/parallel/parallelScavengeHeap.hpp

@@ -246,7 +246,7 @@ class ParallelScavengeHeap : public CollectedHeap {
   }
 
   // Support for loading objects from CDS archive into the heap
-  bool can_load_archived_objects() const override { return UseCompressedOops; }
+  bool can_load_archived_objects() const override { return true; }
   HeapWord* allocate_loaded_archive_space(size_t size) override;
   void complete_loaded_archive_space(MemRegion archive_space) override;
 

src/hotspot/share/gc/serial/serialHeap.hpp

@@ -291,7 +291,7 @@ class SerialHeap : public CollectedHeap {
   void safepoint_synchronize_end() override;
 
   // Support for loading objects from CDS archive into the heap
-  bool can_load_archived_objects() const override { return UseCompressedOops; }
+  bool can_load_archived_objects() const override { return true; }
   HeapWord* allocate_loaded_archive_space(size_t size) override;
   void complete_loaded_archive_space(MemRegion archive_space) override;
 

src/hotspot/share/gc/shenandoah/shenandoahHeap.hpp

@@ -543,7 +543,7 @@ class ShenandoahHeap : public CollectedHeap, public ShenandoahSpaceInfo {
 
 // ---------- CDS archive support
 
-  bool can_load_archived_objects() const override { return UseCompressedOops; }
+  bool can_load_archived_objects() const override { return true; }
   HeapWord* allocate_loaded_archive_space(size_t size) override;
   void complete_loaded_archive_space(MemRegion archive_space) override;
 

src/hotspot/share/prims/whitebox.cpp

@@ -2159,8 +2159,7 @@ WB_ENTRY(jboolean, WB_IsJVMCISupportedByGC(JNIEnv* env))
 WB_END
 
 WB_ENTRY(jboolean, WB_CanWriteJavaHeapArchive(JNIEnv* env))
-  return HeapShared::can_write()
-      && ArchiveHeapLoader::can_use(); // work-around JDK-8341371
+  return HeapShared::can_write();
 WB_END
 
 

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

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2021, 2022, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2021, 2024, 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
@@ -37,16 +37,41 @@
  * @run driver TestEpsilonGCWithCDS
  */
 
+// Below is exactly the same as above, except:
+// - requires vm.bits == "64"
+// - extra argument "false"
+
+/*
+ * @test Loading CDS archived heap objects into EpsilonGC
+ * @bug 8234679 8341371
+ * @requires vm.cds
+ * @requires vm.gc.Epsilon
+ * @requires vm.gc.G1
+ * @requires vm.bits == "64"
+ *
+ * @comment don't run this test if any -XX::+Use???GC options are specified, since they will
+ *          interfere with the test.
+ * @requires vm.gc == null
+ *
+ * @library /test/lib /test/hotspot/jtreg/runtime/cds/appcds
+ * @compile test-classes/Hello.java
+ * @run driver TestEpsilonGCWithCDS false
+ */
 import jdk.test.lib.Platform;
 import jdk.test.lib.process.OutputAnalyzer;
 
 public class TestEpsilonGCWithCDS {
     public final static String HELLO = "Hello World";
     static String helloJar;
+    static boolean useCompressedOops = true;
 
     public static void main(String... args) throws Exception {
         helloJar = JarBuilder.build("hello", "Hello");
 
+        if (args.length > 0 && args[0].equals("false")) {
+            useCompressedOops = false;
+        }
+
         // Check if we can use EpsilonGC during dump time, or run time, or both.
         test(false, true);
         test(true,  false);
@@ -70,6 +95,8 @@ static void test(boolean dumpWithEpsilon, boolean execWithEpsilon, boolean useSm
         String execGC = execWithEpsilon ? Epsilon : G1;
         String small1 = useSmallRegions ? "-Xmx256m" : "-showversion";
         String small2 = useSmallRegions ? "-XX:ObjectAlignmentInBytes=64" : "-showversion";
+        String errMsg = "Cannot use CDS heap data. Selected GC not compatible -XX:-UseCompressedOops";
+        String coops = useCompressedOops ? "-XX:+UseCompressedOops" : "-XX:-UseCompressedOops";
         OutputAnalyzer out;
 
         System.out.println("0. Dump with " + dumpGC);
@@ -79,6 +106,7 @@ static void test(boolean dumpWithEpsilon, boolean execWithEpsilon, boolean useSm
                               dumpGC,
                               small1,
                               small2,
+                              coops,
                               "-Xlog:cds");
         out.shouldContain("Dumping shared data to file:");
         out.shouldHaveExitValue(0);
@@ -89,9 +117,11 @@ static void test(boolean dumpWithEpsilon, boolean execWithEpsilon, boolean useSm
                               execGC,
                               small1,
                               small2,
+                              coops,
                               "-Xlog:cds",
                               "Hello");
         out.shouldContain(HELLO);
+        out.shouldNotContain(errMsg);
         out.shouldHaveExitValue(0);
     }
 }

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

@@ -37,16 +37,41 @@
  * @run driver TestParallelGCWithCDS
  */
 
+// Below is exactly the same as above, except:
+// - requires vm.bits == "64"
+// - extra argument "false"
+
+ /*
+ * @test Loading CDS archived heap objects into ParallelGC
+ * @bug 8274788 8341371
+ * @requires vm.cds
+ * @requires vm.gc.Parallel
+ * @requires vm.gc.G1
+ * @requires vm.bits == "64"
+ *
+ * @comment don't run this test if any -XX::+Use???GC options are specified, since they will
+ *          interfere with the test.
+ * @requires vm.gc == null
+ *
+ * @library /test/lib /test/hotspot/jtreg/runtime/cds/appcds
+ * @compile test-classes/Hello.java
+ * @run driver TestParallelGCWithCDS false
+ */
 import jdk.test.lib.Platform;
 import jdk.test.lib.process.OutputAnalyzer;
 
 public class TestParallelGCWithCDS {
     public final static String HELLO = "Hello World";
     static String helloJar;
+    static boolean useCompressedOops = true;
 
     public static void main(String... args) throws Exception {
         helloJar = JarBuilder.build("hello", "Hello");
 
+        if (args.length > 0 && args[0].equals("false")) {
+            useCompressedOops = false;
+        }
+
         // Check if we can use ParallelGC during dump time, or run time, or both.
         test(false, true);
         test(true,  false);
@@ -69,6 +94,8 @@ static void test(boolean dumpWithParallel, boolean execWithParallel, boolean use
         String execGC = execWithParallel ? Parallel : G1;
         String small1 = useSmallRegions ? "-Xmx256m" : "-showversion";
         String small2 = useSmallRegions ? "-XX:ObjectAlignmentInBytes=64" : "-showversion";
+        String errMsg = "Cannot use CDS heap data. Selected GC not compatible -XX:-UseCompressedOops";
+        String coops = useCompressedOops ? "-XX:+UseCompressedOops" : "-XX:-UseCompressedOops";
         OutputAnalyzer out;
 
         System.out.println("0. Dump with " + dumpGC);
@@ -77,6 +104,7 @@ static void test(boolean dumpWithParallel, boolean execWithParallel, boolean use
                               dumpGC,
                               small1,
                               small2,
+                              coops,
                               "-Xlog:cds");
         out.shouldContain("Dumping shared data to file:");
         out.shouldHaveExitValue(0);
@@ -86,9 +114,11 @@ static void test(boolean dumpWithParallel, boolean execWithParallel, boolean use
                               execGC,
                               small1,
                               small2,
+                              coops,
                               "-Xlog:cds",
                               "Hello");
         out.shouldContain(HELLO);
+        out.shouldNotContain(errMsg);
         out.shouldHaveExitValue(0);
 
         int n = 2;
@@ -109,10 +139,12 @@ static void test(boolean dumpWithParallel, boolean execWithParallel, boolean use
                                       small1,
                                       small2,
                                       xmx,
+                                      coops,
                                       "-Xlog:cds",
                                       "Hello");
                 if (out.getExitValue() == 0) {
                     out.shouldContain(HELLO);
+                    out.shouldNotContain(errMsg);
                 } else {
                     String pattern = "((Too small maximum heap)" +
                                      "|(GC triggered before VM initialization completed)" +