[llvm][hwasan] Decouple use of the TLS global for getting the shadow base and using the frame record feature

[leonardchan]

Summary

This allows for using the frame record feature (which uses __hwasan_tls) independently from however the user wants to access the shadow base, which prior was only usable if shadow wasn't accessed through the TLS variable or ifuncs.

Frame recording can be explicitly set according to ShadowMapping::WithFrameRecord in ShadowMapping::init. Currently, it is only enabled on Fuchsia and if TLS is used, so this should mimic the old behavior.

Added an extra case to prologue.ll that covers this new case.

[mcgrathr]

lgtm

[eugenis]

llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp

1050

I think this will break Linux kernel build.
They use -fsanitize=kernel-hwaddress with one of

-hwasan-instrument-with-calls=1

or

-hwasan-mapping-offset=

The assumption here is that InTls == 0 means there is no thread slot, and therefore no frame recording.

[leonardchan]

llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp

1050

Perhaps we could set the default value of -hwasan-record-stack-history to false so users who don't expect frame recording don't suddenly see TLS being used? It would mean people who do use frame recording won't see it anymore and will need to manually add -hwasan-record-stack-history=1, but maybe that won't break existing users.

[eugenis]

llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp

1050

These are internal compiler flags, they should not have users, ideally. The right thing should happen based on -fsanitize flag and the target triple, and the right thing in this case is to enable frame recording on supported platforms.

I'd rather put this check under InTls, or add something like ShadowMapping::WithFrameRecord and make the decision in ShadowMapping::init.

[leonardchan]

llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp

1050

Updated to add ShadowMapping::WithFrameRecord.

[eugenis]

LGTM with a nit

llvm/test/Instrumentation/HWAddressSanitizer/prologue.ll

30

CHECK-ZERO-OFFSET is never used

[leonardchan]

llvm/test/Instrumentation/HWAddressSanitizer/prologue.ll

30

Woops. Meant to add a RUN case for testing a fuchsia target which should use this. Updated.

llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp

Value *HWAddressSanitizer::getHwasanThreadSlotPtr(IRBuilder<> &IRB, Type *Ty) {
   }
   if (ThreadPtrGlobal)
     return ThreadPtrGlobal;
 
   return nullptr;
 }
 
 void HWAddressSanitizer::emitPrologue(IRBuilder<> &IRB, bool WithFrameRecord) {
-  if (!Mapping.InTls) {
+  if (!Mapping.InTls)
     ShadowBase = getShadowNonTls(IRB);
-    return;
-  }
-
-  if (!WithFrameRecord && TargetTriple.isAndroid()) {
+  else if (!WithFrameRecord && TargetTriple.isAndroid())
     ShadowBase = getDynamicShadowIfunc(IRB);
+
+  if (!WithFrameRecord && ShadowBase)
     return;
-  }
 
   Value *SlotPtr = getHwasanThreadSlotPtr(IRB, IntptrTy);
   assert(SlotPtr);
 
   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) {
     Value *WrapMask = IRB.CreateXor(
         IRB.CreateShl(IRB.CreateAShr(ThreadLong, 56), 12, "", true, true),
         ConstantInt::get(IntptrTy, (uint64_t)-1));
     Value *ThreadLongNew = IRB.CreateAnd(
         IRB.CreateAdd(ThreadLong, ConstantInt::get(IntptrTy, 8)), WrapMask);
     IRB.CreateStore(ThreadLongNew, SlotPtr);
   }
 
+  if (!ShadowBase) {
     // Get shadow base address by aligning RecordPtr up.
     // Note: this is not correct if the pointer is already aligned.
     // Runtime library will make sure this never happens.
     ShadowBase = IRB.CreateAdd(
         IRB.CreateOr(
             ThreadLongMaybeUntagged,
             ConstantInt::get(IntptrTy, (1ULL << kShadowBaseAlignment) - 1)),
         ConstantInt::get(IntptrTy, 1), "hwasan.shadow");
     ShadowBase = IRB.CreateIntToPtr(ShadowBase, Int8PtrTy);
   }
