[compiler-rt] r203648 - tsan: fix handling of pthread_cond_wait in presence of pthread_cancel

[Dmitry Vyukov]

Author: dvyukov
Date: Wed Mar 12 04:48:14 2014
New Revision: 203648

URL: http://llvm.org/viewvc/llvm-project?rev=203648&view=rev
Log:
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


Added:
    compiler-rt/trunk/test/tsan/cond_cancel.c
Modified:
    compiler-rt/trunk/lib/sanitizer_common/sanitizer_common_interceptors.inc
    compiler-rt/trunk/lib/sanitizer_common/sanitizer_linux_libcdep.cc
    compiler-rt/trunk/lib/sanitizer_common/sanitizer_mac.cc
    compiler-rt/trunk/test/tsan/cond_race.cc

Modified: compiler-rt/trunk/lib/sanitizer_common/sanitizer_common_interceptors.inc
URL: http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/lib/sanitizer_common/sanitizer_common_interceptors.inc?rev=203648&r1=203647&r2=203648&view=diff
==============================================================================
--- compiler-rt/trunk/lib/sanitizer_common/sanitizer_common_interceptors.inc (original)
+++ compiler-rt/trunk/lib/sanitizer_common/sanitizer_common_interceptors.inc Wed Mar 12 04:48:14 2014
@@ -2483,6 +2483,21 @@ static void *init_cond(void *c, bool for
   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
   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,
   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;
 }
 

Modified: compiler-rt/trunk/lib/sanitizer_common/sanitizer_linux_libcdep.cc
URL: http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/lib/sanitizer_common/sanitizer_linux_libcdep.cc?rev=203648&r1=203647&r2=203648&view=diff
==============================================================================
--- compiler-rt/trunk/lib/sanitizer_common/sanitizer_linux_libcdep.cc (original)
+++ compiler-rt/trunk/lib/sanitizer_common/sanitizer_linux_libcdep.cc Wed Mar 12 04:48:14 2014
@@ -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

Modified: compiler-rt/trunk/lib/sanitizer_common/sanitizer_mac.cc
URL: http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/lib/sanitizer_common/sanitizer_mac.cc?rev=203648&r1=203647&r2=203648&view=diff
==============================================================================
--- compiler-rt/trunk/lib/sanitizer_common/sanitizer_mac.cc (original)
+++ compiler-rt/trunk/lib/sanitizer_common/sanitizer_mac.cc Wed Mar 12 04:48:14 2014
@@ -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

Added: compiler-rt/trunk/test/tsan/cond_cancel.c
URL: http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/test/tsan/cond_cancel.c?rev=203648&view=auto
==============================================================================
--- compiler-rt/trunk/test/tsan/cond_cancel.c (added)
+++ compiler-rt/trunk/test/tsan/cond_cancel.c Wed Mar 12 04:48:14 2014
@@ -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");
+}

Modified: compiler-rt/trunk/test/tsan/cond_race.cc
URL: http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/test/tsan/cond_race.cc?rev=203648&r1=203647&r2=203648&view=diff
==============================================================================
--- compiler-rt/trunk/test/tsan/cond_race.cc (original)
+++ compiler-rt/trunk/test/tsan/cond_race.cc Wed Mar 12 04:48:14 2014
@@ -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