[compiler-rt][hwasan] Setup hwasan thread handling on Fuchsia

[leonardchan]

Summary

This patch splits up hwasan thread creation between __sanitizer_before_thread_create_hook, __sanitizer_thread_create_hook, and __sanitizer_thread_start_hook. The linux implementation creates the hwasan thread object inside the new thread. On Fuchsia, we know the stack bounds before thread creation, so we can initialize part of the thread object in __sanitizer_before_thread_create_hook, then initialize the stack ring buffer in __sanitizer_thread_start_hook once we enter the thread.

[leonardchan]

The non-fuchsia stuff can be split up in a separate patch when ready, but I'll keep it here for now until we're set on the thread handling.

[mcgrathr]

Usually we like to split changes up into separate small changes for the pure refactoring, and then changes that purely add new Fuchsia-specific code.
I'll do an initial review round of the Fuchsia code here since you've sent it. But then I think this review should be tightened to just the pure refactoring, and we should have a separate review thread for the Fuchsia parts.

compiler-rt/lib/hwasan/hwasan_fuchsia.cpp

27

Specifically call out here that the compiler generates direct TLS references to this and that's why it has a public C ABI name.

For Fuchsia the hwasan will always be in a DSO that is a dependency of libc.so and so always loaded at startup. Hence we can use [[gnu::tls_model("initial-exec")]] here.

33–34

department of redundancy department

35

Nit: I'd call it something more like InitState.

42

What depends on this alignment? I'm not aware of anything that does in the compiler ABI we're using on Fuchsia.
If it's needed, it should have comments.

85–89

Actually we could do all of the allocation and setup except for storing the uptr in __hwasan_tls.
But that would take more refactoring of the common code and it's not clear there's a particular benefit.

138

Most of what's happening in this function is common with the Linux code.
I think a shared function can be factored out and put into hwasan_thread.cpp.

compiler-rt/lib/hwasan/hwasan_thread.cpp

43–46

This is stuff that could be done either in the before-create hook or in the on-start hook. But it's probably not especially worthwhile to move it to before-create as long as we have on-start doing any allocation at all. So to avoid a whole lot more refactoring to move the ringbuffer setup and such, might as well just leave this in on-start too.

64–65

This is probably the only part that actually needs to be different between Linux and Fuchsia.
So this code could just move into an InitStackAndTls(options) that looks like this on Linux and on Fuchsia is just copying values out of options.

compiler-rt/lib/hwasan/hwasan_thread.h

85 ↗

