8292912: Make guard card in CardTable inaccessible

Reviewed-by: tschatzl, sjohanss

src/hotspot/share/gc/g1/g1CardTable.cpp

@@ -52,8 +52,8 @@ void G1CardTable::initialize(G1RegionToSpaceMapper* mapper) {
 
   _byte_map_size = mapper->reserved().byte_size();
 
-  _guard_index = cards_required(_whole_heap.word_size()) - 1;
-  _last_valid_index = _guard_index - 1;
+  size_t num_cards = cards_required(_whole_heap.word_size());
+  _last_valid_index = num_cards - 1;
 
   HeapWord* low_bound  = _whole_heap.start();
   HeapWord* high_bound = _whole_heap.end();

src/hotspot/share/gc/shared/cardTable.cpp

@@ -61,17 +61,14 @@ void CardTable::initialize_card_size() {
   log_info_p(gc, init)("CardTable entry size: " UINT32_FORMAT,  _card_size);
 }
 
-size_t CardTable::compute_byte_map_size() {
-  assert(_guard_index == cards_required(_whole_heap.word_size()) - 1,
-                                        "uninitialized, check declaration order");
+size_t CardTable::compute_byte_map_size(size_t num_bytes) {
   assert(_page_size != 0, "uninitialized, check declaration order");
   const size_t granularity = os::vm_allocation_granularity();
-  return align_up(_guard_index + 1, MAX2(_page_size, granularity));
+  return align_up(num_bytes, MAX2(_page_size, granularity));
 }
 
 CardTable::CardTable(MemRegion whole_heap) :
   _whole_heap(whole_heap),
-  _guard_index(0),
   _last_valid_index(0),
   _page_size(os::vm_page_size()),
   _byte_map_size(0),
@@ -92,10 +89,12 @@ CardTable::~CardTable() {
 }
 
 void CardTable::initialize() {
-  _guard_index = cards_required(_whole_heap.word_size()) - 1;
-  _last_valid_index = _guard_index - 1;
+  size_t num_cards = cards_required(_whole_heap.word_size());
+  _last_valid_index = num_cards - 1;
 
-  _byte_map_size = compute_byte_map_size();
+  // each card takes 1 byte; + 1 for the guard card
+  size_t num_bytes = num_cards + 1;
+  _byte_map_size = compute_byte_map_size(num_bytes);
 
   HeapWord* low_bound  = _whole_heap.start();
   HeapWord* high_bound = _whole_heap.end();
@@ -108,7 +107,7 @@ void CardTable::initialize() {
 
   MemTracker::record_virtual_memory_type((address)heap_rs.base(), mtGC);
 
-  os::trace_page_sizes("Card Table", _guard_index + 1, _guard_index + 1,
+  os::trace_page_sizes("Card Table", num_bytes, num_bytes,
                        _page_size, heap_rs.base(), heap_rs.size());
   if (!heap_rs.is_reserved()) {
     vm_exit_during_initialization("Could not reserve enough space for the "
@@ -124,12 +123,9 @@ void CardTable::initialize() {
   assert(byte_for(low_bound) == &_byte_map[0], "Checking start of map");
   assert(byte_for(high_bound-1) <= &_byte_map[_last_valid_index], "Checking end of map");
 
-  CardValue* guard_card = &_byte_map[_guard_index];
-  HeapWord* guard_page = align_down((HeapWord*)guard_card, _page_size);
-  _guard_region = MemRegion(guard_page, _page_size);
-  os::commit_memory_or_exit((char*)guard_page, _page_size, _page_size,
-                            !ExecMem, "card table last card");
-  *guard_card = last_card;
+  CardValue* guard_card = &_byte_map[num_cards];
+  assert(is_aligned(guard_card, _page_size), "must be on its own OS page");
+  _guard_region = MemRegion((HeapWord*)guard_card, _page_size);
 
   log_trace(gc, barrier)("CardTable::CardTable: ");
   log_trace(gc, barrier)("    &_byte_map[0]: " PTR_FORMAT "  &_byte_map[_last_valid_index]: " PTR_FORMAT,
@@ -172,22 +168,20 @@ HeapWord* CardTable::largest_prev_committed_end(int ind) const {
 }
 
 MemRegion CardTable::committed_unique_to_self(int self, MemRegion mr) const {
+  assert(mr.intersection(_guard_region).is_empty(), "precondition");
   MemRegion result = mr;
   for (int r = 0; r < _cur_covered_regions; r += 1) {
     if (r != self) {
       result = result.minus(_committed[r]);
     }
   }
-  // Never include the guard page.
-  result = result.minus(_guard_region);
   return result;
 }
 
 void CardTable::resize_covered_region(MemRegion new_region) {
   // We don't change the start of a region, only the end.
   assert(_whole_heap.contains(new_region),
            "attempt to cover area not in reserved area");
-  debug_only(verify_guard();)
   // collided is true if the expansion would push into another committed region
   debug_only(bool collided = false;)
   int const ind = find_covering_region_by_base(new_region.start());
@@ -301,7 +295,7 @@ void CardTable::resize_covered_region(MemRegion new_region) {
     } else {
       entry = byte_after(old_region.last());
     }
-    assert(index_for(new_region.last()) <  _guard_index,
+    assert(index_for(new_region.last()) <=  _last_valid_index,
       "The guard card will be overwritten");
     // This line commented out cleans the newly expanded region and
     // not the aligned up expanded region.
@@ -342,7 +336,6 @@ void CardTable::resize_covered_region(MemRegion new_region) {
   // Touch the last card of the covered region to show that it
   // is committed (or SEGV).
   debug_only((void) (*byte_for(_covered[ind].last()));)
-  debug_only(verify_guard();)
 }
 
 // Note that these versions are precise!  The scanning code has to handle the
@@ -384,12 +377,6 @@ uintx CardTable::ct_max_alignment_constraint() {
   return GCCardSizeInBytes * os::vm_page_size();
 }
 
-void CardTable::verify_guard() {
-  // For product build verification
-  guarantee(_byte_map[_guard_index] == last_card,
-            "card table guard has been modified");
-}
-
 void CardTable::invalidate(MemRegion mr) {
   assert(align_down(mr.start(), HeapWordSize) == mr.start(), "Unaligned start");
   assert(align_up  (mr.end(),   HeapWordSize) == mr.end(),   "Unaligned end"  );
@@ -399,10 +386,6 @@ void CardTable::invalidate(MemRegion mr) {
   }
 }
 
-void CardTable::verify() {
-  verify_guard();
-}
-
 #ifndef PRODUCT
 void CardTable::verify_region(MemRegion mr, CardValue val, bool val_equals) {
   CardValue* start    = byte_for(mr.start());

src/hotspot/share/gc/shared/cardTable.hpp

@@ -44,9 +44,6 @@ class CardTable: public CHeapObj<mtGC> {
   // The declaration order of these const fields is important; see the
   // constructor before changing.
   const MemRegion _whole_heap;       // the region covered by the card table
-  size_t          _guard_index;      // index of very last element in the card
-                                     // table; it is set to a guard value
-                                     // (last_card) and should never be modified
   size_t          _last_valid_index; // index of the last valid element
   const size_t    _page_size;        // page size used when mapping _byte_map
   size_t          _byte_map_size;    // in bytes
@@ -65,12 +62,10 @@ class CardTable: public CHeapObj<mtGC> {
   // actually extend onto the card-table space for the next covered region.
   MemRegion* _committed;
 
-  // The last card is a guard card, and we commit the page for it so
-  // we can use the card for verification purposes. We make sure we never
-  // uncommit the MemRegion for that page.
+  // The last card is a guard card; never committed.
   MemRegion _guard_region;
 
-  inline size_t compute_byte_map_size();
+  inline size_t compute_byte_map_size(size_t num_bytes);
 
   // Finds and return the index of the region, if any, to which the given
   // region would be contiguous.  If none exists, assign a new region and
@@ -133,9 +128,8 @@ class CardTable: public CHeapObj<mtGC> {
   // Initialization utilities; covered_words is the size of the covered region
   // in, um, words.
   inline size_t cards_required(size_t covered_words) {
-    // Add one for a guard card, used to detect errors.
-    const size_t words = align_up(covered_words, _card_size_in_words);
-    return words / _card_size_in_words + 1;
+    assert(is_aligned(covered_words, _card_size_in_words), "precondition");
+    return covered_words / _card_size_in_words;
   }
 
   // Dirty the bytes corresponding to "mr" (not all of which must be
@@ -249,9 +243,6 @@ class CardTable: public CHeapObj<mtGC> {
   // Print a description of the memory for the card table
   virtual void print_on(outputStream* st) const;
 
-  void verify();
-  void verify_guard();
-
   // val_equals -> it will check that all cards covered by mr equal val
   // !val_equals -> it will check that all cards covered by mr do not equal val
   void verify_region(MemRegion mr, CardValue val, bool val_equals) PRODUCT_RETURN;

src/hotspot/share/gc/shared/cardTableRS.cpp

@@ -431,7 +431,6 @@ void CardTableRS::verify() {
   // generational heaps.
   VerifyCTGenClosure blk(this);
   GenCollectedHeap::heap()->generation_iterate(&blk, false);
-  CardTable::verify();
 }
 
 CardTableRS::CardTableRS(MemRegion whole_heap) :

src/hotspot/share/gc/shared/vmStructs_gc.hpp

@@ -88,7 +88,6 @@
   nonstatic_field(BarrierSet::FakeRtti,        _concrete_tag,                                 BarrierSet::Name)                      \
                                                                                                                                      \
   nonstatic_field(CardTable,                   _whole_heap,                                   const MemRegion)                       \
-  nonstatic_field(CardTable,                   _guard_index,                                  const size_t)                          \
   nonstatic_field(CardTable,                   _last_valid_index,                             const size_t)                          \
   nonstatic_field(CardTable,                   _page_size,                                    const size_t)                          \
   nonstatic_field(CardTable,                   _byte_map_size,                                const size_t)                          \