[compiler-rt] r310432 - [asan] Refactor thread creation bookkeeping

Vitaly Buka via llvm-commits llvm-commits at lists.llvm.org
Tue Aug 8 17:38:58 PDT 2017


Author: vitalybuka
Date: Tue Aug  8 17:38:57 2017
New Revision: 310432

URL: http://llvm.org/viewvc/llvm-project?rev=310432&view=rev
Log:
[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

Modified:
    compiler-rt/trunk/lib/asan/asan_internal.h
    compiler-rt/trunk/lib/asan/asan_rtl.cc
    compiler-rt/trunk/lib/asan/asan_thread.cc
    compiler-rt/trunk/lib/asan/asan_thread.h
    compiler-rt/trunk/lib/sanitizer_common/sanitizer_thread_registry.cc

Modified: compiler-rt/trunk/lib/asan/asan_internal.h
URL: http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/lib/asan/asan_internal.h?rev=310432&r1=310431&r2=310432&view=diff
==============================================================================
--- compiler-rt/trunk/lib/asan/asan_internal.h (original)
+++ compiler-rt/trunk/lib/asan/asan_internal.h Tue Aug  8 17:38:57 2017
@@ -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.,

Modified: compiler-rt/trunk/lib/asan/asan_rtl.cc
URL: http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/lib/asan/asan_rtl.cc?rev=310432&r1=310431&r2=310432&view=diff
==============================================================================
--- compiler-rt/trunk/lib/asan/asan_rtl.cc (original)
+++ compiler-rt/trunk/lib/asan/asan_rtl.cc Tue Aug  8 17:38:57 2017
@@ -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();
 

Modified: compiler-rt/trunk/lib/asan/asan_thread.cc
URL: http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/lib/asan/asan_thread.cc?rev=310432&r1=310431&r2=310432&view=diff
==============================================================================
--- compiler-rt/trunk/lib/asan/asan_thread.cc (original)
+++ compiler-rt/trunk/lib/asan/asan_thread.cc Tue Aug  8 17:38:57 2017
@@ -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_ca
   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::AsyncSignalSafeLa
   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_),

Modified: compiler-rt/trunk/lib/asan/asan_thread.h
URL: http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/lib/asan/asan_thread.h?rev=310432&r1=310431&r2=310432&view=diff
==============================================================================
--- compiler-rt/trunk/lib/asan/asan_thread.h (original)
+++ compiler-rt/trunk/lib/asan/asan_thread.h Tue Aug  8 17:38:57 2017
@@ -49,6 +49,11 @@ class AsanThreadContext : public ThreadC
 
   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();
 

Modified: compiler-rt/trunk/lib/sanitizer_common/sanitizer_thread_registry.cc
URL: http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/lib/sanitizer_common/sanitizer_thread_registry.cc?rev=310432&r1=310431&r2=310432&view=diff
==============================================================================
--- compiler-rt/trunk/lib/sanitizer_common/sanitizer_thread_registry.cc (original)
+++ compiler-rt/trunk/lib/sanitizer_common/sanitizer_thread_registry.cc Tue Aug  8 17:38:57 2017
@@ -54,8 +54,11 @@ void ThreadContextBase::SetJoined(void *
 }
 
 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,
   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);
   }




More information about the llvm-commits mailing list