[PATCH] D69574: Remove lazy thread-initialisation

2019-11-01 Thread Matthew Malcomson via Phabricator via cfe-commits
mmalcomson added a comment.

Ping @pcc -- does this change to remove lazy thread initialisation look OK?

(I'm looking to start upstreaming hwasan instrumentation to GCC soon, and need 
to know whether GCC must insert the thread initialisation code in function 
prologues)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69574



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D69574: Remove lazy thread-initialisation

2019-10-29 Thread Matthew Malcomson via Phabricator via cfe-commits
mmalcomson created this revision.
mmalcomson added reviewers: eugenis, pcc, Sanitizers.
mmalcomson added a project: Sanitizers.
Herald added subscribers: llvm-commits, cfe-commits, jfb, hiraditya, srhines.
Herald added projects: clang, LLVM.

As I asked in the comments of https://reviews.llvm.org/D69199, if we're no 
longer accounting for the late-binding feature then I believe we can remove 
this lazy thread initialisation complexity.
I've tested with clang and gcc on linux that various threading programs work as 
expected.

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.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D69574

Files:
  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
  llvm/test/Instrumentation/HWAddressSanitizer/lazy-thread-init.ll

Index: llvm/test/Instrumentation/HWAddressSanitizer/lazy-thread-init.ll
===
--- 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 10}
Index: llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp
===
--- llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp
+++ llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp
@@ -284,7 +284,6 @@
 
   FunctionCallee HwasanTagMemoryFunc;
   FunctionCallee HwasanGenerateTagFunc;
-  FunctionCallee HwasanThreadEnterFunc;
 
   Constant *ShadowGlobal;
 
@@ -473,9 +472,6 @@
 
   HWAsanHandleVfork =
   M.getOrInsertFunction("__hwasan_handle_vfork", IRB.getVoidTy(), IntptrTy);
-
-  HwasanThreadEnterFunc =
-  M.getOrInsertFunction("__hwasan_thread_enter", IRB.getVoidTy());
 }
 
 Value *HWAddressSanitizer::getDynamicShadowIfunc(IRBuilder<> ) {
@@ -934,34 +930,13 @@
   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(SplitBlockAndInsertIfThen(
-ThreadLongEqZero, cast(ThreadLongEqZero)->getNextNode(),
-false, MDBuilder(*C).createBranchWeights(1, 10)));
-
-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