[asan][fuchsia] set current thread before reading thread state

[zarvox]

Summary

When enabling stack use-after-free detection, we discovered that we read
the thread ID on the main thread while it is still set to 2^24-1.

This patch moves our call to AsanThread::Init() out of CreateAsanThread,
so that we can call SetCurrentThread first on the main thread.

[mcgrathr]

Thanks for the fix!

compiler-rt/lib/asan/asan_fuchsia.cpp

135

Since that's the invariant and elsewhere there's the invariant that the main thread has tid 0, how about DCHECK_EQ(t->tid(), 0); here? There could also be a DCHECK_NE(tid(), ThreadRegistry::kUnknownTid); in ASanThread::Init, which would have caught this bug.

[zarvox]

Added DCHECKs per mcgrathr's feedback.

[zarvox]

compiler-rt/lib/asan/asan_fuchsia.cpp

135

Yeah, both sound reasonable.

compiler-rt/lib/asan/asan_fuchsia.cpp

 
 struct AsanThread::InitOptions {
   uptr stack_bottom, stack_size;
 };
 
 // Shared setup between thread creation and startup for the initial thread.
 static AsanThread *CreateAsanThread(StackTrace *stack, u32 parent_tid,
                                     uptr user_id, bool detached,
-                                    const char *name, uptr stack_bottom,
-                                    uptr stack_size) {
+                                    const char *name) {
   // In lieu of AsanThread::Create.
   AsanThread *thread = (AsanThread *)MmapOrDie(AsanThreadMmapSize(), __func__);
 
   AsanThreadContext::CreateThreadContextArgs args = {thread, stack};
   u32 tid =
       asanThreadRegistry().CreateThread(user_id, detached, parent_tid, &args);
   asanThreadRegistry().SetThreadName(tid, name);
 
-  // On other systems, AsanThread::Init() is called from the new
-  // thread itself.  But on Fuchsia we already know the stack address
-  // range beforehand, so we can do most of the setup right now.
-  const AsanThread::InitOptions options = {stack_bottom, stack_size};
-  thread->Init(&options);
-
   return thread;
 }
 
 // This gets the same arguments passed to Init by CreateAsanThread, above.
 // We're in the creator thread before the new thread is actually started,
 // but its stack address range is already known.  We don't bother tracking
 // the static TLS address range because the system itself already uses an
 // ASan-aware allocator for that.
AsanThread *CreateMainThread() {
   char name[ZX_MAX_NAME_LEN];
   CHECK_NE(__sanitizer::MainThreadStackBase, 0);
   CHECK_GT(__sanitizer::MainThreadStackSize, 0);
   AsanThread *t = CreateAsanThread(
       nullptr, 0, reinterpret_cast<uptr>(self), true,
       _zx_object_get_property(thrd_get_zx_handle(self), ZX_PROP_NAME, name,
                               sizeof(name)) == ZX_OK
           ? name
-          : nullptr,
-      __sanitizer::MainThreadStackBase, __sanitizer::MainThreadStackSize);
+          : nullptr);
+  // We need to set the current thread before calling AsanThread::Init() below,
+  // since it reads the thread ID.
   SetCurrentThread(t);
+  DCHECK_EQ(t->tid(), 0);
+
+  const AsanThread::InitOptions options = {__sanitizer::MainThreadStackBase,
+                                           __sanitizer::MainThreadStackSize};
+  t->Init(&options);
+
   return t;
 }
 
 // This is called before each thread creation is attempted.  So, in
 // its first call, the calling thread is the initial and sole thread.
 static void *BeforeThreadCreateHook(uptr user_id, bool detached,
                                     const char *name, uptr stack_bottom,
                                     uptr stack_size) {
   EnsureMainThreadIDIsCorrect();
   // Strict init-order checking is thread-hostile.
   if (flags()->strict_init_order) StopInitOrderChecking();
 
   GET_STACK_TRACE_THREAD;
   u32 parent_tid = GetCurrentTidOrInvalid();
 
-  return CreateAsanThread(&stack, parent_tid, user_id, detached, name,
-                          stack_bottom, stack_size);
+  AsanThread *thread =
+      CreateAsanThread(&stack, parent_tid, user_id, detached, name);
+
+  // On other systems, AsanThread::Init() is called from the new
+  // thread itself.  But on Fuchsia we already know the stack address
+  // range beforehand, so we can do most of the setup right now.
+  const AsanThread::InitOptions options = {stack_bottom, stack_size};
+  thread->Init(&options);
+  return thread;
 }
 
 // This is called after creating a new thread (in the creating thread),
 // with the pointer returned by BeforeThreadCreateHook (above).
 static void ThreadCreateHook(void *hook, bool aborted) {
   AsanThread *thread = static_cast<AsanThread *>(hook);
   if (!aborted) {
     // The thread was created successfully.

compiler-rt/lib/asan/asan_thread.cpp

if (atomic_compare_exchange_strong(
     fake_stack_ = FakeStack::Create(stack_size_log);
     SetTLSFakeStack(fake_stack_);
     return fake_stack_;
   }
   return nullptr;
 }
 
 void AsanThread::Init(const InitOptions *options) {
+  DCHECK_NE(tid(), ThreadRegistry::kUnknownTid);
   next_stack_top_ = next_stack_bottom_ = 0;
   atomic_store(&stack_switching_, false, memory_order_release);
   CHECK_EQ(this->stack_size(), 0U);
   SetThreadStackAndTls(options);
   if (stack_top_ != stack_bottom_) {
     CHECK_GT(this->stack_size(), 0U);
     CHECK(AddrIsInMem(stack_bottom_));
     CHECK(AddrIsInMem(stack_top_ - 1));