Added support for clang thread-safety analysis

The annotations have been added to SDL_mutex.h and have been made public so applications can enable this for their own code.

Clang assumes that locking and unlocking can't fail, but SDL has the concept of a NULL mutex, so the mutex functions have been changed not to report errors if a mutex hasn't been initialized. We do have mutexes that might be accessed when they are NULL, notably in the event system, so this is an important change.

This commit cleans up a bunch of rare race conditions in the joystick and game controller code so now everything should be completely protected by the joystick lock.

To test this, change the compiler to "clang -Wthread-safety -Werror=thread-safety -DSDL_THREAD_SAFETY_ANALYSIS"
This commit is contained in:
Sam Lantinga
2022-12-13 14:03:40 -08:00
parent 02493579b5
commit 5c29b58e95
41 changed files with 1618 additions and 1134 deletions

View File

@@ -70,7 +70,7 @@ void SDL_DestroyMutex(SDL_mutex *mutex)
}
/* Lock the mutex */
int SDL_LockMutex(SDL_mutex *mutex)
int SDL_LockMutex(SDL_mutex *mutex) SDL_NO_THREAD_SAFETY_ANALYSIS /* clang doesn't know about NULL mutexes */
{
#if SDL_THREADS_DISABLED
return 0;
@@ -78,7 +78,7 @@ int SDL_LockMutex(SDL_mutex *mutex)
SDL_threadID this_thread;
if (mutex == NULL) {
return SDL_InvalidParamError("mutex");
return 0;
}
this_thread = SDL_ThreadID();
@@ -108,7 +108,7 @@ int SDL_TryLockMutex(SDL_mutex *mutex)
SDL_threadID this_thread;
if (mutex == NULL) {
return SDL_InvalidParamError("mutex");
return 0;
}
this_thread = SDL_ThreadID();
@@ -131,13 +131,13 @@ int SDL_TryLockMutex(SDL_mutex *mutex)
}
/* Unlock the mutex */
int SDL_mutexV(SDL_mutex *mutex)
int SDL_UnlockMutex(SDL_mutex *mutex) SDL_NO_THREAD_SAFETY_ANALYSIS /* clang doesn't know about NULL mutexes */
{
#if SDL_THREADS_DISABLED
return 0;
#else
if (mutex == NULL) {
return SDL_InvalidParamError("mutex");
return 0;
}
/* If we don't own the mutex, we can't unlock it */

View File

@@ -51,10 +51,10 @@ void SDL_DestroyMutex(SDL_mutex *mutex)
}
/* Lock the mutex */
int SDL_LockMutex(SDL_mutex *mutex)
int SDL_LockMutex(SDL_mutex *mutex) SDL_NO_THREAD_SAFETY_ANALYSIS /* clang doesn't know about NULL mutexes */
{
if (mutex == NULL) {
return SDL_InvalidParamError("mutex");
return 0;
}
RecursiveLock_Lock(&mutex->lock);
@@ -66,17 +66,17 @@ int SDL_LockMutex(SDL_mutex *mutex)
int SDL_TryLockMutex(SDL_mutex *mutex)
{
if (mutex == NULL) {
return SDL_InvalidParamError("mutex");
return 0;
}
return RecursiveLock_TryLock(&mutex->lock);
}
/* Unlock the mutex */
int SDL_mutexV(SDL_mutex *mutex)
int SDL_UnlockMutex(SDL_mutex *mutex) SDL_NO_THREAD_SAFETY_ANALYSIS /* clang doesn't know about NULL mutexes */
{
if (mutex == NULL) {
return SDL_InvalidParamError("mutex");
return 0;
}
RecursiveLock_Unlock(&mutex->lock);

View File

@@ -67,26 +67,11 @@ void SDL_DestroyMutex(SDL_mutex *mutex)
}
}
/* Try to lock the mutex */
#if 0
int
SDL_TryLockMutex(SDL_mutex * mutex)
{
if (mutex == NULL)
{
return SDL_InvalidParamError("mutex");
}
// Not yet implemented.
return 0;
}
#endif
/* Lock the mutex */
int SDL_LockMutex(SDL_mutex *mutex)
int SDL_LockMutex(SDL_mutex *mutex) SDL_NO_THREAD_SAFETY_ANALYSIS /* clang doesn't know about NULL mutexes */
{
if (mutex == NULL) {
return SDL_InvalidParamError("mutex");
return 0;
}
RMutex rmutex;
@@ -96,11 +81,26 @@ int SDL_LockMutex(SDL_mutex *mutex)
return 0;
}
/* Try to lock the mutex */
#if 0
int
SDL_TryLockMutex(SDL_mutex *mutex)
{
if (mutex == NULL)
{
return 0;
}
// Not yet implemented.
return 0;
}
#endif
/* Unlock the mutex */
int SDL_UnlockMutex(SDL_mutex *mutex)
int SDL_UnlockMutex(SDL_mutex *mutex) SDL_NO_THREAD_SAFETY_ANALYSIS /* clang doesn't know about NULL mutexes */
{
if (mutex == NULL) {
return SDL_InvalidParamError("mutex");
return 0;
}
RMutex rmutex;

View File

@@ -72,6 +72,27 @@ void SDL_DestroyMutex(SDL_mutex *mutex)
}
}
/* Lock the mutex */
int SDL_LockMutex(SDL_mutex *mutex) SDL_NO_THREAD_SAFETY_ANALYSIS /* clang doesn't know about NULL mutexes */
{
#if SDL_THREADS_DISABLED
return 0;
#else
SceInt32 res = 0;
if (mutex == NULL) {
return 0;
}
res = sceKernelLockLwMutex(&mutex->lock, 1, NULL);
if (res != SCE_KERNEL_ERROR_OK) {
return SDL_SetError("Error trying to lock mutex: %lx", res);
}
return 0;
#endif /* SDL_THREADS_DISABLED */
}
/* Try to lock the mutex */
int SDL_TryLockMutex(SDL_mutex *mutex)
{
@@ -79,8 +100,9 @@ int SDL_TryLockMutex(SDL_mutex *mutex)
return 0;
#else
SceInt32 res = 0;
if (mutex == NULL) {
return SDL_InvalidParamError("mutex");
return 0;
}
res = sceKernelTryLockLwMutex(&mutex->lock, 1);
@@ -100,28 +122,8 @@ int SDL_TryLockMutex(SDL_mutex *mutex)
#endif /* SDL_THREADS_DISABLED */
}
/* Lock the mutex */
int SDL_mutexP(SDL_mutex *mutex)
{
#if SDL_THREADS_DISABLED
return 0;
#else
SceInt32 res = 0;
if (mutex == NULL) {
return SDL_InvalidParamError("mutex");
}
res = sceKernelLockLwMutex(&mutex->lock, 1, NULL);
if (res != SCE_KERNEL_ERROR_OK) {
return SDL_SetError("Error trying to lock mutex: %lx", res);
}
return 0;
#endif /* SDL_THREADS_DISABLED */
}
/* Unlock the mutex */
int SDL_mutexV(SDL_mutex *mutex)
int SDL_UnlockMutex(SDL_mutex *mutex) SDL_NO_THREAD_SAFETY_ANALYSIS /* clang doesn't know about NULL mutexes */
{
#if SDL_THREADS_DISABLED
return 0;
@@ -129,7 +131,7 @@ int SDL_mutexV(SDL_mutex *mutex)
SceInt32 res = 0;
if (mutex == NULL) {
return SDL_InvalidParamError("mutex");
return 0;
}
res = sceKernelUnlockLwMutex(&mutex->lock, 1);
@@ -140,6 +142,7 @@ int SDL_mutexV(SDL_mutex *mutex)
return 0;
#endif /* SDL_THREADS_DISABLED */
}
#endif /* SDL_THREAD_PSP */
/* vi: set ts=4 sw=4 expandtab: */

