From f1366c552480f7c6b2b46b03e19bb798b3a47c66 Mon Sep 17 00:00:00 2001
From: Peter Collingbourne <peter@pcc.me.uk>
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<Function>(
+            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
-- 
2.34.1