changeset 52488:b7bfd64e43a6

8210523: runtime/appcds/cacheObject/DifferentHeapSizes.java crash Reviewed-by: jiangli, ccheung
author iklam
date Wed, 12 Sep 2018 17:45:22 -0700
parents e3411e5e473d
children 96b76dca2be8
files src/hotspot/share/classfile/compactHashtable.cpp src/hotspot/share/classfile/compactHashtable.inline.hpp src/hotspot/share/memory/filemap.cpp src/hotspot/share/memory/filemap.hpp src/hotspot/share/memory/heapShared.cpp src/hotspot/share/memory/heapShared.hpp src/hotspot/share/memory/heapShared.inline.hpp src/hotspot/share/memory/metaspaceShared.cpp test/hotspot/jtreg/runtime/appcds/cacheObject/DifferentHeapSizes.java test/lib/jdk/test/lib/cds/CDSTestUtils.java
diffstat 10 files changed, 65 insertions(+), 35 deletions(-) [+]
line wrap: on
line diff
--- a/src/hotspot/share/classfile/compactHashtable.cpp	Wed Sep 12 16:28:06 2018 -0700
+++ b/src/hotspot/share/classfile/compactHashtable.cpp	Wed Sep 12 17:45:22 2018 -0700
@@ -282,7 +282,7 @@
   CompactHashtable_OopIterator(OopClosure *cl) : _closure(cl) {}
   inline void do_value(address base_address, u4 offset) const {
     narrowOop v = (narrowOop)offset;
-    oop obj = HeapShared::decode_with_archived_oop_encoding_mode(v);
+    oop obj = HeapShared::decode_from_archive(v);
     _closure->do_oop(&obj);
   }
 };
--- a/src/hotspot/share/classfile/compactHashtable.inline.hpp	Wed Sep 12 16:28:06 2018 -0700
+++ b/src/hotspot/share/classfile/compactHashtable.inline.hpp	Wed Sep 12 17:45:22 2018 -0700
@@ -48,7 +48,7 @@
 inline oop CompactHashtable<T, N>::decode_entry(CompactHashtable<oop, char>* const t,
                                                 u4 offset, const char* name, int len) {
   narrowOop v = (narrowOop)offset;
-  oop string = HeapShared::decode_with_archived_oop_encoding_mode(v);
+  oop string = HeapShared::decode_from_archive(v);
   if (java_lang_String::equals(string, (jchar*)name, len)) {
     return string;
   }
--- a/src/hotspot/share/memory/filemap.cpp	Wed Sep 12 16:28:06 2018 -0700
+++ b/src/hotspot/share/memory/filemap.cpp	Wed Sep 12 17:45:22 2018 -0700
@@ -854,7 +854,7 @@
   if (with_current_oop_encoding_mode) {
     return (address)CompressedOops::decode_not_null(offset_of_space(spc));
   } else {
-    return (address)HeapShared::decode_with_archived_oop_encoding_mode(offset_of_space(spc));
+    return (address)HeapShared::decode_from_archive(offset_of_space(spc));
   }
 }
 
