From dbbabfd4f4bfbceb391d6f7f7fb4ec9c4e1b104d Mon Sep 17 00:00:00 2001 From: William Kemper Date: Fri, 19 Jun 2026 19:10:36 +0000 Subject: [PATCH] 8386798: Shenandoah: Missing load barrier when making assertions about mark bitmap Reviewed-by: xpeng, kdnilsen --- .../share/gc/shenandoah/shenandoahAsserts.cpp | 10 ++++++++++ .../share/gc/shenandoah/shenandoahAsserts.hpp | 7 +++++++ .../share/gc/shenandoah/shenandoahFreeSet.cpp | 12 +++++------- .../gc/shenandoah/shenandoahHeapRegion.cpp | 17 ++--------------- .../gc/shenandoah/shenandoahMarkingContext.cpp | 6 ++++-- 5 files changed, 28 insertions(+), 24 deletions(-) diff --git a/src/hotspot/share/gc/shenandoah/shenandoahAsserts.cpp b/src/hotspot/share/gc/shenandoah/shenandoahAsserts.cpp index 278c04b35d6..2eceeb2eca6 100644 --- a/src/hotspot/share/gc/shenandoah/shenandoahAsserts.cpp +++ b/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); diff --git a/src/hotspot/share/gc/shenandoah/shenandoahAsserts.hpp b/src/hotspot/share/gc/shenandoah/shenandoahAsserts.hpp index 545415a6531..44330d54303 100644 --- a/src/hotspot/share/gc/shenandoah/shenandoahAsserts.hpp +++ b/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 @@ public: 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 @@ public: #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 @@ public: #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) diff --git a/src/hotspot/share/gc/shenandoah/shenandoahFreeSet.cpp b/src/hotspot/share/gc/shenandoah/shenandoahFreeSet.cpp index 3bbca7de1ff..24b3743281a 100644 --- a/src/hotspot/share/gc/shenandoah/shenandoahFreeSet.cpp +++ b/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 @@ -36,7 +36,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) { @@ -1685,11 +1684,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 { diff --git a/src/hotspot/share/gc/shenandoah/shenandoahHeapRegion.cpp b/src/hotspot/share/gc/shenandoah/shenandoahHeapRegion.cpp index 063c55ac9c3..66eaed2c222 100644 --- a/src/hotspot/share/gc/shenandoah/shenandoahHeapRegion.cpp +++ b/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,7 +32,6 @@ #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" @@ -40,14 +39,11 @@ #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; diff --git a/src/hotspot/share/gc/shenandoah/shenandoahMarkingContext.cpp b/src/hotspot/share/gc/shenandoah/shenandoahMarkingContext.cpp index 0bcdfa9fd2c..87629cefb0d 100644 --- a/src/hotspot/share/gc/shenandoah/shenandoahMarkingContext.cpp +++ b/src/hotspot/share/gc/shenandoah/shenandoahMarkingContext.cpp @@ -1,7 +1,7 @@ /* - * Copyright (c) 2018, 2026, Red Hat, Inc. All rights reserved. + * Copyright (c) 2018, 2021, Red Hat, Inc. All rights reserved. * Copyright Amazon.com Inc. or its affiliates. All Rights Reserved. - * Copyright (c) 2026, 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 @@ -91,6 +91,8 @@ 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; }