[ASan] Use metadata to pass source-level information from Clang to ASan.

Instead of creating global variables for source locations and global names,
just create metadata nodes and strings. They will be transformed into actual
globals in the instrumentation pass (if necessary). This approach is more
flexible:
1) we don't have to ensure that our custom globals survive all the optimizations
2) if globals are discarded for some reason, we will simply ignore metadata for them
   and won't have to erase corresponding globals
3) metadata for source locations can be reused for other purposes: e.g. we may
   attach source location metadata to alloca instructions and provide better descriptions
   for stack variables in ASan error reports.

No functionality change.

llvm-svn: 214604

diff --git a/clang/lib/CodeGen/SanitizerMetadata.cpp b/clang/lib/CodeGen/SanitizerMetadata.cpp
index 4619152..dd8c133 100644 (file)
--- a/clang/lib/CodeGen/SanitizerMetadata.cpp
+++ b/clang/lib/CodeGen/SanitizerMetadata.cpp
@@ -28,35 +28,15 @@ void SanitizerMetadata::reportGlobalToASan(llvm::GlobalVariable *GV,
   IsDynInit &= !CGM.getSanitizerBlacklist().isIn(*GV, "init");
   IsBlacklisted |= CGM.getSanitizerBlacklist().isIn(*GV);
 
-  llvm::GlobalVariable *LocDescr = nullptr;
-  llvm::GlobalVariable *GlobalName = nullptr;
+  llvm::Value *LocDescr = nullptr;
+  llvm::Value *GlobalName = nullptr;
   llvm::LLVMContext &VMContext = CGM.getLLVMContext();
   if (!IsBlacklisted) {
     // Don't generate source location and global name if it is blacklisted -
     // it won't be instrumented anyway.
-    PresumedLoc PLoc = CGM.getContext().getSourceManager().getPresumedLoc(Loc);
-    if (PLoc.isValid()) {
-      llvm::Constant *LocData[] = {
-          CGM.GetAddrOfConstantCString(PLoc.getFilename()),
-          llvm::ConstantInt::get(llvm::Type::getInt32Ty(VMContext),
-                                 PLoc.getLine()),
-          llvm::ConstantInt::get(llvm::Type::getInt32Ty(VMContext),
-                                 PLoc.getColumn()),
-      };
-      auto LocStruct = llvm::ConstantStruct::getAnon(LocData);
-      LocDescr = new llvm::GlobalVariable(
-          CGM.getModule(), LocStruct->getType(), true,
-          llvm::GlobalValue::PrivateLinkage, LocStruct, ".asan_loc_descr");
-      LocDescr->setUnnamedAddr(true);
-      // Add LocDescr to llvm.compiler.used, so that it won't be removed by
-      // the optimizer before the ASan instrumentation pass.
-      CGM.addCompilerUsedGlobal(LocDescr);
-    }
-    if (!Name.empty()) {
-      GlobalName = CGM.GetAddrOfConstantCString(Name);
-      // GlobalName shouldn't be removed by the optimizer.
-      CGM.addCompilerUsedGlobal(GlobalName);
-    }
+    LocDescr = getLocationMetadata(Loc);
+    if (!Name.empty())
+      GlobalName = llvm::MDString::get(VMContext, Name);
   }
 
   llvm::Value *GlobalMetadata[] = {
@@ -86,3 +66,17 @@ void SanitizerMetadata::disableSanitizerForGlobal(llvm::GlobalVariable *GV) {
   if (CGM.getLangOpts().Sanitize.Address)
     reportGlobalToASan(GV, SourceLocation(), "", false, true);
 }
+
+llvm::MDNode *SanitizerMetadata::getLocationMetadata(SourceLocation Loc) {
+  PresumedLoc PLoc = CGM.getContext().getSourceManager().getPresumedLoc(Loc);
+  if (!PLoc.isValid())
+    return nullptr;
+  llvm::LLVMContext &VMContext = CGM.getLLVMContext();
+  llvm::Value *LocMetadata[] = {
+      llvm::MDString::get(VMContext, PLoc.getFilename()),
+      llvm::ConstantInt::get(llvm::Type::getInt32Ty(VMContext), PLoc.getLine()),
+      llvm::ConstantInt::get(llvm::Type::getInt32Ty(VMContext),
+                             PLoc.getColumn()),
+  };
+  return llvm::MDNode::get(VMContext, LocMetadata);
+}

diff --git a/clang/lib/CodeGen/SanitizerMetadata.h b/clang/lib/CodeGen/SanitizerMetadata.h
index 3062b4a..6d66cbe 100644 (file)
--- a/clang/lib/CodeGen/SanitizerMetadata.h
+++ b/clang/lib/CodeGen/SanitizerMetadata.h
@@ -18,6 +18,7 @@
 
 namespace llvm {
 class GlobalVariable;
+class MDNode;
 }
 
 namespace clang {
@@ -40,6 +41,8 @@ public:
                           StringRef Name, bool IsDynInit = false,
                           bool IsBlacklisted = false);
   void disableSanitizerForGlobal(llvm::GlobalVariable *GV);
+private:
+  llvm::MDNode *getLocationMetadata(SourceLocation Loc);
 };
 }  // end namespace CodeGen
 }  // end namespace clang

