SDL icon indicating copy to clipboard operation
SDL copied to clipboard
verified publisher icon libsdl-org

Infinite recursion when tracking allocation count on a non-gnu toolchain without gcc atomics and no in-line assembly spinlock

Open madebr opened this issue 1 month ago • 0 comments

While testing TinyCC, I found a infinite recursion when configuring with -DCMAKE_C_FLAGS="-DSDL_TRACK_ALLOCATION_COUNT". This is because TinyCC does not support the gcc atomic built-ins and currently does not use the inline assembly spinlock. (EDIT: this is fixed in git main. The patch below simulates a bad configuration)

This results in the following backtrace (with added whitespace to show the infinite recursion):

...
SDL_LockSpinlock_REAL SDL_spinlock.c:153

enterLock SDL_atomic.c:85
SDL_CompareAndSwapAtomicInt_REAL SDL_atomic.c:111
SDL_AddAtomicInt_REAL SDL_atomic.c:251
SDL_calloc_REAL SDL_malloc.c:6483
SDL_CreateMutex_REAL SDL_sysmutex.c:34
SDL_TryLockSpinlock_REAL SDL_spinlock.c:135
SDL_LockSpinlock_REAL SDL_spinlock.c:153

enterLock SDL_atomic.c:85
SDL_CompareAndSwapAtomicInt_REAL SDL_atomic.c:111
SDL_AddAtomicInt_REAL SDL_atomic.c:251
SDL_calloc_REAL SDL_malloc.c:6483
SDL_CreateMutex_REAL SDL_sysmutex.c:34
SDL_TryLockSpinlock_REAL SDL_spinlock.c:135
SDL_LockSpinlock_REAL SDL_spinlock.c:153

enterLock SDL_atomic.c:85
SDL_CompareAndSwapAtomicInt_REAL SDL_atomic.c:111
SDL_AddAtomicInt_REAL SDL_atomic.c:251
SDL_calloc_REAL SDL_malloc.c:6483
SDL_CreateMutex_REAL SDL_sysmutex.c:34
SDL_TryLockSpinlock_REAL SDL_spinlock.c:135
SDL_LockSpinlock_REAL SDL_spinlock.c:153

enterLock SDL_atomic.c:85
SDL_CompareAndSwapAtomicInt_REAL SDL_atomic.c:111
SDL_AddAtomicInt_REAL SDL_atomic.c:251
SDL_calloc_REAL SDL_malloc.c:6483
SDL_CreateMutex_REAL SDL_sysmutex.c:34
SDL_TryLockSpinlock_REAL SDL_spinlock.c:135
SDL_LockSpinlock_REAL SDL_spinlock.c:153

