[tsan] Properly describe GCD worker threads in reports

[kubamracek]

Summary

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:

==================
WARNING: ThreadSanitizer: data race (pid=18771)
  Write of size 8 at 0x00010ac221a0 by thread T3:
    ...

  Previous write of size 8 at 0x00010ac221a0 by thread T1:
    ...

  Location is global 'global' at 0x00010ac221a0 (workerthreads.mm.tmp+0x0001000021a0)

  Thread T3 (tid=5981759, running) created by thread T-1
    [failed to restore the stack]

  Thread T1 (tid=5981757, running) created by thread T-1
    [failed to restore the stack]

SUMMARY: ThreadSanitizer: data race workerthreads.mm:23 in __main_block_invoke_2
==================

This patch tries to avoid that and instead print:

==================
WARNING: ThreadSanitizer: data race (pid=18843)
  Write of size 8 at 0x0001044431a0 by thread T3:
    ...

  Previous write of size 8 at 0x0001044431a0 by thread T1:
    ...

  Location is global 'global' at 0x0001044431a0 (workerthreads.mm.tmp+0x0001000021a0)

  Thread T3 (tid=5989400, running) is a GCD worker thread

  Thread T1 (tid=5989398, running) is a GCD worker thread

SUMMARY: ThreadSanitizer: data race workerthreads.mm:23 in __main_block_invoke_2
==================

[dvyukov]

lib/tsan/rtl/tsan_report.cc

239

This feels like an abuse of kInvalidTid.
Currently we just say that we don't know who is the parent of GCD threads on creation. Which looks legit. But now you are saying: if we don't know a parent of any thread, then it must be a GCD worker thread.

Please mark GCD worker threads more explicitly (either by introducing another special tid, or by marking them with a new flag).
tsan_report.cc is meant to be as dump as possible and just format what it receives. So preferably this should also be a new flag in ReportThread.

[kubamracek]

Updating the patch to introduce a "workerthread" field in ThreadContextBase.

[dvyukov]

Thanks!
Now this also improves asan.