diff --git a/clang/test/CodeGen/asan-globals.cpp b/clang/test/CodeGen/asan-globals.cpp
index d9ecc64..a9b9b5f 100644 (file)
--- a/clang/test/CodeGen/asan-globals.cpp
+++ b/clang/test/CodeGen/asan-globals.cpp
@@ -5,28 +5,24 @@
 // REQUIRES: shell
 
 int global;
-// CHECK: [[GLOBAL_LOC:@.asan_loc_descr[0-9]*]] = private unnamed_addr constant {{.*}} i32 [[@LINE-1]], i32 5
-// CHECK: [[GLOBAL_NAME:@.str[0-9]+]] = private unnamed_addr constant {{.*}} c"global\00"
 int dyn_init_global = global;
-// CHECK: [[DYN_INIT_LOC:@.asan_loc_descr[0-9]*]] = {{.*}} i32 [[@LINE-1]], i32 5
-// CHECK: [[DYN_INIT_NAME:@.str[0-9]+]] = private unnamed_addr constant {{.*}} c"dyn_init_global\00"
 int blacklisted_global;
 
 void func() {
   static int static_var = 0;
-  // CHECK: [[STATIC_LOC:@.asan_loc_descr[0-9]*]] = {{.*}} i32 [[@LINE-1]], i32 14
-  // CHECK: [[STATIC_NAME:@.str[0-9]+]] = private unnamed_addr constant {{.*}} c"static_var\00"
   const char *literal = "Hello, world!";
-  // CHECK: [[LITERAL_LOC:@.asan_loc_descr[0-9]*]] = {{.*}} i32 [[@LINE-1]], i32 25
-  // CHECK: [[LITERAL_NAME:@.str[0-9]+]] = private unnamed_addr constant {{.*}} c"<string literal>\00"
 }
 
 // CHECK: !llvm.asan.globals = !{![[GLOBAL:[0-9]+]], ![[DYN_INIT_GLOBAL:[0-9]+]], ![[BLACKLISTED_GLOBAL:[0-9]+]], ![[STATIC_VAR:[0-9]+]], ![[LITERAL:[0-9]+]]}
-// CHECK: ![[GLOBAL]] = metadata !{{{.*}} [[GLOBAL_LOC]], {{.*}} [[GLOBAL_NAME]], i1 false, i1 false}
-// CHECK: ![[DYN_INIT_GLOBAL]] = metadata !{{{.*}} [[DYN_INIT_LOC]], {{.*}} [[DYN_INIT_NAME]], i1 true, i1 false}
+// CHECK: ![[GLOBAL]] = metadata !{{{.*}} metadata ![[GLOBAL_LOC:[0-9]+]], metadata !"global", i1 false, i1 false}
+// CHECK: ![[GLOBAL_LOC]] = metadata !{metadata !"{{.*}}asan-globals.cpp", i32 7, i32 5}
+// CHECK: ![[DYN_INIT_GLOBAL]] = metadata !{{{.*}} metadata ![[DYN_INIT_LOC:[0-9]+]], metadata !"dyn_init_global", i1 true, i1 false}
+// CHECK: ![[DYN_INIT_LOC]] = metadata !{metadata !"{{.*}}asan-globals.cpp", i32 8, i32 5}
 // CHECK: ![[BLACKLISTED_GLOBAL]] = metadata !{{{.*}}, null, null, i1 false, i1 true}
