Skip to content

Commit 87f477b

Browse files
pccpcc
committedJan 4, 2019
hwasan: Implement lazy thread initialization for the interceptor ABI.
The problem is similar to D55986 but for threads: a process with the interceptor hwasan library loaded might have some threads started by instrumented libraries and some by uninstrumented libraries, and we need to be able to run instrumented code on the latter. The solution is to perform per-thread initialization lazily. If a function needs to access shadow memory or add itself to the per-thread ring buffer its prologue checks to see whether the value in the sanitizer TLS slot is null, and if so it calls __hwasan_thread_enter and reloads from the TLS slot. The runtime does the same thing if it needs to access this data structure. This change means that the code generator needs to know whether we are targeting the interceptor runtime, since we don't want to pay the cost of lazy initialization when targeting a platform with native hwasan support. A flag -fsanitize-hwaddress-abi={interceptor,platform} has been introduced for selecting the runtime ABI to target. The default ABI is set to interceptor since it's assumed that it will be more common that users will be compiling application code than platform code. Because we can no longer assume that the TLS slot is initialized, the pthread_create interceptor is no longer necessary, so it has been removed. Ideally, lazy initialization should only cost one instruction in the hot path, but at present the call may cause us to spill arguments to the stack, which means more instructions in the hot path (or theoretically in the cold path if the spills are moved with shrink wrapping). With an appropriately chosen calling convention for the per-thread initialization function (TODO) the hot path should always need just one instruction and the cold path should need two instructions with no spilling required. Differential Revision: https://reviews.llvm.org/D56038 llvm-svn: 350429
1 parent ff92a1a commit 87f477b

File tree

13 files changed

+106
-33
lines changed

13 files changed

+106
-33
lines changed
 

‎clang/include/clang/Basic/CodeGenOptions.h

