From 29480c6c746cd3eda90b7060233bdc04a3f2e3dd Mon Sep 17 00:00:00 2001 From: Drew Fisher Date: Sat, 24 Oct 2020 12:26:40 -0700 Subject: [PATCH] [asan][fuchsia] set current thread before reading thread state 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. Reviewed By: mcgrathr Differential Revision: https://reviews.llvm.org/D89606 --- compiler-rt/lib/asan/asan_fuchsia.cpp | 31 +++++++++++++++++++------------ compiler-rt/lib/asan/asan_thread.cpp | 1 + 2 files changed, 20 insertions(+), 12 deletions(-) diff --git a/compiler-rt/lib/asan/asan_fuchsia.cpp b/compiler-rt/lib/asan/asan_fuchsia.cpp index ec15abf..6c61344 100644 --- a/compiler-rt/lib/asan/asan_fuchsia.cpp +++ b/compiler-rt/lib/asan/asan_fuchsia.cpp @@ -91,8 +91,7 @@ struct AsanThread::InitOptions { // 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__); @@ -101,12 +100,6 @@ static AsanThread *CreateAsanThread(StackTrace *stack, u32 parent_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; } @@ -135,9 +128,16 @@ AsanThread *CreateMainThread() { _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; } @@ -153,8 +153,15 @@ static void *BeforeThreadCreateHook(uptr user_id, bool detached, 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), diff --git a/compiler-rt/lib/asan/asan_thread.cpp b/compiler-rt/lib/asan/asan_thread.cpp index 58cdc29..68138c2 100644 --- a/compiler-rt/lib/asan/asan_thread.cpp +++ b/compiler-rt/lib/asan/asan_thread.cpp @@ -218,6 +218,7 @@ FakeStack *AsanThread::AsyncSignalSafeLazyInitFakeStack() { } 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); -- 2.7.4