From b337599da27b7659d816f3c4e91566543bbbcc25 Mon Sep 17 00:00:00 2001 From: Aleksey Shipilev Date: Wed, 6 May 2026 14:41:18 +0000 Subject: [PATCH] 8383810: Shenandoah: Simplify native CAS barriers Reviewed-by: rkennke, xpeng, kdnilsen --- .../shenandoahBarrierSet.inline.hpp | 80 ++++++++---- .../gc/shenandoah/TestWeakReferenceCAS.java | 117 ++++++++++++++++++ 2 files changed, 173 insertions(+), 24 deletions(-) create mode 100644 test/hotspot/jtreg/gc/shenandoah/TestWeakReferenceCAS.java diff --git a/src/hotspot/share/gc/shenandoah/shenandoahBarrierSet.inline.hpp b/src/hotspot/share/gc/shenandoah/shenandoahBarrierSet.inline.hpp index fbb5a4350bb..f4a6d2b9038 100644 --- a/src/hotspot/share/gc/shenandoah/shenandoahBarrierSet.inline.hpp +++ b/src/hotspot/share/gc/shenandoah/shenandoahBarrierSet.inline.hpp @@ -249,29 +249,35 @@ inline oop ShenandoahBarrierSet::oop_load(DecoratorSet decorators, T* addr) { template inline oop ShenandoahBarrierSet::oop_cmpxchg(DecoratorSet decorators, T* addr, oop compare_value, oop new_value) { - oop res; - oop expected = compare_value; - do { - compare_value = expected; - res = RawAccess<>::oop_atomic_cmpxchg(addr, compare_value, new_value); - expected = res; - } while ((compare_value != expected) && (resolve_forwarded(compare_value) == resolve_forwarded(expected))); + shenandoah_assert_not_in_cset_except(nullptr, compare_value, (compare_value == nullptr || ShenandoahHeap::heap()->cancelled_gc())); + shenandoah_assert_not_in_cset_except(nullptr, new_value, (new_value == nullptr || ShenandoahHeap::heap()->cancelled_gc())); - // Note: We don't need a keep-alive-barrier here. We already enqueue any loaded reference for SATB anyway, - // because it must be the previous value. - res = load_reference_barrier(decorators, res, static_cast(nullptr)); - satb_enqueue(res); - return res; + // Handle the previous value through SATB, as we are about to perform the store. + oop prev = RawAccess<>::oop_load(addr); + satb_enqueue(prev); + + // Perform LRB on location to fix it up for this and all following accesses. + // This guarantees there are no false negatives due to concurrent evacuation, + // and the value loaded later by CAS is sanitized by some LRB, or is null. + load_reference_barrier(decorators, prev, addr); + + return RawAccess<>::oop_atomic_cmpxchg(addr, compare_value, new_value); } template inline oop ShenandoahBarrierSet::oop_xchg(DecoratorSet decorators, T* addr, oop new_value) { - oop previous = RawAccess<>::oop_atomic_xchg(addr, new_value); - // Note: We don't need a keep-alive-barrier here. We already enqueue any loaded reference for SATB anyway, - // because it must be the previous value. - previous = load_reference_barrier(decorators, previous, static_cast(nullptr)); - satb_enqueue(previous); - return previous; + shenandoah_assert_not_in_cset_except(nullptr, new_value, (new_value == nullptr || ShenandoahHeap::heap()->cancelled_gc())); + + // Handle the previous value through SATB, as we are about to perform the store. + oop prev = RawAccess<>::oop_load(addr); + satb_enqueue(prev); + + // Perform LRB on location to fix it up for this and all following accesses. + // This is purely opportunistic: we would not have any false negatives here. + // This guarantees the value loaded later by XCHG is sanitized by some LRB, or is null. + load_reference_barrier(decorators, prev, addr); + + return RawAccess<>::oop_atomic_xchg(addr, new_value); } template @@ -338,7 +344,8 @@ inline void ShenandoahBarrierSet::AccessBarrier::oop_st template template inline oop ShenandoahBarrierSet::AccessBarrier::oop_atomic_cmpxchg_not_in_heap(T* addr, oop compare_value, oop new_value) { - assert((decorators & (AS_NO_KEEPALIVE | ON_UNKNOWN_OOP_REF)) == 0, "must be absent"); + assert((decorators & AS_NO_KEEPALIVE) == 0, "CAS only with keep-alive"); + assert((decorators & ON_STRONG_OOP_REF) != 0, "CAS only for strong refs"); ShenandoahBarrierSet* bs = ShenandoahBarrierSet::barrier_set(); return bs->oop_cmpxchg(decorators, addr, compare_value, new_value); } @@ -346,7 +353,8 @@ inline oop ShenandoahBarrierSet::AccessBarrier::oop_ato template template inline oop ShenandoahBarrierSet::AccessBarrier::oop_atomic_cmpxchg_in_heap(T* addr, oop compare_value, oop new_value) { - assert((decorators & (AS_NO_KEEPALIVE | ON_UNKNOWN_OOP_REF)) == 0, "must be absent"); + assert((decorators & AS_NO_KEEPALIVE) == 0, "CAS only with keep-alive"); + assert((decorators & ON_STRONG_OOP_REF) != 0, "CAS only for strong refs"); ShenandoahBarrierSet* bs = ShenandoahBarrierSet::barrier_set(); oop result = bs->oop_cmpxchg(decorators, addr, compare_value, new_value); if (ShenandoahCardBarrier) { @@ -357,9 +365,20 @@ inline oop ShenandoahBarrierSet::AccessBarrier::oop_ato template inline oop ShenandoahBarrierSet::AccessBarrier::oop_atomic_cmpxchg_in_heap_at(oop base, ptrdiff_t offset, oop compare_value, oop new_value) { - assert((decorators & AS_NO_KEEPALIVE) == 0, "must be absent"); + assert((decorators & AS_NO_KEEPALIVE) == 0, "CAS only with keep-alive"); + assert((decorators & (ON_STRONG_OOP_REF | ON_UNKNOWN_OOP_REF)) != 0, "CAS only for strong refs OR unknown refs (Unsafe)"); ShenandoahBarrierSet* bs = ShenandoahBarrierSet::barrier_set(); + + // Unsafe.compareAndExchange/Set come here with ON_UNKNOWN_OOP_REF set. + // These are normally strong refs, but one can use Unsafe on Reference.referent. + // We cannot deal with that case. If application does Unsafe operations on + // Reference.referent field, this likely breaks weak reference semantics already. + // We upgrade the access to strong in (sometimes futile) attempt to maintain heap + // integrity, and assert in debug builds for better diagnostics. DecoratorSet resolved_decorators = AccessBarrierSupport::resolve_possibly_unknown_oop_ref_strength(base, offset); + assert((resolved_decorators & ON_STRONG_OOP_REF) != 0, "Application error: CAS on weak location"); + resolved_decorators = (resolved_decorators & ~ON_DECORATOR_MASK) | ON_STRONG_OOP_REF; + auto addr = AccessInternal::oop_field_addr(base, offset); oop result = bs->oop_cmpxchg(resolved_decorators, addr, compare_value, new_value); if (ShenandoahCardBarrier) { @@ -371,7 +390,8 @@ inline oop ShenandoahBarrierSet::AccessBarrier::oop_ato template template inline oop ShenandoahBarrierSet::AccessBarrier::oop_atomic_xchg_not_in_heap(T* addr, oop new_value) { - assert((decorators & (AS_NO_KEEPALIVE | ON_UNKNOWN_OOP_REF)) == 0, "must be absent"); + assert((decorators & AS_NO_KEEPALIVE) == 0, "XCHG only with keep-alive"); + assert((decorators & ON_STRONG_OOP_REF) != 0, "XCHG only for strong refs"); ShenandoahBarrierSet* bs = ShenandoahBarrierSet::barrier_set(); return bs->oop_xchg(decorators, addr, new_value); } @@ -379,7 +399,8 @@ inline oop ShenandoahBarrierSet::AccessBarrier::oop_ato template template inline oop ShenandoahBarrierSet::AccessBarrier::oop_atomic_xchg_in_heap(T* addr, oop new_value) { - assert((decorators & (AS_NO_KEEPALIVE | ON_UNKNOWN_OOP_REF)) == 0, "must be absent"); + assert((decorators & AS_NO_KEEPALIVE) == 0, "XCHG only with keep-alive"); + assert((decorators & ON_STRONG_OOP_REF) != 0, "XCHG only for strong refs"); ShenandoahBarrierSet* bs = ShenandoahBarrierSet::barrier_set(); oop result = bs->oop_xchg(decorators, addr, new_value); if (ShenandoahCardBarrier) { @@ -390,9 +411,20 @@ inline oop ShenandoahBarrierSet::AccessBarrier::oop_ato template inline oop ShenandoahBarrierSet::AccessBarrier::oop_atomic_xchg_in_heap_at(oop base, ptrdiff_t offset, oop new_value) { - assert((decorators & AS_NO_KEEPALIVE) == 0, "must be absent"); + assert((decorators & AS_NO_KEEPALIVE) == 0, "XCHG only with keep-alive"); + assert((decorators & (ON_STRONG_OOP_REF | ON_UNKNOWN_OOP_REF)) != 0, "XCHG only for strong refs OR unknown refs (Unsafe)"); ShenandoahBarrierSet* bs = ShenandoahBarrierSet::barrier_set(); + + // Unsafe.getAndSet comes here with ON_UNKNOWN_OOP_REF set. + // These are normally strong refs, but one can use Unsafe on Reference.referent. + // We cannot deal with that case. If application does Unsafe operations on + // Reference.referent field, this likely breaks weak reference semantics already. + // We upgrade the access to strong in (sometimes futile) attempt to maintain heap + // integrity, and assert in debug builds for better diagnostics. DecoratorSet resolved_decorators = AccessBarrierSupport::resolve_possibly_unknown_oop_ref_strength(base, offset); + assert((resolved_decorators & ON_STRONG_OOP_REF) != 0, "Application error: XCHG on weak location"); + resolved_decorators = (resolved_decorators & ~ON_DECORATOR_MASK) | ON_STRONG_OOP_REF; + auto addr = AccessInternal::oop_field_addr(base, offset); oop result = bs->oop_xchg(resolved_decorators, addr, new_value); if (ShenandoahCardBarrier) { diff --git a/test/hotspot/jtreg/gc/shenandoah/TestWeakReferenceCAS.java b/test/hotspot/jtreg/gc/shenandoah/TestWeakReferenceCAS.java new file mode 100644 index 00000000000..2373bc1c905 --- /dev/null +++ b/test/hotspot/jtreg/gc/shenandoah/TestWeakReferenceCAS.java @@ -0,0 +1,117 @@ +/* + * Copyright Amazon.com Inc. 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 + * under the terms of the GNU General Public License version 2 only, as + * published by the Free Software Foundation. + * + * This code is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License + * version 2 for more details (a copy is included in the LICENSE file that + * accompanied this code). + * + * You should have received a copy of the GNU General Public License version + * 2 along with this work; if not, write to the Free Software Foundation, + * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA. + * + * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA + * or visit www.oracle.com if you need additional information or have any + * questions. + */ + +/** + * @test + * @summary Test that weak CAS attempt on Reference.referent is handled properly + * @requires vm.gc.Shenandoah + * @library /test/lib + * @modules java.base/jdk.internal.misc:+open + * + * @run driver TestWeakReferenceCAS + */ + +import java.util.*; +import java.lang.ref.*; +import java.lang.reflect.*; +import jdk.internal.misc.Unsafe; + +import jdk.test.lib.Platform; +import jdk.test.lib.process.ProcessTools; +import jdk.test.lib.process.OutputAnalyzer; + +public class TestWeakReferenceCAS { + + public static void main(String[] args) throws Exception { + if (args.length == 0) { + driver("CAS"); + driver("CAE"); + driver("GAS"); + } else { + switch (args[0]) { + case "CAS": + Test.testCAS(); + break; + case "CAE": + Test.testCAE(); + break; + case "GAS": + Test.testGAS(); + break; + default: + throw new IllegalArgumentException("Unknown test mode: " + args[0]); + } + } + } + + private static void driver(String test) throws Exception { + OutputAnalyzer output = ProcessTools.executeLimitedTestJava( + "-Xmx128m", + "--add-exports", "java.base/jdk.internal.misc=ALL-UNNAMED", + "--add-opens", "java.base/java.lang.ref=ALL-UNNAMED", + "-XX:+UseShenandoahGC", + TestWeakReferenceCAS.class.getName(), + test); + if (Platform.isDebugBuild()) { + output.shouldNotHaveExitValue(0); + output.shouldContain("Application error:"); + } else { + output.shouldHaveExitValue(0); + } + } + + static class Test { + static final Unsafe UNSAFE = Unsafe.getUnsafe(); + static final long OFFSET; + + static { + try { + Field f = Reference.class.getDeclaredField("referent"); + OFFSET = UNSAFE.objectFieldOffset(f); + } catch (Exception e) { + throw new RuntimeException(e); + } + } + + static void testCAS() { + Object obj = new Object(); + Object next = new Object(); + WeakReference ref = new WeakReference<>(obj); + UNSAFE.compareAndSetReference(ref, OFFSET, obj, next); + } + + static void testCAE() { + Object obj = new Object(); + Object next = new Object(); + WeakReference ref = new WeakReference<>(obj); + UNSAFE.compareAndExchangeReference(ref, OFFSET, obj, next); + } + + static void testGAS() { + Object obj = new Object(); + Object next = new Object(); + WeakReference ref = new WeakReference<>(obj); + UNSAFE.getAndSetReference(ref, OFFSET, next); + } + } +}