Skip to content

Commit 91167e2

Browse files
author
David Spickett
committed
[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
1 parent bc728d5 commit 91167e2

File tree

5 files changed

+28
-83
lines changed

5 files changed

+28
-83
lines changed

clang/include/clang/Driver/Options.td

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1032,10 +1032,16 @@ def fno_sanitize_address_use_odr_indicator
10321032
: Flag<["-"], "fno-sanitize-address-use-odr-indicator">,
10331033
Group<f_clang_Group>,
10341034
HelpText<"Disable ODR indicator globals">;
1035+
// Note: This flag was introduced when it was necessary to distinguish between
1036+
// ABI for correct codegen. This is no longer needed, but the flag is
1037+
// not removed since targeting either ABI will behave the same.
1038+
// This way we cause no disturbance to existing scripts & code, and if we
1039+
// want to use this flag in the future we will cause no disturbance then
1040+
// either.
10351041
def fsanitize_hwaddress_abi_EQ
10361042
: Joined<["-"], "fsanitize-hwaddress-abi=">,
10371043
Group<f_clang_Group>,
1038-
HelpText<"Select the HWAddressSanitizer ABI to target (interceptor or platform, default interceptor)">;
1044+
HelpText<"Select the HWAddressSanitizer ABI to target (interceptor or platform, default interceptor). This option is currently unused.">;
10391045
def fsanitize_recover : Flag<["-"], "fsanitize-recover">, Group<f_clang_Group>;
10401046
def fno_sanitize_recover : Flag<["-"], "fno-sanitize-recover">,
10411047
Flags<[CoreOption, DriverOption]>,

compiler-rt/lib/hwasan/hwasan_interceptors.cpp

Lines changed: 18 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -202,23 +202,33 @@ INTERCEPTOR_ALIAS(__sanitizer_struct_mallinfo, mallinfo);
202202
INTERCEPTOR_ALIAS(int, mallopt, int cmd, int value);
203203
INTERCEPTOR_ALIAS(void, malloc_stats, void);
204204
#endif
205-
#endif // HWASAN_WITH_INTERCEPTORS
206205

206+
struct ThreadStartArg {
207+
thread_callback_t callback;
208+
void *param;
209+
};
210+
211+
static void *HwasanThreadStartFunc(void *arg) {
212+
__hwasan_thread_enter();
213+
ThreadStartArg A = *reinterpret_cast<ThreadStartArg*>(arg);
214+
UnmapOrDie(arg, GetPageSizeCached());
215+
return A.callback(A.param);
216+
}
207217

208-
#if HWASAN_WITH_INTERCEPTORS && !defined(__aarch64__)
209-
INTERCEPTOR(int, pthread_create, void *th, void *attr,
210-
void *(*callback)(void *), void *param) {
218+
INTERCEPTOR(int, pthread_create, void *th, void *attr, void *(*callback)(void*),
219+
void * param) {
211220
ScopedTaggingDisabler disabler;
221+
ThreadStartArg *A = reinterpret_cast<ThreadStartArg *> (MmapOrDie(
222+
GetPageSizeCached(), "pthread_create"));
223+
*A = {callback, param};
212224
int res = REAL(pthread_create)(UntagPtr(th), UntagPtr(attr),
213-
callback, param);
225+
&HwasanThreadStartFunc, A);
214226
return res;
215227
}
216-
#endif
217228

218-
#if HWASAN_WITH_INTERCEPTORS
219229
DEFINE_REAL(int, vfork)
220230
DECLARE_EXTERN_INTERCEPTOR_AND_WRAPPER(int, vfork)
221-
#endif
231+
#endif // HWASAN_WITH_INTERCEPTORS
222232

223233
#if HWASAN_WITH_INTERCEPTORS && defined(__aarch64__)
224234
// Get and/or change the set of blocked signals.
@@ -331,9 +341,7 @@ void InitializeInterceptors() {
331341
#if defined(__linux__)
332342
INTERCEPT_FUNCTION(vfork);
333343
#endif // __linux__
334-
#if !defined(__aarch64__)
335344
INTERCEPT_FUNCTION(pthread_create);
336-
#endif // __aarch64__
337345
#endif
338346

339347
inited = 1;

compiler-rt/lib/hwasan/hwasan_linux.cpp

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -354,12 +354,7 @@ void AndroidTestTlsSlot() {}
354354
#endif
355355

356356
Thread *GetCurrentThread() {
357-
uptr *ThreadLong = GetCurrentThreadLongPtr();
358-
#if HWASAN_WITH_INTERCEPTORS
359-
if (!*ThreadLong)
360-
__hwasan_thread_enter();
361-
#endif
362-
auto *R = (StackAllocationsRingBuffer *)ThreadLong;
357+
auto *R = (StackAllocationsRingBuffer *)GetCurrentThreadLongPtr();
363358
return hwasanThreadList().GetThreadByBufferAddress((uptr)(R->Next()));
364359
}
365360

llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp

Lines changed: 2 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -284,7 +284,6 @@ class HWAddressSanitizer {
284284

285285
FunctionCallee HwasanTagMemoryFunc;
286286
FunctionCallee HwasanGenerateTagFunc;
287-
FunctionCallee HwasanThreadEnterFunc;
288287

289288
Constant *ShadowGlobal;
290289

@@ -473,9 +472,6 @@ void HWAddressSanitizer::initializeCallbacks(Module &M) {
473472

474473
HWAsanHandleVfork =
475474
M.getOrInsertFunction("__hwasan_handle_vfork", IRB.getVoidTy(), IntptrTy);
476-
477-
HwasanThreadEnterFunc =
478-
M.getOrInsertFunction("__hwasan_thread_enter", IRB.getVoidTy());
479475
}
480476

481477
Value *HWAddressSanitizer::getDynamicShadowIfunc(IRBuilder<> &IRB) {
@@ -934,34 +930,13 @@ void HWAddressSanitizer::emitPrologue(IRBuilder<> &IRB, bool WithFrameRecord) {
934930
Value *SlotPtr = getHwasanThreadSlotPtr(IRB, IntptrTy);
935931
assert(SlotPtr);
936932

937-
Instruction *ThreadLong = IRB.CreateLoad(IntptrTy, SlotPtr);
938-
939-
Function *F = IRB.GetInsertBlock()->getParent();
940-
if (F->getFnAttribute("hwasan-abi").getValueAsString() == "interceptor") {
941-
Value *ThreadLongEqZero =
942-
IRB.CreateICmpEQ(ThreadLong, ConstantInt::get(IntptrTy, 0));
943-
auto *Br = cast<BranchInst>(SplitBlockAndInsertIfThen(
944-
ThreadLongEqZero, cast<Instruction>(ThreadLongEqZero)->getNextNode(),
945-
false, MDBuilder(*C).createBranchWeights(1, 100000)));
946-
947-
IRB.SetInsertPoint(Br);
948-
// FIXME: This should call a new runtime function with a custom calling
949-
// convention to avoid needing to spill all arguments here.
950-
IRB.CreateCall(HwasanThreadEnterFunc);
951-
LoadInst *ReloadThreadLong = IRB.CreateLoad(IntptrTy, SlotPtr);
952-
953-
IRB.SetInsertPoint(&*Br->getSuccessor(0)->begin());
954-
PHINode *ThreadLongPhi = IRB.CreatePHI(IntptrTy, 2);
955-
ThreadLongPhi->addIncoming(ThreadLong, ThreadLong->getParent());
956-
ThreadLongPhi->addIncoming(ReloadThreadLong, ReloadThreadLong->getParent());
957-
ThreadLong = ThreadLongPhi;
958-
}
959-
933+
Value *ThreadLong = IRB.CreateLoad(IntptrTy, SlotPtr);
960934
// Extract the address field from ThreadLong. Unnecessary on AArch64 with TBI.
961935
Value *ThreadLongMaybeUntagged =
962936
TargetTriple.isAArch64() ? ThreadLong : untagPointer(IRB, ThreadLong);
963937

964938
if (WithFrameRecord) {
939+
Function *F = IRB.GetInsertBlock()->getParent();
965940
StackBaseTag = IRB.CreateAShr(ThreadLong, 3);
966941

967942
// Prepare ring buffer data.

llvm/test/Instrumentation/HWAddressSanitizer/lazy-thread-init.ll

Lines changed: 0 additions & 39 deletions
This file was deleted.

0 commit comments

Comments
 (0)