tsan: fix false positive for pthread_cond_clockwait

Message ID CAJbH2PzQg9aSBK9i0zX1dLM8vqr3M7GTESN6ACD71TcVL_U6pw@mail.gmail.com
State New
Headers show
Series
  • tsan: fix false positive for pthread_cond_clockwait
Related show

Commit Message

Jonathan Wakely via Gcc-patches May 7, 2021, 5:07 p.m.
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;
+}
 }

Comments

Martin Liška May 13, 2021, 7:33 a.m. | #1
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;

>   }

>
Jonathan Wakely via Gcc-patches May 13, 2021, 5:10 p.m. | #2
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;

> >   }

> >

>
Martin Liška May 14, 2021, 10:10 a.m. | #3
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

Patch

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;