pthread_cond_clockwait isn't added to TSAN_INTERCEPTORS which leads to false positives regarding double locking of a mutex. This was uncovered by a user reporting an issue to the google sanitizer github: https://github.com/google/sanitizers/issues/1259
This patch copies code from the fix made in llvm: https://github.com/llvm/llvm-project/commit/16eb853ffdd1a1ad7c95455b7795c5f004402e46 However, because the tsan related source code hasn't been kept in sync with llvm, I had to make some modifications. Given that this is my first contibution to gcc, let me know if I've missed anything. Met vriendelijke groet, Michael de Lang +++ b/gcc/testsuite/g++.dg/tsan/pthread_cond_clockwait.C @@ -0,0 +1,31 @@ +// Test pthread_cond_clockwait not generating false positives with tsan +// { dg-do run { target { { *-*-linux* *-*-gnu* *-*-uclinux* } && pthread } } } +// { dg-options "-fsanitize=thread -lpthread" } + +#include <pthread.h> + +pthread_cond_t cv; +pthread_mutex_t mtx; + +void *fn(void *vp) { + pthread_mutex_lock(&mtx); + pthread_cond_signal(&cv); + pthread_mutex_unlock(&mtx); + return NULL; +} + +int main() { + pthread_mutex_lock(&mtx); + + pthread_t tid; + pthread_create(&tid, NULL, fn, NULL); + + struct timespec ts; + clock_gettime(CLOCK_MONOTONIC, &ts); + ts.tv_sec += 10; + pthread_cond_clockwait(&cv, &mtx, CLOCK_MONOTONIC, &ts); + pthread_mutex_unlock(&mtx); + + pthread_join(tid, NULL); + return 0; +} diff --git a/libsanitizer/tsan/tsan_interceptors_posix.cpp b/libsanitizer/tsan/tsan_interceptors_posix.cpp index aa04d8dfb67..7b3d0a917de 100644 --- a/libsanitizer/tsan/tsan_interceptors_posix.cpp +++ b/libsanitizer/tsan/tsan_interceptors_posix.cpp @@ -1126,7 +1126,10 @@ struct CondMutexUnlockCtx { ScopedInterceptor *si; ThreadState *thr; uptr pc; + void *c; void *m; + void *abstime; + __sanitizer_clockid_t clock; }; static void cond_mutex_unlock(CondMutexUnlockCtx *arg) { @@ -1152,19 +1155,18 @@ INTERCEPTOR(int, pthread_cond_init, void *c, void *a) { } static int cond_wait(ThreadState *thr, uptr pc, ScopedInterceptor *si, - int (*fn)(void *c, void *m, void *abstime), void *c, - void *m, void *t) { + int (*fn)(void *arg), void *c, + void *m, void *t, __sanitizer_clockid_t clock) { MemoryAccessRange(thr, pc, (uptr)c, sizeof(uptr), false); MutexUnlock(thr, pc, (uptr)m); - CondMutexUnlockCtx arg = {si, thr, pc, m}; + CondMutexUnlockCtx arg = {si, thr, pc, c, m, t, clock}; int res = 0; // This ensures that we handle mutex lock even in case of pthread_cancel. // See test/tsan/cond_cancel.cpp. { // Enable signal delivery while the thread is blocked. BlockingCall bc(thr); - res = call_pthread_cancel_with_cleanup( - fn, c, m, t, (void (*)(void *arg))cond_mutex_unlock, &arg); + res = call_pthread_cancel_with_cleanup(fn, (void (*)(void *arg))cond_mutex_unlock, &arg); } if (res == errno_EOWNERDEAD) MutexRepair(thr, pc, (uptr)m); MutexPostLock(thr, pc, (uptr)m, MutexFlagDoPreLockOnPostLock); @@ -1174,25 +1176,34 @@ static int cond_wait(ThreadState *thr, uptr pc, ScopedInterceptor *si, INTERCEPTOR(int, pthread_cond_wait, void *c, void *m) { void *cond = init_cond(c); SCOPED_TSAN_INTERCEPTOR(pthread_cond_wait, cond, m); - return cond_wait(thr, pc, &si, (int (*)(void *c, void *m, void *abstime))REAL( - pthread_cond_wait), - cond, m, 0); + return cond_wait(thr, pc, &si, [](void *a) { CondMutexUnlockCtx *arg = (CondMutexUnlockCtx*)a; return REAL(pthread_cond_wait)(arg->c, arg->m); }, + cond, m, 0, 0); } INTERCEPTOR(int, pthread_cond_timedwait, void *c, void *m, void *abstime) { void *cond = init_cond(c); SCOPED_TSAN_INTERCEPTOR(pthread_cond_timedwait, cond, m, abstime); - return cond_wait(thr, pc, &si, REAL(pthread_cond_timedwait), cond, m, - abstime); + return cond_wait(thr, pc, &si, [](void *a) { CondMutexUnlockCtx *arg = (CondMutexUnlockCtx*)a; return REAL(pthread_cond_timedwait)(arg->c, arg->m, arg->abstime); }, cond, m, + abstime, 0); } +#if SANITIZER_LINUX +INTERCEPTOR(int, pthread_cond_clockwait, void *c, void *m, __sanitizer_clockid_t clock, void *abstime) { + void *cond = init_cond(c); + SCOPED_TSAN_INTERCEPTOR(pthread_cond_clockwait, cond, m, clock, abstime); + return cond_wait(thr, pc, &si, + [](void *a) { CondMutexUnlockCtx *arg = (CondMutexUnlockCtx*)a; return REAL(pthread_cond_clockwait)(arg->c, arg->m, arg->clock, arg->abstime); }, + cond, m, abstime, clock); +} +#endif + #if SANITIZER_MAC INTERCEPTOR(int, pthread_cond_timedwait_relative_np, void *c, void *m, void *reltime) { void *cond = init_cond(c); SCOPED_TSAN_INTERCEPTOR(pthread_cond_timedwait_relative_np, cond, m, reltime); - return cond_wait(thr, pc, &si, REAL(pthread_cond_timedwait_relative_np), cond, - m, reltime); + return cond_wait(thr, pc, &si, [](void *a) { CondMutexUnlockCtx *arg = (CondMutexUnlockCtx*)a; return REAL(pthread_cond_timedwait_relative_np)(arg->c, arg->m, arg->abstime); }, cond, + m, reltime, 0); } #endif @@ -2697,6 +2708,9 @@ void InitializeInterceptors() { TSAN_INTERCEPT_VER(pthread_cond_wait, PTHREAD_ABI_BASE); TSAN_INTERCEPT_VER(pthread_cond_timedwait, PTHREAD_ABI_BASE); TSAN_INTERCEPT_VER(pthread_cond_destroy, PTHREAD_ABI_BASE); +#if SANITIZER_LINUX + TSAN_INTERCEPT(pthread_cond_clockwait); +#endif TSAN_INTERCEPT(pthread_mutex_init); TSAN_INTERCEPT(pthread_mutex_destroy); diff --git a/libsanitizer/tsan/tsan_platform.h b/libsanitizer/tsan/tsan_platform.h index 16169cab666..d973136f7ae 100644 --- a/libsanitizer/tsan/tsan_platform.h +++ b/libsanitizer/tsan/tsan_platform.h @@ -1040,9 +1040,8 @@ int ExtractRecvmsgFDs(void *msg, int *fds, int nfd); uptr ExtractLongJmpSp(uptr *env); void ImitateTlsWrite(ThreadState *thr, uptr tls_addr, uptr tls_size); -int call_pthread_cancel_with_cleanup(int(*fn)(void *c, void *m, - void *abstime), void *c, void *m, void *abstime, - void(*cleanup)(void *arg), void *arg); +int call_pthread_cancel_with_cleanup(int(*fn)(void *arg), + void(*cleanup)(void *arg), void *cleanup_arg); void DestroyThreadState(); void PlatformCleanUpThreadState(ThreadState *thr); diff --git a/libsanitizer/tsan/tsan_platform_linux.cpp b/libsanitizer/tsan/tsan_platform_linux.cpp index d136dcb1cec..d0ac995dfb2 100644 --- a/libsanitizer/tsan/tsan_platform_linux.cpp +++ b/libsanitizer/tsan/tsan_platform_linux.cpp @@ -443,14 +443,13 @@ void ImitateTlsWrite(ThreadState *thr, uptr tls_addr, uptr tls_size) { // Note: this function runs with async signals enabled, // so it must not touch any tsan state. -int call_pthread_cancel_with_cleanup(int(*fn)(void *c, void *m, - void *abstime), void *c, void *m, void *abstime, +int call_pthread_cancel_with_cleanup(int(*fn)(void *arg), void(*cleanup)(void *arg), void *arg) { // pthread_cleanup_push/pop are hardcore macros mess. // We can't intercept nor call them w/o including pthread.h. int res; pthread_cleanup_push(cleanup, arg); - res = fn(c, m, abstime); + res = fn(arg); pthread_cleanup_pop(0); return res; } diff --git a/libsanitizer/tsan/tsan_platform_mac.cpp b/libsanitizer/tsan/tsan_platform_mac.cpp index ec2c5fb1621..59427b9cb6c 100644 --- a/libsanitizer/tsan/tsan_platform_mac.cpp +++ b/libsanitizer/tsan/tsan_platform_mac.cpp @@ -306,14 +306,13 @@ void ImitateTlsWrite(ThreadState *thr, uptr tls_addr, uptr tls_size) { #if !SANITIZER_GO // Note: this function runs with async signals enabled, // so it must not touch any tsan state. -int call_pthread_cancel_with_cleanup(int(*fn)(void *c, void *m, - void *abstime), void *c, void *m, void *abstime, +int call_pthread_cancel_with_cleanup(int(*fn)(void *arg), void(*cleanup)(void *arg), void *arg) { // pthread_cleanup_push/pop are hardcore macros mess. // We can't intercept nor call them w/o including pthread.h. int res; pthread_cleanup_push(cleanup, arg); - res = fn(c, m, abstime); + res = fn(arg); pthread_cleanup_pop(0); return res; }