+2
Original file line numberDiff line numberDiff line change
@@ -286,6 +286,8 @@ class CodeGenOptions : public CodeGenOptionsBase {
286286
/// Set of XRay instrumentation kinds to emit.
287287
XRayInstrSet XRayInstrumentationBundle;
288288

289+
std::vector<std::string> DefaultFunctionAttrs;
290+
289291
public:
290292
// Define accessors/mutators for code generation options of enumeration type.
291293
#define CODEGENOPT(Name, Bits, Default)

‎clang/include/clang/Driver/CC1Options.td

+2
Original file line numberDiff line numberDiff line change
@@ -163,6 +163,8 @@ let Flags = [CC1Option, CC1AsOption, NoDriverOption] in {
163163
def debug_info_kind_EQ : Joined<["-"], "debug-info-kind=">;
164164
def debug_info_macro : Flag<["-"], "debug-info-macro">,
165165
HelpText<"Emit macro debug information">;
166+
def default_function_attr : Separate<["-"], "default-function-attr">,
167+
HelpText<"Apply given attribute to all functions">;
166168
def dwarf_version_EQ : Joined<["-"], "dwarf-version=">;
167169
def debugger_tuning_EQ : Joined<["-"], "debugger-tuning=">;
168170
def fdebug_compilation_dir : Separate<["-"], "fdebug-compilation-dir">,

‎clang/include/clang/Driver/Options.td

+4
Original file line numberDiff line numberDiff line change
@@ -998,6 +998,10 @@ def fno_sanitize_address_use_odr_indicator
998998
: Flag<["-"], "fno-sanitize-address-use-odr-indicator">,
999999
Group<f_clang_Group>,
10001000
HelpText<"Disable ODR indicator globals">;
1001+
def fsanitize_hwaddress_abi_EQ
1002+
: Joined<["-"], "fsanitize-hwaddress-abi=">,
1003+
Group<f_clang_Group>,
1004+
HelpText<"Select the HWAddressSanitizer ABI to target (interceptor or platform, default interceptor)">;
10011005
def fsanitize_recover : Flag<["-"], "fsanitize-recover">, Group<f_clang_Group>;
10021006
def fno_sanitize_recover : Flag<["-"], "fno-sanitize-recover">,
10031007
Flags<[CoreOption, DriverOption]>,

‎clang/include/clang/Driver/SanitizerArgs.h

+1
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,7 @@ class SanitizerArgs {
3939
bool AsanPoisonCustomArrayCookie = false;
4040
bool AsanGlobalsDeadStripping = false;
4141
bool AsanUseOdrIndicator = false;
42+
std::string HwasanAbi;
4243
bool LinkCXXRuntimes = false;
4344
bool NeedPIE = false;
4445
bool SafeStackRuntime = false;

‎clang/lib/CodeGen/CGCall.cpp

+6
Original file line numberDiff line numberDiff line change
@@ -1816,6 +1816,12 @@ void CodeGenModule::ConstructDefaultFnAttrList(StringRef Name, bool HasOptnone,
18161816
if (CodeGenOpts.FlushDenorm)
18171817
FuncAttrs.addAttribute("nvptx-f32ftz", "true");
18181818
}
1819+
1820+
for (StringRef Attr : CodeGenOpts.DefaultFunctionAttrs) {
1821+
StringRef Var, Value;
1822+
std::tie(Var, Value) = Attr.split('=');
1823+
FuncAttrs.addAttribute(Var, Value);
1824+
}
18191825
}
18201826

18211827
void CodeGenModule::AddDefaultFnAttrs(llvm::Function &F) {

‎clang/lib/Driver/SanitizerArgs.cpp

+17
Original file line numberDiff line numberDiff line change
@@ -741,6 +741,18 @@ SanitizerArgs::SanitizerArgs(const ToolChain &TC,
741741
AsanUseAfterScope = false;
742742
}
743743

744+
if (AllAddedKinds & HWAddress) {
745+
if (Arg *HwasanAbiArg =
746+
Args.getLastArg(options::OPT_fsanitize_hwaddress_abi_EQ)) {
747+
HwasanAbi = HwasanAbiArg->getValue();
748+
if (HwasanAbi != "platform" && HwasanAbi != "interceptor")
749+
D.Diag(clang::diag::err_drv_invalid_value)
750+
<< HwasanAbiArg->getAsString(Args) << HwasanAbi;
751+
} else {
752+
HwasanAbi = "interceptor";
753+
}
754+
}
755+
744756
if (AllAddedKinds & SafeStack) {
745757
// SafeStack runtime is built into the system on Fuchsia.
746758
SafeStackRuntime = !TC.getTriple().isOSFuchsia();
@@ -913,6 +925,11 @@ void SanitizerArgs::addArgs(const ToolChain &TC, const llvm::opt::ArgList &Args,
913925
if (AsanUseOdrIndicator)
914926
CmdArgs.push_back("-fsanitize-address-use-odr-indicator");
915927

928+
if (!HwasanAbi.empty()) {
929+
CmdArgs.push_back("-default-function-attr");
930+
CmdArgs.push_back(Args.MakeArgString("hwasan-abi=" + HwasanAbi));
931+
}
932+
916933
// MSan: Workaround for PR16386.
917934
// ASan: This is mainly to help LSan with cases such as
918935
// https://github.com/google/sanitizers/issues/373

‎clang/lib/Frontend/CompilerInvocation.cpp

+2
Original file line numberDiff line numberDiff line change
@@ -1319,6 +1319,8 @@ static bool ParseCodeGenArgs(CodeGenOptions &Opts, ArgList &Args, InputKind IK,
13191319

13201320
Opts.SpeculativeLoadHardening = Args.hasArg(OPT_mspeculative_load_hardening);
13211321

1322+
Opts.DefaultFunctionAttrs = Args.getAllArgValues(OPT_default_function_attr);
1323+
13221324
return Success;
13231325
}
13241326

‎clang/test/CodeGen/default-function-attr.c

+6
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
// RUN: %clang_cc1 -default-function-attr foo=bar -emit-llvm %s -o - | FileCheck %s
2+
3+
// CHECK: define void @foo() #[[X:[0-9]+]]
4+
void foo() {}
5+
6+
// CHECK: attributes #[[X]] = {{.*}} "foo"="bar"

‎clang/test/Driver/fsanitize.c

+8
Original file line numberDiff line numberDiff line change
@@ -837,3 +837,11 @@
837837
//
838838
// RUN: %clang -target x86_64-linux-gnu -fsanitize=scudo,kernel-memory %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-SCUDO-KMSAN
839839
// CHECK-SCUDO-KMSAN: error: invalid argument '-fsanitize=kernel-memory' not allowed with '-fsanitize=scudo'
840+
841+
// RUN: %clang -target x86_64-linux-gnu -fsanitize=hwaddress %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-HWASAN-INTERCEPTOR-ABI
842+
// RUN: %clang -target x86_64-linux-gnu -fsanitize=hwaddress -fsanitize-hwaddress-abi=interceptor %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-HWASAN-INTERCEPTOR-ABI
843+
// RUN: %clang -target x86_64-linux-gnu -fsanitize=hwaddress -fsanitize-hwaddress-abi=platform %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-HWASAN-PLATFORM-ABI
844+
// RUN: %clang -target x86_64-linux-gnu -fsanitize=hwaddress -fsanitize-hwaddress-abi=foo %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-HWASAN-FOO-ABI
845+
// CHECK-HWASAN-INTERCEPTOR-ABI: "-default-function-attr" "hwasan-abi=interceptor"
846+
// CHECK-HWASAN-PLATFORM-ABI: "-default-function-attr" "hwasan-abi=platform"
847+
// CHECK-HWASAN-FOO-ABI: error: invalid value 'foo' in '-fsanitize-hwaddress-abi=foo'

‎compiler-rt/lib/hwasan/hwasan_interceptors.cc

-30
Original file line numberDiff line numberDiff line change
@@ -217,35 +217,6 @@ INTERCEPTOR_ALIAS(void, malloc_stats, void);
217217
#endif
218218
#endif // HWASAN_WITH_INTERCEPTORS
219219

220-
221-
#if HWASAN_WITH_INTERCEPTORS
222-
extern "C" int pthread_attr_init(void *attr);
223-
extern "C" int pthread_attr_destroy(void *attr);
224-
225-
struct ThreadStartArg {
226-
thread_callback_t callback;
227-
void *param;
228-
};
229-
230-
static void *HwasanThreadStartFunc(void *arg) {
231-
__hwasan_thread_enter();
232-
ThreadStartArg A = *reinterpret_cast<ThreadStartArg*>(arg);
233-
UnmapOrDie(arg, GetPageSizeCached());
234-
return A.callback(A.param);
235-
}
236-
237-
INTERCEPTOR(int, pthread_create, void *th, void *attr, void *(*callback)(void*),
238-
void * param) {
239-
ScopedTaggingDisabler disabler;
240-
ThreadStartArg *A = reinterpret_cast<ThreadStartArg *> (MmapOrDie(
241-
GetPageSizeCached(), "pthread_create"));
242-
*A = {callback, param};
243-
int res = REAL(pthread_create)(UntagPtr(th), UntagPtr(attr),
244-
&HwasanThreadStartFunc, A);
245-
return res;
246-
}
247-
#endif // HWASAN_WITH_INTERCEPTORS
248-
249220
static void BeforeFork() {
250221
StackDepotLockAll();
251222
}
@@ -285,7 +256,6 @@ void InitializeInterceptors() {
285256
INTERCEPT_FUNCTION(fork);
286257

287258
#if HWASAN_WITH_INTERCEPTORS
288-
INTERCEPT_FUNCTION(pthread_create);
289259
INTERCEPT_FUNCTION(realloc);
290260
INTERCEPT_FUNCTION(free);
291261
#endif

‎compiler-rt/lib/hwasan/hwasan_linux.cc

+6-1
Original file line numberDiff line numberDiff line change
@@ -302,7 +302,12 @@ void AndroidTestTlsSlot() {}
302302
#endif
303303

304304
Thread *GetCurrentThread() {
305-
auto *R = (StackAllocationsRingBuffer*)GetCurrentThreadLongPtr();
305+
uptr *ThreadLong = GetCurrentThreadLongPtr();
306+
#if HWASAN_WITH_INTERCEPTORS
307+
if (!*ThreadLong)
308+
__hwasan_thread_enter();
309+
#endif
310+
auto *R = (StackAllocationsRingBuffer *)ThreadLong;
306311
return hwasanThreadList().GetThreadByBufferAddress((uptr)(R->Next()));
307312
}
308313

‎llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp

+27-2
Original file line numberDiff line numberDiff line change
@@ -264,6 +264,7 @@ class HWAddressSanitizer : public FunctionPass {
264264

265265
Function *HwasanTagMemoryFunc;
266266
Function *HwasanGenerateTagFunc;
267+
Function *HwasanThreadEnterFunc;
267268

268269
Constant *ShadowGlobal;
269270

@@ -391,6 +392,9 @@ void HWAddressSanitizer::initializeCallbacks(Module &M) {
391392
HWAsanMemset = checkSanitizerInterfaceFunction(M.getOrInsertFunction(
392393
MemIntrinCallbackPrefix + "memset", IRB.getInt8PtrTy(),
393394
IRB.getInt8PtrTy(), IRB.getInt32Ty(), IntptrTy));
395+
396+
HwasanThreadEnterFunc = checkSanitizerInterfaceFunction(
397+
M.getOrInsertFunction("__hwasan_thread_enter", IRB.getVoidTy()));
394398
}
395399

396400
Value *HWAddressSanitizer::getDynamicShadowNonTls(IRBuilder<> &IRB) {
@@ -806,14 +810,35 @@ Value *HWAddressSanitizer::emitPrologue(IRBuilder<> &IRB,
806810
Value *SlotPtr = getHwasanThreadSlotPtr(IRB, IntptrTy);
807811
assert(SlotPtr);
808812

809-
Value *ThreadLong = IRB.CreateLoad(SlotPtr);
813+
Instruction *ThreadLong = IRB.CreateLoad(SlotPtr);
814+
815+
Function *F = IRB.GetInsertBlock()->getParent();
816+
if (F->getFnAttribute("hwasan-abi").getValueAsString() == "interceptor") {
817+
Value *ThreadLongEqZero =
818+
IRB.CreateICmpEQ(ThreadLong, ConstantInt::get(IntptrTy, 0));
819+
auto *Br = cast<BranchInst>(SplitBlockAndInsertIfThen(
820+
ThreadLongEqZero, cast<Instruction>(ThreadLongEqZero)->getNextNode(),
821+
false, MDBuilder(*C).createBranchWeights(1, 100000)));
822+
823+
IRB.SetInsertPoint(Br);
824+
// FIXME: This should call a new runtime function with a custom calling
825+
// convention to avoid needing to spill all arguments here.
826+
IRB.CreateCall(HwasanThreadEnterFunc);
827+
LoadInst *ReloadThreadLong = IRB.CreateLoad(SlotPtr);
828+
829+
IRB.SetInsertPoint(&*Br->getSuccessor(0)->begin());
830+
PHINode *ThreadLongPhi = IRB.CreatePHI(IntptrTy, 2);
831+
ThreadLongPhi->addIncoming(ThreadLong, ThreadLong->getParent());
832+
ThreadLongPhi->addIncoming(ReloadThreadLong, ReloadThreadLong->getParent());
833+
ThreadLong = ThreadLongPhi;
834+
}
835+
810836
// Extract the address field from ThreadLong. Unnecessary on AArch64 with TBI.
811837
Value *ThreadLongMaybeUntagged =
812838
TargetTriple.isAArch64() ? ThreadLong : untagPointer(IRB, ThreadLong);
813839

814840
if (WithFrameRecord) {
815841
// Prepare ring buffer data.
816-
Function *F = IRB.GetInsertBlock()->getParent();
817842
auto PC = IRB.CreatePtrToInt(F, IntptrTy);
818843
auto GetStackPointerFn =
819844
Intrinsic::getDeclaration(F->getParent(), Intrinsic::frameaddress);

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

+25
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
; RUN: opt -S -hwasan < %s | FileCheck %s
2+
3+
target triple = "x86_64-unknown-linux-gnu"
4+
5+
declare void @bar([16 x i32]* %p)
6+
7+
define void @foo() sanitize_hwaddress "hwasan-abi"="interceptor" {
8+
; CHECK: [[LOAD:%[^ ]*]] = load i64, i64* @__hwasan_tls
9+
; CHECK: [[ICMP:%[^ ]*]] = icmp eq i64 [[LOAD]], 0
10+
; CHECK: br i1 [[ICMP]], label %[[INIT:[^,]*]], label %[[CONT:[^,]*]], !prof [[PROF:![0-9]+]]
11+
12+
; CHECK: [[INIT]]:
13+
; CHECK: call void @__hwasan_thread_enter()
14+
; CHECK: [[RELOAD:%[^ ]*]] = load i64, i64* @__hwasan_tls
15+
; CHECK: br label %[[CONT]]
16+
17+
; CHECK: [[CONT]]:
18+
; CHECK: phi i64 [ [[LOAD]], %0 ], [ [[RELOAD]], %[[INIT]] ]
19+
20+
%p = alloca [16 x i32]
21+
call void @bar([16 x i32]* %p)
22+
ret void
23+
}
24+
25+
; CHECK: [[PROF]] = !{!"branch_weights", i32 1, i32 100000}

0 commit comments

Comments
 (0)