8222838: Shenandoah: SEGV on accessing cset bitmap for NULL ptr
Reviewed-by: rkennke
--- 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) {
--- 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;
}
};
--- 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());