@@ -880,7 +880,7 @@
     CDSFileMapRegion* si = space_at(i);
     size_t size = si->_used;
     if (size > 0) {
-      address s = start_address_with_current_oop_encoding_mode(si);
+      address s = start_address_as_decoded_with_current_oop_encoding_mode(si);
       address e = s + size;
       if (start > s) {
         start = s;
@@ -972,21 +972,20 @@
   HeapShared::init_narrow_oop_decoding(narrow_oop_base() + delta, narrow_oop_shift());
 
   CDSFileMapRegion* si = space_at(MetaspaceShared::first_string);
-  address relocated_strings_bottom = start_address_with_archived_oop_encoding_mode(si);
-  if (!is_aligned(relocated_strings_bottom + delta, HeapRegion::GrainBytes)) {
+  address relocated_strings_bottom = start_address_as_decoded_from_archive(si);
+  if (!is_aligned(relocated_strings_bottom, HeapRegion::GrainBytes)) {
     // Align the bottom of the string regions at G1 region boundary. This will avoid
     // the situation where the highest open region and the lowest string region sharing
     // the same G1 region. Otherwise we will fail to map the open regions.
     size_t align = size_t(relocated_strings_bottom) % HeapRegion::GrainBytes;
     delta -= align;
-    assert(is_aligned(relocated_strings_bottom + delta, HeapRegion::GrainBytes), "must be");
-
     log_info(cds)("CDS heap data need to be relocated lower by a further " SIZE_FORMAT
-                  " bytes to be aligned with HeapRegion::GrainBytes", align);
-
+                  " bytes to " INTX_FORMAT " to be aligned with HeapRegion::GrainBytes", align, delta);
     HeapShared::init_narrow_oop_decoding(narrow_oop_base() + delta, narrow_oop_shift());
     _heap_pointers_need_patching = true;
+    relocated_strings_bottom = start_address_as_decoded_from_archive(si);
   }
+  assert(is_aligned(relocated_strings_bottom, HeapRegion::GrainBytes), "must be");
 
   // First, map string regions as closed archive heap regions.
   // GC does not write into the regions.
@@ -1032,7 +1031,7 @@
     si = space_at(i);
     size_t size = si->_used;
     if (size > 0) {
-      HeapWord* start = (HeapWord*)start_address_with_archived_oop_encoding_mode(si);
+      HeapWord* start = (HeapWord*)start_address_as_decoded_from_archive(si);
       regions[region_num] = MemRegion(start, size / HeapWordSize);
       region_num ++;
       log_info(cds)("Trying to map heap data: region[%d] at " INTPTR_FORMAT ", size = " SIZE_FORMAT_W(8) " bytes",
@@ -1242,7 +1241,7 @@
   if (MetaspaceShared::is_heap_region(idx)) {
     assert(DumpSharedSpaces, "The following doesn't work at runtime");
     return si->_used > 0 ?
-          (char*)start_address_with_current_oop_encoding_mode(si) : NULL;
+          (char*)start_address_as_decoded_with_current_oop_encoding_mode(si) : NULL;
   } else {
     return si->_addr._base;
   }
--- a/src/hotspot/share/memory/filemap.hpp	Wed Sep 12 16:28:06 2018 -0700
+++ b/src/hotspot/share/memory/filemap.hpp	Wed Sep 12 17:45:22 2018 -0700
@@ -335,12 +335,12 @@
   }
 
   // The starting address of spc, as calculated with CompressedOop::decode_non_null()
-  address start_address_with_current_oop_encoding_mode(CDSFileMapRegion* spc) {
+  address start_address_as_decoded_with_current_oop_encoding_mode(CDSFileMapRegion* spc) {
     return decode_start_address(spc, true);
   }
 
-  // The starting address of spc, as calculated with HeapShared::decode_with_archived_oop_encoding_mode()
-  address start_address_with_archived_oop_encoding_mode(CDSFileMapRegion* spc) {
+  // The starting address of spc, as calculated with HeapShared::decode_from_archive()
+  address start_address_as_decoded_from_archive(CDSFileMapRegion* spc) {
     return decode_start_address(spc, false);
   }
 
--- a/src/hotspot/share/memory/heapShared.cpp	Wed Sep 12 16:28:06 2018 -0700
+++ b/src/hotspot/share/memory/heapShared.cpp	Wed Sep 12 17:45:22 2018 -0700
@@ -834,7 +834,7 @@
     narrowOop* p = _start + offset;
     narrowOop v = *p;
     assert(!CompressedOops::is_null(v), "null oops should have been filtered out at dump time");
-    oop o = HeapShared::decode_with_archived_oop_encoding_mode(v);
+    oop o = HeapShared::decode_from_archive(v);
     RawAccess<IS_NOT_NULL>::oop_store(p, o);
     return true;
   }
--- a/src/hotspot/share/memory/heapShared.hpp	Wed Sep 12 16:28:06 2018 -0700
+++ b/src/hotspot/share/memory/heapShared.hpp	Wed Sep 12 17:45:22 2018 -0700
@@ -137,7 +137,7 @@
 
   static size_t build_archived_subgraph_info_records(int num_records);
 
-  // Used by decode_with_archived_oop_encoding_mode
+  // Used by decode_from_archive
   static address _narrow_oop_base;
   static int     _narrow_oop_shift;
 
@@ -194,7 +194,7 @@
   // than Universe::narrow_oop_{base,shift} -- see FileMapInfo::map_heap_regions_impl.
   // To decode them, do not use CompressedOops::decode_not_null. Use this
   // function instead.
-  inline static oop decode_with_archived_oop_encoding_mode(narrowOop v) NOT_CDS_JAVA_HEAP_RETURN_(NULL);
+  inline static oop decode_from_archive(narrowOop v) NOT_CDS_JAVA_HEAP_RETURN_(NULL);
 
   static void init_narrow_oop_decoding(address base, int shift) NOT_CDS_JAVA_HEAP_RETURN;
 
--- a/src/hotspot/share/memory/heapShared.inline.hpp	Wed Sep 12 16:28:06 2018 -0700
+++ b/src/hotspot/share/memory/heapShared.inline.hpp	Wed Sep 12 17:45:22 2018 -0700
@@ -30,7 +30,7 @@
 
 #if INCLUDE_CDS_JAVA_HEAP
 
-inline oop HeapShared::decode_with_archived_oop_encoding_mode(narrowOop v) {
+inline oop HeapShared::decode_from_archive(narrowOop v) {
   assert(!CompressedOops::is_null(v), "narrow oop value can never be zero");
   oop result = (oop)(void*)((uintptr_t)_narrow_oop_base + ((uintptr_t)v << _narrow_oop_shift));
   assert(check_obj_alignment(result), "address not aligned: " INTPTR_FORMAT, p2i((void*) result));
--- a/src/hotspot/share/memory/metaspaceShared.cpp	Wed Sep 12 16:28:06 2018 -0700
+++ b/src/hotspot/share/memory/metaspaceShared.cpp	Wed Sep 12 17:45:22 2018 -0700
@@ -1944,7 +1944,7 @@
   assert(archive_heap_region_fixed(),
          "must be called after archive heap regions are fixed");
   if (!CompressedOops::is_null(v)) {
-    oop obj = HeapShared::decode_with_archived_oop_encoding_mode(v);
+    oop obj = HeapShared::decode_from_archive(v);
     return G1CollectedHeap::heap()->materialize_archived_object(obj);
   }
   return NULL;
@@ -2021,7 +2021,7 @@
              "Archived heap object is not allowed");
       assert(MetaspaceShared::open_archive_heap_region_mapped(),
              "Open archive heap region is not mapped");
-      *p = HeapShared::decode_with_archived_oop_encoding_mode(o);
+      *p = HeapShared::decode_from_archive(o);
     }
   }
 
--- a/test/hotspot/jtreg/runtime/appcds/cacheObject/DifferentHeapSizes.java	Wed Sep 12 16:28:06 2018 -0700
+++ b/test/hotspot/jtreg/runtime/appcds/cacheObject/DifferentHeapSizes.java	Wed Sep 12 17:45:22 2018 -0700
@@ -40,6 +40,8 @@
 import jdk.test.lib.cds.CDSTestUtils;
 
 public class DifferentHeapSizes {
+    static final String DEDUP = "-XX:+UseStringDeduplication"; // This increases code coverage.
+
     static class Scenario {
         int dumpSize;   // in MB
         int runSizes[]; // in MB
@@ -59,7 +61,6 @@
     };
 
     public static void main(String[] args) throws Exception {
-        String dedup = "-XX:+UseStringDeduplication"; // This increases code coverage.
         JarBuilder.getOrCreateHelloJar();
         String appJar = TestCommon.getTestJar("hello.jar");
         String appClasses[] = TestCommon.list("Hello");
@@ -71,7 +72,7 @@
             for (int runSize : s.runSizes) {
                 String runXmx = "-Xmx" + runSize + "m";
                 CDSTestUtils.Result result = TestCommon.run("-cp", appJar, "-showversion",
-                        "-Xlog:cds", runXmx, dedup, "Hello");
+                        "-Xlog:cds", runXmx, DEDUP, "Hello");
                 if (runSize < 32768) {
                     result
                         .assertNormalExit("Hello World")
@@ -80,21 +81,49 @@
                             out.shouldNotContain(CDSTestUtils.MSG_RANGE_ALREADT_IN_USE);
                         });
                 } else {
-                    result.assertAbnormalExit("Unable to use shared archive: UseCompressedOops and UseCompressedClassPointers must be on for UseSharedSpaces.");
+                    result.assertAbnormalExit(CDSTestUtils.MSG_COMPRESSION_MUST_BE_USED);
                 }
             }
         }
-        String flag = "HeapBaseMinAddress";
-        String xxflag = "-XX:" + flag + "=";
-        String mx = "-Xmx128m";
-        long base = WhiteBox.getWhiteBox().getSizeTVMFlag(flag).longValue();
 
-        TestCommon.dump(appJar, appClasses, mx, xxflag + base);
-        TestCommon.run("-cp", appJar, "-showversion", "-Xlog:cds", mx, xxflag + (base + 256 * 1024 * 1024), dedup, "Hello")
-            .assertNormalExit("Hello World")
-            .assertNormalExit(out -> {
-                    out.shouldNotContain(CDSTestUtils.MSG_RANGE_NOT_WITHIN_HEAP);
-                    out.shouldNotContain(CDSTestUtils.MSG_RANGE_ALREADT_IN_USE);
-                });
+        // Test various settings of -XX:HeapBaseMinAddress that would trigger
+        // "CDS heap data need to be relocated because the desired range ... is outside of the heap"
+        long default_base = WhiteBox.getWhiteBox().getSizeTVMFlag("HeapBaseMinAddress").longValue();
+        long M = 1024 * 1024;
+        long bases[] = new long[] {
+            /* dump xmx */   /* run xmx */   /* dump base */             /* run base */
+            128 * M,         128 * M,        default_base,               default_base + 256L * 1024 * 1024,
+            128 * M,         16376 * M,      0x0000000119200000L,        -1,
+        };
+
+        for (int i = 0; i < bases.length; i += 4) {
+            String dump_xmx  = getXmx(bases[i+0]);
+            String run_xmx   = getXmx(bases[i+1]);
+            String dump_base = getHeapBaseMinAddress(bases[i+2]);
+            String run_base  = getHeapBaseMinAddress(bases[i+3]);
+
+            TestCommon.dump(appJar, appClasses, dump_xmx, dump_base);
+            TestCommon.run("-cp", appJar, "-showversion", "-Xlog:cds", run_xmx, run_base, DEDUP, "Hello")
+                .assertNormalExit("Hello World")
+                .assertNormalExit(out -> {
+                        out.shouldNotContain(CDSTestUtils.MSG_RANGE_NOT_WITHIN_HEAP);
+                        out.shouldNotContain(CDSTestUtils.MSG_RANGE_ALREADT_IN_USE);
+                    });
+        }
+    }
+
+    static String getXmx(long value) {
+        if (value < 0) {
+            return "-showversion"; // This is a harmless command line arg
+        } else {
+            return "-Xmx" + (value / 1024 / 1024) + "m";
+        }
+    }
+    static String getHeapBaseMinAddress(long value) {
+        if (value < 0) {
+            return "-showversion"; // This is a harmless command line arg
+        } else {
+            return "-XX:HeapBaseMinAddress=0x" + Long.toHexString(value);
+        }
     }
 }
--- a/test/lib/jdk/test/lib/cds/CDSTestUtils.java	Wed Sep 12 16:28:06 2018 -0700
+++ b/test/lib/jdk/test/lib/cds/CDSTestUtils.java	Wed Sep 12 17:45:22 2018 -0700
@@ -40,6 +40,8 @@
         "UseSharedSpaces: Unable to allocate region, range is not within java heap.";
     public static final String MSG_RANGE_ALREADT_IN_USE =
         "Unable to allocate region, java heap range is already in use.";
+    public static final String MSG_COMPRESSION_MUST_BE_USED =
+        "Unable to use shared archive: UseCompressedOops and UseCompressedClassPointers must be on for UseSharedSpaces.";
 
     public interface Checker {
         public void check(OutputAnalyzer output) throws Exception;