From f1366c552480f7c6b2b46b03e19bb798b3a47c66 Mon Sep 17 00:00:00 2001 From: Peter Collingbourne Date: Thu, 22 Aug 2013 20:08:08 +0000 Subject: [PATCH] DataFlowSanitizer: Prefix the name of each instrumented function with "dfs$". DFSan changes the ABI of each function in the module. This makes it possible for a function with the native ABI to be called with the instrumented ABI, or vice versa, thus possibly invoking undefined behavior. A simple way of statically detecting instances of this problem is to prepend the prefix "dfs$" to the name of each instrumented-ABI function. This will not catch every such problem; in particular function pointers passed across the instrumented-native barrier cannot be used on the other side. These problems could potentially be caught dynamically. Differential Revision: http://llvm-reviews.chandlerc.com/D1373 git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@189052 91177308-0d34-0410-b5e6-96231b3b80d8 --- .../Instrumentation/DataFlowSanitizer.cpp | 58 ++++++++++++++++--- .../DataFlowSanitizer/abilist.ll | 4 +- .../DataFlowSanitizer/args-unreachable-bb.ll | 4 +- .../DataFlowSanitizer/arith.ll | 10 ++-- .../Instrumentation/DataFlowSanitizer/call.ll | 4 +- .../DataFlowSanitizer/debug-nonzero-labels.ll | 4 +- .../Instrumentation/DataFlowSanitizer/load.ll | 8 +-- .../DataFlowSanitizer/memset.ll | 2 +- .../DataFlowSanitizer/prefix-rename.ll | 14 +++++ .../DataFlowSanitizer/store.ll | 8 +-- 10 files changed, 87 insertions(+), 29 deletions(-) create mode 100644 test/Instrumentation/DataFlowSanitizer/prefix-rename.ll diff --git a/lib/Transforms/Instrumentation/DataFlowSanitizer.cpp b/lib/Transforms/Instrumentation/DataFlowSanitizer.cpp index 7159cc04994..9a469117514 100644 --- a/lib/Transforms/Instrumentation/DataFlowSanitizer.cpp +++ b/lib/Transforms/Instrumentation/DataFlowSanitizer.cpp @@ -179,11 +179,13 @@ class DataFlowSanitizer : public ModulePass { Value *getShadowAddress(Value *Addr, Instruction *Pos); Value *combineShadows(Value *V1, Value *V2, Instruction *Pos); - bool isInstrumented(Function *F); + bool isInstrumented(const Function *F); + bool isInstrumented(const GlobalAlias *GA); FunctionType *getArgsFunctionType(FunctionType *T); FunctionType *getCustomFunctionType(FunctionType *T); InstrumentedABI getInstrumentedABI(); WrapperKind getWrapperKind(Function *F); + void addGlobalNamePrefix(GlobalValue *GV); public: DataFlowSanitizer(StringRef ABIListFile = StringRef(), @@ -343,10 +345,14 @@ bool DataFlowSanitizer::doInitialization(Module &M) { return true; } -bool DataFlowSanitizer::isInstrumented(Function *F) { +bool DataFlowSanitizer::isInstrumented(const Function *F) { return !ABIList->isIn(*F, "uninstrumented"); } +bool DataFlowSanitizer::isInstrumented(const GlobalAlias *GA) { + return !ABIList->isIn(*GA, "uninstrumented"); +} + DataFlowSanitizer::InstrumentedABI DataFlowSanitizer::getInstrumentedABI() { return ClArgsABI ? IA_Args : IA_TLS; } @@ -362,6 +368,25 @@ DataFlowSanitizer::WrapperKind DataFlowSanitizer::getWrapperKind(Function *F) { return WK_Warning; } +void DataFlowSanitizer::addGlobalNamePrefix(GlobalValue *GV) { + std::string GVName = GV->getName(), Prefix = "dfs$"; + GV->setName(Prefix + GVName); + + // Try to change the name of the function in module inline asm. We only do + // this for specific asm directives, currently only ".symver", to try to avoid + // corrupting asm which happens to contain the symbol name as a substring. + // Note that the substitution for .symver assumes that the versioned symbol + // also has an instrumented name. + std::string Asm = GV->getParent()->getModuleInlineAsm(); + std::string SearchStr = ".symver " + GVName + ","; + size_t Pos = Asm.find(SearchStr); + if (Pos != std::string::npos) { + Asm.replace(Pos, SearchStr.size(), + ".symver " + Prefix + GVName + "," + Prefix); + GV->getParent()->setModuleInlineAsm(Asm); + } +} + bool DataFlowSanitizer::runOnModule(Module &M) { if (!DL) return false; @@ -415,6 +440,21 @@ bool DataFlowSanitizer::runOnModule(Module &M) { FnsToInstrument.push_back(&*i); } + // Give function aliases prefixes when necessary. + for (Module::alias_iterator i = M.alias_begin(), e = M.alias_end(); i != e;) { + GlobalAlias *GA = &*i; + ++i; + // Don't stop on weak. We assume people aren't playing games with the + // instrumentedness of overridden weak aliases. + if (Function *F = dyn_cast( + GA->resolveAliasedGlobal(/*stopOnWeak=*/false))) { + bool GAInst = isInstrumented(GA), FInst = isInstrumented(F); + if (GAInst && FInst) { + addGlobalNamePrefix(GA); + } + } + } + AttrBuilder B; B.addAttribute(Attribute::ReadOnly).addAttribute(Attribute::ReadNone); ReadOnlyNoneAttrs = AttributeSet::get(*Ctx, AttributeSet::FunctionIndex, B); @@ -427,12 +467,13 @@ bool DataFlowSanitizer::runOnModule(Module &M) { Function &F = **i; FunctionType *FT = F.getFunctionType(); - if (FT->getNumParams() == 0 && !FT->isVarArg() && - FT->getReturnType()->isVoidTy()) - continue; + bool IsZeroArgsVoidRet = (FT->getNumParams() == 0 && !FT->isVarArg() && + FT->getReturnType()->isVoidTy()); if (isInstrumented(&F)) { - if (getInstrumentedABI() == IA_Args) { + // Instrumented functions get a 'dfs$' prefix. This allows us to more + // easily identify cases of mismatching ABIs. + if (getInstrumentedABI() == IA_Args && !IsZeroArgsVoidRet) { FunctionType *NewFT = getArgsFunctionType(FT); Function *NewF = Function::Create(NewFT, F.getLinkage(), "", &M); NewF->copyAttributesFrom(&F); @@ -463,13 +504,16 @@ bool DataFlowSanitizer::runOnModule(Module &M) { NewF->takeName(&F); F.eraseFromParent(); *i = NewF; + addGlobalNamePrefix(NewF); + } else { + addGlobalNamePrefix(&F); } // Hopefully, nobody will try to indirectly call a vararg // function... yet. } else if (FT->isVarArg()) { UnwrappedFnMap[&F] = &F; *i = 0; - } else { + } else if (!IsZeroArgsVoidRet || getWrapperKind(&F) == WK_Custom) { // Build a wrapper function for F. The wrapper simply calls F, and is // added to FnsToInstrument so that any instrumentation according to its // WrapperKind is done in the second pass below. diff --git a/test/Instrumentation/DataFlowSanitizer/abilist.ll b/test/Instrumentation/DataFlowSanitizer/abilist.ll index bf550f5d476..33432212128 100644 --- a/test/Instrumentation/DataFlowSanitizer/abilist.ll +++ b/test/Instrumentation/DataFlowSanitizer/abilist.ll @@ -16,7 +16,7 @@ declare void @custom1(i32 %a, i32 %b) declare i32 @custom2(i32 %a, i32 %b) -; CHECK: @f +; CHECK: @"dfs$f" define void @f() { ; CHECK: %[[LABELRETURN:.*]] = alloca i16 @@ -37,7 +37,7 @@ define void @f() { ; CHECK: insertvalue {{.*}}[[RVSHADOW]], 1 ; CHECK: ret { i32, i16 } -; CHECK: @g +; CHECK: @"dfs$g" define i32 (i32, i32)* @g() { ; CHECK: ret {{.*}} @"dfsw$custom2" ret i32 (i32, i32)* @custom2 diff --git a/test/Instrumentation/DataFlowSanitizer/args-unreachable-bb.ll b/test/Instrumentation/DataFlowSanitizer/args-unreachable-bb.ll index b91f321ec74..a699f7523c1 100644 --- a/test/Instrumentation/DataFlowSanitizer/args-unreachable-bb.ll +++ b/test/Instrumentation/DataFlowSanitizer/args-unreachable-bb.ll @@ -1,7 +1,7 @@ ; RUN: opt < %s -dfsan -verify -dfsan-args-abi -S | FileCheck %s target datalayout = "e-p:64:64:64-i1:8:8-i8:8:8-i16:16:16-i32:32:32-i64:64:64-f32:32:32-f64:64:64-v64:64:64-v128:128:128-a0:0:64-s0:64:64-f80:128:128-n8:16:32:64-S128" -; CHECK-LABEL: @unreachable_bb1 +; CHECK-LABEL: @"dfs$unreachable_bb1" define i8 @unreachable_bb1() { ; CHECK: ret { i8, i16 } { i8 1, i16 0 } ; CHECK-NOT: bb2: @@ -21,7 +21,7 @@ bb4: declare void @abort() noreturn -; CHECK-LABEL: @unreachable_bb2 +; CHECK-LABEL: @"dfs$unreachable_bb2" define i8 @unreachable_bb2() { call void @abort() noreturn ; CHECK-NOT: i8 12 diff --git a/test/Instrumentation/DataFlowSanitizer/arith.ll b/test/Instrumentation/DataFlowSanitizer/arith.ll index ecb77a26008..d75c396e141 100644 --- a/test/Instrumentation/DataFlowSanitizer/arith.ll +++ b/test/Instrumentation/DataFlowSanitizer/arith.ll @@ -2,7 +2,7 @@ target datalayout = "e-p:64:64:64-i1:8:8-i8:8:8-i16:16:16-i32:32:32-i64:64:64-f32:32:32-f64:64:64-v64:64:64-v128:128:128-a0:0:64-s0:64:64-f80:128:128-n8:16:32:64-S128" define i8 @add(i8 %a, i8 %b) { - ; CHECK: @add + ; CHECK: @"dfs$add" ; CHECK: load{{.*}}__dfsan_arg_tls ; CHECK: load{{.*}}__dfsan_arg_tls ; CHECK: call{{.*}}__dfsan_union @@ -14,7 +14,7 @@ define i8 @add(i8 %a, i8 %b) { } define i8 @sub(i8 %a, i8 %b) { - ; CHECK: @sub + ; CHECK: @"dfs$sub" ; CHECK: load{{.*}}__dfsan_arg_tls ; CHECK: load{{.*}}__dfsan_arg_tls ; CHECK: call{{.*}}__dfsan_union @@ -26,7 +26,7 @@ define i8 @sub(i8 %a, i8 %b) { } define i8 @mul(i8 %a, i8 %b) { - ; CHECK: @mul + ; CHECK: @"dfs$mul" ; CHECK: load{{.*}}__dfsan_arg_tls ; CHECK: load{{.*}}__dfsan_arg_tls ; CHECK: call{{.*}}__dfsan_union @@ -38,7 +38,7 @@ define i8 @mul(i8 %a, i8 %b) { } define i8 @sdiv(i8 %a, i8 %b) { - ; CHECK: @sdiv + ; CHECK: @"dfs$sdiv" ; CHECK: load{{.*}}__dfsan_arg_tls ; CHECK: load{{.*}}__dfsan_arg_tls ; CHECK: call{{.*}}__dfsan_union @@ -50,7 +50,7 @@ define i8 @sdiv(i8 %a, i8 %b) { } define i8 @udiv(i8 %a, i8 %b) { - ; CHECK: @udiv + ; CHECK: @"dfs$udiv" ; CHECK: load{{.*}}__dfsan_arg_tls ; CHECK: load{{.*}}__dfsan_arg_tls ; CHECK: call{{.*}}__dfsan_union diff --git a/test/Instrumentation/DataFlowSanitizer/call.ll b/test/Instrumentation/DataFlowSanitizer/call.ll index c374246d4c6..813f4c1e76f 100644 --- a/test/Instrumentation/DataFlowSanitizer/call.ll +++ b/test/Instrumentation/DataFlowSanitizer/call.ll @@ -7,10 +7,10 @@ target datalayout = "e-p:64:64:64-i1:8:8-i8:8:8-i16:16:16-i32:32:32-i64:64:64-f3 declare i32 @f(i32) declare float @llvm.sqrt.f32(float) -; CHECK: @call +; CHECK: @"dfs$call" define i32 @call() { ; CHECK: store{{.*}}__dfsan_arg_tls - ; CHECK: call{{.*}}@f + ; CHECK: call{{.*}}@"dfs$f" ; CHECK: load{{.*}}__dfsan_retval_tls %r = call i32 @f(i32 0) diff --git a/test/Instrumentation/DataFlowSanitizer/debug-nonzero-labels.ll b/test/Instrumentation/DataFlowSanitizer/debug-nonzero-labels.ll index 4329fd112d7..6bcd5c5f0c1 100644 --- a/test/Instrumentation/DataFlowSanitizer/debug-nonzero-labels.ll +++ b/test/Instrumentation/DataFlowSanitizer/debug-nonzero-labels.ll @@ -3,14 +3,14 @@ target datalayout = "e-p:64:64:64-i1:8:8-i8:8:8-i16:16:16-i32:32:32-i64:64:64-f3 declare i32 @g() -; CHECK: define { i32, i16 } @f(i32, i16) +; CHECK: define { i32, i16 } @"dfs$f"(i32, i16) define i32 @f(i32) { ; CHECK: [[LOCALLABELALLOCA:%.*]] = alloca i16 ; CHECK: [[ARGCMP:%.*]] = icmp ne i16 %1, 0 ; CHECK: br i1 [[ARGCMP]] %i = alloca i32 store i32 %0, i32* %i - ; CHECK: [[CALL:%.*]] = call { i32, i16 } @g() + ; CHECK: [[CALL:%.*]] = call { i32, i16 } @"dfs$g"() ; CHECK: [[CALLLABEL:%.*]] = extractvalue { i32, i16 } [[CALL]], 1 ; CHECK: [[CALLCMP:%.*]] = icmp ne i16 [[CALLLABEL]], 0 ; CHECK: br i1 [[CALLCMP]] diff --git a/test/Instrumentation/DataFlowSanitizer/load.ll b/test/Instrumentation/DataFlowSanitizer/load.ll index d12a17a478b..6431213f8be 100644 --- a/test/Instrumentation/DataFlowSanitizer/load.ll +++ b/test/Instrumentation/DataFlowSanitizer/load.ll @@ -2,7 +2,7 @@ target datalayout = "e-p:64:64:64-i1:8:8-i8:8:8-i16:16:16-i32:32:32-i64:64:64-f32:32:32-f64:64:64-v64:64:64-v128:128:128-a0:0:64-s0:64:64-f80:128:128-n8:16:32:64-S128" define i8 @load8(i8* %p) { - ; CHECK: @load8 + ; CHECK: @"dfs$load8" ; CHECK: ptrtoint ; CHECK: and ; CHECK: mul @@ -15,7 +15,7 @@ define i8 @load8(i8* %p) { } define i16 @load16(i16* %p) { - ; CHECK: @load16 + ; CHECK: @"dfs$load16" ; CHECK: ptrtoint ; CHECK: and ; CHECK: mul @@ -31,7 +31,7 @@ define i16 @load16(i16* %p) { } define i32 @load32(i32* %p) { - ; CHECK: @load32 + ; CHECK: @"dfs$load32" ; CHECK: ptrtoint ; CHECK: and ; CHECK: mul @@ -54,7 +54,7 @@ define i32 @load32(i32* %p) { } define i64 @load64(i64* %p) { - ; CHECK: @load64 + ; CHECK: @"dfs$load64" ; CHECK: ptrtoint ; CHECK: and ; CHECK: mul diff --git a/test/Instrumentation/DataFlowSanitizer/memset.ll b/test/Instrumentation/DataFlowSanitizer/memset.ll index 2cc25db96fc..062ef1ac9f4 100644 --- a/test/Instrumentation/DataFlowSanitizer/memset.ll +++ b/test/Instrumentation/DataFlowSanitizer/memset.ll @@ -4,7 +4,7 @@ target datalayout = "e-p:64:64:64-i1:8:8-i8:8:8-i16:16:16-i32:32:32-i64:64:64-f3 declare void @llvm.memset.p0i8.i64(i8* nocapture, i8, i64, i32, i1) define void @ms(i8* %p, i8 %v) { - ; CHECK-LABEL: @ms(i8*, i8, i16, i16) + ; CHECK-LABEL: @"dfs$ms"(i8*, i8, i16, i16) ; CHECK: call void @__dfsan_set_label(i16 %3, i8* %0, i64 1) call void @llvm.memset.p0i8.i64(i8* %p, i8 %v, i64 1, i32 1, i1 1) ret void diff --git a/test/Instrumentation/DataFlowSanitizer/prefix-rename.ll b/test/Instrumentation/DataFlowSanitizer/prefix-rename.ll new file mode 100644 index 00000000000..1a5646074d2 --- /dev/null +++ b/test/Instrumentation/DataFlowSanitizer/prefix-rename.ll @@ -0,0 +1,14 @@ +; RUN: opt < %s -dfsan -S | FileCheck %s +; RUN: opt < %s -dfsan -dfsan-args-abi -S | FileCheck %s +target datalayout = "e-p:64:64:64-i1:8:8-i8:8:8-i16:16:16-i32:32:32-i64:64:64-f32:32:32-f64:64:64-v64:64:64-v128:128:128-a0:0:64-s0:64:64-f80:128:128-n8:16:32:64-S128" + +; CHECK: module asm ".symver dfs$f1,dfs$f@@version1" +module asm ".symver f1,f@@version1" + +; CHECK: @"dfs$f2" = alias {{.*}} @"dfs$f1" +@f2 = alias void ()* @f1 + +; CHECK: define void @"dfs$f1" +define void @f1() { + ret void +} diff --git a/test/Instrumentation/DataFlowSanitizer/store.ll b/test/Instrumentation/DataFlowSanitizer/store.ll index 0c0aa496813..95091777a32 100644 --- a/test/Instrumentation/DataFlowSanitizer/store.ll +++ b/test/Instrumentation/DataFlowSanitizer/store.ll @@ -2,7 +2,7 @@ target datalayout = "e-p:64:64:64-i1:8:8-i8:8:8-i16:16:16-i32:32:32-i64:64:64-f32:32:32-f64:64:64-v64:64:64-v128:128:128-a0:0:64-s0:64:64-f80:128:128-n8:16:32:64-S128" define void @store8(i8 %v, i8* %p) { - ; CHECK: @store8 + ; CHECK: @"dfs$store8" ; CHECK: load{{.*}}__dfsan_arg_tls ; CHECK: ptrtoint ; CHECK: and @@ -16,7 +16,7 @@ define void @store8(i8 %v, i8* %p) { } define void @store16(i16 %v, i16* %p) { - ; CHECK: @store16 + ; CHECK: @"dfs$store16" ; CHECK: load{{.*}}__dfsan_arg_tls ; CHECK: ptrtoint ; CHECK: and @@ -32,7 +32,7 @@ define void @store16(i16 %v, i16* %p) { } define void @store32(i32 %v, i32* %p) { - ; CHECK: @store32 + ; CHECK: @"dfs$store32" ; CHECK: load{{.*}}__dfsan_arg_tls ; CHECK: ptrtoint ; CHECK: and @@ -52,7 +52,7 @@ define void @store32(i32 %v, i32* %p) { } define void @store64(i64 %v, i64* %p) { - ; CHECK: @store64 + ; CHECK: @"dfs$store64" ; CHECK: load{{.*}}__dfsan_arg_tls ; CHECK: ptrtoint ; CHECK: and