-// CHECK: ![[STATIC_VAR]] = metadata !{{{.*}} [[STATIC_LOC]], {{.*}} [[STATIC_NAME]], i1 false, i1 false}
-// CHECK: ![[LITERAL]] = metadata !{{{.*}} [[LITERAL_LOC]], {{.*}} [[LITERAL_NAME]], i1 false, i1 false}
+// CHECK: ![[STATIC_VAR]] = metadata !{{{.*}} metadata ![[STATIC_LOC:[0-9]+]], metadata !"static_var", i1 false, i1 false}
+// CHECK: ![[STATIC_LOC]] = metadata !{metadata !"{{.*}}asan-globals.cpp", i32 12, i32 14}
+// CHECK: ![[LITERAL]] = metadata !{{{.*}} metadata ![[LITERAL_LOC:[0-9]+]], metadata !"<string literal>", i1 false, i1 false}
+// CHECK: ![[LITERAL_LOC]] = metadata !{metadata !"{{.*}}asan-globals.cpp", i32 13, i32 25}
 
 // BLACKLIST-SRC: !llvm.asan.globals = !{![[GLOBAL:[0-9]+]], ![[DYN_INIT_GLOBAL:[0-9]+]], ![[BLACKLISTED_GLOBAL:[0-9]+]], ![[STATIC_VAR:[0-9]+]], ![[LITERAL:[0-9]+]]}
 // BLACKLIST-SRC: ![[GLOBAL]] = metadata !{{{.*}} null, null, i1 false, i1 true}

