tsan: fix handling of pthread_cond_wait in presence of pthread_cancel

if the thread is cancelled in pthread_cond_wait, it locks the mutex before
processing pthread_cleanup stack
but tsan was missing that, thus reporting false double-lock/wrong-unlock errors
see the test for details

llvm-svn: 203648

5 files changed, 89 insertions, 4 deletions

diff --git a/compiler-rt/lib/sanitizer_common/sanitizer_common_interceptors.inc b/compiler-rt/lib/sanitizer_common/sanitizer_common_interceptors.inc
index b6ae222aca4..010799aa13f 100644
--- a/compiler-rt/lib/sanitizer_common/sanitizer_common_interceptors.inc
+++ b/compiler-rt/lib/sanitizer_common/sanitizer_common_interceptors.inc
@@ -2483,6 +2483,21 @@ static void *init_cond(void *c, bool force = false) {
   return (void*)cond;
 }
 
+struct CondMutexUnlockCtx {
+  void *ctx;
+  void *m;
+};
+
+static void cond_mutex_unlock(CondMutexUnlockCtx *arg) {
+  COMMON_INTERCEPTOR_MUTEX_LOCK(arg->ctx, arg->m);
+}
+
+namespace __sanitizer {
+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);
+}  // namespace __sanitizer
+
 INTERCEPTOR(int, pthread_cond_init, void *c, void *a) {
   void *cond = init_cond(c, true);
   void *ctx;
@@ -2497,8 +2512,12 @@ INTERCEPTOR(int, pthread_cond_wait, void *c, void *m) {
   COMMON_INTERCEPTOR_ENTER(ctx, pthread_cond_wait, cond, m);
   COMMON_INTERCEPTOR_MUTEX_UNLOCK(ctx, m);
   COMMON_INTERCEPTOR_READ_RANGE(ctx, c, sizeof(uptr));
-  int res = REAL(pthread_cond_wait)(cond, m);
-  COMMON_INTERCEPTOR_MUTEX_LOCK(ctx, m);
+  CondMutexUnlockCtx arg = {ctx, m};
+  // This ensures that we handle mutex lock even in case of pthread_cancel.
+  // See test/tsan/cond_cancel.cc.
+  int res = __sanitizer::call_pthread_cancel_with_cleanup(
+      (int(*)(void *c, void *m, void *abstime))REAL(pthread_cond_wait),
+      cond, m, 0, (void(*)(void *arg))cond_mutex_unlock, &arg);
   return res;
 }
 
@@ -2508,8 +2527,12 @@ INTERCEPTOR(int, pthread_cond_timedwait, void *c, void *m, void *abstime) {
   COMMON_INTERCEPTOR_ENTER(ctx, pthread_cond_timedwait, cond, m, abstime);
   COMMON_INTERCEPTOR_MUTEX_UNLOCK(ctx, m);
   COMMON_INTERCEPTOR_READ_RANGE(ctx, c, sizeof(uptr));
-  int res = REAL(pthread_cond_timedwait)(cond, m, abstime);
-  COMMON_INTERCEPTOR_MUTEX_LOCK(ctx, m);
+  CondMutexUnlockCtx arg = {ctx, m};
+  // This ensures that we handle mutex lock even in case of pthread_cancel.
+  // See test/tsan/cond_cancel.cc.
+  int res = __sanitizer::call_pthread_cancel_with_cleanup(
+      REAL(pthread_cond_timedwait), cond, m, abstime,
+      (void(*)(void *arg))cond_mutex_unlock, &arg);
   return res;
 }
 
diff --git a/compiler-rt/lib/sanitizer_common/sanitizer_linux_libcdep.cc b/compiler-rt/lib/sanitizer_common/sanitizer_linux_libcdep.cc
index 2e3fbd2b851..94e99e0aa49 100644
--- a/compiler-rt/lib/sanitizer_common/sanitizer_linux_libcdep.cc
+++ b/compiler-rt/lib/sanitizer_common/sanitizer_linux_libcdep.cc
@@ -515,6 +515,18 @@ void SetIndirectCallWrapper(uptr wrapper) {
   indirect_call_wrapper = wrapper;
 }
 
+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) {
+  // 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);
+  pthread_cleanup_pop(1);
+  return res;
+}
+
 }  // namespace __sanitizer
 
 #endif  // SANITIZER_FREEBSD || SANITIZER_LINUX
diff --git a/compiler-rt/lib/sanitizer_common/sanitizer_mac.cc b/compiler-rt/lib/sanitizer_common/sanitizer_mac.cc
index 747513de50e..b6b3a016033 100644
--- a/compiler-rt/lib/sanitizer_common/sanitizer_mac.cc
+++ b/compiler-rt/lib/sanitizer_common/sanitizer_mac.cc
@@ -302,6 +302,18 @@ MacosVersion GetMacosVersion() {
   return result;
 }
 
+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) {
+  // 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);
+  pthread_cleanup_pop(1);
+  return res;
+}
+
 }  // namespace __sanitizer
 
 #endif  // SANITIZER_MAC
diff --git a/compiler-rt/test/tsan/cond_cancel.c b/compiler-rt/test/tsan/cond_cancel.c
new file mode 100644
index 00000000000..e37621ddacc
--- /dev/null
+++ b/compiler-rt/test/tsan/cond_cancel.c
@@ -0,0 +1,37 @@
+// RUN: %clang_tsan -O1 %s -o %t && %t 2>&1 | FileCheck %s
+// CHECK-NOT: WARNING
+// CHECK: OK
+
+#include <stdio.h>
+#include <stdlib.h>
+#include <pthread.h>
+#include <unistd.h>
+
+pthread_mutex_t m;
+pthread_cond_t c;
+int x;
+
+void *thr1(void *p) {
+  pthread_mutex_lock(&m);
+  pthread_cleanup_push((void(*)(void *arg))pthread_mutex_unlock, &m);
+  while (x == 0)
+    pthread_cond_wait(&c, &m);
+  pthread_cleanup_pop(1);
+  return 0;
+}
+
+int main() {
+  pthread_t th;
+
+  pthread_mutex_init(&m, 0);
+  pthread_cond_init(&c, 0);
+
+  pthread_create(&th, 0, thr1, 0);
+  sleep(1);  // let it block on cond var
+  pthread_cancel(th);
+
+  pthread_join(th, 0);
+  pthread_mutex_lock(&m);
+  pthread_mutex_unlock(&m);
+  fprintf(stderr, "OK\n");
+}
diff --git a/compiler-rt/test/tsan/cond_race.cc b/compiler-rt/test/tsan/cond_race.cc
index de9f18d2194..08d6901e6f0 100644
--- a/compiler-rt/test/tsan/cond_race.cc
+++ b/compiler-rt/test/tsan/cond_race.cc
@@ -1,4 +1,5 @@
 // RUN: %clang_tsan -O1 %s -o %t && not %t 2>&1 | FileCheck %s
+// CHECK-NOT: unlock of unlocked mutex
 // CHECK: ThreadSanitizer: data race
 // CHECK: pthread_cond_signal