From 95557ad48c190e1c59fe92517939b2b44e182b38 Mon Sep 17 00:00:00 2001 From: subotac <73706465+subotac@users.noreply.github.com> Date: Mon, 3 Aug 2026 12:27:58 +0300 Subject: [PATCH] fixes #26027; use valid compare-exchange failure orders (#26066) Fixes #26027. Map the single-order compare-exchange failure ordering from `release` to `relaxed` and from `acquire-release` to `acquire`. Apply the mapping to the trivial and non-trivial strong and weak overloads, and correct the explicit-order test cases. Tested `tests/stdlib/concurrency/tatomics.nim` across C/C++, refc/orc, and native/C++ atomics (8 combinations). Also verified the original GCC 16.1 assertion reproducer. --- lib/pure/concurrency/atomics.nim | 27 +++++++++++++-------- tests/stdlib/concurrency/tatomics.nim | 34 +++++++++++++-------------- 2 files changed, 34 insertions(+), 27 deletions(-) diff --git a/lib/pure/concurrency/atomics.nim b/lib/pure/concurrency/atomics.nim index 818f1b37ac..9c652afdc1 100644 --- a/lib/pure/concurrency/atomics.nim +++ b/lib/pure/concurrency/atomics.nim @@ -256,12 +256,8 @@ else: cast[T](interlockedExchange(addr(location.value), cast[int64](desired))) proc compareExchange*[T: Trivial](location: var Atomic[T]; expected: var T; desired: T; success, failure: MemoryOrder): bool {.inline.} = cast[T](interlockedCompareExchange(addr(location.value), cast[nonAtomicType(T)](desired), cast[nonAtomicType(T)](expected))) == expected - proc compareExchange*[T: Trivial](location: var Atomic[T]; expected: var T; desired: T; order: MemoryOrder = moSequentiallyConsistent): bool {.inline.} = - compareExchange(location, expected, desired, order, order) proc compareExchangeWeak*[T: Trivial](location: var Atomic[T]; expected: var T; desired: T; success, failure: MemoryOrder): bool {.inline.} = compareExchange(location, expected, desired, success, failure) - proc compareExchangeWeak*[T: Trivial](location: var Atomic[T]; expected: var T; desired: T; order: MemoryOrder = moSequentiallyConsistent): bool {.inline.} = - compareExchangeWeak(location, expected, desired, order, order) proc fetchAdd*[T: SomeInteger](location: var Atomic[T]; value: T; order: MemoryOrder = moSequentiallyConsistent): T {.inline.} = var currentValue = location.load() @@ -358,13 +354,9 @@ else: cast[T](atomic_exchange_explicit(addr(location.value), cast[nonAtomicType(T)](desired), order)) proc compareExchange*[T: Trivial](location: var Atomic[T]; expected: var T; desired: T; success, failure: MemoryOrder): bool {.inline.} = atomic_compare_exchange_strong_explicit(addr(location.value), cast[ptr nonAtomicType(T)](addr(expected)), cast[nonAtomicType(T)](desired), success, failure) - proc compareExchange*[T: Trivial](location: var Atomic[T]; expected: var T; desired: T; order: MemoryOrder = moSequentiallyConsistent): bool {.inline.} = - compareExchange(location, expected, desired, order, order) proc compareExchangeWeak*[T: Trivial](location: var Atomic[T]; expected: var T; desired: T; success, failure: MemoryOrder): bool {.inline.} = atomic_compare_exchange_weak_explicit(addr(location.value), cast[ptr nonAtomicType(T)](addr(expected)), cast[nonAtomicType(T)](desired), success, failure) - proc compareExchangeWeak*[T: Trivial](location: var Atomic[T]; expected: var T; desired: T; order: MemoryOrder = moSequentiallyConsistent): bool {.inline.} = - compareExchangeWeak(location, expected, desired, order, order) # Numerical operations proc fetchAdd*[T: SomeInteger](location: var Atomic[T]; value: T; order: MemoryOrder = moSequentiallyConsistent): T {.inline.} = @@ -378,6 +370,21 @@ else: proc fetchXor*[T: SomeInteger](location: var Atomic[T]; value: T; order: MemoryOrder = moSequentiallyConsistent): T {.inline.} = cast[T](atomic_fetch_xor_explicit(addr(location.value), cast[nonAtomicType(T)](value), order)) + func compareExchangeFailureOrder(order: MemoryOrder): MemoryOrder {.inline.} = + case order + of moRelease: + moRelaxed + of moAcquireRelease: + moAcquire + else: + order + + proc compareExchange*[T: Trivial](location: var Atomic[T]; expected: var T; desired: T; order: MemoryOrder = moSequentiallyConsistent): bool {.inline.} = + compareExchange(location, expected, desired, order, compareExchangeFailureOrder(order)) + + proc compareExchangeWeak*[T: Trivial](location: var Atomic[T]; expected: var T; desired: T; order: MemoryOrder = moSequentiallyConsistent): bool {.inline.} = + compareExchangeWeak(location, expected, desired, order, compareExchangeFailureOrder(order)) + template withLock[T: not Trivial](location: var Atomic[T]; order: MemoryOrder; body: untyped): untyped = while testAndSet(location.guard, moAcquire): discard try: @@ -411,10 +418,10 @@ else: compareExchange(location, expected, desired, success, failure) proc compareExchange*[T: not Trivial](location: var Atomic[T]; expected: var T; desired: T; order: MemoryOrder = moSequentiallyConsistent): bool {.inline.} = - compareExchange(location, expected, desired, order, order) + compareExchange(location, expected, desired, order, compareExchangeFailureOrder(order)) proc compareExchangeWeak*[T: not Trivial](location: var Atomic[T]; expected: var T; desired: T; order: MemoryOrder = moSequentiallyConsistent): bool {.inline.} = - compareExchangeWeak(location, expected, desired, order, order) + compareExchangeWeak(location, expected, desired, order, compareExchangeFailureOrder(order)) proc atomicInc*[T: SomeInteger](location: var Atomic[T]; value: T = 1) {.inline.} = ## Atomically increments the atomic integer by some `value`. diff --git a/tests/stdlib/concurrency/tatomics.nim b/tests/stdlib/concurrency/tatomics.nim index 08f2e7d3ee..44760395eb 100644 --- a/tests/stdlib/concurrency/tatomics.nim +++ b/tests/stdlib/concurrency/tatomics.nim @@ -49,7 +49,7 @@ block trivialExchange: doAssert location.load == 6 -block trivialCompareExchangeDoesExchange: +block trivialCompareExchangeDoesExchange: # bug #26027 var location: Atomic[int] var expected = 1 location.store(1) @@ -115,11 +115,11 @@ block trivialCompareExchangeSuccessFailureDoesExchange: doAssert expected == 3 doAssert location.load == 4 expected = 4 - doAssert location.compareExchange(expected, 5, moRelease, moRelease) + doAssert location.compareExchange(expected, 5, moRelease, moRelaxed) doAssert expected == 4 doAssert location.load == 5 expected = 5 - doAssert location.compareExchange(expected, 6, moAcquireRelease, moAcquireRelease) + doAssert location.compareExchange(expected, 6, moAcquireRelease, moAcquire) doAssert expected == 5 doAssert location.load == 6 @@ -140,11 +140,11 @@ block trivialCompareExchangeSuccessFailureDoesNotExchange: doAssert expected == 1 doAssert location.load == 1 expected = 10 - doAssert not location.compareExchange(expected, 5, moRelease, moRelease) + doAssert not location.compareExchange(expected, 5, moRelease, moRelaxed) doAssert expected == 1 doAssert location.load == 1 expected = 10 - doAssert not location.compareExchange(expected, 6, moAcquireRelease, moAcquireRelease) + doAssert not location.compareExchange(expected, 6, moAcquireRelease, moAcquire) doAssert expected == 1 doAssert location.load == 1 @@ -215,11 +215,11 @@ block trivialCompareExchangeWeakSuccessFailureDoesExchange: doAssert expected == 3 doAssert location.load == 4 expected = 4 - doAssert location.compareExchangeWeak(expected, 5, moRelease, moRelease) + doAssert location.compareExchangeWeak(expected, 5, moRelease, moRelaxed) doAssert expected == 4 doAssert location.load == 5 expected = 5 - doAssert location.compareExchangeWeak(expected, 6, moAcquireRelease, moAcquireRelease) + doAssert location.compareExchangeWeak(expected, 6, moAcquireRelease, moAcquire) doAssert expected == 5 doAssert location.load == 6 @@ -240,11 +240,11 @@ block trivialCompareExchangeWeakSuccessFailureDoesNotExchange: doAssert expected == 1 doAssert location.load == 1 expected = 10 - doAssert not location.compareExchangeWeak(expected, 5, moRelease, moRelease) + doAssert not location.compareExchangeWeak(expected, 5, moRelease, moRelaxed) doAssert expected == 1 doAssert location.load == 1 expected = 10 - doAssert not location.compareExchangeWeak(expected, 6, moAcquireRelease, moAcquireRelease) + doAssert not location.compareExchangeWeak(expected, 6, moAcquireRelease, moAcquire) doAssert expected == 1 doAssert location.load == 1 @@ -349,11 +349,11 @@ block objectCompareExchangeSuccessFailureDoesExchange: doAssert expected == Object(val: 3) doAssert location.load == Object(val: 4) expected = Object(val: 4) - doAssert location.compareExchange(expected, Object(val: 5), moRelease, moRelease) + doAssert location.compareExchange(expected, Object(val: 5), moRelease, moRelaxed) doAssert expected == Object(val: 4) doAssert location.load == Object(val: 5) expected = Object(val: 5) - doAssert location.compareExchange(expected, Object(val: 6), moAcquireRelease, moAcquireRelease) + doAssert location.compareExchange(expected, Object(val: 6), moAcquireRelease, moAcquire) doAssert expected == Object(val: 5) doAssert location.load == Object(val: 6) @@ -374,11 +374,11 @@ block objectCompareExchangeSuccessFailureDoesNotExchange: doAssert expected == Object(val: 1) doAssert location.load == Object(val: 1) expected = Object(val: 10) - doAssert not location.compareExchange(expected, Object(val: 5), moRelease, moRelease) + doAssert not location.compareExchange(expected, Object(val: 5), moRelease, moRelaxed) doAssert expected == Object(val: 1) doAssert location.load == Object(val: 1) expected = Object(val: 10) - doAssert not location.compareExchange(expected, Object(val: 6), moAcquireRelease, moAcquireRelease) + doAssert not location.compareExchange(expected, Object(val: 6), moAcquireRelease, moAcquire) doAssert expected == Object(val: 1) doAssert location.load == Object(val: 1) @@ -449,11 +449,11 @@ block objectCompareExchangeWeakSuccessFailureDoesExchange: doAssert expected == Object(val: 3) doAssert location.load == Object(val: 4) expected = Object(val: 4) - doAssert location.compareExchangeWeak(expected, Object(val: 5), moRelease, moRelease) + doAssert location.compareExchangeWeak(expected, Object(val: 5), moRelease, moRelaxed) doAssert expected == Object(val: 4) doAssert location.load == Object(val: 5) expected = Object(val: 5) - doAssert location.compareExchangeWeak(expected, Object(val: 6), moAcquireRelease, moAcquireRelease) + doAssert location.compareExchangeWeak(expected, Object(val: 6), moAcquireRelease, moAcquire) doAssert expected == Object(val: 5) doAssert location.load == Object(val: 6) @@ -474,11 +474,11 @@ block objectCompareExchangeWeakSuccessFailureDoesNotExchange: doAssert expected == Object(val: 1) doAssert location.load == Object(val: 1) expected = Object(val: 10) - doAssert not location.compareExchangeWeak(expected, Object(val: 5), moRelease, moRelease) + doAssert not location.compareExchangeWeak(expected, Object(val: 5), moRelease, moRelaxed) doAssert expected == Object(val: 1) doAssert location.load == Object(val: 1) expected = Object(val: 10) - doAssert not location.compareExchangeWeak(expected, Object(val: 6), moAcquireRelease, moAcquireRelease) + doAssert not location.compareExchangeWeak(expected, Object(val: 6), moAcquireRelease, moAcquire) doAssert expected == Object(val: 1) doAssert location.load == Object(val: 1)