[ASan] Improve blacklisting of global variables.

This commit changes the way we blacklist global variables in ASan.
Now the global is excluded from instrumentation (either regular
bounds checking, or initialization-order checking) if:

1) Global is explicitly blacklisted by its mangled name.
This part is left unchanged.

2) SourceLocation of a global is in blacklisted source file.
This changes the old behavior, where instead of looking at the
SourceLocation of a variable we simply considered llvm::Module
identifier. This was wrong, as identifier may not correspond to
the file name, and we incorrectly disabled instrumentation
for globals coming from #include'd files.

3) Global is blacklisted by type.
Now we build the type of a global variable using Clang machinery
(QualType::getAsString()), instead of llvm::StructType::getName().

After this commit, the active users of ASan blacklist files
may have to revisit them (this is a backwards-incompatible change).

llvm-svn: 220097

clang/docs/AddressSanitizer.rst

@@ -165,9 +165,9 @@ problems happening in certain source files or with certain global variables.
     # Disable out-of-bound checks for global:
     global:bad_array
     # Disable out-of-bound checks for global instances of a given class ...
-    type:class.Namespace::BadClassName
+    type:Namespace::BadClassName
     # ... or a given struct. Use wildcard to deal with anonymous namespace.
-    type:struct.Namespace2::*::BadStructName
+    type:Namespace2::*::BadStructName
     # Disable initialization-order checks for globals:
     global:bad_init_global=init
     type:*BadInitClassSubstring*=init

clang/include/clang/Basic/SanitizerBlacklist.h

@@ -21,10 +21,6 @@
 #include "llvm/Support/SpecialCaseList.h"
 #include <memory>
 
