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

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);

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);

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) {

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

compiler-rt/test/tsan/ignore_lib6.cpp.supp

@@ -0,0 +1 @@
+called_from_lib:/libignore_lib.so$