[compiler-rt] 2dcbdba - tsan: fix pthread_detach with called_from_lib suppressions
Dmitry Vyukov via llvm-commits
llvm-commits at lists.llvm.org
Wed Feb 26 03:59:59 PST 2020
Author: Dmitry Vyukov
Date: 2020-02-26T12:59:49+01:00
New Revision: 2dcbdba85406e498b885570c05e9573ddc3e4a81
URL: https://github.com/llvm/llvm-project/commit/2dcbdba85406e498b885570c05e9573ddc3e4a81
DIFF: https://github.com/llvm/llvm-project/commit/2dcbdba85406e498b885570c05e9573ddc3e4a81.diff
LOG: tsan: fix pthread_detach with called_from_lib suppressions
Generally we ignore interceptors coming from called_from_lib-suppressed libraries.
However, we must not ignore critical interceptors like e.g. pthread_create,
otherwise runtime will lost track of threads.
pthread_detach is one of these interceptors we should not ignore as it affects
thread states and behavior of pthread_join which we don't ignore as well.
Currently we can produce very obscure false positives. For more context see:
https://groups.google.com/forum/#!topic/thread-sanitizer/ecH2P0QUqPs
The added test captures this pattern.
While we are here rename ThreadTid to ThreadConsumeTid to make it clear that
it's not just a "getter", it resets user_id to 0. This lead to confusion recently.
Reviewed in https://reviews.llvm.org/D74828
Added:
compiler-rt/test/tsan/ignore_lib6.cpp
compiler-rt/test/tsan/ignore_lib6.cpp.supp
Modified:
compiler-rt/lib/tsan/rtl/tsan_interceptors_posix.cpp
compiler-rt/lib/tsan/rtl/tsan_rtl.h
compiler-rt/lib/tsan/rtl/tsan_rtl_thread.cpp
Removed:
################################################################################
diff --git a/compiler-rt/lib/tsan/rtl/tsan_interceptors_posix.cpp b/compiler-rt/lib/tsan/rtl/tsan_interceptors_posix.cpp
index 8aea1e4ec051..a623f4fe589d 100644
--- a/compiler-rt/lib/tsan/rtl/tsan_interceptors_posix.cpp
+++ b/compiler-rt/lib/tsan/rtl/tsan_interceptors_posix.cpp
@@ -1016,7 +1016,7 @@ TSAN_INTERCEPTOR(int, pthread_create,
TSAN_INTERCEPTOR(int, pthread_join, void *th, void **ret) {
SCOPED_INTERCEPTOR_RAW(pthread_join, th, ret);
- int tid = ThreadTid(thr, pc, (uptr)th);
+ int tid = ThreadConsumeTid(thr, pc, (uptr)th);
ThreadIgnoreBegin(thr, pc);
int res = BLOCK_REAL(pthread_join)(th, ret);
ThreadIgnoreEnd(thr, pc);
@@ -1029,8 +1029,8 @@ TSAN_INTERCEPTOR(int, pthread_join, void *th, void **ret) {
DEFINE_REAL_PTHREAD_FUNCTIONS
TSAN_INTERCEPTOR(int, pthread_detach, void *th) {
- SCOPED_TSAN_INTERCEPTOR(pthread_detach, th);
- int tid = ThreadTid(thr, pc, (uptr)th);
+ SCOPED_INTERCEPTOR_RAW(pthread_detach, th);
+ int tid = ThreadConsumeTid(thr, pc, (uptr)th);
int res = REAL(pthread_detach)(th);
if (res == 0) {
ThreadDetach(thr, pc, tid);
@@ -1050,8 +1050,8 @@ TSAN_INTERCEPTOR(void, pthread_exit, void *retval) {
#if SANITIZER_LINUX
TSAN_INTERCEPTOR(int, pthread_tryjoin_np, void *th, void **ret) {
- SCOPED_TSAN_INTERCEPTOR(pthread_tryjoin_np, th, ret);
- int tid = ThreadTid(thr, pc, (uptr)th);
+ SCOPED_INTERCEPTOR_RAW(pthread_tryjoin_np, th, ret);
+ int tid = ThreadConsumeTid(thr, pc, (uptr)th);
ThreadIgnoreBegin(thr, pc);
int res = REAL(pthread_tryjoin_np)(th, ret);
ThreadIgnoreEnd(thr, pc);
@@ -1064,8 +1064,8 @@ TSAN_INTERCEPTOR(int, pthread_tryjoin_np, void *th, void **ret) {
TSAN_INTERCEPTOR(int, pthread_timedjoin_np, void *th, void **ret,
const struct timespec *abstime) {
- SCOPED_TSAN_INTERCEPTOR(pthread_timedjoin_np, th, ret, abstime);
- int tid = ThreadTid(thr, pc, (uptr)th);
+ SCOPED_INTERCEPTOR_RAW(pthread_timedjoin_np, th, ret, abstime);
+ int tid = ThreadConsumeTid(thr, pc, (uptr)th);
ThreadIgnoreBegin(thr, pc);
int res = BLOCK_REAL(pthread_timedjoin_np)(th, ret, abstime);
ThreadIgnoreEnd(thr, pc);
diff --git a/compiler-rt/lib/tsan/rtl/tsan_rtl.h b/compiler-rt/lib/tsan/rtl/tsan_rtl.h
index c38fc43a9f84..20f7a99157ab 100644
--- a/compiler-rt/lib/tsan/rtl/tsan_rtl.h
+++ b/compiler-rt/lib/tsan/rtl/tsan_rtl.h
@@ -775,7 +775,7 @@ int ThreadCreate(ThreadState *thr, uptr pc, uptr uid, bool detached);
void ThreadStart(ThreadState *thr, int tid, tid_t os_id,
ThreadType thread_type);
void ThreadFinish(ThreadState *thr);
-int ThreadTid(ThreadState *thr, uptr pc, uptr uid);
+int ThreadConsumeTid(ThreadState *thr, uptr pc, uptr uid);
void ThreadJoin(ThreadState *thr, uptr pc, int tid);
void ThreadDetach(ThreadState *thr, uptr pc, int tid);
void ThreadFinalize(ThreadState *thr);
diff --git a/compiler-rt/lib/tsan/rtl/tsan_rtl_thread.cpp b/compiler-rt/lib/tsan/rtl/tsan_rtl_thread.cpp
index 0ac1ee99c470..98beb5aad644 100644
--- a/compiler-rt/lib/tsan/rtl/tsan_rtl_thread.cpp
+++ b/compiler-rt/lib/tsan/rtl/tsan_rtl_thread.cpp
@@ -285,19 +285,34 @@ void ThreadFinish(ThreadState *thr) {
ctx->thread_registry->FinishThread(thr->tid);
}
-static bool FindThreadByUid(ThreadContextBase *tctx, void *arg) {
- uptr uid = (uptr)arg;
- if (tctx->user_id == uid && tctx->status != ThreadStatusInvalid) {
+struct ConsumeThreadContext {
+ uptr uid;
+ ThreadContextBase *tctx;
+};
+
+static bool ConsumeThreadByUid(ThreadContextBase *tctx, void *arg) {
+ ConsumeThreadContext *findCtx = (ConsumeThreadContext *)arg;
+ if (tctx->user_id == findCtx->uid && tctx->status != ThreadStatusInvalid) {
+ if (findCtx->tctx) {
+ // Ensure that user_id is unique. If it's not the case we are screwed.
+ // Something went wrong before, but now there is no way to recover.
+ // Returning a wrong thread is not an option, it may lead to very hard
+ // to debug false positives (e.g. if we join a wrong thread).
+ Report("ThreadSanitizer: dup thread with used id 0x%zx\n", findCtx->uid);
+ Die();
+ }
+ findCtx->tctx = tctx;
tctx->user_id = 0;
- return true;
}
return false;
}
-int ThreadTid(ThreadState *thr, uptr pc, uptr uid) {
- int res = ctx->thread_registry->FindThread(FindThreadByUid, (void*)uid);
- DPrintf("#%d: ThreadTid uid=%zu tid=%d\n", thr->tid, uid, res);
- return res;
+int ThreadConsumeTid(ThreadState *thr, uptr pc, uptr uid) {
+ ConsumeThreadContext findCtx = {uid, nullptr};
+ ctx->thread_registry->FindThread(ConsumeThreadByUid, &findCtx);
+ int tid = findCtx.tctx ? findCtx.tctx->tid : ThreadRegistry::kUnknownTid;
+ DPrintf("#%d: ThreadTid uid=%zu tid=%d\n", thr->tid, uid, tid);
+ return tid;
}
void ThreadJoin(ThreadState *thr, uptr pc, int tid) {
diff --git a/compiler-rt/test/tsan/ignore_lib6.cpp b/compiler-rt/test/tsan/ignore_lib6.cpp
new file mode 100644
index 000000000000..b03bde3765e3
--- /dev/null
+++ b/compiler-rt/test/tsan/ignore_lib6.cpp
@@ -0,0 +1,74 @@
+// RUN: rm -rf %t-dir
+// RUN: mkdir %t-dir
+// RUN: %clangxx_tsan -O1 %s -DLIB -fPIC -fno-sanitize=thread -shared -o %t-dir/libignore_lib.so
+// RUN: %clangxx_tsan -O1 %s %link_libcxx_tsan -o %t-dir/executable
+// RUN: %env_tsan_opts=suppressions='%s.supp' %run %t-dir/executable 2>&1 | FileCheck %s
+
+// Copied from ignore_lib5.cpp:
+// REQUIRES: stable-runtime
+// UNSUPPORTED: powerpc64le
+// UNSUPPORTED: netbsd
+
+// Test that pthread_detach works in libraries ignored by called_from_lib.
+// For more context see:
+// https://groups.google.com/forum/#!topic/thread-sanitizer/ecH2P0QUqPs
+
+#include "test.h"
+#include <dlfcn.h>
+#include <errno.h>
+#include <libgen.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <string>
+#include <sys/mman.h>
+
+#ifndef LIB
+
+void *thr(void *arg) {
+ *(volatile long long *)arg = 1;
+ return 0;
+}
+
+int main(int argc, char **argv) {
+ std::string lib = std::string(dirname(argv[0])) + "/libignore_lib.so";
+ void *h = dlopen(lib.c_str(), RTLD_GLOBAL | RTLD_NOW);
+ if (h == 0)
+ exit(printf("failed to load the library (%d)\n", errno));
+ void (*libfunc)() = (void (*)())dlsym(h, "libfunc");
+ if (libfunc == 0)
+ exit(printf("failed to find the func (%d)\n", errno));
+ libfunc();
+
+ const int kThreads = 10;
+ pthread_t t[kThreads];
+ volatile long long data[kThreads];
+ for (int i = 0; i < kThreads; i++)
+ pthread_create(&t[i], 0, thr, (void *)&data[i]);
+ for (int i = 0; i < kThreads; i++) {
+ pthread_join(t[i], 0);
+ data[i] = 2;
+ }
+ fprintf(stderr, "DONE\n");
+}
+
+// CHECK-NOT: WARNING: ThreadSanitizer:
+// CHECK: DONE
+// CHECK-NOT: WARNING: ThreadSanitizer:
+
+#else // #ifdef LIB
+
+void *thr(void *p) {
+ sleep(1);
+ pthread_detach(pthread_self());
+ return 0;
+}
+
+extern "C" void libfunc() {
+ const int kThreads = 10;
+ pthread_t t[kThreads];
+ for (int i = 0; i < kThreads; i++)
+ pthread_create(&t[i], 0, thr, 0);
+ sleep(2);
+}
+
+#endif // #ifdef LIB
diff --git a/compiler-rt/test/tsan/ignore_lib6.cpp.supp b/compiler-rt/test/tsan/ignore_lib6.cpp.supp
new file mode 100644
index 000000000000..78ec27c94acd
--- /dev/null
+++ b/compiler-rt/test/tsan/ignore_lib6.cpp.supp
@@ -0,0 +1 @@
+called_from_lib:/libignore_lib.so$
More information about the llvm-commits
mailing list