Remove lazy thread-initialisation

[mmalcomson]

Summary

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.

[eugenis]

LGTM keeping the flag looks like the right thing to do.
I'll leave this for @pcc to accept.

[mmalcomson]

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)

[pcc]

LGTM

clang/include/clang/Driver/Options.td

 def fsanitize_address_use_odr_indicator
     : Flag<["-"], "fsanitize-address-use-odr-indicator">,
       Group<f_clang_Group>,
       HelpText<"Enable ODR indicator globals to avoid false ODR violation reports in partially sanitized programs at the cost of an increase in binary size">;
 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]>,
                            Group<f_clang_Group>;
 def fsanitize_recover_EQ : CommaJoined<["-"], "fsanitize-recover=">,
                            Group<f_clang_Group>,
                            HelpText<"Enable recovery for specified sanitizers">;
 def fno_sanitize_recover_EQ

compiler-rt/lib/hwasan/hwasan_interceptors.cpp

 #if !SANITIZER_FREEBSD && !SANITIZER_NETBSD
 INTERCEPTOR_ALIAS(void *, memalign, SIZE_T alignment, SIZE_T size);
 INTERCEPTOR_ALIAS(void *, pvalloc, SIZE_T size);
 INTERCEPTOR_ALIAS(void, cfree, void *ptr);
 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.
 extern "C" int sigprocmask(int __how, const __hw_sigset_t *__restrict __set,
                            __hw_sigset_t *__restrict __oset);
 #define SIG_BLOCK 0
 #define SIG_SETMASK 2
 extern "C" int __sigjmp_save(__hw_sigjmp_buf env, int savemask) {
void InitializeInterceptors() {
   CHECK_EQ(inited, 0);
 
   INTERCEPT_FUNCTION(fork);
 
 #if HWASAN_WITH_INTERCEPTORS
 #if defined(__linux__)
   INTERCEPT_FUNCTION(vfork);
 #endif  // __linux__
-#if !defined(__aarch64__)
   INTERCEPT_FUNCTION(pthread_create);
-#endif  // __aarch64__
 #endif
 
   inited = 1;
 }
 } // namespace __hwasan

compiler-rt/lib/hwasan/hwasan_linux.cpp

void AndroidTestTlsSlot() {
   }
   *tls_ptr = old_value;
 }
 #else
 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()));
 }
 
 struct AccessInfo {
   uptr addr;
   uptr size;
   bool is_store;
   bool is_load;

llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp

private:
 
   Function *HwasanCtorFunction;
 
   FunctionCallee HwasanMemoryAccessCallback[2][kNumberOfAccessSizes];
   FunctionCallee HwasanMemoryAccessCallbackSized[2];
 
   FunctionCallee HwasanTagMemoryFunc;
   FunctionCallee HwasanGenerateTagFunc;
-  FunctionCallee HwasanThreadEnterFunc;
 
   Constant *ShadowGlobal;
 
   Value *LocalDynamicShadow = nullptr;
   Value *StackBaseTag = nullptr;
   GlobalValue *ThreadPtrGlobal = nullptr;
 };
 
HWAsanMemcpy = M.getOrInsertFunction(MemIntrinCallbackPrefix + "memcpy",
                                        IRB.getInt8PtrTy(), IRB.getInt8PtrTy(),
                                        IRB.getInt8PtrTy(), IntptrTy);
   HWAsanMemset = M.getOrInsertFunction(MemIntrinCallbackPrefix + "memset",
                                        IRB.getInt8PtrTy(), IRB.getInt8PtrTy(),
                                        IRB.getInt32Ty(), IntptrTy);
 
   HWAsanHandleVfork =
       M.getOrInsertFunction("__hwasan_handle_vfork", IRB.getVoidTy(), IntptrTy);
-
-  HwasanThreadEnterFunc =
-      M.getOrInsertFunction("__hwasan_thread_enter", IRB.getVoidTy());
 }
 
 Value *HWAddressSanitizer::getDynamicShadowIfunc(IRBuilder<> &IRB) {
   // An empty inline asm with input reg == output reg.
   // An opaque no-op cast, basically.
   InlineAsm *Asm = InlineAsm::get(
       FunctionType::get(Int8PtrTy, {ShadowGlobal->getType()}, false),
       StringRef(""), StringRef("=r,0"),
void HWAddressSanitizer::emitPrologue(IRBuilder<> &IRB, bool WithFrameRecord) {
   if (!WithFrameRecord && TargetTriple.isAndroid()) {
     LocalDynamicShadow = getDynamicShadowIfunc(IRB);
     return;
   }
 
   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.
     Value *PC;
     if (TargetTriple.getArch() == Triple::aarch64)
       PC = readRegister(IRB, "pc");
     else
       PC = IRB.CreatePtrToInt(F, IntptrTy);

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

-; 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}