lib/asan/asan_mac.cc

 
 ALWAYS_INLINE
 void asan_register_worker_thread(int parent_tid, StackTrace *stack) {
   AsanThread *t = GetCurrentThread();
   if (!t) {
     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);
   }
 }
 
 // For use by only those functions that allocated the context via
 // alloc_asan_context().
 extern "C"
 void asan_dispatch_call_block_and_release(void *block) {

lib/asan/asan_thread.cc

void AsanThread::Init() {
   VReport(1, "T%d: stack [%p,%p) size 0x%zx; local=%p\n", tid(),
           (void *)stack_bottom_, (void *)stack_top_, stack_top_ - stack_bottom_,
           &local);
 }
 
 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);
 
   if (common_flags()->use_sigaltstack) SetAlternateSignalStack();
 
   if (!start_routine_) {
     // start_routine_ == 0 if we're on the main thread or on one of the
     // OS X libdispatch worker threads. But nobody is supposed to call

lib/lsan/lsan_thread.cc

void ThreadStart(u32 tid, uptr os_id) {
   uptr stack_size = 0;
   uptr tls_size = 0;
   GetThreadStackAndTls(tid == 0, &args.stack_begin, &stack_size,
                        &args.tls_begin, &tls_size);
   args.stack_end = args.stack_begin + stack_size;
   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() {
   thread_registry->FinishThread(GetCurrentThread());
 }
 
 ThreadContext *CurrentThreadContext() {
   if (!thread_registry) return nullptr;

lib/sanitizer_common/sanitizer_thread_registry.h

public:
   u64 unique_id;  // Unique thread ID.
   u32 reuse_count;  // Number of times this tid was reused.
   uptr os_id;     // PID (used for reporting).
   uptr user_id;   // Some opaque user thread id (e.g. pthread_t).
   char name[64];  // As annotated by user.
 
   ThreadStatus status;
   bool detached;
+  bool workerthread;
 
   u32 parent_tid;
   ThreadContextBase *next;  // For storing thread contexts in a list.
 
   void SetName(const char *new_name);
 
   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();
 
   // The following methods may be overriden by subclasses.
   // Some of them take opaque arg that may be optionally be used
   // by subclasses.
   virtual void OnDead() {}
ThreadContextBase *FindThreadContextLocked(FindThreadCallback cb,
                                              void *arg);
   ThreadContextBase *FindThreadContextByOsIDLocked(uptr os_id);
 
   void SetThreadName(u32 tid, const char *name);
   void SetThreadNameByUserId(uptr user_id, const char *name);
   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_;
   const u32 max_threads_;
   const u32 thread_quarantine_size_;
   const u32 max_reuse_;
 
   BlockingMutex mtx_;

lib/sanitizer_common/sanitizer_thread_registry.cc

 
 #include "sanitizer_thread_registry.h"
 
 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';
 }
 
 ThreadContextBase::~ThreadContextBase() {
   // ThreadContextBase should never be deleted.
   CHECK(0);
 }
 

 }
 
 void ThreadContextBase::SetFinished() {
   if (!detached)
     status = ThreadStatusFinished;
   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);
 }
 
 void ThreadContextBase::SetCreated(uptr _user_id, u64 _unique_id,
                                    bool _detached, u32 _parent_tid, void *arg) {
   status = ThreadStatusCreated;
   user_id = _user_id;
   unique_id = _unique_id;
void ThreadRegistry::FinishThread(u32 tid) {
   CHECK_EQ(ThreadStatusRunning, tctx->status);
   tctx->SetFinished();
   if (tctx->detached) {
     tctx->SetDead();
     QuarantinePush(tctx);
   }
 }
 
-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) {
   if (tctx->tid == 0)
     return;  // Don't reuse the main thread.  It's a special snowflake.
   dead_threads_.push_back(tctx);
   if (dead_threads_.size() <= thread_quarantine_size_)
     return;

lib/tsan/go/tsan_go.cc

 void __tsan_free(uptr p, uptr sz) {
   ctx->metamap.FreeRange(get_cur_proc(), p, sz);
 }
 
 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) {
   ThreadFinish(thr);
   internal_free(thr);
 }
 
 void __tsan_proc_create(Processor **pproc) {

lib/tsan/rtl/tsan_interceptors.cc

if (pthread_setspecific(g_thread_finalize_key,
       Die();
     }
     ThreadIgnoreEnd(thr, 0);
 #endif
     while ((tid = atomic_load(&p->tid, memory_order_acquire)) == 0)
       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);
   // Prevent the callback from being tail called,
   // it mixes up stack traces.
   volatile int foo = 42;
   foo++;
   return res;

lib/tsan/rtl/tsan_platform_mac.cc

if (event == PTHREAD_INTROSPECTION_THREAD_CREATE) {
     if (thread == pthread_self()) {
       // The current thread is a newly created GCD worker thread.
       ThreadState *thr = cur_thread();
       Processor *proc = ProcCreate();
       ProcWire(proc, thr);
       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()) {
       ThreadState *thr = cur_thread();
       if (thr->tctx) {
         DestroyThreadState();
       }
     }

lib/tsan/rtl/tsan_report.h

  private:
   explicit ReportLocation(ReportLocationType type);
 };
 
 struct ReportThread {
   int id;
   uptr os_id;
   bool running;
+  bool workerthread;
   char *name;
-  int parent_tid;
+  u32 parent_tid;
   ReportStack *stack;
 };
 
 struct ReportMutex {
   u64 id;
   uptr addr;
   bool destroyed;
   ReportStack *stack;

lib/tsan/rtl/tsan_report.cc

static void PrintThread(const ReportThread *rt) {
   Decorator d;
   if (rt->id == 0)  // Little sense in describing the main thread.
     return;
   Printf("%s", d.ThreadDescription());
   Printf("  Thread T%d", rt->id);
   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",
+  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");
   Printf("%s", d.EndThreadDescription());
   PrintStack(rt->stack);
 }
 
 static void PrintSleep(const ReportStack *s) {

lib/tsan/rtl/tsan_rtl.h

 void ThreadIgnoreEnd(ThreadState *thr, uptr pc);
 void ThreadIgnoreSyncBegin(ThreadState *thr, uptr pc);
 void ThreadIgnoreSyncEnd(ThreadState *thr, uptr pc);
 
 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);
 void ThreadDetach(ThreadState *thr, uptr pc, int tid);
 void ThreadFinalize(ThreadState *thr);
 void ThreadSetName(ThreadState *thr, const char *name);
 int ThreadCount(ThreadState *thr);
 void ProcessPendingSignals(ThreadState *thr);

lib/tsan/rtl/tsan_rtl.cc

 #endif
 
   VPrintf(1, "***** Running under ThreadSanitizer v2 (pid %d) *****\n",
           (int)internal_getpid());
 
   // 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
   ctx->initialized = true;
 
 #if !SANITIZER_GO
   Symbolizer::LateInitialize();
 #endif

lib/tsan/rtl/tsan_rtl_report.cc

void ScopedReport::AddThread(const ThreadContext *tctx, bool suppressable) {
   void *mem = internal_alloc(MBlockReportThread, sizeof(ReportThread));
   ReportThread *rt = new(mem) ReportThread;
   rep_->threads.PushBack(rt);
   rt->id = tctx->tid;
   rt->os_id = tctx->os_id;
   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)
     rt->stack->suppressable = suppressable;
 }
 
 #if !SANITIZER_GO
 static bool FindThreadByUidLockedCallback(ThreadContextBase *tctx, void *arg) {

lib/tsan/rtl/tsan_rtl_thread.cc

int ThreadCreate(ThreadState *thr, uptr pc, uptr uid, bool detached) {
   u32 parent_tid = thr ? thr->tid : kInvalidTid;  // No parent for GCD workers.
   int tid =
       ctx->thread_registry->CreateThread(uid, detached, parent_tid, &args);
   DPrintf("#%d: ThreadCreate tid=%d uid=%zu\n", parent_tid, tid, uid);
   StatSet(thr, StatThreadMaxAlive, ctx->thread_registry->GetMaxAliveThreads());
   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;
   uptr tls_size = 0;
 #if !SANITIZER_GO
   GetThreadStackAndTls(tid == 0, &stk_addr, &stk_size, &tls_addr, &tls_size);
 
   if (tid) {
if (tls_addr && tls_size) {
       MemoryRangeImitateWrite(thr, /*pc=*/ 2,
           thr_end, tls_addr + tls_size - thr_end);
     }
   }
 #endif
 
   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);
   tr->Unlock();
 
 #if !SANITIZER_GO
   if (ctx->after_multithreaded_fork) {
     thr->ignore_interceptors++;

test/tsan/Darwin/workerthreads.mm

+// 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.