[asan] Refactor thread creation bookkeeping

Summary:
This is a pure refactoring change.  It paves the way for OS-specific
implementations, such as Fuchsia's, that can do most of the
per-thread bookkeeping work in the creator thread before the new
thread actually starts.  This model is simpler and cleaner, avoiding
some race issues that the interceptor code for thread creation has
to do for the existing OS-specific implementations.

Submitted on behalf of Roland McGrath.

Reviewers: vitalybuka, alekseyshl, kcc

Reviewed By: alekseyshl

Subscribers: phosek, filcab, llvm-commits, kubamracek

Tags: #sanitizers

Differential Revision: https://reviews.llvm.org/D36385

llvm-svn: 310432

5 files changed, 53 insertions, 23 deletions

diff --git a/compiler-rt/lib/asan/asan_internal.h b/compiler-rt/lib/asan/asan_internal.h
index 3cfd75f4961..19133e5291a 100644
--- a/compiler-rt/lib/asan/asan_internal.h
+++ b/compiler-rt/lib/asan/asan_internal.h
@@ -84,6 +84,9 @@ void *AsanDoesNotSupportStaticLinkage();
 void AsanCheckDynamicRTPrereqs();
 void AsanCheckIncompatibleRT();
 
+// asan_thread.cc
+AsanThread *CreateMainThread();
+
 // Support function for __asan_(un)register_image_globals. Searches for the
 // loaded image containing `needle' and then enumerates all global metadata
 // structures declared in that image, applying `op' (e.g.,
