Message ID | CAJbH2PzQg9aSBK9i0zX1dLM8vqr3M7GTESN6ACD71TcVL_U6pw@mail.gmail.com |
---|---|
State | New |
Headers | show |
Series | tsan: fix false positive for pthread_cond_clockwait | expand |
On 5/7/21 7:07 PM, Michael de Lang via Gcc-patches wrote: > 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 Hello. Thank you for looking into this. > > However, because the tsan related source code hasn't been kept in sync > with llvm, I had to make some modifications. We merge from master rougtly twice a year. I've just merged LLVM upstream to our master. > > Given that this is my first contibution to gcc, let me know if I've > missed anything. Please take a look at the following steps: https://gcc.gnu.org/contribute.html We still want your test-case, can you please resend the patch on the current master? Thanks! Cheers, Martin > > 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; > } >
Thanks for updating LLVM to upstream. I've added the rebased patch below. Met vriendelijke groet, Michael de Lang gcc/ChangeLog * g++.dg/tsan/pthread_cond_clockwait.C: new testcase diff --git a/gcc/testsuite/g++.dg/tsan/pthread_cond_clockwait.C b/gcc/testsuite/g++.dg/tsan/pthread_cond_clockwait.C new file mode 100644 index 00000000000..82d6a5c8329 --- /dev/null +++ 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; +} On Thu, 13 May 2021 at 09:33, Martin Liška <mliska@suse.cz> wrote: > > On 5/7/21 7:07 PM, Michael de Lang via Gcc-patches wrote: > > 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 > > Hello. > > Thank you for looking into this. > > > > > However, because the tsan related source code hasn't been kept in sync > > with llvm, I had to make some modifications. > > We merge from master rougtly twice a year. I've just merged LLVM upstream to our master. > > > > > Given that this is my first contibution to gcc, let me know if I've > > missed anything. > > Please take a look at the following steps: > https://gcc.gnu.org/contribute.html > > We still want your test-case, can you please resend the patch on the current master? > > Thanks! > Cheers, > Martin > > > > > 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; > > } > > >
On 5/13/21 7:10 PM, Michael de Lang wrote:
> |Thanks for updating LLVM to upstream. I've added the rebased patch below.|
Thanks, I've just tested and pushed your commit:
https://gcc.gnu.org/git/?p=gcc.git;a=commit;h=80b4ce1a5190ebe764b1009afae57dcef45f92c2
Martin
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;