From be41a98ac222f33ed5558d86e1cede67249e99b5 Mon Sep 17 00:00:00 2001 From: Dmitry Vyukov Date: Sat, 21 Mar 2020 13:34:50 +0100 Subject: [PATCH] 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. --- compiler-rt/lib/tsan/rtl/tsan_rtl.cc | 9 ++++ .../test/tsan/pthread_atfork_deadlock2.c | 49 +++++++++++++++++++ 2 files changed, 58 insertions(+) create mode 100644 compiler-rt/test/tsan/pthread_atfork_deadlock2.c diff --git a/compiler-rt/lib/tsan/rtl/tsan_rtl.cc b/compiler-rt/lib/tsan/rtl/tsan_rtl.ccc index fe469faad2a2..13c9b770f50a 100644 --- a/compiler-rt/lib/tsan/rtl/tsan_rtl.cc +++ b/compiler-rt/lib/tsan/rtl/tsan_rtl.cc @@ -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 +#include +#include + +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