(On Diff #351313)

I think these need to be left uniformly uninitialized to guarantee they can be "linker-initialized".
At any rate, there's no reason one pointer member here should be initialized and the others not.

[leonardchan]

Rebased against recent refactoring commits and addressed some comments.

I think I can omit some functions like GetCurrentThread or GetRingBufferSize which are small enough to merit their own changes .

[leonardchan]

compiler-rt/lib/hwasan/hwasan_fuchsia.cpp

42

Added.

138

Updated such that this just calls InitStackRingBuffer which handles the stack ring buffer initialization.

compiler-rt/lib/hwasan/hwasan_thread.cpp

43–46

With the refactoring from D104248, this bit will be done in the before-create hook without any changes to this code.

64–65

This is done in D104248.

[leonardchan]

Ok. Ready for reviews.

[leonardchan]

Let me know if this should be split up also. I think this might be small and and simple enough for one patch.

[mcgrathr]

lgtm

compiler-rt/lib/hwasan/CMakeLists.txt

 include_directories(..)
 
 # Runtime library sources and build flags.
 set(HWASAN_RTL_SOURCES
   hwasan.cpp
   hwasan_allocator.cpp
   hwasan_allocation_functions.cpp
   hwasan_dynamic_shadow.cpp
   hwasan_exceptions.cpp
+  hwasan_fuchsia.cpp
   hwasan_globals.cpp
   hwasan_interceptors.cpp
   hwasan_interceptors_vfork.S
   hwasan_linux.cpp
   hwasan_memintrinsics.cpp
   hwasan_poisoning.cpp
   hwasan_report.cpp
   hwasan_setjmp.S

compiler-rt/lib/hwasan/hwasan_fuchsia.cpp

+//===-- hwasan_fuchsia.cpp --------------------------------------*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+///
+/// \file
+/// This file is a part of HWAddressSanitizer and contains Fuchsia-specific
+/// code.
+///
+//===----------------------------------------------------------------------===//
+
+#include "sanitizer_common/sanitizer_fuchsia.h"
+#if SANITIZER_FUCHSIA
+
+#include "hwasan.h"
+#include "hwasan_interface_internal.h"
+#include "hwasan_report.h"
+#include "hwasan_thread.h"
+#include "hwasan_thread_list.h"
+
+// This TLS variable contains the location of the stack ring buffer and can be
+// used to always find the hwasan thread object associated with the current
+// running thread.
+[[gnu::tls_model("initial-exec")]]
+SANITIZER_INTERFACE_ATTRIBUTE
+THREADLOCAL uptr __hwasan_tls;
+
+namespace __hwasan {
+
+// These are known parameters passed to the hwasan runtime on thread creation.
+struct Thread::InitState {
+  uptr stack_bottom, stack_top;
+};
+
+static void FinishThreadInitialization(Thread *thread);
+
+void InitThreads() {
+  // This is the minimal alignment needed for the storage where hwasan threads
+  // and their stack ring buffers are placed. This alignment is necessary so the
+  // stack ring buffer can perform a simple calculation to get the next element
+  // in the RB. The instructions for this calculation are emitted by the
+  // compiler. (Full explanation in hwasan_thread_list.h.)
+  uptr alloc_size = UINT64_C(1) << kShadowBaseAlignment;
+  uptr thread_start = reinterpret_cast<uptr>(
+      MmapAlignedOrDieOnFatalError(alloc_size, alloc_size, __func__));
+
+  InitThreadList(thread_start, alloc_size);
+
+  // Create the hwasan thread object for the current (main) thread. Stack info
+  // for this thread is known from information passed via
+  // __sanitizer_startup_hook.
+  const Thread::InitState state = {
+      .stack_bottom = __sanitizer::MainThreadStackBase,
+      .stack_top =
+          __sanitizer::MainThreadStackBase + __sanitizer::MainThreadStackSize,
+  };
+  FinishThreadInitialization(hwasanThreadList().CreateCurrentThread(&state));
+}
+
+uptr *GetCurrentThreadLongPtr() { return &__hwasan_tls; }
+
+// This is called from the parent thread before the new thread is created. Here
+// we can propagate known info like the stack bounds to Thread::Init before
+// jumping into the thread. We cannot initialize the stack ring buffer yet since
+// we have not entered the new thread.
+static void *BeforeThreadCreateHook(uptr user_id, bool detached,
+                                    const char *name, uptr stack_bottom,
+                                    uptr stack_size) {
+  const Thread::InitState state = {
+      .stack_bottom = stack_bottom,
+      .stack_top = stack_bottom + stack_size,
+  };
+  return hwasanThreadList().CreateCurrentThread(&state);
+}
+
+// This sets the stack top and bottom according to the InitState passed to
+// CreateCurrentThread above.
+void Thread::InitStackAndTls(const InitState *state) {
+  CHECK_NE(state->stack_bottom, 0);
+  CHECK_NE(state->stack_top, 0);
+  stack_bottom_ = state->stack_bottom;
+  stack_top_ = state->stack_top;
+  tls_end_ = tls_begin_ = 0;
+}
+
+// This is called after creating a new thread with the pointer returned by
+// BeforeThreadCreateHook. We are still in the creating thread and should check
+// if it was actually created correctly.
+static void ThreadCreateHook(void *hook, bool aborted) {
+  Thread *thread = static_cast<Thread *>(hook);
+  if (!aborted) {
+    // The thread was created successfully.
+    // ThreadStartHook can already be running in the new thread.
+  } else {
+    // The thread wasn't created after all.
+    // Clean up everything we set up in BeforeThreadCreateHook.
+    atomic_signal_fence(memory_order_seq_cst);
+    hwasanThreadList().ReleaseThread(thread);
+  }
+}
+
+// This is called in the newly-created thread before it runs anything else,
+// with the pointer returned by BeforeThreadCreateHook (above). Here we can
+// setup the stack ring buffer.
+static void ThreadStartHook(void *hook, thrd_t self) {
+  Thread *thread = static_cast<Thread *>(hook);
+  FinishThreadInitialization(thread);
+  thread->InitRandomState();
+}
+
+// This is the function that sets up the stack ring buffer and enables us to use
+// GetCurrentThread. This function should only be called while IN the thread
+// that we want to create the hwasan thread object for so __hwasan_tls can be
+// properly referenced.
+static void FinishThreadInitialization(Thread *thread) {
+  CHECK_NE(thread, nullptr);
+
+  // The ring buffer is located immediately before the thread object.
+  uptr stack_buffer_size = hwasanThreadList().GetRingBufferSize();
+  uptr stack_buffer_start = reinterpret_cast<uptr>(thread) - stack_buffer_size;
+  thread->InitStackRingBuffer(stack_buffer_start, stack_buffer_size);
+}
+
+static void ThreadExitHook(void *hook, thrd_t self) {
+  Thread *thread = static_cast<Thread *>(hook);
+  atomic_signal_fence(memory_order_seq_cst);
+  hwasanThreadList().ReleaseThread(thread);
+}
+
+}  // namespace __hwasan
+
+extern "C" {
+
+void *__sanitizer_before_thread_create_hook(thrd_t thread, bool detached,
+                                            const char *name, void *stack_base,
+                                            size_t stack_size) {
+  return __hwasan::BeforeThreadCreateHook(
+      reinterpret_cast<uptr>(thread), detached, name,
+      reinterpret_cast<uptr>(stack_base), stack_size);
+}
+
+void __sanitizer_thread_create_hook(void *hook, thrd_t thread, int error) {
+  __hwasan::ThreadCreateHook(hook, error != thrd_success);
+}
+
+void __sanitizer_thread_start_hook(void *hook, thrd_t self) {
+  __hwasan::ThreadStartHook(hook, reinterpret_cast<uptr>(self));
+}
+
+void __sanitizer_thread_exit_hook(void *hook, thrd_t self) {
+  __hwasan::ThreadExitHook(hook, self);
+}
+
+}  // extern "C"
+
+#endif  // SANITIZER_FUCHSIA

compiler-rt/lib/hwasan/hwasan_thread.cpp

 }
 
 void Thread::Init(uptr stack_buffer_start, uptr stack_buffer_size,
                   const InitState *state) {
   CHECK_EQ(0, unique_id_);  // try to catch bad stack reuse
   CHECK_EQ(0, stack_top_);
   CHECK_EQ(0, stack_bottom_);
 
   static u64 unique_id;
   unique_id_ = unique_id++;
   if (auto sz = flags()->heap_history_size)
     heap_allocations_ = HeapAllocationsRingBuffer::New(sz);
 
   InitStackAndTls(state);
+#if !SANITIZER_FUCHSIA
+  // Do not initialize the stack ring buffer just yet on Fuchsia. Threads will
+  // be initialized before we enter the thread itself, so we will instead call
+  // this later.
   InitStackRingBuffer(stack_buffer_start, stack_buffer_size);
+#endif
 }
 
 void Thread::InitStackRingBuffer(uptr stack_buffer_start,
                                  uptr stack_buffer_size) {
   HwasanTSDThreadInit();  // Only needed with interceptors.
   uptr *ThreadLong = GetCurrentThreadLongPtr();
   // The following implicitly sets (this) as the current thread.
   stack_allocations_ = new (ThreadLong)
       StackAllocationsRingBuffer((void *)stack_buffer_start, stack_buffer_size);
   // Check that it worked.
   CHECK_EQ(GetCurrentThread(), this);
 
   // ScopedTaggingDisable needs GetCurrentThread to be set up.
   ScopedTaggingDisabler disabler;
 
   if (stack_bottom_) {
     int local;
     CHECK(AddrIsInStack((uptr)&local));
     CHECK(MemIsApp(stack_bottom_));
     CHECK(MemIsApp(stack_top_ - 1));
   }
 
   if (flags()->verbose_threads) {
     if (IsMainThread()) {