SDL_InitDynamicAPI SDL_dynapi.c:587
SDL_LogMessageV_DEFAULT SDL_dynapi_procs.h:671
SDL_Log SDL_dynapi.c:260
main app.c:8
Patch to force disable gcc atomics and in-line assembly spin lock 
--- a/src/atomic/SDL_atomic.c
+++ b/src/atomic/SDL_atomic.c
@@ -95,17 +95,17 @@ static SDL_INLINE void leaveLock(void *a)
 
 bool SDL_CompareAndSwapAtomicInt(SDL_AtomicInt *a, int oldval, int newval)
 {
-#ifdef HAVE_MSC_ATOMICS
+#ifdef XX_HAVE_MSC_ATOMICS
     SDL_COMPILE_TIME_ASSERT(atomic_cas, sizeof(long) == sizeof(a->value));
     return _InterlockedCompareExchange((long *)&a->value, (long)newval, (long)oldval) == (long)oldval;
-#elif defined(HAVE_GCC_ATOMICS)
+#elif 0//defined(HAVE_GCC_ATOMICS)
     return __sync_bool_compare_and_swap(&a->value, oldval, newval);
-#elif defined(SDL_PLATFORM_MACOS) // this is deprecated in 10.12 sdk; favor gcc atomics.
+#elif 0//defined(SDL_PLATFORM_MACOS) // this is deprecated in 10.12 sdk; favor gcc atomics.
     return OSAtomicCompareAndSwap32Barrier(oldval, newval, &a->value);
-#elif defined(SDL_PLATFORM_SOLARIS)
+#elif 0//defined(SDL_PLATFORM_SOLARIS)
     SDL_COMPILE_TIME_ASSERT(atomic_cas, sizeof(uint_t) == sizeof(a->value));
     return ((int)atomic_cas_uint((volatile uint_t *)&a->value, (uint_t)oldval, (uint_t)newval) == oldval);
-#elif defined(EMULATE_CAS)
+#elif 1//defined(EMULATE_CAS)
     bool result = false;
 
     enterLock(a);
@@ -123,17 +123,17 @@ bool SDL_CompareAndSwapAtomicInt(SDL_AtomicInt *a, int oldval, int newval)
 
 bool SDL_CompareAndSwapAtomicU32(SDL_AtomicU32 *a, Uint32 oldval, Uint32 newval)
 {
-#ifdef HAVE_MSC_ATOMICS
+#ifdef XX_HAVE_MSC_ATOMICS
     SDL_COMPILE_TIME_ASSERT(atomic_cas, sizeof(long) == sizeof(a->value));
     return _InterlockedCompareExchange((long *)&a->value, (long)newval, (long)oldval) == (long)oldval;
-#elif defined(HAVE_GCC_ATOMICS)
+#elif 0//defined(HAVE_GCC_ATOMICS)
     return __sync_bool_compare_and_swap(&a->value, oldval, newval);
-#elif defined(SDL_PLATFORM_MACOS) // this is deprecated in 10.12 sdk; favor gcc atomics.
+#elif 0//defined(SDL_PLATFORM_MACOS) // this is deprecated in 10.12 sdk; favor gcc atomics.
     return OSAtomicCompareAndSwap32Barrier((int32_t)oldval, (int32_t)newval, (int32_t *)&a->value);
-#elif defined(SDL_PLATFORM_SOLARIS)
+#elif 0//defined(SDL_PLATFORM_SOLARIS)
     SDL_COMPILE_TIME_ASSERT(atomic_cas, sizeof(uint_t) == sizeof(a->value));
     return ((Uint32)atomic_cas_uint((volatile uint_t *)&a->value, (uint_t)oldval, (uint_t)newval) == oldval);
-#elif defined(EMULATE_CAS)
+#elif 1//defined(EMULATE_CAS)
     bool result = false;
 
     enterLock(a);
@@ -151,15 +151,15 @@ bool SDL_CompareAndSwapAtomicU32(SDL_AtomicU32 *a, Uint32 oldval, Uint32 newval)
 
 bool SDL_CompareAndSwapAtomicPointer(void **a, void *oldval, void *newval)
 {
-#ifdef HAVE_MSC_ATOMICS
+#ifdef XX_HAVE_MSC_ATOMICS
     return _InterlockedCompareExchangePointer(a, newval, oldval) == oldval;
-#elif defined(HAVE_GCC_ATOMICS)
+#elif 0//defined(HAVE_GCC_ATOMICS)
     return __sync_bool_compare_and_swap(a, oldval, newval);
-#elif defined(SDL_PLATFORM_MACOS) && defined(__LP64__)  // this is deprecated in 10.12 sdk; favor gcc atomics.
+#elif 0//defined(SDL_PLATFORM_MACOS) && defined(__LP64__)  // this is deprecated in 10.12 sdk; favor gcc atomics.
     return OSAtomicCompareAndSwap64Barrier((int64_t)oldval, (int64_t)newval, (int64_t *)a);
-#elif defined(SDL_PLATFORM_MACOS) && !defined(__LP64__) // this is deprecated in 10.12 sdk; favor gcc atomics.
+#elif 0//defined(SDL_PLATFORM_MACOS) && !defined(__LP64__) // this is deprecated in 10.12 sdk; favor gcc atomics.
     return OSAtomicCompareAndSwap32Barrier((int32_t)oldval, (int32_t)newval, (int32_t *)a);
-#elif defined(SDL_PLATFORM_SOLARIS)
+#elif 0//defined(SDL_PLATFORM_SOLARIS)
     return (atomic_cas_ptr(a, oldval, newval) == oldval);
 #elif defined(EMULATE_CAS)
     bool result = false;
@@ -179,12 +179,12 @@ bool SDL_CompareAndSwapAtomicPointer(void **a, void *oldval, void *newval)
 
 int SDL_SetAtomicInt(SDL_AtomicInt *a, int v)
 {
-#ifdef HAVE_MSC_ATOMICS
+#ifdef XX_HAVE_MSC_ATOMICS
     SDL_COMPILE_TIME_ASSERT(atomic_set, sizeof(long) == sizeof(a->value));
     return _InterlockedExchange((long *)&a->value, v);
-#elif defined(HAVE_GCC_ATOMICS)
+#elif 0//defined(HAVE_GCC_ATOMICS)
     return __sync_lock_test_and_set(&a->value, v);
-#elif defined(SDL_PLATFORM_SOLARIS)
+#elif 0//defined(SDL_PLATFORM_SOLARIS)
     SDL_COMPILE_TIME_ASSERT(atomic_set, sizeof(uint_t) == sizeof(a->value));
     return (int)atomic_swap_uint((volatile uint_t *)&a->value, v);
 #else
@@ -198,12 +198,12 @@ int SDL_SetAtomicInt(SDL_AtomicInt *a, int v)
 
 Uint32 SDL_SetAtomicU32(SDL_AtomicU32 *a, Uint32 v)
 {
-#ifdef HAVE_MSC_ATOMICS
+#ifdef XX_HAVE_MSC_ATOMICS
     SDL_COMPILE_TIME_ASSERT(atomic_set, sizeof(long) == sizeof(a->value));
     return _InterlockedExchange((long *)&a->value, v);
-#elif defined(HAVE_GCC_ATOMICS)
+#elif 0//defined(HAVE_GCC_ATOMICS)
     return __sync_lock_test_and_set(&a->value, v);
-#elif defined(SDL_PLATFORM_SOLARIS)
+#elif 0//defined(SDL_PLATFORM_SOLARIS)
     SDL_COMPILE_TIME_ASSERT(atomic_set, sizeof(uint_t) == sizeof(a->value));
     return (Uint32)atomic_swap_uint((volatile uint_t *)&a->value, v);
 #else
@@ -217,11 +217,11 @@ Uint32 SDL_SetAtomicU32(SDL_AtomicU32 *a, Uint32 v)
 
 void *SDL_SetAtomicPointer(void **a, void *v)
 {
-#ifdef HAVE_MSC_ATOMICS
+#ifdef XXX_HAVE_MSC_ATOMICS
     return _InterlockedExchangePointer(a, v);
-#elif defined(HAVE_GCC_ATOMICS)
+#elif 0//defined(HAVE_GCC_ATOMICS)
     return __sync_lock_test_and_set(a, v);
-#elif defined(SDL_PLATFORM_SOLARIS)
+#elif 0//defined(SDL_PLATFORM_SOLARIS)
     return atomic_swap_ptr(a, v);
 #else
     void *value;
@@ -234,12 +234,12 @@ void *SDL_SetAtomicPointer(void **a, void *v)
 
 int SDL_AddAtomicInt(SDL_AtomicInt *a, int v)
 {
-#ifdef HAVE_MSC_ATOMICS
+#ifdef XXX_HAVE_MSC_ATOMICS
     SDL_COMPILE_TIME_ASSERT(atomic_add, sizeof(long) == sizeof(a->value));
     return _InterlockedExchangeAdd((long *)&a->value, v);
-#elif defined(HAVE_GCC_ATOMICS)
+#elif 0//defined(HAVE_GCC_ATOMICS)
     return __sync_fetch_and_add(&a->value, v);
-#elif defined(SDL_PLATFORM_SOLARIS)
+#elif 0//defined(SDL_PLATFORM_SOLARIS)
     int pv = a->value;
     membar_consumer();
     atomic_add_int((volatile uint_t *)&a->value, v);
@@ -255,12 +255,12 @@ int SDL_AddAtomicInt(SDL_AtomicInt *a, int v)
 
 Uint32 SDL_AddAtomicU32(SDL_AtomicU32 *a, int v)
 {
-#ifdef HAVE_MSC_ATOMICS
+#ifdef XX_HAVE_MSC_ATOMICS
     SDL_COMPILE_TIME_ASSERT(atomic_add, sizeof(long) == sizeof(a->value));
     return (Uint32)_InterlockedExchangeAdd((long *)&a->value, v);
-#elif defined(HAVE_GCC_ATOMICS)
+#elif 0//defined(HAVE_GCC_ATOMICS)
     return __sync_fetch_and_add(&a->value, v);
-#elif defined(SDL_PLATFORM_SOLARIS)
+#elif 0//defined(SDL_PLATFORM_SOLARIS)
     Uint32 pv = a->value;
     membar_consumer();
     atomic_add_int((volatile uint_t *)&a->value, v);
@@ -276,16 +276,16 @@ Uint32 SDL_AddAtomicU32(SDL_AtomicU32 *a, int v)
 
 int SDL_GetAtomicInt(SDL_AtomicInt *a)
 {
-#ifdef HAVE_ATOMIC_LOAD_N
+#ifdef XX_HAVE_ATOMIC_LOAD_N
     return __atomic_load_n(&a->value, __ATOMIC_SEQ_CST);
-#elif defined(HAVE_MSC_ATOMICS)
+#elif 0//defined(HAVE_MSC_ATOMICS)
     SDL_COMPILE_TIME_ASSERT(atomic_get, sizeof(long) == sizeof(a->value));
     return _InterlockedOr((long *)&a->value, 0);
-#elif defined(HAVE_GCC_ATOMICS)
+#elif 0//defined(HAVE_GCC_ATOMICS)
     return __sync_or_and_fetch(&a->value, 0);
-#elif defined(SDL_PLATFORM_MACOS) // this is deprecated in 10.12 sdk; favor gcc atomics.
+#elif 0//defined(SDL_PLATFORM_MACOS) // this is deprecated in 10.12 sdk; favor gcc atomics.
     return sizeof(a->value) == sizeof(uint32_t) ? OSAtomicOr32Barrier(0, (volatile uint32_t *)&a->value) : OSAtomicAdd64Barrier(0, (volatile int64_t *)&a->value);
-#elif defined(SDL_PLATFORM_SOLARIS)
+#elif 0//defined(SDL_PLATFORM_SOLARIS)
     return atomic_or_uint_nv((volatile uint_t *)&a->value, 0);
 #else
     int value;
@@ -298,16 +298,16 @@ int SDL_GetAtomicInt(SDL_AtomicInt *a)
 
 Uint32 SDL_GetAtomicU32(SDL_AtomicU32 *a)
 {
-#ifdef HAVE_ATOMIC_LOAD_N
+#ifdef XX_HAVE_ATOMIC_LOAD_N
     return __atomic_load_n(&a->value, __ATOMIC_SEQ_CST);
-#elif defined(HAVE_MSC_ATOMICS)
+#elif 0//defined(HAVE_MSC_ATOMICS)
     SDL_COMPILE_TIME_ASSERT(atomic_get, sizeof(long) == sizeof(a->value));
     return (Uint32)_InterlockedOr((long *)&a->value, 0);
-#elif defined(HAVE_GCC_ATOMICS)
+#elif 0//defined(HAVE_GCC_ATOMICS)
     return __sync_or_and_fetch(&a->value, 0);
-#elif defined(SDL_PLATFORM_MACOS) // this is deprecated in 10.12 sdk; favor gcc atomics.
+#elif 0//defined(SDL_PLATFORM_MACOS) // this is deprecated in 10.12 sdk; favor gcc atomics.
     return OSAtomicOr32Barrier(0, (volatile uint32_t *)&a->value);
-#elif defined(SDL_PLATFORM_SOLARIS)
+#elif 0//defined(SDL_PLATFORM_SOLARIS)
     SDL_COMPILE_TIME_ASSERT(atomic_get, sizeof(uint_t) == sizeof(a->value));
     return (Uint32)atomic_or_uint_nv((volatile uint_t *)&a->value, 0);
 #else
diff --git a/src/atomic/SDL_spinlock.c b/src/atomic/SDL_spinlock.c
index 75bacda7c..80ca214db 100644
--- a/src/atomic/SDL_spinlock.c
+++ b/src/atomic/SDL_spinlock.c
@@ -91,7 +91,7 @@ bool SDL_TryLockSpinlock(SDL_SpinLock *lock)
         : "cc", "memory");
     return result == 0;
 
-#elif defined(__GNUC__) && (defined(__i386__) || defined(__x86_64__))
+#elif 0//defined(__GNUC__) && (defined(__i386__) || defined(__x86_64__))
     int result;
     __asm__ __volatile__(
         "lock ; xchgl %0, (%1)\n"
--- a/src/stdlib/SDL_malloc.c
+++ b/src/stdlib/SDL_malloc.c
@@ -6366,7 +6366,10 @@ static struct
 };
 
 // Define this if you want to track the number of allocations active
-// #define SDL_TRACK_ALLOCATION_COUNT
+#ifndef SDL_TRACK_ALLOCATION_COUNT
+// Force allocation count tracking
+#define SDL_TRACK_ALLOCATION_COUNT
+#endif
 #ifdef SDL_TRACK_ALLOCATION_COUNT
 #define INCREMENT_ALLOCATION_COUNT()    (void)SDL_AtomicIncRef(&s_mem.num_allocations)
 #define DECREMENT_ALLOCATION_COUNT()    (void)SDL_AtomicDecRef(&s_mem.num_allocations)

For TinyCC on x86/x86_64, I can break the cycle by the following patch (part of https://github.com/libsdl-org/SDL/pull/14464):

--- a/src/atomic/SDL_spinlock.c
+++ b/src/atomic/SDL_spinlock.c
@@ -91,7 +91,7 @@ bool SDL_TryLockSpinlock(SDL_SpinLock *lock)
         : "cc", "memory");
     return result == 0;
 
-#elif defined(__GNUC__) && (defined(__i386__) || defined(__x86_64__))
+#elif (defined(__GNUC__) || defined(__TINYC__)) && (defined(__i386__) || defined(__x86_64__))
     int result;
     __asm__ __volatile__(
         "lock ; xchgl %0, (%1)\n"

It's fixed for tinycc on x86/x86_64, but might re-appear for other compilers/architectures.

madebr avatar Nov 15 '25 15:11 madebr