[compiler-rt][hwasan] Refactor Thread::Init

[leonardchan]

Summary

This allows for other implementations to define their own version of Thread::Init. This will be the case for Fuchsia where much of the thread initialization can be broken up between different thread hooks (__sanitizer_before_thread_create_hook, __sanitizer_thread_create_hook, __sanitizer_thread_start_hook). Namely, setting up the heap ring buffer and stack info and can be setup before thread creation. The stack ring buffer can also be setup before thread creation, but storing it into __hwasan_tls can only be done on the thread start hook since it's only then we can access __hwasan_tls for that thread correctly.

[mcgrathr]

compiler-rt/lib/hwasan/hwasan_linux.cpp

430 ↗

(On Diff #351937)

Most of this code can actually be reused for Fuchsia (just not necessarily in Thread::Init).
It's probably better to split it up for reuse rather than just moving the whole thing to linux-specific.

[leonardchan]

compiler-rt/lib/hwasan/hwasan_linux.cpp

430 ↗

(On Diff #351937)

So maybe something like the current patch? Then on Fuchsia we could (1) have a custom implementation of InitStackAndTls, (2) wrap the call to Thread::InitStackRingBuffer in Thread::Init with a #if SANITIZER_FUCHSIA so it's not called before thread creation, then (2) call Thread::InitStackRingBuffer in the __sanitizer_thread_start_hook hook.

[leonardchan]

compiler-rt/lib/hwasan/hwasan_linux.cpp

430 ↗

(On Diff #351937)

Sorry. Meant wrapping with a #if !SANITIZER_FUCHSIA.

[vitalybuka]

compiler-rt/lib/hwasan/hwasan_linux.cpp

430 ↗

(On Diff #351937)

in other sanitizers only GetThreadStackAndTls is platform specific

compiler-rt/lib/hwasan/hwasan_thread.cpp

51

clang-format: please reformat the code

please fix

[mcgrathr]

compiler-rt/lib/hwasan/hwasan_linux.cpp

430 ↗

(On Diff #351937)

in other sanitizers only GetThreadStackAndTls is platform specific

That's not entirely accurate. AsanThread::SetThreadStackAndTls might be a model here. But asan on Fuchsia also splits up the initialization into pre-creation and on-created-thread portions (AsanThread::Init is called before thread creation, and AsanThreadRegistry::StartThread is called on the created thread).

[leonardchan]

Rebased and formatted.

[vitalybuka]

compiler-rt/lib/hwasan/hwasan_linux.cpp

430 ↗

(On Diff #351937)

Patch itself is LGTM.
But I am not sure if @mcgrathr is asking for particular changes.

[leonardchan]

Will submit around end of day if there's no more comments.

compiler-rt/lib/hwasan/hwasan_thread.h

 #include "sanitizer_common/sanitizer_ring_buffer.h"
 
 namespace __hwasan {
 
 typedef __sanitizer::CompactRingBuffer<uptr> StackAllocationsRingBuffer;
 
 class Thread {
  public:
-  void Init(uptr stack_buffer_start, uptr stack_buffer_size);  // Must be called from the thread itself.
+  void Init(uptr stack_buffer_start, uptr stack_buffer_size);
   void InitRandomState();
+  void InitStackAndTls();
+
+  // Must be called from the thread itself.
+  void InitStackRingBuffer(uptr stack_buffer_start, uptr stack_buffer_size);
+
   void Destroy();
 
   uptr stack_top() { return stack_top_; }
   uptr stack_bottom() { return stack_bottom_; }
   uptr stack_size() { return stack_top() - stack_bottom(); }
   uptr tls_begin() { return tls_begin_; }
   uptr tls_end() { return tls_end_; }
   bool IsMainThread() { return unique_id_ == 0; }

compiler-rt/lib/hwasan/hwasan_thread.cpp

void Thread::Init(uptr stack_buffer_start, uptr stack_buffer_size) {
   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();
+  InitStackAndRingBuffer(stack_buffer_start, stack_buffer_size);
+}
+
+void Thread::InitStackAndTls() {
+  uptr tls_size;
+  uptr stack_size;
+  GetThreadStackAndTls(IsMainThread(), &stack_bottom_, &stack_size, &tls_begin_,
+                       &tls_size);
+  stack_top_ = stack_bottom_ + stack_size;
+  tls_end_ = tls_begin_ + tls_size;
+}
+
+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;
 
-  uptr tls_size;
-  uptr stack_size;
-  GetThreadStackAndTls(IsMainThread(), &stack_bottom_, &stack_size, &tls_begin_,
-                       &tls_size);
-  stack_top_ = stack_bottom_ + stack_size;
-  tls_end_ = tls_begin_ + tls_size;
-
   if (stack_bottom_) {
     int local;
     CHECK(AddrIsInStack((uptr)&local));
     CHECK(MemIsApp(stack_bottom_));
     CHECK(MemIsApp(stack_top_ - 1));
   }
 
   if (flags()->verbose_threads) {