View File

@@ -74,14 +74,14 @@ void SDL_DestroyMutex(SDL_mutex *mutex)
}
/* Lock the mutex */
int SDL_LockMutex(SDL_mutex *mutex)
int SDL_LockMutex(SDL_mutex *mutex) SDL_NO_THREAD_SAFETY_ANALYSIS /* clang doesn't know about NULL mutexes */
{
#if FAKE_RECURSIVE_MUTEX
pthread_t this_thread;
#endif
if (mutex == NULL) {
return SDL_InvalidParamError("mutex");
return 0;
}
#if FAKE_RECURSIVE_MUTEX
@@ -117,7 +117,7 @@ int SDL_TryLockMutex(SDL_mutex *mutex)
#endif
if (mutex == NULL) {
return SDL_InvalidParamError("mutex");
return 0;
}
retval = 0;
@@ -153,10 +153,10 @@ int SDL_TryLockMutex(SDL_mutex *mutex)
return retval;
}
int SDL_UnlockMutex(SDL_mutex *mutex)
int SDL_UnlockMutex(SDL_mutex *mutex) SDL_NO_THREAD_SAFETY_ANALYSIS /* clang doesn't know about NULL mutexes */
{
if (mutex == NULL) {
return SDL_InvalidParamError("mutex");
return 0;
}
#if FAKE_RECURSIVE_MUTEX

View File

@@ -55,12 +55,12 @@ SDL_DestroyMutex(SDL_mutex *mutex)
}
}
/* Lock the semaphore */
/* Lock the mutex */
extern "C" int
SDL_mutexP(SDL_mutex *mutex)
SDL_LockMutex(SDL_mutex *mutex) SDL_NO_THREAD_SAFETY_ANALYSIS /* clang doesn't know about NULL mutexes */
{
if (mutex == NULL) {
return SDL_InvalidParamError("mutex");
return 0;
}
try {
@@ -75,8 +75,9 @@ SDL_mutexP(SDL_mutex *mutex)
int SDL_TryLockMutex(SDL_mutex *mutex)
{
int retval = 0;
if (mutex == NULL) {
return SDL_InvalidParamError("mutex");
return 0;
}
if (mutex->cpp_mutex.try_lock() == false) {
@@ -87,10 +88,10 @@ int SDL_TryLockMutex(SDL_mutex *mutex)
/* Unlock the mutex */
extern "C" int
SDL_mutexV(SDL_mutex *mutex)
SDL_UnlockMutex(SDL_mutex *mutex) SDL_NO_THREAD_SAFETY_ANALYSIS /* clang doesn't know about NULL mutexes */
{
if (mutex == NULL) {
return SDL_InvalidParamError("mutex");
return 0;
}
mutex->cpp_mutex.unlock();

View File

@@ -68,6 +68,27 @@ void SDL_DestroyMutex(SDL_mutex *mutex)
}
}
/* Lock the mutex */
int SDL_LockMutex(SDL_mutex *mutex) SDL_NO_THREAD_SAFETY_ANALYSIS /* clang doesn't know about NULL mutexes */
{
#if SDL_THREADS_DISABLED
return 0;
#else
SceInt32 res = 0;
if (mutex == NULL) {
return 0;
}
res = sceKernelLockLwMutex(&mutex->lock, 1, NULL);
if (res != SCE_KERNEL_OK) {
return SDL_SetError("Error trying to lock mutex: %x", res);
}
return 0;
#endif /* SDL_THREADS_DISABLED */
}
/* Try to lock the mutex */
int SDL_TryLockMutex(SDL_mutex *mutex)
{
@@ -75,8 +96,9 @@ int SDL_TryLockMutex(SDL_mutex *mutex)
return 0;
#else
SceInt32 res = 0;
if (mutex == NULL) {
return SDL_InvalidParamError("mutex");
return 0;
}
res = sceKernelTryLockLwMutex(&mutex->lock, 1);
@@ -96,28 +118,8 @@ int SDL_TryLockMutex(SDL_mutex *mutex)
#endif /* SDL_THREADS_DISABLED */
}
/* Lock the mutex */
int SDL_mutexP(SDL_mutex *mutex)
{
#if SDL_THREADS_DISABLED
return 0;
#else
SceInt32 res = 0;
if (mutex == NULL) {
return SDL_InvalidParamError("mutex");
}
res = sceKernelLockLwMutex(&mutex->lock, 1, NULL);
if (res != SCE_KERNEL_OK) {
return SDL_SetError("Error trying to lock mutex: %x", res);
}
return 0;
#endif /* SDL_THREADS_DISABLED */
}
/* Unlock the mutex */
int SDL_mutexV(SDL_mutex *mutex)
int SDL_UnlockMutex(SDL_mutex *mutex) SDL_NO_THREAD_SAFETY_ANALYSIS /* clang doesn't know about NULL mutexes */
{
#if SDL_THREADS_DISABLED
return 0;
@@ -125,7 +127,7 @@ int SDL_mutexV(SDL_mutex *mutex)
SceInt32 res = 0;
if (mutex == NULL) {
return SDL_InvalidParamError("mutex");
return 0;
}
res = sceKernelUnlockLwMutex(&mutex->lock, 1);

View File

@@ -74,13 +74,13 @@ static void SDL_DestroyMutex_srw(SDL_mutex *mutex)
}
}
static int SDL_LockMutex_srw(SDL_mutex *_mutex)
static int SDL_LockMutex_srw(SDL_mutex *_mutex) SDL_NO_THREAD_SAFETY_ANALYSIS /* clang doesn't know about NULL mutexes */
{
SDL_mutex_srw *mutex = (SDL_mutex_srw *)_mutex;
DWORD this_thread;
if (mutex == NULL) {
return SDL_InvalidParamError("mutex");
return 0;
}
this_thread = GetCurrentThreadId();
@@ -106,7 +106,7 @@ static int SDL_TryLockMutex_srw(SDL_mutex *_mutex)
int retval = 0;
if (mutex == NULL) {
return SDL_InvalidParamError("mutex");
return 0;
}
this_thread = GetCurrentThreadId();
@@ -124,12 +124,12 @@ static int SDL_TryLockMutex_srw(SDL_mutex *_mutex)
return retval;
}
static int SDL_UnlockMutex_srw(SDL_mutex *_mutex)
static int SDL_UnlockMutex_srw(SDL_mutex *_mutex) SDL_NO_THREAD_SAFETY_ANALYSIS /* clang doesn't know about NULL mutexes */
{
SDL_mutex_srw *mutex = (SDL_mutex_srw *)_mutex;
if (mutex == NULL) {
return SDL_InvalidParamError("mutex");
return 0;
}
if (mutex->owner == GetCurrentThreadId()) {
@@ -189,11 +189,11 @@ static void SDL_DestroyMutex_cs(SDL_mutex *mutex_)
}
/* Lock the mutex */
static int SDL_LockMutex_cs(SDL_mutex *mutex_)
static int SDL_LockMutex_cs(SDL_mutex *mutex_) SDL_NO_THREAD_SAFETY_ANALYSIS /* clang doesn't know about NULL mutexes */
{
SDL_mutex_cs *mutex = (SDL_mutex_cs *)mutex_;
if (mutex == NULL) {
return SDL_InvalidParamError("mutex");
return 0;
}
EnterCriticalSection(&mutex->cs);
@@ -206,7 +206,7 @@ static int SDL_TryLockMutex_cs(SDL_mutex *mutex_)
SDL_mutex_cs *mutex = (SDL_mutex_cs *)mutex_;
int retval = 0;
if (mutex == NULL) {
return SDL_InvalidParamError("mutex");
return 0;
}
if (TryEnterCriticalSection(&mutex->cs) == 0) {
@@ -216,11 +216,11 @@ static int SDL_TryLockMutex_cs(SDL_mutex *mutex_)
}
/* Unlock the mutex */
static int SDL_UnlockMutex_cs(SDL_mutex *mutex_)
static int SDL_UnlockMutex_cs(SDL_mutex *mutex_) SDL_NO_THREAD_SAFETY_ANALYSIS /* clang doesn't know about NULL mutexes */
{
SDL_mutex_cs *mutex = (SDL_mutex_cs *)mutex_;
if (mutex == NULL) {
return SDL_InvalidParamError("mutex");
return 0;
}
LeaveCriticalSection(&mutex->cs);