+}
 
 Value *HWAddressSanitizer::readRegister(IRBuilder<> &IRB, StringRef Name) {
   Module *M = IRB.GetInsertBlock()->getParent()->getParent();
   Function *ReadRegister =
       Intrinsic::getDeclaration(M, Intrinsic::read_register, IntptrTy);
   MDNode *MD = MDNode::get(*C, {MDString::get(*C, Name)});
   Value *Args[] = {MetadataAsValue::get(*C, MD)};
   return IRB.CreateCall(ReadRegister, Args);

llvm/test/Instrumentation/HWAddressSanitizer/alloca.ll

 ; Test alloca instrumentation.
 ;
-; RUN: opt < %s -hwasan -hwasan-with-ifunc=1 -S | FileCheck %s --check-prefixes=CHECK,DYNAMIC-SHADOW,NO-UAR-TAGS
-; RUN: opt < %s -hwasan -hwasan-mapping-offset=0 -S | FileCheck %s --check-prefixes=CHECK,ZERO-BASED-SHADOW,NO-UAR-TAGS
-; RUN: opt < %s -hwasan -hwasan-with-ifunc=1 -hwasan-uar-retag-to-zero=0 -S | FileCheck %s --check-prefixes=CHECK,DYNAMIC-SHADOW,UAR-TAGS
+; RUN: opt < %s -hwasan -hwasan-record-stack-history=0 -hwasan-with-ifunc=1 -S | FileCheck %s --check-prefixes=CHECK,DYNAMIC-SHADOW,NO-UAR-TAGS
+; RUN: opt < %s -hwasan -hwasan-record-stack-history=0 -hwasan-mapping-offset=0 -S | FileCheck %s --check-prefixes=CHECK,ZERO-BASED-SHADOW,NO-UAR-TAGS
+; RUN: opt < %s -hwasan -hwasan-record-stack-history=0 -hwasan-with-ifunc=1 -hwasan-uar-retag-to-zero=0 -S | FileCheck %s --check-prefixes=CHECK,DYNAMIC-SHADOW,UAR-TAGS
 
 target datalayout = "e-m:e-i8:8:32-i16:16:32-i64:64-i128:128-n32:64-S128"
 target triple = "aarch64--linux-android10000"
 
 declare void @use32(i32*)
 
 define void @test_alloca() sanitize_hwaddress {
 ; CHECK-LABEL: @test_alloca(

llvm/test/Instrumentation/HWAddressSanitizer/kernel-alloca.ll

 ; Test kernel hwasan instrumentation for alloca.
 ;
-; RUN: opt < %s -hwasan -hwasan-kernel=1 -S | FileCheck %s
+; RUN: opt < %s -hwasan -hwasan-record-stack-history=0 -hwasan-kernel=1 -S | FileCheck %s
 
 target datalayout = "e-m:e-i8:8:32-i16:16:32-i64:64-i128:128-n32:64-S128"
 target triple = "aarch64--linux-android"
 
 declare void @use32(i32*)
 
 define void @test_alloca() sanitize_hwaddress {
 ; CHECK-LABEL: @test_alloca(

llvm/test/Instrumentation/HWAddressSanitizer/prologue.ll

 ; Test -hwasan-with-ifunc flag.
 ;
 ; RUN: opt -hwasan -S < %s | \
-; RUN:     FileCheck %s --check-prefixes=CHECK,CHECK-NOGLOBAL,CHECK-TLS,CHECK-HISTORY
+; RUN:     FileCheck %s --check-prefixes=CHECK,CHECK-NOGLOBAL,CHECK-TLS,CHECK-HISTORY,CHECK-THREAD-PTR
 ; RUN: opt -hwasan -S -hwasan-with-ifunc=0 -hwasan-with-tls=1 -hwasan-record-stack-history=1 < %s | \
-; RUN:     FileCheck %s --check-prefixes=CHECK,CHECK-NOGLOBAL,CHECK-TLS,CHECK-HISTORY
+; RUN:     FileCheck %s --check-prefixes=CHECK,CHECK-NOGLOBAL,CHECK-TLS,CHECK-HISTORY,CHECK-THREAD-PTR
 ; RUN: opt -hwasan -S -hwasan-with-ifunc=0 -hwasan-with-tls=1 -hwasan-record-stack-history=0 < %s | \
 ; RUN:     FileCheck %s --check-prefixes=CHECK,CHECK-NOGLOBAL,CHECK-IFUNC,CHECK-NOHISTORY
-; RUN: opt -hwasan -S -hwasan-with-ifunc=0 -hwasan-with-tls=0 < %s | \
+; RUN: opt -hwasan -S -hwasan-record-stack-history=0 -hwasan-with-ifunc=0 -hwasan-with-tls=0 < %s | \
 ; RUN:     FileCheck %s --check-prefixes=CHECK,CHECK-GLOBAL,CHECK-NOHISTORY
-; RUN: opt -hwasan -S -hwasan-with-ifunc=1  -hwasan-with-tls=0 < %s | \
+; RUN: opt -hwasan -S -hwasan-record-stack-history=0 -hwasan-with-ifunc=1  -hwasan-with-tls=0 < %s | \
 ; RUN:     FileCheck %s --check-prefixes=CHECK,CHECK-IFUNC,CHECK-NOHISTORY
+; RUN: opt -hwasan -S -hwasan-record-stack-history=1 -hwasan-with-ifunc=0 -hwasan-with-tls=0 < %s | \
+; RUN:     FileCheck %s --check-prefixes=CHECK,CHECK-GLOBAL,CHECK-HISTORY,CHECK-THREAD-PTR
 
 target datalayout = "e-m:e-i8:8:32-i16:16:32-i64:64-i128:128-n32:64-S128"
 target triple = "aarch64--linux-android22"
 
 ; CHECK-IFUNC: @__hwasan_shadow = external global [0 x i8]
 ; CHECK-NOIFUNC: @__hwasan_shadow_memory_dynamic_address = external global i64
 
 define i32 @test_load(i32* %a) sanitize_hwaddress {
 ; First instrumentation in the function must be to load the dynamic shadow
 ; address into a local variable.
 ; CHECK-LABEL: @test_load
 ; CHECK: entry:
 
 ; CHECK-NOGLOBAL:   %[[A:[^ ]*]] = call i8* asm "", "=r,0"([0 x i8]* @__hwasan_shadow)
 ; CHECK-NOGLOBAL:   @llvm.hwasan.check.memaccess(i8* %[[A]]
 
 ; CHECK-GLOBAL: load i8*, i8** @__hwasan_shadow_memory_dynamic_address
 
 ; "store i64" is only used to update stack history (this input IR intentionally does not use any i64)
 ; W/o any allocas, the history is not updated, even if it is enabled explicitly with -hwasan-record-stack-history=1
 ; CHECK-NOT: store i64
 
 ; CHECK: ret i32
 

 ; CHECK-LABEL: @test_alloca
 ; CHECK: entry:
 
 ; CHECK-IFUNC:   %[[A:[^ ]*]] = call i8* asm "", "=r,0"([0 x i8]* @__hwasan_shadow)
 ; CHECK-IFUNC:   getelementptr i8, i8* %[[A]]
 
 ; CHECK-GLOBAL: load i8*, i8** @__hwasan_shadow_memory_dynamic_address
 
-; CHECK-TLS:   %[[A:[^ ]*]] = call i8* @llvm.thread.pointer()
-; CHECK-TLS:   %[[B:[^ ]*]] = getelementptr i8, i8* %[[A]], i32 48
-; CHECK-TLS:   %[[C:[^ ]*]] = bitcast i8* %[[B]] to i64*
-; CHECK-TLS:   %[[D:[^ ]*]] = load i64, i64* %[[C]]
-; CHECK-TLS:   %[[E:[^ ]*]] = ashr i64 %[[D]], 3
+; CHECK-THREAD-PTR:   %[[A:[^ ]*]] = call i8* @llvm.thread.pointer()
+; CHECK-THREAD-PTR:   %[[B:[^ ]*]] = getelementptr i8, i8* %[[A]], i32 48
+; CHECK-THREAD-PTR:   %[[C:[^ ]*]] = bitcast i8* %[[B]] to i64*
+; CHECK-THREAD-PTR:   %[[D:[^ ]*]] = load i64, i64* %[[C]]
+; CHECK-THREAD-PTR:   %[[E:[^ ]*]] = ashr i64 %[[D]], 3
 
 ; CHECK-NOHISTORY-NOT: store i64
 
 ; CHECK-HISTORY: call i64 @llvm.read_register.i64(metadata [[MD:![0-9]*]])
 ; CHECK-HISTORY: %[[PTR:[^ ]*]] = inttoptr i64 %[[D]] to i64*
 ; CHECK-HISTORY: store i64 %{{.*}}, i64* %[[PTR]]
 ; CHECK-HISTORY: %[[D1:[^ ]*]] = ashr i64 %[[D]], 56
 ; CHECK-HISTORY: %[[D2:[^ ]*]] = shl nuw nsw i64 %[[D1]], 12