[tsan] Properly describe GCD worker threads in reports

When dealing with GCD worker threads, TSan currently prints weird things like "created by thread T-1" and "[failed to restore the stack]" in reports. This patch avoids that and instead prints "Thread T3 (...) is a GCD worker thread". Differential Revision: https://reviews.llvm.org/D29103 llvm-svn: 293882
llvm · Feb 2, 2017 · bba1d40 · bba1d40
1 parent e396d8e
commit bba1d40
Show file tree

Hide file tree

Showing 15 changed files with 76 additions and 20 deletions.
diff --git a/compiler-rt/lib/asan/asan_mac.cc b/compiler-rt/lib/asan/asan_mac.cc
@@ -138,7 +138,8 @@ void asan_register_worker_thread(int parent_tid, StackTrace *stack) {
     t = AsanThread::Create(/* start_routine */ nullptr, /* arg */ nullptr,
                            parent_tid, stack, /* detached */ true);
     t->Init();
-    asanThreadRegistry().StartThread(t->tid(), 0, 0);
+    asanThreadRegistry().StartThread(t->tid(), GetTid(),
+                                     /* workerthread */ true, 0);
     SetCurrentThread(t);
   }
 }

diff --git a/compiler-rt/lib/asan/asan_thread.cc b/compiler-rt/lib/asan/asan_thread.cc
@@ -239,7 +239,8 @@ void AsanThread::Init() {
 thread_return_t AsanThread::ThreadStart(
     uptr os_id, atomic_uintptr_t *signal_thread_is_registered) {
   Init();
-  asanThreadRegistry().StartThread(tid(), os_id, nullptr);
+  asanThreadRegistry().StartThread(tid(), os_id, /*workerthread*/ false,
+                                   nullptr);
   if (signal_thread_is_registered)
     atomic_store(signal_thread_is_registered, 1, memory_order_release);
 

diff --git a/compiler-rt/lib/lsan/lsan_thread.cc b/compiler-rt/lib/lsan/lsan_thread.cc
@@ -97,7 +97,7 @@ void ThreadStart(u32 tid, uptr os_id) {
   args.tls_end = args.tls_begin + tls_size;
   GetAllocatorCacheRange(&args.cache_begin, &args.cache_end);
   args.dtls = DTLS_Get();
-  thread_registry->StartThread(tid, os_id, &args);
+  thread_registry->StartThread(tid, os_id, /*workerthread*/ false, &args);
 }
 
 void ThreadFinish() {

diff --git a/compiler-rt/lib/sanitizer_common/sanitizer_thread_registry.cc b/compiler-rt/lib/sanitizer_common/sanitizer_thread_registry.cc
@@ -19,7 +19,7 @@ namespace __sanitizer {
 ThreadContextBase::ThreadContextBase(u32 tid)
     : tid(tid), unique_id(0), reuse_count(), os_id(0), user_id(0),
       status(ThreadStatusInvalid),
-      detached(false), parent_tid(0), next(0) {
+      detached(false), workerthread(false), parent_tid(0), next(0) {
   name[0] = '\0';
 }
 
@@ -59,9 +59,10 @@ void ThreadContextBase::SetFinished() {
   OnFinished();
 }
 
-void ThreadContextBase::SetStarted(uptr _os_id, void *arg) {
+void ThreadContextBase::SetStarted(uptr _os_id, bool _workerthread, void *arg) {
   status = ThreadStatusRunning;
   os_id = _os_id;
+  workerthread = _workerthread;
   OnStarted(arg);
 }
 
@@ -266,14 +267,15 @@ void ThreadRegistry::FinishThread(u32 tid) {
   }
 }
 
-void ThreadRegistry::StartThread(u32 tid, uptr os_id, void *arg) {
+void ThreadRegistry::StartThread(u32 tid, uptr os_id, bool workerthread,
+                                 void *arg) {
   BlockingMutexLock l(&mtx_);
   running_threads_++;
   CHECK_LT(tid, n_contexts_);
   ThreadContextBase *tctx = threads_[tid];
   CHECK_NE(tctx, 0);
   CHECK_EQ(ThreadStatusCreated, tctx->status);
-  tctx->SetStarted(os_id, arg);
+  tctx->SetStarted(os_id, workerthread, arg);
 }
 
 void ThreadRegistry::QuarantinePush(ThreadContextBase *tctx) {

diff --git a/compiler-rt/lib/sanitizer_common/sanitizer_thread_registry.h b/compiler-rt/lib/sanitizer_common/sanitizer_thread_registry.h
@@ -45,6 +45,7 @@ class ThreadContextBase {
 
   ThreadStatus status;
   bool detached;
+  bool workerthread;
 
   u32 parent_tid;
   ThreadContextBase *next;  // For storing thread contexts in a list.
@@ -54,7 +55,7 @@ class ThreadContextBase {
   void SetDead();
   void SetJoined(void *arg);
   void SetFinished();
-  void SetStarted(uptr _os_id, void *arg);
+  void SetStarted(uptr _os_id, bool _workerthread, void *arg);
   void SetCreated(uptr _user_id, u64 _unique_id, bool _detached,
                   u32 _parent_tid, void *arg);
   void Reset();
@@ -115,7 +116,7 @@ class ThreadRegistry {
   void DetachThread(u32 tid, void *arg);
   void JoinThread(u32 tid, void *arg);
   void FinishThread(u32 tid);
-  void StartThread(u32 tid, uptr os_id, void *arg);
+  void StartThread(u32 tid, uptr os_id, bool workerthread, void *arg);
 
  private:
   const ThreadContextFactory context_factory_;

diff --git a/compiler-rt/lib/tsan/go/tsan_go.cc b/compiler-rt/lib/tsan/go/tsan_go.cc
@@ -214,7 +214,7 @@ void __tsan_go_start(ThreadState *parent, ThreadState **pthr, void *pc) {
   ThreadState *thr = AllocGoroutine();
   *pthr = thr;
   int goid = ThreadCreate(parent, (uptr)pc, 0, true);
-  ThreadStart(thr, goid, 0);
+  ThreadStart(thr, goid, 0, /*workerthread*/ false);
 }
 
 void __tsan_go_end(ThreadState *thr) {

diff --git a/compiler-rt/lib/tsan/rtl/tsan_interceptors.cc b/compiler-rt/lib/tsan/rtl/tsan_interceptors.cc
@@ -881,7 +881,7 @@ extern "C" void *__tsan_thread_start_func(void *arg) {
       internal_sched_yield();
     Processor *proc = ProcCreate();
     ProcWire(proc, thr);
-    ThreadStart(thr, tid, GetTid());
+    ThreadStart(thr, tid, GetTid(), /*workerthread*/ false);
     atomic_store(&p->tid, 0, memory_order_release);
   }
   void *res = callback(param);

diff --git a/compiler-rt/lib/tsan/rtl/tsan_platform_mac.cc b/compiler-rt/lib/tsan/rtl/tsan_platform_mac.cc
@@ -207,7 +207,7 @@ static void my_pthread_introspection_hook(unsigned int event, pthread_t thread,
       ThreadState *parent_thread_state = nullptr;  // No parent.
       int tid = ThreadCreate(parent_thread_state, 0, (uptr)thread, true);
       CHECK_NE(tid, 0);
-      ThreadStart(thr, tid, GetTid());
+      ThreadStart(thr, tid, GetTid(), /*workerthread*/ true);
     }
   } else if (event == PTHREAD_INTROSPECTION_THREAD_TERMINATE) {
     if (thread == pthread_self()) {

diff --git a/compiler-rt/lib/tsan/rtl/tsan_report.cc b/compiler-rt/lib/tsan/rtl/tsan_report.cc
@@ -235,9 +235,15 @@ static void PrintThread(const ReportThread *rt) {
   if (rt->name && rt->name[0] != '\0')
     Printf(" '%s'", rt->name);
   char thrbuf[kThreadBufSize];
-  Printf(" (tid=%zu, %s) created by %s",
-    rt->os_id, rt->running ? "running" : "finished",
-    thread_name(thrbuf, rt->parent_tid));
+  const char *thread_status = rt->running ? "running" : "finished";
+  if (rt->workerthread) {
+    Printf(" (tid=%zu, %s) is a GCD worker thread\n", rt->os_id, thread_status);
+    Printf("\n");
+    Printf("%s", d.EndThreadDescription());
+    return;
+  }
+  Printf(" (tid=%zu, %s) created by %s", rt->os_id, thread_status,
+         thread_name(thrbuf, rt->parent_tid));
   if (rt->stack)
     Printf(" at:");
   Printf("\n");

diff --git a/compiler-rt/lib/tsan/rtl/tsan_report.h b/compiler-rt/lib/tsan/rtl/tsan_report.h
@@ -89,8 +89,9 @@ struct ReportThread {
   int id;
   uptr os_id;
   bool running;
+  bool workerthread;
   char *name;
-  int parent_tid;
+  u32 parent_tid;
   ReportStack *stack;
 };
 

diff --git a/compiler-rt/lib/tsan/rtl/tsan_rtl.cc b/compiler-rt/lib/tsan/rtl/tsan_rtl.cc
@@ -381,7 +381,7 @@ void Initialize(ThreadState *thr) {
   // Initialize thread 0.
   int tid = ThreadCreate(thr, 0, 0, true);
   CHECK_EQ(tid, 0);
-  ThreadStart(thr, tid, GetTid());
+  ThreadStart(thr, tid, GetTid(), /*workerthread*/ false);
 #if TSAN_CONTAINS_UBSAN
   __ubsan::InitAsPlugin();
 #endif

diff --git a/compiler-rt/lib/tsan/rtl/tsan_rtl.h b/compiler-rt/lib/tsan/rtl/tsan_rtl.h
@@ -713,7 +713,7 @@ void FuncEntry(ThreadState *thr, uptr pc);
 void FuncExit(ThreadState *thr);
 
 int ThreadCreate(ThreadState *thr, uptr pc, uptr uid, bool detached);
-void ThreadStart(ThreadState *thr, int tid, uptr os_id);
+void ThreadStart(ThreadState *thr, int tid, uptr os_id, bool workerthread);
 void ThreadFinish(ThreadState *thr);
 int ThreadTid(ThreadState *thr, uptr pc, uptr uid);
 void ThreadJoin(ThreadState *thr, uptr pc, int tid);

diff --git a/compiler-rt/lib/tsan/rtl/tsan_rtl_report.cc b/compiler-rt/lib/tsan/rtl/tsan_rtl_report.cc
@@ -202,6 +202,7 @@ void ScopedReport::AddThread(const ThreadContext *tctx, bool suppressable) {
   rt->running = (tctx->status == ThreadStatusRunning);
   rt->name = internal_strdup(tctx->name);
   rt->parent_tid = tctx->parent_tid;
+  rt->workerthread = tctx->workerthread;
   rt->stack = 0;
   rt->stack = SymbolizeStackId(tctx->creation_stack_id);
   if (rt->stack)

diff --git a/compiler-rt/lib/tsan/rtl/tsan_rtl_thread.cc b/compiler-rt/lib/tsan/rtl/tsan_rtl_thread.cc
@@ -236,7 +236,7 @@ int ThreadCreate(ThreadState *thr, uptr pc, uptr uid, bool detached) {
   return tid;
 }
 
-void ThreadStart(ThreadState *thr, int tid, uptr os_id) {
+void ThreadStart(ThreadState *thr, int tid, uptr os_id, bool workerthread) {
   uptr stk_addr = 0;
   uptr stk_size = 0;
   uptr tls_addr = 0;
@@ -266,7 +266,7 @@ void ThreadStart(ThreadState *thr, int tid, uptr os_id) {
 
   ThreadRegistry *tr = ctx->thread_registry;
   OnStartedArgs args = { thr, stk_addr, stk_size, tls_addr, tls_size };
-  tr->StartThread(tid, os_id, &args);
+  tr->StartThread(tid, os_id, workerthread, &args);
 
   tr->Lock();
   thr->tctx = (ThreadContext*)tr->GetThreadLocked(tid);

diff --git a/compiler-rt/test/tsan/Darwin/workerthreads.mm b/compiler-rt/test/tsan/Darwin/workerthreads.mm
@@ -0,0 +1,43 @@
+// RUN: %clang_tsan %s -o %t -framework Foundation
+// RUN: %deflake %run %t 2>&1 | FileCheck %s
+
+#import <Foundation/Foundation.h>
+
+#import "../test.h"
+
+long global;
+
+int main() {
+  fprintf(stderr, "Hello world.\n");
+  print_address("addr=", 1, &global);
+  barrier_init(&barrier, 2);
+
+  global = 42;
+  dispatch_async(dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_DEFAULT, 0), ^{
+    global = 43;
+    barrier_wait(&barrier);
+  });
+
+  dispatch_async(dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_DEFAULT, 0), ^{
+    barrier_wait(&barrier);
+    global = 44;
+
+    dispatch_sync(dispatch_get_main_queue(), ^{
+      CFRunLoopStop(CFRunLoopGetCurrent());
+    });
+  });
+
+  CFRunLoopRun();
+  fprintf(stderr, "Done.\n");
+}
+
+// CHECK: Hello world.
+// CHECK: WARNING: ThreadSanitizer: data race
+// CHECK: Write of size 8
+// CHECK: Previous write of size 8
+// CHECK: Location is global
+// CHECK: Thread {{.*}} is a GCD worker thread
+// CHECK-NOT: failed to restore the stack
+// CHECK: Thread {{.*}} is a GCD worker thread
+// CHECK-NOT: failed to restore the stack
+// CHECK: Done.