diff --git a/compiler-rt/lib/asan/asan_globals.cc b/compiler-rt/lib/asan/asan_globals.cc
index a844201..dc1cb41 100644 (file)
--- a/compiler-rt/lib/asan/asan_globals.cc
+++ b/compiler-rt/lib/asan/asan_globals.cc
@@ -73,8 +73,13 @@ ALWAYS_INLINE void PoisonRedZones(const Global &g) {
 
 static void ReportGlobal(const Global &g, const char *prefix) {
   Report("%s Global[%p]: beg=%p size=%zu/%zu name=%s module=%s dyn_init=%zu\n",
-         prefix, &g, (void*)g.beg, g.size, g.size_with_redzone, g.name,
+         prefix, &g, (void *)g.beg, g.size, g.size_with_redzone, g.name,
          g.module_name, g.has_dynamic_init);
+  if (g.location) {
+    Report("  location (%p): name=%s[%p], %d %d\n", g.location,
+           g.location->filename, g.location->filename, g.location->line_no,
+           g.location->column_no);
+  }
 }
 
 bool DescribeAddressIfGlobal(uptr addr, uptr size) {

diff --git a/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp b/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
index ad3abed..02d68d4 100644 (file)
--- a/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
+++ b/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
@@ -212,15 +212,34 @@ STATISTIC(NumOptimizedAccessesToGlobalVar,
           "Number of optimized accesses to global vars");
 
 namespace {
+/// Frontend-provided metadata for source location.
+struct LocationMetadata {
+  StringRef Filename;
+  int LineNo;
+  int ColumnNo;
+
+  LocationMetadata() : Filename(), LineNo(0), ColumnNo(0) {}
+
+  bool empty() const { return Filename.empty(); }
+
+  void parse(MDNode *MDN) {
+    assert(MDN->getNumOperands() == 3);
+    MDString *MDFilename = cast<MDString>(MDN->getOperand(0));
+    Filename = MDFilename->getString();
+    LineNo = cast<ConstantInt>(MDN->getOperand(1))->getLimitedValue();
+    ColumnNo = cast<ConstantInt>(MDN->getOperand(2))->getLimitedValue();
+  }
+};
+
 /// Frontend-provided metadata for global variables.
 class GlobalsMetadata {
  public:
   struct Entry {
     Entry()
-        : SourceLoc(nullptr), Name(nullptr), IsDynInit(false),
+        : SourceLoc(), Name(), IsDynInit(false),
           IsBlacklisted(false) {}
-    GlobalVariable *SourceLoc;
-    GlobalVariable *Name;
+    LocationMetadata SourceLoc;
+    StringRef Name;
     bool IsDynInit;
     bool IsBlacklisted;
   };
@@ -244,15 +263,11 @@ class GlobalsMetadata {
       // We can already have an entry for GV if it was merged with another
       // global.
       Entry &E = Entries[GV];
-      if (Value *Loc = MDN->getOperand(1)) {
-        GlobalVariable *GVLoc = cast<GlobalVariable>(Loc);
-        E.SourceLoc = GVLoc;
-        addSourceLocationGlobal(GVLoc);
-      }
+      if (Value *Loc = MDN->getOperand(1))
+        E.SourceLoc.parse(cast<MDNode>(Loc));
       if (Value *Name = MDN->getOperand(2)) {
-        GlobalVariable *GVName = cast<GlobalVariable>(Name);
-        E.Name = GVName;
-        InstrumentationGlobals.insert(GVName);
+        MDString *MDName = cast<MDString>(Name);
+        E.Name = MDName->getString();
       }
       ConstantInt *IsDynInit = cast<ConstantInt>(MDN->getOperand(3));
       E.IsDynInit |= IsDynInit->isOne();
@@ -267,31 +282,9 @@ class GlobalsMetadata {
     return (Pos != Entries.end()) ? Pos->second : Entry();
   }
 
-  /// Check if the global was generated by the instrumentation
-  /// (we don't want to instrument it again in this case).
-  bool isInstrumentationGlobal(GlobalVariable *G) const {
-    return InstrumentationGlobals.count(G);
-  }
-
  private:
   bool inited_;
   DenseMap<GlobalVariable*, Entry> Entries;
-  // Globals generated by the frontend instrumentation.
-  DenseSet<GlobalVariable*> InstrumentationGlobals;
-
-  void addSourceLocationGlobal(GlobalVariable *SourceLocGV) {
-    // Source location global is a struct with layout:
-    // {
-    //    filename,
-    //    i32 line_number,
-    //    i32 column_number,
-    // }
-    InstrumentationGlobals.insert(SourceLocGV);
-    ConstantStruct *Contents =
-        cast<ConstantStruct>(SourceLocGV->getInitializer());
-    GlobalVariable *FilenameGV = cast<GlobalVariable>(Contents->getOperand(0));
-    InstrumentationGlobals.insert(FilenameGV);
-  }
 };
 
 /// This struct defines the shadow mapping using the rule:
@@ -616,6 +609,22 @@ static GlobalVariable *createPrivateGlobalForString(
   return GV;
 }
 
+/// \brief Create a global describing a source location.
+static GlobalVariable *createPrivateGlobalForSourceLoc(Module &M,
+                                                       LocationMetadata MD) {
+  Constant *LocData[] = {
+      createPrivateGlobalForString(M, MD.Filename, true),
+      ConstantInt::get(Type::getInt32Ty(M.getContext()), MD.LineNo),
+      ConstantInt::get(Type::getInt32Ty(M.getContext()), MD.ColumnNo),
+  };
+  auto LocStruct = ConstantStruct::getAnon(LocData);
+  auto GV = new GlobalVariable(M, LocStruct->getType(), true,
+                               GlobalValue::PrivateLinkage, LocStruct,
+                               kAsanGenPrefix);
+  GV->setUnnamedAddr(true);
+  return GV;
+}
+
 static bool GlobalWasGeneratedByAsan(GlobalVariable *G) {
   return G->getName().find(kAsanGenPrefix) == 0;
 }
@@ -920,7 +929,6 @@ bool AddressSanitizerModule::ShouldInstrumentGlobal(GlobalVariable *G) {
   DEBUG(dbgs() << "GLOBAL: " << *G << "\n");
 
   if (GlobalsMD.get(G).IsBlacklisted) return false;
-  if (GlobalsMD.isInstrumentationGlobal(G)) return false;
   if (!Ty->isSized()) return false;
   if (!G->hasInitializer()) return false;
   if (GlobalWasGeneratedByAsan(G)) return false;  // Our own global.
@@ -1062,11 +1070,11 @@ bool AddressSanitizerModule::InstrumentGlobals(IRBuilder<> &IRB, Module &M) {
     GlobalVariable *G = GlobalsToChange[i];
 
     auto MD = GlobalsMD.get(G);
-    // Create string holding the global name unless it was provided by
-    // the metadata.
-    GlobalVariable *Name =
-        MD.Name ? MD.Name : createPrivateGlobalForString(M, G->getName(),
-                                                         /*AllowMerging*/ true);
+    // Create string holding the global name (use global name from metadata
+    // if it's available, otherwise just write the name of global variable).
+    GlobalVariable *Name = createPrivateGlobalForString(
+        M, MD.Name.empty() ? G->getName() : MD.Name,
+        /*AllowMerging*/ true);
 
     PointerType *PtrTy = cast<PointerType>(G->getType());
     Type *Ty = PtrTy->getElementType();
@@ -1108,16 +1116,21 @@ bool AddressSanitizerModule::InstrumentGlobals(IRBuilder<> &IRB, Module &M) {
     NewGlobal->takeName(G);
     G->eraseFromParent();
 
+    Constant *SourceLoc;
+    if (!MD.SourceLoc.empty()) {
+      auto SourceLocGlobal = createPrivateGlobalForSourceLoc(M, MD.SourceLoc);
+      SourceLoc = ConstantExpr::getPointerCast(SourceLocGlobal, IntptrTy);
+    } else {
+      SourceLoc = ConstantInt::get(IntptrTy, 0);
+    }
+
     Initializers[i] = ConstantStruct::get(
         GlobalStructTy, ConstantExpr::getPointerCast(NewGlobal, IntptrTy),
         ConstantInt::get(IntptrTy, SizeInBytes),
         ConstantInt::get(IntptrTy, SizeInBytes + RightRedzoneSize),
         ConstantExpr::getPointerCast(Name, IntptrTy),
         ConstantExpr::getPointerCast(ModuleName, IntptrTy),
-        ConstantInt::get(IntptrTy, MD.IsDynInit),
-        MD.SourceLoc ? ConstantExpr::getPointerCast(MD.SourceLoc, IntptrTy)
-                     : ConstantInt::get(IntptrTy, 0),
-        NULL);
+        ConstantInt::get(IntptrTy, MD.IsDynInit), SourceLoc, NULL);
 
     if (ClInitializers && MD.IsDynInit)
       HasDynamicallyInitializedGlobals = true;

diff --git a/llvm/test/Instrumentation/AddressSanitizer/global_metadata.ll b/llvm/test/Instrumentation/AddressSanitizer/global_metadata.ll
index 4dcd53b..fd5a8c6 100644 (file)
--- a/llvm/test/Instrumentation/AddressSanitizer/global_metadata.ll
+++ b/llvm/test/Instrumentation/AddressSanitizer/global_metadata.ll
@@ -11,28 +11,18 @@ target triple = "x86_64-unknown-linux-gnu"
 @.str = private unnamed_addr constant [14 x i8] c"Hello, world!\00", align 1
 @llvm.global_ctors = appending global [1 x { i32, void ()*, i8* }] [{ i32, void ()*, i8* } { i32 65535, void ()* @_GLOBAL__sub_I_asan_globals.cpp, i8* null }]
 
-; Sanitizer location descriptors:
-@.str1 = private unnamed_addr constant [22 x i8] c"/tmp/asan-globals.cpp\00", align 1
-@.asan_loc_descr = private unnamed_addr constant { [22 x i8]*, i32, i32 } { [22 x i8]* @.str1, i32 5, i32 5 }
-@.asan_loc_descr1 = private unnamed_addr constant { [22 x i8]*, i32, i32 } { [22 x i8]* @.str1, i32 7, i32 5 }
-@.asan_loc_descr2 = private unnamed_addr constant { [22 x i8]*, i32, i32 } { [22 x i8]* @.str1, i32 12, i32 14 }
-@.asan_loc_descr4 = private unnamed_addr constant { [22 x i8]*, i32, i32 } { [22 x i8]* @.str1, i32 14, i32 25 }
-
-; Global names:
-@.str2 = private unnamed_addr constant [7 x i8] c"global\00", align 1
-@.str3 = private unnamed_addr constant [16 x i8] c"dyn_init_global\00", align 1
-@.str4 = private unnamed_addr constant [11 x i8] c"static_var\00", align 1
-@.str5 = private unnamed_addr constant [17 x i8] c"<string literal>\00", align 1
-
-; Check that globals were instrumented, but sanitizer location descriptors weren't:
+; Check that globals were instrumented:
 ; CHECK: @global = global { i32, [60 x i8] } zeroinitializer, align 32
 ; CHECK: @.str = internal unnamed_addr constant { [14 x i8], [50 x i8] } { [14 x i8] c"Hello, world!\00", [50 x i8] zeroinitializer }, align 32
-; CHECK: @.asan_loc_descr = private unnamed_addr constant { [22 x i8]*, i32, i32 } { [22 x i8]* @.str1, i32 5, i32 5 }
-; CHECK: @.str2 = private unnamed_addr constant [7 x i8] c"global\00", align 1
+
+; Check emitted location descriptions:
+; CHECK: [[VARNAME:@__asan_gen_[0-9]+]] = private unnamed_addr constant [7 x i8] c"global\00", align 1
+; CHECK: [[FILENAME:@__asan_gen_[0-9]+]] = private unnamed_addr constant [22 x i8] c"/tmp/asan-globals.cpp\00", align 1
+; CHECK: [[LOCDESCR:@__asan_gen_[0-9]+]] = private unnamed_addr constant { [22 x i8]*, i32, i32 } { [22 x i8]* [[FILENAME]], i32 5, i32 5 }
 
 ; Check that location decriptors and global names were passed into __asan_register_globals:
-; CHECK: i64 ptrtoint ([7 x i8]* @.str2 to i64)
-; CHECK: i64 ptrtoint ({ [22 x i8]*, i32, i32 }* @.asan_loc_descr to i64)
+; CHECK: i64 ptrtoint ([7 x i8]* [[VARNAME]] to i64)
+; CHECK: i64 ptrtoint ({ [22 x i8]*, i32, i32 }* [[LOCDESCR]] to i64)
 
 ; Function Attrs: nounwind sanitize_address
 define internal void @__cxx_global_var_init() #0 section ".text.startup" {
@@ -63,9 +53,15 @@ attributes #1 = { nounwind sanitize_address "less-precise-fpmad"="false" "no-fra
 !llvm.asan.globals = !{!0, !1, !2, !3, !4}
 !llvm.ident = !{!5}
 
-!0 = metadata !{i32* @global, { [22 x i8]*, i32, i32 }* @.asan_loc_descr, [7 x i8]* @.str2, i1 false, i1 false}
-!1 = metadata !{i32* @dyn_init_global, { [22 x i8]*, i32, i32 }* @.asan_loc_descr1, [16 x i8]* @.str3, i1 true, i1 false}
+!0 = metadata !{i32* @global, metadata !6, metadata !"global", i1 false, i1 false}
+!1 = metadata !{i32* @dyn_init_global, metadata !7, metadata !"dyn_init_global", i1 true, i1 false}
 !2 = metadata !{i32* @blacklisted_global, null, null, i1 false, i1 true}
-!3 = metadata !{i32* @_ZZ4funcvE10static_var, { [22 x i8]*, i32, i32 }* @.asan_loc_descr2, [11 x i8]* @.str4, i1 false, i1 false}
-!4 = metadata !{[14 x i8]* @.str, { [22 x i8]*, i32, i32 }* @.asan_loc_descr4, [17 x i8]* @.str5, i1 false, i1 false}
+!3 = metadata !{i32* @_ZZ4funcvE10static_var, metadata !8, metadata !"static_var", i1 false, i1 false}
+!4 = metadata !{[14 x i8]* @.str, metadata !9, metadata !"<string literal>", i1 false, i1 false}
+
 !5 = metadata !{metadata !"clang version 3.5.0 (211282)"}
+
+!6 = metadata !{metadata !"/tmp/asan-globals.cpp", i32 5, i32 5}
+!7 = metadata !{metadata !"/tmp/asan-globals.cpp", i32 7, i32 5}
+!8 = metadata !{metadata !"/tmp/asan-globals.cpp", i32 12, i32 14}
+!9 = metadata !{metadata !"/tmp/asan-globals.cpp", i32 14, i32 25}