[clang] 91167e2 - [hwasan] Remove lazy thread-initialisation
David Spickett via cfe-commits
cfe-commits at lists.llvm.org
Mon Nov 4 03:19:52 PST 2019
Author: David Spickett
Date: 2019-11-04T10:58:46Z
New Revision: 91167e22eca535025f093335acece573bf19c525
URL: https://github.com/llvm/llvm-project/commit/91167e22eca535025f093335acece573bf19c525
DIFF: https://github.com/llvm/llvm-project/commit/91167e22eca535025f093335acece573bf19c525.diff
LOG: [hwasan] Remove lazy thread-initialisation
This was an experiment made possible by a non-standard feature of the
Android dynamic loader.
It required introducing a flag to tell the compiler which ABI was being
targeted.
This flag is no longer needed, since the generated code now works for
both ABI's.
We leave that flag untouched for backwards compatibility. This also
means that if we need to distinguish between targeted ABI's again
we can do that without disturbing any existing workflows.
We leave a comment in the source code and mention in the help text to
explain this for any confused person reading the code in the future.
Patch by Matthew Malcomson
Differential Revision: https://reviews.llvm.org/D69574
Added:
Modified:
clang/include/clang/Driver/Options.td
compiler-rt/lib/hwasan/hwasan_interceptors.cpp
compiler-rt/lib/hwasan/hwasan_linux.cpp
llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp
Removed:
llvm/test/Instrumentation/HWAddressSanitizer/lazy-thread-init.ll
################################################################################
diff --git a/clang/include/clang/Driver/Options.td b/clang/include/clang/Driver/Options.td
index 722427bc35a6..3093dc65fce4 100644
--- a/clang/include/clang/Driver/Options.td
+++ b/clang/include/clang/Driver/Options.td
@@ -1032,10 +1032,16 @@ def fno_sanitize_address_use_odr_indicator
: Flag<["-"], "fno-sanitize-address-use-odr-indicator">,
Group<f_clang_Group>,
HelpText<"Disable ODR indicator globals">;
+// Note: This flag was introduced when it was necessary to distinguish between
+// ABI for correct codegen. This is no longer needed, but the flag is
+// not removed since targeting either ABI will behave the same.
+// This way we cause no disturbance to existing scripts & code, and if we
+// want to use this flag in the future we will cause no disturbance then
+// either.
def fsanitize_hwaddress_abi_EQ
: Joined<["-"], "fsanitize-hwaddress-abi=">,
Group<f_clang_Group>,
- HelpText<"Select the HWAddressSanitizer ABI to target (interceptor or platform, default interceptor)">;
+ HelpText<"Select the HWAddressSanitizer ABI to target (interceptor or platform, default interceptor). This option is currently unused.">;
def fsanitize_recover : Flag<["-"], "fsanitize-recover">, Group<f_clang_Group>;
def fno_sanitize_recover : Flag<["-"], "fno-sanitize-recover">,
Flags<[CoreOption, DriverOption]>,
diff --git a/compiler-rt/lib/hwasan/hwasan_interceptors.cpp b/compiler-rt/lib/hwasan/hwasan_interceptors.cpp
index 4f9bd3469eb1..44e569ee6d72 100644
--- a/compiler-rt/lib/hwasan/hwasan_interceptors.cpp
+++ b/compiler-rt/lib/hwasan/hwasan_interceptors.cpp
@@ -202,23 +202,33 @@ INTERCEPTOR_ALIAS(__sanitizer_struct_mallinfo, mallinfo);
INTERCEPTOR_ALIAS(int, mallopt, int cmd, int value);
INTERCEPTOR_ALIAS(void, malloc_stats, void);
#endif
-#endif // HWASAN_WITH_INTERCEPTORS
+struct ThreadStartArg {
+ thread_callback_t callback;
+ void *param;
+};
+
+static void *HwasanThreadStartFunc(void *arg) {
+ __hwasan_thread_enter();
+ ThreadStartArg A = *reinterpret_cast<ThreadStartArg*>(arg);
+ UnmapOrDie(arg, GetPageSizeCached());
+ return A.callback(A.param);
+}
-#if HWASAN_WITH_INTERCEPTORS && !defined(__aarch64__)
-INTERCEPTOR(int, pthread_create, void *th, void *attr,
- void *(*callback)(void *), void *param) {
+INTERCEPTOR(int, pthread_create, void *th, void *attr, void *(*callback)(void*),
+ void * param) {
ScopedTaggingDisabler disabler;
+ ThreadStartArg *A = reinterpret_cast<ThreadStartArg *> (MmapOrDie(
+ GetPageSizeCached(), "pthread_create"));
+ *A = {callback, param};
int res = REAL(pthread_create)(UntagPtr(th), UntagPtr(attr),
- callback, param);
+ &HwasanThreadStartFunc, A);
return res;
}
-#endif
-#if HWASAN_WITH_INTERCEPTORS
DEFINE_REAL(int, vfork)
DECLARE_EXTERN_INTERCEPTOR_AND_WRAPPER(int, vfork)
-#endif
+#endif // HWASAN_WITH_INTERCEPTORS
#if HWASAN_WITH_INTERCEPTORS && defined(__aarch64__)
// Get and/or change the set of blocked signals.
@@ -331,9 +341,7 @@ void InitializeInterceptors() {
#if defined(__linux__)
INTERCEPT_FUNCTION(vfork);
#endif // __linux__
-#if !defined(__aarch64__)
INTERCEPT_FUNCTION(pthread_create);
-#endif // __aarch64__
#endif
inited = 1;
diff --git a/compiler-rt/lib/hwasan/hwasan_linux.cpp b/compiler-rt/lib/hwasan/hwasan_linux.cpp
index dfef11883a28..ed0f30161b02 100644
--- a/compiler-rt/lib/hwasan/hwasan_linux.cpp
+++ b/compiler-rt/lib/hwasan/hwasan_linux.cpp
@@ -354,12 +354,7 @@ void AndroidTestTlsSlot() {}
#endif
Thread *GetCurrentThread() {
- uptr *ThreadLong = GetCurrentThreadLongPtr();
-#if HWASAN_WITH_INTERCEPTORS
- if (!*ThreadLong)
- __hwasan_thread_enter();
-#endif
- auto *R = (StackAllocationsRingBuffer *)ThreadLong;
+ auto *R = (StackAllocationsRingBuffer *)GetCurrentThreadLongPtr();
return hwasanThreadList().GetThreadByBufferAddress((uptr)(R->Next()));
}
diff --git a/llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp b/llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp
index f87132ee4758..697f2f7eec66 100644
--- a/llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp
+++ b/llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp
@@ -284,7 +284,6 @@ class HWAddressSanitizer {
FunctionCallee HwasanTagMemoryFunc;
FunctionCallee HwasanGenerateTagFunc;
- FunctionCallee HwasanThreadEnterFunc;
Constant *ShadowGlobal;
@@ -473,9 +472,6 @@ void HWAddressSanitizer::initializeCallbacks(Module &M) {
HWAsanHandleVfork =
M.getOrInsertFunction("__hwasan_handle_vfork", IRB.getVoidTy(), IntptrTy);
-
- HwasanThreadEnterFunc =
- M.getOrInsertFunction("__hwasan_thread_enter", IRB.getVoidTy());
}
Value *HWAddressSanitizer::getDynamicShadowIfunc(IRBuilder<> &IRB) {
@@ -934,34 +930,13 @@ void HWAddressSanitizer::emitPrologue(IRBuilder<> &IRB, bool WithFrameRecord) {
Value *SlotPtr = getHwasanThreadSlotPtr(IRB, IntptrTy);
assert(SlotPtr);
- Instruction *ThreadLong = IRB.CreateLoad(IntptrTy, SlotPtr);
-
- Function *F = IRB.GetInsertBlock()->getParent();
- if (F->getFnAttribute("hwasan-abi").getValueAsString() == "interceptor") {
- Value *ThreadLongEqZero =
- IRB.CreateICmpEQ(ThreadLong, ConstantInt::get(IntptrTy, 0));
- auto *Br = cast<BranchInst>(SplitBlockAndInsertIfThen(
- ThreadLongEqZero, cast<Instruction>(ThreadLongEqZero)->getNextNode(),
- false, MDBuilder(*C).createBranchWeights(1, 100000)));
-
- IRB.SetInsertPoint(Br);
- // FIXME: This should call a new runtime function with a custom calling
- // convention to avoid needing to spill all arguments here.
- IRB.CreateCall(HwasanThreadEnterFunc);
- LoadInst *ReloadThreadLong = IRB.CreateLoad(IntptrTy, SlotPtr);
-
- IRB.SetInsertPoint(&*Br->getSuccessor(0)->begin());
- PHINode *ThreadLongPhi = IRB.CreatePHI(IntptrTy, 2);
- ThreadLongPhi->addIncoming(ThreadLong, ThreadLong->getParent());
- ThreadLongPhi->addIncoming(ReloadThreadLong, ReloadThreadLong->getParent());
- ThreadLong = ThreadLongPhi;
- }
-
+ Value *ThreadLong = IRB.CreateLoad(IntptrTy, SlotPtr);
// Extract the address field from ThreadLong. Unnecessary on AArch64 with TBI.
Value *ThreadLongMaybeUntagged =
TargetTriple.isAArch64() ? ThreadLong : untagPointer(IRB, ThreadLong);
if (WithFrameRecord) {
+ Function *F = IRB.GetInsertBlock()->getParent();
StackBaseTag = IRB.CreateAShr(ThreadLong, 3);
// Prepare ring buffer data.
diff --git a/llvm/test/Instrumentation/HWAddressSanitizer/lazy-thread-init.ll b/llvm/test/Instrumentation/HWAddressSanitizer/lazy-thread-init.ll
deleted file mode 100644
index 6c445ed16735..000000000000
--- a/llvm/test/Instrumentation/HWAddressSanitizer/lazy-thread-init.ll
+++ /dev/null
@@ -1,39 +0,0 @@
-; RUN: opt -S -hwasan < %s | FileCheck %s
-
-target triple = "aarch64--linux-android"
-
-declare i32 @bar([16 x i32]* %p)
-
-define void @alloca() sanitize_hwaddress "hwasan-abi"="interceptor" {
- ; CHECK: alloca [16 x i32]
- ; CHECK: [[A:%[^ ]*]] = call i8* @llvm.thread.pointer()
- ; CHECK: [[B:%[^ ]*]] = getelementptr i8, i8* [[A]], i32 48
- ; CHECK: [[C:%[^ ]*]] = bitcast i8* [[B]] to i64*
- ; CHECK: [[LOAD:%[^ ]*]] = load i64, i64* [[C]]
- ; CHECK: [[ICMP:%[^ ]*]] = icmp eq i64 [[LOAD]], 0
- ; CHECK: br i1 [[ICMP]], label %[[INIT:[^,]*]], label %[[CONT:[^,]*]], !prof [[PROF:![0-9]+]]
-
- ; CHECK: [[INIT]]:
- ; CHECK: call void @__hwasan_thread_enter()
- ; CHECK: [[RELOAD:%[^ ]*]] = load i64, i64* [[C]]
- ; CHECK: br label %[[CONT]]
-
- ; CHECK: [[CONT]]:
- ; CHECK: phi i64 [ [[LOAD]], %0 ], [ [[RELOAD]], %[[INIT]] ]
- ; CHECK: alloca i8
-
- %p = alloca [16 x i32]
- %size = call i32 @bar([16 x i32]* %p)
- %q = alloca i8, i32 %size
- ret void
-}
-
-define i32 @load(i32* %p) sanitize_hwaddress "hwasan-abi"="interceptor" {
- ; CHECK: [[SHADOW:%[^ ]*]] = call i8* asm "", "=r,0"([0 x i8]* @__hwasan_shadow)
- ; CHECK-NOT: icmp
- ; CHECK: call void @llvm.hwasan.check.memaccess(i8* [[SHADOW]],
- %v = load i32, i32* %p
- ret i32 %v
-}
-
-; CHECK: [[PROF]] = !{!"branch_weights", i32 1, i32 100000}
More information about the cfe-commits
mailing list