[compiler-rt] be41a98 - tsan: fix deadlock with pthread_atfork callbacks

[Dmitry Vyukov via llvm-commits]

Author: Dmitry Vyukov
Date: 2020-03-21T14:36:51+01:00
New Revision: be41a98ac222f33ed5558d86e1cede67249e99b5

URL: https://github.com/llvm/llvm-project/commit/be41a98ac222f33ed5558d86e1cede67249e99b5
DIFF: https://github.com/llvm/llvm-project/commit/be41a98ac222f33ed5558d86e1cede67249e99b5.diff

LOG: tsan: fix deadlock with pthread_atfork callbacks

This fixes the bug reported at:
https://groups.google.com/forum/#!topic/thread-sanitizer/e_zB9gYqFHM

A pthread_atfork callback triggers a data race
and we deadlock on the report_mtx. Ignore memory access
in the pthread_atfork callbacks to prevent the deadlock.

Added: 
    compiler-rt/test/tsan/pthread_atfork_deadlock2.c

Modified: 
    compiler-rt/lib/tsan/rtl/tsan_rtl.cpp

Removed: 
    


################################################################################
diff  --git a/compiler-rt/lib/tsan/rtl/tsan_rtl.cpp b/compiler-rt/lib/tsan/rtl/tsan_rtl.cpp
index fe469faad2a2..13c9b770f50a 100644
--- a/compiler-rt/lib/tsan/rtl/tsan_rtl.cpp
+++ b/compiler-rt/lib/tsan/rtl/tsan_rtl.cpp
@@ -495,14 +495,23 @@ int Finalize(ThreadState *thr) {
 void ForkBefore(ThreadState *thr, uptr pc) {
   ctx->thread_registry->Lock();
   ctx->report_mtx.Lock();
+  // Ignore memory accesses in the pthread_atfork callbacks.
+  // If any of them triggers a data race we will deadlock
+  // on the report_mtx.
+  // We could ignore interceptors and sync operations as well,
+  // but so far it's unclear if it will do more good or harm.
+  // Unnecessarily ignoring things can lead to false positives later.
+  ThreadIgnoreBegin(thr, pc);
 }
 
 void ForkParentAfter(ThreadState *thr, uptr pc) {
+  ThreadIgnoreEnd(thr, pc);  // Begin is in ForkBefore.
   ctx->report_mtx.Unlock();
   ctx->thread_registry->Unlock();
 }
 
 void ForkChildAfter(ThreadState *thr, uptr pc) {
+  ThreadIgnoreEnd(thr, pc);  // Begin is in ForkBefore.
   ctx->report_mtx.Unlock();
   ctx->thread_registry->Unlock();
 

diff  --git a/compiler-rt/test/tsan/pthread_atfork_deadlock2.c b/compiler-rt/test/tsan/pthread_atfork_deadlock2.c
new file mode 100644
index 000000000000..700507c1e637
--- /dev/null
+++ b/compiler-rt/test/tsan/pthread_atfork_deadlock2.c
@@ -0,0 +1,49 @@
+// RUN: %clang_tsan -O1 %s -o %t && %run %t 2>&1 | FileCheck %s
+// Regression test for
+// https://groups.google.com/d/msg/thread-sanitizer/e_zB9gYqFHM/DmAiTsrLAwAJ
+// pthread_atfork() callback triggers a data race and we deadlocked
+// on the report_mtx as we lock it around fork.
+#include "test.h"
+#include <sys/types.h>
+#include <sys/wait.h>
+#include <errno.h>
+
+int glob = 0;
+
+void *worker(void *unused) {
+  glob++;
+  barrier_wait(&barrier);
+  return NULL;
+}
+
+void atfork() {
+  glob++;
+}
+
+int main() {
+  barrier_init(&barrier, 2);
+  pthread_atfork(atfork, NULL, NULL);
+  pthread_t t;
+  pthread_create(&t, NULL, worker, NULL);
+  barrier_wait(&barrier);
+  pid_t pid = fork();
+  if (pid < 0) {
+    fprintf(stderr, "fork failed: %d\n", errno);
+    return 1;
+  }
+  if (pid == 0) {
+    fprintf(stderr, "CHILD\n");
+    return 0;
+  }
+  if (pid != waitpid(pid, NULL, 0)) {
+    fprintf(stderr, "waitpid failed: %d\n", errno);
+    return 1;
+  }
+  pthread_join(t, NULL);
+  fprintf(stderr, "PARENT\n");
+  return 0;
+}
+
+// CHECK-NOT: ThreadSanitizer: data race
+// CHECK: CHILD
+// CHECK: PARENT