8380390: Shenandoah: Missing store barrier when resetting bitmaps

8386798: Shenandoah: Missing load barrier when making assertions about mark bitmap

Reviewed-by: xpeng, kdnilsen
Backport-of: a13dd29

Author: William Kemper

src/hotspot/share/gc/shenandoah/shenandoahAsserts.cpp

@@ -31,6 +31,7 @@
 #include "gc/shenandoah/shenandoahUtils.hpp"
 #include "memory/resourceArea.hpp"
 #include "oops/oop.inline.hpp"
+#include "runtime/orderAccess.hpp"
 #include "runtime/os.hpp"
 #include "utilities/vmError.hpp"
 
@@ -425,6 +426,15 @@ void ShenandoahAsserts::assert_marked_strong(void *interior_loc, oop obj, const
   }
 }
 
+void ShenandoahAsserts::assert_bitmap_clear_above_top(ShenandoahHeapRegion* region) {
+  ShenandoahMarkingContext* const ctx = ShenandoahHeap::heap()->marking_context();
+  const HeapWord* top_bitmap = ctx->top_bitmap(region);
+  // Make sure that top is loaded before any of the marks from the bitmap are loaded. If another
+  // thread has cleared the bitmap we must not allow any stale reads.
+  OrderAccess::loadload();
+  assert(ctx->is_bitmap_range_within_region_clear(top_bitmap, region->end()), "Bitmap above top_bitmap() must be clear");
+}
+
 void ShenandoahAsserts::assert_mark_complete(HeapWord* obj, const char* file, int line) {
   const ShenandoahHeap* heap = ShenandoahHeap::heap();
   const ShenandoahHeapRegion* region = heap->heap_region_containing(obj);

src/hotspot/share/gc/shenandoah/shenandoahAsserts.hpp

@@ -31,6 +31,8 @@
 #include "runtime/mutex.hpp"
 #include "utilities/formatBuffer.hpp"
 
+class ShenandoahHeapRegion;
+
 typedef FormatBuffer<8192> ShenandoahMessageBuffer;
 
 class ShenandoahAsserts {
@@ -65,6 +67,7 @@ class ShenandoahAsserts {
   static void assert_marked(void* interior_loc, oop obj, const char* file, int line);
   static void assert_marked_weak(void* interior_loc, oop obj, const char* file, int line);
   static void assert_marked_strong(void* interior_loc, oop obj, const char* file, int line);
+  static void assert_bitmap_clear_above_top(ShenandoahHeapRegion* region);
 
   // Assert that marking is complete for the generation where this obj resides
   static void assert_mark_complete(HeapWord* obj, const char* file, int line);
@@ -137,6 +140,9 @@ class ShenandoahAsserts {
 #define shenandoah_assert_marked_strong(interior_loc, obj) \
                     ShenandoahAsserts::assert_marked_strong(interior_loc, obj, __FILE__, __LINE__)
 
+#define shenandoah_assert_clear_above_top(region) \
+                    ShenandoahAsserts::assert_bitmap_clear_above_top(region)
+
 #define shenandoah_assert_mark_complete(obj) \
                     ShenandoahAsserts::assert_mark_complete(obj, __FILE__, __LINE__)
 
@@ -227,6 +233,7 @@ class ShenandoahAsserts {
 #define shenandoah_assert_marked_strong_except(interior_loc, obj, exception)
 #define shenandoah_assert_marked_strong(interior_loc, obj)
 
+#define shenandoah_assert_clear_above_top(region)
 #define shenandoah_assert_mark_complete(obj)
 
 #define shenandoah_assert_in_cset_if(interior_loc, obj, condition)

src/hotspot/share/gc/shenandoah/shenandoahFreeSet.cpp

@@ -1,7 +1,7 @@
 /*
  * Copyright (c) 2016, 2021, Red Hat, Inc. All rights reserved.
  * Copyright Amazon.com Inc. or its affiliates. All Rights Reserved.
- * Copyright (c) 2025, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2025, 2026, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -35,7 +35,6 @@
 #include "gc/shenandoah/shenandoahYoungGeneration.hpp"
 #include "logging/logStream.hpp"
 #include "memory/resourceArea.hpp"
-#include "runtime/orderAccess.hpp"
 
 static const char* partition_name(ShenandoahFreeSetPartitionId t) {
   switch (t) {
@@ -1510,11 +1509,10 @@ HeapWord* ShenandoahFreeSet::try_allocate_in(ShenandoahHeapRegion* r, Shenandoah
       // coalesce-and-fill processing.
       r->end_preemptible_coalesce_and_fill();
     }
-#ifdef ASSERT
-    ShenandoahMarkingContext* const ctx = _heap->marking_context();
-    assert(ctx->top_at_mark_start(r) == r->bottom(), "Newly established allocation region starts with TAMS equal to bottom");
-    assert(ctx->is_bitmap_range_within_region_clear(ctx->top_bitmap(r), r->end()), "Bitmap above top_bitmap() must be clear");
-#endif
+
+    assert(_heap->marking_context()->top_at_mark_start(r) == r->bottom(),
+      "Newly established allocation region (%zu) must start with TAMS equal to bottom", r->index());
+    shenandoah_assert_clear_above_top(r);
     log_debug(gc, free)("Using new region (%zu) for %s (" PTR_FORMAT ").",
                         r->index(), req.type_string(), p2i(&req));
   } else {

src/hotspot/share/gc/shenandoah/shenandoahHeapRegion.cpp

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2023, 2025, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2023, 2026, Oracle and/or its affiliates. All rights reserved.
  * Copyright (c) 2013, 2020, Red Hat, Inc. All rights reserved.
  * Copyright Amazon.com Inc. or its affiliates. All Rights Reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
@@ -32,22 +32,18 @@
 #include "gc/shenandoah/shenandoahGeneration.hpp"
 #include "gc/shenandoah/shenandoahHeap.inline.hpp"
 #include "gc/shenandoah/shenandoahHeapRegion.hpp"
-#include "gc/shenandoah/shenandoahHeapRegionSet.inline.hpp"
 #include "gc/shenandoah/shenandoahMarkingContext.inline.hpp"
 #include "gc/shenandoah/shenandoahOldGeneration.hpp"
 #include "gc/shenandoah/shenandoahScanRemembered.inline.hpp"
 #include "gc/shenandoah/shenandoahYoungGeneration.hpp"
 #include "jfr/jfrEvents.hpp"
 #include "memory/allocation.hpp"
 #include "memory/iterator.inline.hpp"
-#include "memory/resourceArea.hpp"
 #include "memory/universe.hpp"
 #include "oops/oop.inline.hpp"
 #include "runtime/globals_extension.hpp"
 #include "runtime/java.hpp"
-#include "runtime/mutexLocker.hpp"
 #include "runtime/os.hpp"
-#include "runtime/safepoint.hpp"
 #include "utilities/powerOfTwo.hpp"
 
 size_t ShenandoahHeapRegion::RegionCount = 0;
@@ -846,16 +842,7 @@ void ShenandoahHeapRegion::set_affiliation(ShenandoahAffiliation new_affiliation
                   p2i(top()), p2i(ctx->top_at_mark_start(this)), p2i(_update_watermark.load_relaxed()), p2i(ctx->top_bitmap(this)));
   }
 
-#ifdef ASSERT
-  {
-    size_t idx = this->index();
-    HeapWord* top_bitmap = ctx->top_bitmap(this);
-
-    assert(ctx->is_bitmap_range_within_region_clear(top_bitmap, _end),
-           "Region %zu, bitmap should be clear between top_bitmap: " PTR_FORMAT " and end: " PTR_FORMAT, idx,
-           p2i(top_bitmap), p2i(_end));
-  }
-#endif
+  shenandoah_assert_clear_above_top(this);
 
   if (region_affiliation == new_affiliation) {
     return;

src/hotspot/share/gc/shenandoah/shenandoahHeapRegion.hpp

@@ -280,6 +280,7 @@ class ShenandoahHeapRegion {
   // clears the self_fwd bits. Safety-net reset on region recycle.
   ShenandoahSharedFlag _has_self_forwards;
 
+  // This is only read/written by a gc worker to avoid unnecessary bitmap resets
   bool _needs_bitmap_reset;
 
 public:

src/hotspot/share/gc/shenandoah/shenandoahMarkingContext.cpp

@@ -1,7 +1,7 @@
 /*
  * Copyright (c) 2018, 2021, Red Hat, Inc. All rights reserved.
  * Copyright Amazon.com Inc. or its affiliates. All Rights Reserved.
- * Copyright (c) 2025, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2025, 2026, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -27,6 +27,7 @@
 #include "gc/shared/markBitMap.inline.hpp"
 #include "gc/shenandoah/shenandoahHeap.inline.hpp"
 #include "gc/shenandoah/shenandoahMarkingContext.hpp"
+#include "runtime/orderAccess.hpp"
 
 ShenandoahMarkingContext::ShenandoahMarkingContext(MemRegion heap_region, MemRegion bitmap_region, size_t num_regions) :
   _mark_bit_map(heap_region, bitmap_region),
@@ -90,6 +91,9 @@ void ShenandoahMarkingContext::clear_bitmap(ShenandoahHeapRegion* r) {
 
   if (top_bitmap > bottom) {
     _mark_bit_map.clear_range_large(MemRegion(bottom, top_bitmap));
+    // All bitmap writes must complete before we update top at bitmap. If these writes were reordered,
+    // other threads could see stale marks above top, which is not valid.
+    OrderAccess::storestore();
     _top_bitmaps[r->index()] = bottom;
   }