diff --git a/compiler-rt/lib/asan/asan_rtl.cc b/compiler-rt/lib/asan/asan_rtl.cc
index e32872452b9..6e83671abb5 100644
--- a/compiler-rt/lib/asan/asan_rtl.cc
+++ b/compiler-rt/lib/asan/asan_rtl.cc
@@ -451,13 +451,8 @@ static void AsanInitInternal() {
   InitTlsSize();
 
   // Create main thread.
-  AsanThread *main_thread = AsanThread::Create(
-      /* start_routine */ nullptr, /* arg */ nullptr, /* parent_tid */ 0,
-      /* stack */ nullptr, /* detached */ true);
+  AsanThread *main_thread = CreateMainThread();
   CHECK_EQ(0, main_thread->tid());
-  SetCurrentThread(main_thread);
-  main_thread->ThreadStart(internal_getpid(),
-                           /* signal_thread_is_registered */ nullptr);
   force_interface_symbols();  // no-op.
   SanitizerInitializeUnwinder();
 
diff --git a/compiler-rt/lib/asan/asan_thread.cc b/compiler-rt/lib/asan/asan_thread.cc
index 74426f675de..c41d3ba9450 100644
--- a/compiler-rt/lib/asan/asan_thread.cc
+++ b/compiler-rt/lib/asan/asan_thread.cc
@@ -27,11 +27,6 @@ namespace __asan {
 
 // AsanThreadContext implementation.
 
-struct CreateThreadContextArgs {
-  AsanThread *thread;
-  StackTrace *stack;
-};
-
 void AsanThreadContext::OnCreated(void *arg) {
   CreateThreadContextArgs *args = static_cast<CreateThreadContextArgs*>(arg);
   if (args->stack)
@@ -88,7 +83,7 @@ AsanThread *AsanThread::Create(thread_callback_t start_routine, void *arg,
   AsanThread *thread = (AsanThread*)MmapOrDie(size, __func__);
   thread->start_routine_ = start_routine;
   thread->arg_ = arg;
-  CreateThreadContextArgs args = { thread, stack };
+  AsanThreadContext::CreateThreadContextArgs args = {thread, stack};
   asanThreadRegistry().CreateThread(*reinterpret_cast<uptr *>(thread), detached,
                                     parent_tid, &args);
 
@@ -223,12 +218,12 @@ FakeStack *AsanThread::AsyncSignalSafeLazyInitFakeStack() {
   return nullptr;
 }
 
-void AsanThread::Init() {
+void AsanThread::Init(const InitOptions *options) {
   next_stack_top_ = next_stack_bottom_ = 0;
   atomic_store(&stack_switching_, false, memory_order_release);
   fake_stack_ = nullptr;  // Will be initialized lazily if needed.
   CHECK_EQ(this->stack_size(), 0U);
-  SetThreadStackAndTls();
+  SetThreadStackAndTls(options);
   CHECK_GT(this->stack_size(), 0U);
   CHECK(AddrIsInMem(stack_bottom_));
   CHECK(AddrIsInMem(stack_top_ - 1));
@@ -274,7 +269,21 @@ thread_return_t AsanThread::ThreadStart(
   return res;
 }
 
-void AsanThread::SetThreadStackAndTls() {
+AsanThread *CreateMainThread() {
+  AsanThread *main_thread = AsanThread::Create(
+      /* start_routine */ nullptr, /* arg */ nullptr, /* parent_tid */ 0,
+      /* stack */ nullptr, /* detached */ true);
+  SetCurrentThread(main_thread);
+  main_thread->ThreadStart(internal_getpid(),
+                           /* signal_thread_is_registered */ nullptr);
+  return main_thread;
+}
+
+// This implementation doesn't use the argument, which is just passed down
+// from the caller of Init (which see, above).  It's only there to support
+// OS-specific implementations that need more information passed through.
+void AsanThread::SetThreadStackAndTls(const InitOptions *options) {
+  DCHECK_EQ(options, nullptr);
   uptr tls_size = 0;
   uptr stack_size = 0;
   GetThreadStackAndTls(tid() == 0, const_cast<uptr *>(&stack_bottom_),
diff --git a/compiler-rt/lib/asan/asan_thread.h b/compiler-rt/lib/asan/asan_thread.h
index 424f9e68dfe..d34942492bf 100644
--- a/compiler-rt/lib/asan/asan_thread.h
+++ b/compiler-rt/lib/asan/asan_thread.h
@@ -49,6 +49,11 @@ class AsanThreadContext : public ThreadContextBase {
 
   void OnCreated(void *arg) override;
   void OnFinished() override;
+
+  struct CreateThreadContextArgs {
+    AsanThread *thread;
+    StackTrace *stack;
+  };
 };
 
 // AsanThreadContext objects are never freed, so we need many of them.
@@ -62,7 +67,9 @@ class AsanThread {
   static void TSDDtor(void *tsd);
   void Destroy();
 
-  void Init();  // Should be called from the thread itself.
+  struct InitOptions;
+  void Init(const InitOptions *options = nullptr);
+
   thread_return_t ThreadStart(tid_t os_id,
                               atomic_uintptr_t *signal_thread_is_registered);
 
@@ -128,7 +135,9 @@ class AsanThread {
  private:
   // NOTE: There is no AsanThread constructor. It is allocated
   // via mmap() and *must* be valid in zero-initialized state.
-  void SetThreadStackAndTls();
+
+  void SetThreadStackAndTls(const InitOptions *options);
+
   void ClearShadowForThreadStackAndTLS();
   FakeStack *AsyncSignalSafeLazyInitFakeStack();
 
diff --git a/compiler-rt/lib/sanitizer_common/sanitizer_thread_registry.cc b/compiler-rt/lib/sanitizer_common/sanitizer_thread_registry.cc
index 84c55fc811b..79f3caad21e 100644
--- a/compiler-rt/lib/sanitizer_common/sanitizer_thread_registry.cc
+++ b/compiler-rt/lib/sanitizer_common/sanitizer_thread_registry.cc
@@ -54,8 +54,11 @@ void ThreadContextBase::SetJoined(void *arg) {
 }
 
 void ThreadContextBase::SetFinished() {
-  if (!detached)
-    status = ThreadStatusFinished;
+  // ThreadRegistry::FinishThread calls here in ThreadStatusCreated state
+  // for a thread that never actually started.  In that case the thread
+  // should go to ThreadStatusFinished regardless of whether it was created
+  // as detached.
+  if (!detached || status == ThreadStatusCreated) status = ThreadStatusFinished;
   OnFinished();
 }
 
@@ -252,18 +255,29 @@ void ThreadRegistry::JoinThread(u32 tid, void *arg) {
   QuarantinePush(tctx);
 }
 
+// Normally this is called when the thread is about to exit.  If
+// called in ThreadStatusCreated state, then this thread was never
+// really started.  We just did CreateThread for a prospective new
+// thread before trying to create it, and then failed to actually
+// create it, and so never called StartThread.
 void ThreadRegistry::FinishThread(u32 tid) {
   BlockingMutexLock l(&mtx_);
   CHECK_GT(alive_threads_, 0);
   alive_threads_--;
-  CHECK_GT(running_threads_, 0);
-  running_threads_--;
   CHECK_LT(tid, n_contexts_);
   ThreadContextBase *tctx = threads_[tid];
   CHECK_NE(tctx, 0);
-  CHECK_EQ(ThreadStatusRunning, tctx->status);
+  bool dead = tctx->detached;
+  if (tctx->status == ThreadStatusRunning) {
+    CHECK_GT(running_threads_, 0);
+    running_threads_--;
+  } else {
+    // The thread never really existed.
+    CHECK_EQ(tctx->status, ThreadStatusCreated);
+    dead = true;
+  }
   tctx->SetFinished();
-  if (tctx->detached) {
+  if (dead) {
     tctx->SetDead();
     QuarantinePush(tctx);
   }