8254028: G1 incorrectly updates scan_top for collection set regions during preparation of evacuation

Reviewed-by: kbarrett

Author: Thomas Schatzl

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

@@ -906,14 +906,27 @@ void G1RemSet::scan_collection_set_regions(G1ParScanThreadState* pss,
   }
 }
 
-void G1RemSet::prepare_region_for_scan(HeapRegion* region) {
-  uint hrm_index = region->hrm_index();
+#ifdef ASSERT
+void G1RemSet::assert_scan_top_is_null(uint hrm_index) {
+  assert(_scan_state->scan_top(hrm_index) == NULL,
+         "scan_top of region %u is unexpectedly " PTR_FORMAT,
+         hrm_index, p2i(_scan_state->scan_top(hrm_index)));
+}
+#endif
+
+void G1RemSet::prepare_region_for_scan(HeapRegion* r) {
+  uint hrm_index = r->hrm_index();
 
-  if (region->is_old_or_humongous_or_archive()) {
-    _scan_state->set_scan_top(hrm_index, region->top());
+  // Only update non-collection set old regions, others must have already been set
+  // to NULL (don't scan) in the initialization.
+  if (r->in_collection_set()) {
+    assert_scan_top_is_null(hrm_index);
+  } else if (r->is_old_or_humongous_or_archive()) {
+    _scan_state->set_scan_top(hrm_index, r->top());
   } else {
-    assert(region->in_collection_set() || region->is_free(),
-           "Should only be free or in the collection set at this point %s", region->get_type_str());
+    assert_scan_top_is_null(hrm_index);
+    assert(r->is_free(),
+           "Region %u should be free region but is %s", hrm_index, r->get_type_str());
   }
 }
 

src/hotspot/share/gc/g1/g1RemSet.hpp

@@ -67,6 +67,8 @@ class G1RemSet: public CHeapObj<mtGC> {
   G1HotCardCache*        _hot_card_cache;
 
   void print_merge_heap_roots_stats();
+
+  void assert_scan_top_is_null(uint hrm_index) PRODUCT_RETURN;
 public:
 
   typedef CardTable::CardValue CardValue;