From 30de669b32b4b31c67c36171524cd539167fa916 Mon Sep 17 00:00:00 2001 From: Cameron Gutman Date: Sun, 19 Apr 2026 14:06:44 -0500 Subject: [PATCH] atomic: Fix missing full memory barrier on GCC/Clang __sync_lock_test_and_set() is designed for creating locks, not as a general atomic exchange function. As a result, it only provides an acquire memory barrier and isn't guaranteed to actually store the provided value (though it does on architectures we care about). __atomic_exchange_n() is supported on GCC/Clang for the last ~10 years, so let's use that instead if available. We will keep the __sync_lock_test_and_set() fallback around for ancient platforms, but add a full memory barrier to match the documented behavior. --- CMakeLists.txt | 1 + src/atomic/SDL_atomic.c | 28 +++++++++++++++++++++++++--- 2 files changed, 26 insertions(+), 3 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 00c90ba60a..ff79c018a4 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -2812,6 +2812,7 @@ elseif(PS2) gskit dmakit ps2_drivers + atomic ) elseif(OS2) diff --git a/src/atomic/SDL_atomic.c b/src/atomic/SDL_atomic.c index 5f4057bd34..35c3ca42cd 100644 --- a/src/atomic/SDL_atomic.c +++ b/src/atomic/SDL_atomic.c @@ -35,12 +35,18 @@ #include #endif -/* The __atomic_load_n() intrinsic showed up in different times for different compilers. */ -#if defined(__GNUC__) && (__GNUC__ >= 5) +/* The __atomic intrinsics showed up in different times for different compilers. */ +#if (defined(__GNUC__) && (__GNUC__ >= 5)) || (defined(__clang__) && defined(HAVE_GCC_ATOMICS)) #define HAVE_ATOMIC_LOAD_N 1 -#elif _SDL_HAS_BUILTIN(__atomic_load_n) || (defined(__clang__) && defined(HAVE_GCC_ATOMICS)) +#define HAVE_ATOMIC_EXCHANGE_N 1 +#else +#if _SDL_HAS_BUILTIN(__atomic_load_n) #define HAVE_ATOMIC_LOAD_N 1 #endif +#if _SDL_HAS_BUILTIN(__atomic_exchange_n) +#define HAVE_ATOMIC_EXCHANGE_N 1 +#endif +#endif /* *INDENT-OFF* */ /* clang-format off */ #if defined(__WATCOMC__) && defined(__386__) @@ -182,7 +188,15 @@ int SDL_AtomicSet(SDL_atomic_t *a, int v) return _InterlockedExchange((long *)&a->value, v); #elif defined(HAVE_WATCOM_ATOMICS) return _SDL_xchg_watcom(&a->value, v); +#elif defined(HAVE_ATOMIC_EXCHANGE_N) + return __atomic_exchange_n(&a->value, v, __ATOMIC_SEQ_CST); #elif defined(HAVE_GCC_ATOMICS) + /* __sync_lock_test_and_set() is designed for locking rather than a + generic atomic exchange, so it only provides an acquire barrier + and may not store the exact value on all architectures. We prefer + __atomic_exchange_n() instead on all modern compilers. + */ + __sync_synchronize(); return __sync_lock_test_and_set(&a->value, v); #elif defined(__SOLARIS__) return (int)atomic_swap_uint((volatile uint_t *)&a->value, v); @@ -201,7 +215,15 @@ void *SDL_AtomicSetPtr(void **a, void *v) return _InterlockedExchangePointer(a, v); #elif defined(HAVE_WATCOM_ATOMICS) return (void *)_SDL_xchg_watcom((int *)a, (long)v); +#elif defined(HAVE_ATOMIC_EXCHANGE_N) + return __atomic_exchange_n(a, v, __ATOMIC_SEQ_CST); #elif defined(HAVE_GCC_ATOMICS) + /* __sync_lock_test_and_set() is designed for locking rather than a + generic atomic exchange, so it only provides an acquire barrier + and may not store the exact value on all architectures. We prefer + __atomic_exchange_n() instead on all modern compilers. + */ + __sync_synchronize(); return __sync_lock_test_and_set(a, v); #elif defined(__SOLARIS__) return atomic_swap_ptr(a, v);