[PATCH] D104085: [compiler-rt][hwasan] Setup hwasan thread handling on Fuchsia

Leonard Chan via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Wed Jun 23 16:04:24 PDT 2021


leonardchan planned changes to this revision.
leonardchan added inline comments.


================
Comment at: compiler-rt/lib/hwasan/hwasan_fuchsia.cpp:41
+void InitThreads() {
+  uptr alloc_size = UINT64_C(1) << kShadowBaseAlignment;
+  uptr thread_start = reinterpret_cast<uptr>(
----------------
mcgrathr wrote:
> 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.
> 
Added.


================
Comment at: compiler-rt/lib/hwasan/hwasan_fuchsia.cpp:137
+// __hwasan_tls can be referenced.
+static void FinishThreadInitialization(Thread *thread) {
+  CHECK_NE(thread, nullptr);
----------------
mcgrathr wrote:
> 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.
> 
Updated such that this just calls `InitStackRingBuffer` which handles the stack ring buffer initialization.


================
Comment at: compiler-rt/lib/hwasan/hwasan_thread.cpp:42-45
-  static u64 unique_id;
-  unique_id_ = unique_id++;
-  if (auto sz = flags()->heap_history_size)
-    heap_allocations_ = HeapAllocationsRingBuffer::New(sz);
----------------
mcgrathr wrote:
> 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.
With the refactoring from D104248, this bit will be done in the before-create hook without any changes to this code.


================
Comment at: compiler-rt/lib/hwasan/hwasan_thread.cpp:58-63
-  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;
----------------
mcgrathr wrote:
> 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. 
> 
This is done in D104248.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D104085/new/

https://reviews.llvm.org/D104085



More information about the cfe-commits mailing list