[PATCH] D104085: [compiler-rt][hwasan] Setup hwasan thread handling on Fuchsia
This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG966386514bec: [compiler-rt][hwasan] Setup hwasan thread
handling on Fuchsia (authored by leonardchan).
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D104085/new/
https://reviews.llvm.org/D104085
Files:
compiler-rt/lib/hwasan/CMakeLists.txt
compiler-rt/lib/hwasan/hwasan_fuchsia.cpp
compiler-rt/lib/hwasan/hwasan_thread.cpp
Index: compiler-rt/lib/hwasan/hwasan_thread.cpp
===
--- compiler-rt/lib/hwasan/hwasan_thread.cpp
+++ compiler-rt/lib/hwasan/hwasan_thread.cpp
@@ -46,7 +46,12 @@
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,
Index: compiler-rt/lib/hwasan/hwasan_fuchsia.cpp
===
--- /dev/null
+++ compiler-rt/lib/hwasan/hwasan_fuchsia.cpp
@@ -0,0 +1,159 @@
+//===-- 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(
+ 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());
+}
+
+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();
+}
+
+// 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
[PATCH] D104085: [compiler-rt][hwasan] Setup hwasan thread handling on Fuchsia
mcgrathr accepted this revision. mcgrathr added a comment. This revision is now accepted and ready to land. lgtm 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
[PATCH] D104085: [compiler-rt][hwasan] Setup hwasan thread handling on Fuchsia
leonardchan updated this revision to Diff 355637.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D104085/new/
https://reviews.llvm.org/D104085
Files:
compiler-rt/lib/hwasan/CMakeLists.txt
compiler-rt/lib/hwasan/hwasan_fuchsia.cpp
compiler-rt/lib/hwasan/hwasan_thread.cpp
Index: compiler-rt/lib/hwasan/hwasan_thread.cpp
===
--- compiler-rt/lib/hwasan/hwasan_thread.cpp
+++ compiler-rt/lib/hwasan/hwasan_thread.cpp
@@ -46,7 +46,12 @@
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,
Index: compiler-rt/lib/hwasan/hwasan_fuchsia.cpp
===
--- /dev/null
+++ compiler-rt/lib/hwasan/hwasan_fuchsia.cpp
@@ -0,0 +1,159 @@
+//===-- 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(
+ 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());
+}
+
+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();
+}
+
+// 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(hook);
+ if (!aborted) {
+// The thread was created successfully.
+// ThreadStartHook can
[PATCH] D104085: [compiler-rt][hwasan] Setup hwasan thread handling on Fuchsia
leonardchan updated this revision to Diff 355368.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D104085/new/
https://reviews.llvm.org/D104085
Files:
compiler-rt/lib/hwasan/CMakeLists.txt
compiler-rt/lib/hwasan/hwasan_fuchsia.cpp
compiler-rt/lib/hwasan/hwasan_thread.cpp
Index: compiler-rt/lib/hwasan/hwasan_thread.cpp
===
--- compiler-rt/lib/hwasan/hwasan_thread.cpp
+++ compiler-rt/lib/hwasan/hwasan_thread.cpp
@@ -46,7 +46,12 @@
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,
Index: compiler-rt/lib/hwasan/hwasan_fuchsia.cpp
===
--- /dev/null
+++ compiler-rt/lib/hwasan/hwasan_fuchsia.cpp
@@ -0,0 +1,158 @@
+//===-- 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.
+SANITIZER_INTERFACE_ATTRIBUTE
+[[gnu::tls_model("initial-exec")]] 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(
+ 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());
+}
+
+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();
+}
+
+// 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(hook);
+ if (!aborted) {
+// The thread was created successfully.
+// ThreadStartHook can
[PATCH] D104085: [compiler-rt][hwasan] Setup hwasan thread handling on Fuchsia
leonardchan added a comment. Let me know if this should be split up also. I think this might be small and and simple enough for one patch. 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
[PATCH] D104085: [compiler-rt][hwasan] Setup hwasan thread handling on Fuchsia
leonardchan updated this revision to Diff 355323.
leonardchan added a comment.
Ok. Ready for reviews.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D104085/new/
https://reviews.llvm.org/D104085
Files:
compiler-rt/lib/hwasan/CMakeLists.txt
compiler-rt/lib/hwasan/hwasan_fuchsia.cpp
compiler-rt/lib/hwasan/hwasan_thread.cpp
Index: compiler-rt/lib/hwasan/hwasan_thread.cpp
===
--- compiler-rt/lib/hwasan/hwasan_thread.cpp
+++ compiler-rt/lib/hwasan/hwasan_thread.cpp
@@ -46,7 +46,12 @@
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,
Index: compiler-rt/lib/hwasan/hwasan_fuchsia.cpp
===
--- /dev/null
+++ compiler-rt/lib/hwasan/hwasan_fuchsia.cpp
@@ -0,0 +1,159 @@
+//===-- 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.
+SANITIZER_INTERFACE_ATTRIBUTE
+[[gnu::tls_model("initial-exec")]] 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(
+ 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());
+}
+
+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();
+}
+
+// This sets the stack top and bottom according to the InitState passed to
+// CreateCurrentThread above.
+void Thread::InitStackAndTls(uptr stack_buffer_start, uptr stack_buffer_size,
+ 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) {
+
[PATCH] D104085: [compiler-rt][hwasan] Setup hwasan thread handling on Fuchsia
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(
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(), _bottom_, _size,
_begin_,
- _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
[PATCH] D104085: [compiler-rt][hwasan] Setup hwasan thread handling on Fuchsia
leonardchan updated this revision to Diff 354098.
leonardchan marked 7 inline comments as done.
leonardchan edited the summary of this revision.
leonardchan added a comment.
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 .
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D104085/new/
https://reviews.llvm.org/D104085
Files:
compiler-rt/lib/hwasan/CMakeLists.txt
compiler-rt/lib/hwasan/hwasan_fuchsia.cpp
compiler-rt/lib/hwasan/hwasan_thread.cpp
compiler-rt/lib/hwasan/hwasan_thread_list.h
Index: compiler-rt/lib/hwasan/hwasan_thread_list.h
===
--- compiler-rt/lib/hwasan/hwasan_thread_list.h
+++ compiler-rt/lib/hwasan/hwasan_thread_list.h
@@ -171,6 +171,9 @@
return stats_;
}
+ // Reuse this value so external users don't need to call RingBufferSize().
+ uptr GetRingBufferSize() const { return ring_buffer_size_; }
+
private:
Thread *AllocThread() {
SpinMutexLock l(_space_mutex_);
Index: compiler-rt/lib/hwasan/hwasan_thread.cpp
===
--- compiler-rt/lib/hwasan/hwasan_thread.cpp
+++ compiler-rt/lib/hwasan/hwasan_thread.cpp
@@ -46,7 +46,12 @@
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,
Index: compiler-rt/lib/hwasan/hwasan_fuchsia.cpp
===
--- /dev/null
+++ compiler-rt/lib/hwasan/hwasan_fuchsia.cpp
@@ -0,0 +1,167 @@
+//===-- 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.
+SANITIZER_INTERFACE_ATTRIBUTE
+[[gnu::tls_model("initial-exec")]] 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(
+ 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());
+}
+
+uptr *GetCurrentThreadLongPtr() { return &__hwasan_tls; }
+
+Thread *GetCurrentThread() {
+ uptr *ThreadLongPtr = GetCurrentThreadLongPtr();
+ if (UNLIKELY(*ThreadLongPtr == 0))
+return nullptr;
+ auto *R = (StackAllocationsRingBuffer *)ThreadLongPtr;
+ return hwasanThreadList().GetThreadByBufferAddress((uptr)R->Next());
+}
+
+// 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
[PATCH] D104085: [compiler-rt][hwasan] Setup hwasan thread handling on Fuchsia
mcgrathr added a comment.
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.
Comment at: compiler-rt/lib/hwasan/hwasan_fuchsia.cpp:26
+// used to always find the hwasan thread object associated with the current
+// running thread.
+SANITIZER_INTERFACE_ATTRIBUTE
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.
Comment at: compiler-rt/lib/hwasan/hwasan_fuchsia.cpp:32-33
+
+// These are known parameters passed to the hwasan runtime on thread creation
+// when creating a thread.
+struct Thread::InitOptions {
department of redundancy department
Comment at: compiler-rt/lib/hwasan/hwasan_fuchsia.cpp:34
+// when creating a thread.
+struct Thread::InitOptions {
+ uptr stack_bottom, stack_top;
Nit: I'd call it something more like InitState.
Comment at: compiler-rt/lib/hwasan/hwasan_fuchsia.cpp:41
+void InitThreads() {
+ uptr alloc_size = UINT64_C(1) << kShadowBaseAlignment;
+ uptr thread_start = reinterpret_cast(
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.
Comment at: compiler-rt/lib/hwasan/hwasan_fuchsia.cpp:84-88
+// are known before thread creation. However, we cannot setup the stack ring
+// buffer just yet because it is stored in the global __hwasan_tls, which we
can
+// only correctly access once in the new thread. This will be set up in the
+// ThreadStartHook where we can safely reference __hwasan_tls while in the new
+// thread.
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.
Comment at: compiler-rt/lib/hwasan/hwasan_fuchsia.cpp:137
+// __hwasan_tls can be referenced.
+static void FinishThreadInitialization(Thread *thread) {
+ CHECK_NE(thread, nullptr);
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.
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);
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.
Comment at: compiler-rt/lib/hwasan/hwasan_thread.cpp:58-63
- uptr tls_size;
- uptr stack_size;
- GetThreadStackAndTls(IsMainThread(), _bottom_, _size,
_begin_,
- _size);
- stack_top_ = stack_bottom_ + stack_size;
- tls_end_ = tls_begin_ + tls_size;
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.
Comment at: compiler-rt/lib/hwasan/hwasan_thread.h:85
HeapAllocationsRingBuffer *heap_allocations_;
- StackAllocationsRingBuffer *stack_allocations_;
+ StackAllocationsRingBuffer *stack_allocations_ = nullptr;
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.
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
[PATCH] D104085: [compiler-rt][hwasan] Setup hwasan thread handling on Fuchsia
leonardchan added a comment. 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. 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
[PATCH] D104085: [compiler-rt][hwasan] Setup hwasan thread handling on Fuchsia
leonardchan created this revision.
leonardchan added reviewers: phosek, mcgrathr.
leonardchan added a project: Sanitizers.
Herald added subscribers: jfb, mgorny, dberris.
leonardchan requested review of this revision.
Herald added a subscriber: Sanitizers.
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.
Some refactoring:
- Move `Thread::Init` from `hwasan_thread.cpp` into `hwasan_linux.cpp` and
`hwasan_fuchsia.cpp` so they can have separate implementations. The linux
implementation uses the same logic of making hwasan thread object while in the
thread itself. The fuchsia implementation separates this into two functions:
one for initializing tls, stack, and heap_allocation members; and one for
initializing the stack ring buffer once we enter the thread.
- `Thread::Init` accepts an optional `InitOptions` struct pointer which will
initialize a hwasan thread from miscellaneous parameters. In the fuchsia
implementation, this is just the stack bounds.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D104085
Files:
compiler-rt/lib/hwasan/CMakeLists.txt
compiler-rt/lib/hwasan/hwasan.cpp
compiler-rt/lib/hwasan/hwasan_fuchsia.cpp
compiler-rt/lib/hwasan/hwasan_linux.cpp
compiler-rt/lib/hwasan/hwasan_thread.cpp
compiler-rt/lib/hwasan/hwasan_thread.h
compiler-rt/lib/hwasan/hwasan_thread_list.h
Index: compiler-rt/lib/hwasan/hwasan_thread_list.h
===
--- compiler-rt/lib/hwasan/hwasan_thread_list.h
+++ compiler-rt/lib/hwasan/hwasan_thread_list.h
@@ -85,7 +85,7 @@
RoundUpTo(ring_buffer_size_ + sizeof(Thread), ring_buffer_size_ * 2);
}
- Thread *CreateCurrentThread() {
+ Thread *CreateCurrentThread(const Thread::InitOptions *options = nullptr) {
Thread *t = nullptr;
{
SpinMutexLock l(_list_mutex_);
@@ -104,7 +104,7 @@
SpinMutexLock l(_list_mutex_);
live_list_.push_back(t);
}
-t->Init((uptr)t - ring_buffer_size_, ring_buffer_size_);
+t->Init((uptr)t - ring_buffer_size_, ring_buffer_size_, options);
AddThreadStats(t);
return t;
}
@@ -171,6 +171,9 @@
return stats_;
}
+ // Reuse this value so external users don't need to call RingBufferSize().
+ uptr GetRingBufferSize() const { return ring_buffer_size_; }
+
private:
Thread *AllocThread() {
SpinMutexLock l(_space_mutex_);
Index: compiler-rt/lib/hwasan/hwasan_thread.h
===
--- compiler-rt/lib/hwasan/hwasan_thread.h
+++ compiler-rt/lib/hwasan/hwasan_thread.h
@@ -23,7 +23,17 @@
class Thread {
public:
- void Init(uptr stack_buffer_start, uptr stack_buffer_size); // Must be called from the thread itself.
+ // These are optional parameters that can be passed to Init.
+ struct InitOptions;
+
+ void Init(uptr stack_buffer_start, uptr stack_buffer_size,
+const InitOptions *options = nullptr);
+
+ void InitStackAllocations(StackAllocationsRingBuffer *allocations) {
+CHECK_EQ(stack_allocations_, nullptr);
+stack_allocations_ = allocations;
+ }
+
void InitRandomState();
void Destroy();
@@ -72,7 +82,7 @@
AllocatorCache allocator_cache_;
HeapAllocationsRingBuffer *heap_allocations_;
- StackAllocationsRingBuffer *stack_allocations_;
+ StackAllocationsRingBuffer *stack_allocations_ = nullptr;
u64 unique_id_; // counting from zero.
Index: compiler-rt/lib/hwasan/hwasan_thread.cpp
===
--- compiler-rt/lib/hwasan/hwasan_thread.cpp
+++ compiler-rt/lib/hwasan/hwasan_thread.cpp
@@ -34,51 +34,6 @@
stack_allocations_->push(0);
}
-void Thread::Init(uptr stack_buffer_start, uptr stack_buffer_size) {
- 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);
-
- 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