-namespace llvm {
-class GlobalVariable;
-}
-
 namespace clang {
 
 class SanitizerBlacklist {
@@ -33,8 +29,8 @@ class SanitizerBlacklist {
 
 public:
   SanitizerBlacklist(StringRef BlacklistPath, SourceManager &SM);
-  bool isIn(const llvm::GlobalVariable &G,
+  bool isBlacklistedGlobal(StringRef GlobalName,
-            StringRef Category = StringRef()) const;
+                           StringRef Category = StringRef()) const;
   bool isBlacklistedType(StringRef MangledTypeName,
                          StringRef Category = StringRef()) const;
   bool isBlacklistedFunction(StringRef FunctionName) const;

clang/lib/Basic/SanitizerBlacklist.cpp

@@ -12,31 +12,16 @@
 //
 //===----------------------------------------------------------------------===//
 #include "clang/Basic/SanitizerBlacklist.h"
-#include "llvm/IR/GlobalValue.h"
-#include "llvm/IR/Module.h"
 
 using namespace clang;
 
-static StringRef GetGlobalTypeString(const llvm::GlobalValue &G) {
-  // Types of GlobalVariables are always pointer types.
-  llvm::Type *GType = G.getType()->getElementType();
-  // For now we support blacklisting struct types only.
-  if (llvm::StructType *SGType = dyn_cast<llvm::StructType>(GType)) {
-    if (!SGType->isLiteral())
-      return SGType->getName();
-  }
-  return "<unknown type>";
-}
-
 SanitizerBlacklist::SanitizerBlacklist(StringRef BlacklistPath,
                                        SourceManager &SM)
     : SCL(llvm::SpecialCaseList::createOrDie(BlacklistPath)), SM(SM) {}
 
-bool SanitizerBlacklist::isIn(const llvm::GlobalVariable &G,
+bool SanitizerBlacklist::isBlacklistedGlobal(StringRef GlobalName,
-                              StringRef Category) const {
+                                             StringRef Category) const {
-  return isBlacklistedFile(G.getParent()->getModuleIdentifier(), Category) ||
+  return SCL->inSection("global", GlobalName, Category);
-         SCL->inSection("global", G.getName(), Category) ||
-         SCL->inSection("type", GetGlobalTypeString(G), Category);
 }
 
 bool SanitizerBlacklist::isBlacklistedType(StringRef MangledTypeName,

clang/lib/CodeGen/CodeGenModule.cpp

@@ -1186,6 +1186,34 @@ bool CodeGenModule::isInSanitizerBlacklist(llvm::Function *Fn,
   return false;
 }
 
+bool CodeGenModule::isInSanitizerBlacklist(llvm::GlobalVariable *GV,
+                                           SourceLocation Loc, QualType Ty,
+                                           StringRef Category) const {
+  // For now globals can be blacklisted only in ASan.
+  if (!LangOpts.Sanitize.Address)
+    return false;
+  const auto &SanitizerBL = getContext().getSanitizerBlacklist();
+  if (SanitizerBL.isBlacklistedGlobal(GV->getName(), Category))
+    return true;
+  if (SanitizerBL.isBlacklistedLocation(Loc, Category))
+    return true;
+  // Check global type.
+  if (!Ty.isNull()) {
+    // Drill down the array types: if global variable of a fixed type is
+    // blacklisted, we also don't instrument arrays of them.
+    while (auto AT = dyn_cast<ArrayType>(Ty.getTypePtr()))
+      Ty = AT->getElementType();
+    Ty = Ty.getCanonicalType().getUnqualifiedType();
+    // We allow to blacklist only record types (classes, structs etc.)
+    if (Ty->isRecordType()) {
+      std::string TypeStr = Ty.getAsString(getContext().getPrintingPolicy());
+      if (SanitizerBL.isBlacklistedType(TypeStr, Category))
+        return true;
+    }
+  }
+  return false;
+}
+
 bool CodeGenModule::MayDeferGeneration(const ValueDecl *Global) {
   // Never defer when EmitAllDecls is specified.
   if (LangOpts.EmitAllDecls)
@@ -2800,7 +2828,8 @@ CodeGenModule::GetAddrOfConstantStringFromLiteral(const StringLiteral *S,
   if (Entry)
     *Entry = GV;
 
-  SanitizerMD->reportGlobalToASan(GV, S->getStrTokenLoc(0), "<string literal>");
+  SanitizerMD->reportGlobalToASan(GV, S->getStrTokenLoc(0), "<string literal>",
+                                  QualType());
   return GV;
 }
 

clang/lib/CodeGen/CodeGenModule.h

@@ -1048,6 +1048,10 @@ public:
 
   bool isInSanitizerBlacklist(llvm::Function *Fn, SourceLocation Loc) const;
 
+  bool isInSanitizerBlacklist(llvm::GlobalVariable *GV, SourceLocation Loc,
+                              QualType Ty,
+                              StringRef Category = StringRef()) const;
+
   SanitizerMetadata *getSanitizerMetadata() {
     return SanitizerMD.get();
   }

clang/lib/CodeGen/SanitizerMetadata.cpp

@@ -12,6 +12,7 @@
 //===----------------------------------------------------------------------===//
 #include "SanitizerMetadata.h"
 #include "CodeGenModule.h"
+#include "clang/AST/Type.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/IR/Constants.h"
 
@@ -22,11 +23,12 @@ SanitizerMetadata::SanitizerMetadata(CodeGenModule &CGM) : CGM(CGM) {}
 
 void SanitizerMetadata::reportGlobalToASan(llvm::GlobalVariable *GV,
                                            SourceLocation Loc, StringRef Name,
-                                           bool IsDynInit, bool IsBlacklisted) {
+                                           QualType Ty, bool IsDynInit,
+                                           bool IsBlacklisted) {
   if (!CGM.getLangOpts().Sanitize.Address)
     return;
-  IsDynInit &= !CGM.getContext().getSanitizerBlacklist().isIn(*GV, "init");
+  IsDynInit &= !CGM.isInSanitizerBlacklist(GV, Loc, Ty, "init");
-  IsBlacklisted |= CGM.getContext().getSanitizerBlacklist().isIn(*GV);
+  IsBlacklisted |= CGM.isInSanitizerBlacklist(GV, Loc, Ty);
 
   llvm::Value *LocDescr = nullptr;
   llvm::Value *GlobalName = nullptr;
@@ -57,14 +59,14 @@ void SanitizerMetadata::reportGlobalToASan(llvm::GlobalVariable *GV,
   std::string QualName;
   llvm::raw_string_ostream OS(QualName);
   D.printQualifiedName(OS);
-  reportGlobalToASan(GV, D.getLocation(), OS.str(), IsDynInit);
+  reportGlobalToASan(GV, D.getLocation(), OS.str(), D.getType(), IsDynInit);
 }
 
 void SanitizerMetadata::disableSanitizerForGlobal(llvm::GlobalVariable *GV) {
   // For now, just make sure the global is not modified by the ASan
   // instrumentation.
   if (CGM.getLangOpts().Sanitize.Address)
-    reportGlobalToASan(GV, SourceLocation(), "", false, true);
+    reportGlobalToASan(GV, SourceLocation(), "", QualType(), false, true);
 }
 
 void SanitizerMetadata::disableSanitizerForInstruction(llvm::Instruction *I) {

clang/lib/CodeGen/SanitizerMetadata.h

@@ -15,6 +15,7 @@
 
 #include "clang/Basic/LLVM.h"
 #include "clang/Basic/SourceLocation.h"
+#include "clang/AST/Type.h"
 
 namespace llvm {
 class GlobalVariable;
@@ -39,7 +40,7 @@ public:
   void reportGlobalToASan(llvm::GlobalVariable *GV, const VarDecl &D,
                           bool IsDynInit = false);
   void reportGlobalToASan(llvm::GlobalVariable *GV, SourceLocation Loc,
-                          StringRef Name, bool IsDynInit = false,
+                          StringRef Name, QualType Ty, bool IsDynInit = false,
                           bool IsBlacklisted = false);
   void disableSanitizerForGlobal(llvm::GlobalVariable *GV);
   void disableSanitizerForInstruction(llvm::Instruction *I);

clang/test/CodeGen/asan-globals.cpp

@@ -1,7 +1,8 @@
+// RUN: echo "int extra_global;" > %t.extra-source.cpp
 // RUN: echo "global:*blacklisted_global*" > %t.blacklist
-// RUN: %clang_cc1 -fsanitize=address -fsanitize-blacklist=%t.blacklist -emit-llvm -o - %s | FileCheck %s
+// RUN: %clang_cc1 -include %t.extra-source.cpp -fsanitize=address -fsanitize-blacklist=%t.blacklist -emit-llvm -o - %s | FileCheck %s
 // RUN: echo "src:%s" > %t.blacklist-src
-// RUN: %clang_cc1 -fsanitize=address -fsanitize-blacklist=%t.blacklist-src -emit-llvm -o - %s | FileCheck %s --check-prefix=BLACKLIST-SRC
+// RUN: %clang_cc1 -include %t.extra-source.cpp -fsanitize=address -fsanitize-blacklist=%t.blacklist-src -emit-llvm -o - %s | FileCheck %s --check-prefix=BLACKLIST-SRC
 // REQUIRES: shell
 
 int global;
@@ -13,18 +14,22 @@ void func() {
   const char *literal = "Hello, world!";
 }
 
-// CHECK: !llvm.asan.globals = !{![[GLOBAL:[0-9]+]], ![[DYN_INIT_GLOBAL:[0-9]+]], ![[BLACKLISTED_GLOBAL:[0-9]+]], ![[STATIC_VAR:[0-9]+]], ![[LITERAL:[0-9]+]]}
+// CHECK: !llvm.asan.globals = !{![[EXTRA_GLOBAL:[0-9]+]], ![[GLOBAL:[0-9]+]], ![[DYN_INIT_GLOBAL:[0-9]+]], ![[BLACKLISTED_GLOBAL:[0-9]+]], ![[STATIC_VAR:[0-9]+]], ![[LITERAL:[0-9]+]]}
+// CHECK: ![[EXTRA_GLOBAL]] = metadata !{{{.*}} metadata ![[EXTRA_GLOBAL_LOC:[0-9]+]], metadata !"extra_global", i1 false, i1 false}
+// CHECK: ![[EXTRA_GLOBAL_LOC]] = metadata !{metadata !"{{.*}}extra-source.cpp", i32 1, i32 5}
 // 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: ![[GLOBAL_LOC]] = metadata !{metadata !"{{.*}}asan-globals.cpp", i32 8, 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: ![[DYN_INIT_LOC]] = metadata !{metadata !"{{.*}}asan-globals.cpp", i32 9, i32 5}
 // CHECK: ![[BLACKLISTED_GLOBAL]] = metadata !{{{.*}}, null, null, i1 false, i1 true}
 // 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: ![[STATIC_LOC]] = metadata !{metadata !"{{.*}}asan-globals.cpp", i32 13, 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}
+// CHECK: ![[LITERAL_LOC]] = metadata !{metadata !"{{.*}}asan-globals.cpp", i32 14, 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: !llvm.asan.globals = !{![[EXTRA_GLOBAL:[0-9]+]], ![[GLOBAL:[0-9]+]], ![[DYN_INIT_GLOBAL:[0-9]+]], ![[BLACKLISTED_GLOBAL:[0-9]+]], ![[STATIC_VAR:[0-9]+]], ![[LITERAL:[0-9]+]]}
+// BLACKLIST-SRC: ![[EXTRA_GLOBAL]] = metadata !{{{.*}} metadata ![[EXTRA_GLOBAL_LOC:[0-9]+]], metadata !"extra_global", i1 false, i1 false}
+// BLACKLIST-SRC: ![[EXTRA_GLOBAL_LOC]] = metadata !{metadata !"{{.*}}extra-source.cpp", i32 1, i32 5}
 // BLACKLIST-SRC: ![[GLOBAL]] = metadata !{{{.*}} null, null, i1 false, i1 true}
 // BLACKLIST-SRC: ![[DYN_INIT_GLOBAL]] = metadata !{{{.*}} null, null, i1 true, i1 true}
 // BLACKLIST-SRC: ![[BLACKLISTED_GLOBAL]] = metadata !{{{.*}}, null, null, i1 false, i1 true}

clang/test/CodeGen/sanitize-init-order.cpp

@@ -2,7 +2,8 @@
 
 // Test blacklist functionality.
 // RUN: echo "global-init-src:%s" > %t-file.blacklist
-// RUN: echo "global-init-type:struct.PODWithCtorAndDtor" > %t-type.blacklist
+// RUN: echo "global-init-type:PODWithCtorAndDtor" > %t-type.blacklist
+// RUN: echo "type:NS::PODWithCtor=init" >> %t-type.blacklist
 // RUN: %clang_cc1 -fsanitize=address -fsanitize-blacklist=%t-file.blacklist -emit-llvm -o - %s | FileCheck %s --check-prefix=BLACKLIST
 // RUN: %clang_cc1 -fsanitize=address -fsanitize-blacklist=%t-type.blacklist -emit-llvm -o - %s | FileCheck %s --check-prefix=BLACKLIST
 // REQUIRES: shell
@@ -25,14 +26,25 @@ struct PODWithCtorAndDtor {
 };
 PODWithCtorAndDtor s3;
 
+namespace NS {
+class PODWithCtor {
+public:
+  PODWithCtor() {}
+};
+
+const volatile PODWithCtor array[5][5];
+}
+
 // Check that ASan init-order checking ignores structs with trivial default
 // constructor.
-// CHECK: !llvm.asan.globals = !{![[GLOB_1:[0-9]+]], ![[GLOB_2:[0-9]+]], ![[GLOB_3:[0-9]]]}
+// CHECK: !llvm.asan.globals = !{![[GLOB_1:[0-9]+]], ![[GLOB_2:[0-9]+]], ![[GLOB_3:[0-9]]], ![[GLOB_4:[0-9]]]}
 // CHECK: ![[GLOB_1]] = metadata !{%struct.PODStruct* {{.*}}, i1 false, i1 false}
 // CHECK: ![[GLOB_2]] = metadata !{%struct.PODWithDtor* {{.*}}, i1 false, i1 false}
 // CHECK: ![[GLOB_3]] = metadata !{%struct.PODWithCtorAndDtor* {{.*}}, i1 true, i1 false}
+// CHECK: ![[GLOB_4]] = metadata !{{{.*}}class.NS::PODWithCtor{{.*}}, i1 true, i1 false}
 
-// BLACKLIST: !llvm.asan.globals = !{![[GLOB_1:[0-9]+]], ![[GLOB_2:[0-9]+]], ![[GLOB_3:[0-9]]]}
+// BLACKLIST: !llvm.asan.globals = !{![[GLOB_1:[0-9]+]], ![[GLOB_2:[0-9]+]], ![[GLOB_3:[0-9]]], ![[GLOB_4:[0-9]]]}
 // BLACKLIST: ![[GLOB_1]] = metadata !{%struct.PODStruct* {{.*}}, i1 false, i1 false}
 // BLACKLIST: ![[GLOB_2]] = metadata !{%struct.PODWithDtor* {{.*}}, i1 false, i1 false}
 // BLACKLIST: ![[GLOB_3]] = metadata !{%struct.PODWithCtorAndDtor* {{.*}}, i1 false, i1 false}
+// BLACKLIST: ![[GLOB_4]] = metadata !{{{.*}}class.NS::PODWithCtor{{.*}}, i1 false, i1 false}