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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
