# HG changeset patch
# User shade
# Date 1556098804 -7200
# Node ID 24eb7720919c0bf2d93f1e9252ecbe5b6387d39c
# Parent  f6f95cb8643e83f88a5edb9efae63c3bf3ec7f97
8222838: Shenandoah: SEGV on accessing cset bitmap for NULL ptr
Reviewed-by: rkennke

diff -r f6f95cb8643e -r 24eb7720919c src/hotspot/share/gc/shenandoah/shenandoahCollectionSet.cpp
--- a/src/hotspot/share/gc/shenandoah/shenandoahCollectionSet.cpp	Wed Apr 24 11:39:56 2019 +0200
+++ b/src/hotspot/share/gc/shenandoah/shenandoahCollectionSet.cpp	Wed Apr 24 11:40:04 2019 +0200
@@ -30,22 +30,51 @@
 #include "runtime/atomic.hpp"
 #include "utilities/copy.hpp"
 
-ShenandoahCollectionSet::ShenandoahCollectionSet(ShenandoahHeap* heap, HeapWord* heap_base) :
+ShenandoahCollectionSet::ShenandoahCollectionSet(ShenandoahHeap* heap, char* heap_base, size_t size) :
   _map_size(heap->num_regions()),
   _region_size_bytes_shift(ShenandoahHeapRegion::region_size_bytes_shift()),
-  _cset_map(NEW_C_HEAP_ARRAY(jbyte, _map_size, mtGC)),
-  _biased_cset_map(_cset_map - ((uintx)heap_base >> _region_size_bytes_shift)),
+  _map_space(align_up(((uintx)heap_base + size) >> _region_size_bytes_shift, os::vm_allocation_granularity())),
+  _cset_map(_map_space.base() + ((uintx)heap_base >> _region_size_bytes_shift)),
+  _biased_cset_map(_map_space.base()),
   _heap(heap),
   _garbage(0),
   _live_data(0),
   _used(0),
   _region_count(0),
   _current_index(0) {
-  // Use 1-byte data type
-  STATIC_ASSERT(sizeof(jbyte) == 1);
+
+  // The collection set map is reserved to cover the entire heap *and* zero addresses.
+  // This is needed to accept in-cset checks for both heap oops and NULLs, freeing
+  // high-performance code from checking for NULL first.
+  //
+  // Since heap_base can be far away, committing the entire map would waste memory.
+  // Therefore, we only commit the parts that are needed to operate: the heap view,
+  // and the zero page.
+  //
+  // Note: we could instead commit the entire map, and piggyback on OS virtual memory
+  // subsystem for mapping not-yet-written-to pages to a single physical backing page,
+  // but this is not guaranteed, and would confuse NMT and other memory accounting tools.
+
+  MemTracker::record_virtual_memory_type(_map_space.base(), mtGC);
+
+  size_t page_size = (size_t)os::vm_page_size();
 
-  // Initialize cset map
+  if (!_map_space.special()) {
+    // Commit entire pages that cover the heap cset map.
+    char* bot_addr = align_down(_cset_map, page_size);
+    char* top_addr = align_up(_cset_map + _map_size, page_size);
+    os::commit_memory_or_exit(bot_addr, pointer_delta(top_addr, bot_addr, 1), false,
+                              "Unable to commit collection set bitmap: heap");
+
+    // Commit the zero page, if not yet covered by heap cset map.
+    if (bot_addr != _biased_cset_map) {
+      os::commit_memory_or_exit(_biased_cset_map, page_size, false,
+                                "Unable to commit collection set bitmap: zero page");
+    }
+  }
+
   Copy::zero_to_bytes(_cset_map, _map_size);
+  Copy::zero_to_bytes(_biased_cset_map, page_size);
 }
 
 void ShenandoahCollectionSet::add_region(ShenandoahHeapRegion* r) {
diff -r f6f95cb8643e -r 24eb7720919c src/hotspot/share/gc/shenandoah/shenandoahCollectionSet.hpp
--- a/src/hotspot/share/gc/shenandoah/shenandoahCollectionSet.hpp	Wed Apr 24 11:39:56 2019 +0200
+++ b/src/hotspot/share/gc/shenandoah/shenandoahCollectionSet.hpp	Wed Apr 24 11:40:04 2019 +0200
@@ -33,9 +33,10 @@
 private:
   size_t const          _map_size;
   size_t const          _region_size_bytes_shift;
-  jbyte* const          _cset_map;
+  ReservedSpace         _map_space;
+  char* const           _cset_map;
   // Bias cset map's base address for fast test if an oop is in cset
-  jbyte* const          _biased_cset_map;
+  char* const           _biased_cset_map;
 
   ShenandoahHeap* const _heap;
 
@@ -49,7 +50,7 @@
   DEFINE_PAD_MINUS_SIZE(1, DEFAULT_CACHE_LINE_SIZE, 0);
 
 public:
-  ShenandoahCollectionSet(ShenandoahHeap* heap, HeapWord* heap_base);
+  ShenandoahCollectionSet(ShenandoahHeap* heap, char* heap_base, size_t size);
 
   // Add region to collection set
   void add_region(ShenandoahHeapRegion* r);
@@ -88,10 +89,10 @@
   void clear();
 
 private:
-  jbyte* map_address() const {
+  char* map_address() const {
     return _cset_map;
   }
-  jbyte* biased_map_address() const {
+  char* biased_map_address() const {
     return _biased_cset_map;
   }
 };
diff -r f6f95cb8643e -r 24eb7720919c src/hotspot/share/gc/shenandoah/shenandoahHeap.cpp
--- a/src/hotspot/share/gc/shenandoah/shenandoahHeap.cpp	Wed Apr 24 11:39:56 2019 +0200
+++ b/src/hotspot/share/gc/shenandoah/shenandoahHeap.cpp	Wed Apr 24 11:40:04 2019 +0200
@@ -259,7 +259,7 @@
 
   _regions = NEW_C_HEAP_ARRAY(ShenandoahHeapRegion*, _num_regions, mtGC);
   _free_set = new ShenandoahFreeSet(this, _num_regions);
-  _collection_set = new ShenandoahCollectionSet(this, (HeapWord*)sh_rs.base());
+  _collection_set = new ShenandoahCollectionSet(this, sh_rs.base(), sh_rs.size());
 
   {
     ShenandoahHeapLocker locker(lock());