SanitizerBlacklist: blacklist functions by their source location.

This commit changes the way we blacklist functions in ASan, TSan,
MSan and UBSan. We used to treat function as "blacklisted"
and turned off instrumentation in it in two cases:

1) Function is explicitly blacklisted by its mangled name.
This part is not changed.

2) Function is located in llvm::Module, whose identifier is
contained in the list of blacklisted sources. This is completely
wrong, as llvm::Module may not correspond to the actual source
file function is defined in. Also, function can be defined in
a header, in which case user had to blacklist the .cpp file
this header was #include'd into, not the header itself.
Such functions could cause other problems - for instance, if the
header was included in multiple source files, compiled
separately and linked into a single executable, we could end up
with both instrumented and non-instrumented version of the same
function participating in the same link.

After this change we will make blacklisting decision based on
the SourceLocation of a function definition. If a function is
not explicitly defined in the source file, (for example, the
function is compiler-generated and responsible for
initialization/destruction of a global variable), then it will
be blacklisted if the corresponding global variable is defined
in blacklisted source file, and will be instrumented otherwise.

After this commit, the active users of blacklist files may have
to revisit them. This is a backwards-incompatible change, but
I don't think it's possible or makes sense to support the
old incorrect behavior.

I plan to make similar change for blacklisting GlobalVariables
(which is ASan-specific).

git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@219997 91177308-0d34-0410-b5e6-96231b3b80d8

diff --git a/include/clang/Basic/SanitizerBlacklist.h b/include/clang/Basic/SanitizerBlacklist.h
index dfe55a18eb9902a049e62edef4b6f64df4ea5e29..79bb75accad389b9b80b87ebaaea35688fcad3ae 100644 (file)
--- a/include/clang/Basic/SanitizerBlacklist.h
+++ b/include/clang/Basic/SanitizerBlacklist.h
@@ -23,7 +23,6 @@
 
 namespace llvm {
 class GlobalVariable;
-class Function;
 }
 
 namespace clang {
@@ -34,7 +33,6 @@ class SanitizerBlacklist {
 
 public:
   SanitizerBlacklist(StringRef BlacklistPath, SourceManager &SM);
-  bool isIn(const llvm::Function &F) const;
   bool isIn(const llvm::GlobalVariable &G,
             StringRef Category = StringRef()) const;
   bool isBlacklistedType(StringRef MangledTypeName,

diff --git a/lib/Basic/SanitizerBlacklist.cpp b/lib/Basic/SanitizerBlacklist.cpp
index f0221331ed2037ccb43c19e43e9ca4f6298110a7..2f4bdb51138c1561e891c83680113917c9dea2b5 100644 (file)
--- a/lib/Basic/SanitizerBlacklist.cpp
+++ b/lib/Basic/SanitizerBlacklist.cpp
@@ -12,7 +12,6 @@
 //
 //===----------------------------------------------------------------------===//
 #include "clang/Basic/SanitizerBlacklist.h"
-#include "llvm/IR/Function.h"
 #include "llvm/IR/GlobalValue.h"
 #include "llvm/IR/Module.h"
 
@@ -33,11 +32,6 @@ SanitizerBlacklist::SanitizerBlacklist(StringRef BlacklistPath,
                                        SourceManager &SM)
     : SCL(llvm::SpecialCaseList::createOrDie(BlacklistPath)), SM(SM) {}
 
-bool SanitizerBlacklist::isIn(const llvm::Function &F) const {
-  return isBlacklistedFile(F.getParent()->getModuleIdentifier()) ||
-         isBlacklistedFunction(F.getName());
-}
-
 bool SanitizerBlacklist::isIn(const llvm::GlobalVariable &G,
                               StringRef Category) const {
   return isBlacklistedFile(G.getParent()->getModuleIdentifier(), Category) ||

diff --git a/lib/CodeGen/CGDeclCXX.cpp b/lib/CodeGen/CGDeclCXX.cpp
index dcfc45ff84fcd178c3cc70cc3813f398bb6b2d02..0ba4ea5d335e10af582e92ea5b3b0a6ec536117c 100644 (file)
--- a/lib/CodeGen/CGDeclCXX.cpp
+++ b/lib/CodeGen/CGDeclCXX.cpp
@@ -167,7 +167,8 @@ llvm::Constant *CodeGenFunction::createAtExitStub(const VarDecl &VD,
     llvm::raw_svector_ostream Out(FnName);
     CGM.getCXXABI().getMangleContext().mangleDynamicAtExitDestructor(&VD, Out);
   }
-  llvm::Function *fn = CGM.CreateGlobalInitOrDestructFunction(ty, FnName.str());
+  llvm::Function *fn = CGM.CreateGlobalInitOrDestructFunction(ty, FnName.str(),
+                                                              VD.getLocation());
 
   CodeGenFunction CGF(CGM);
 
@@ -219,9 +220,8 @@ void CodeGenFunction::EmitCXXGuardedInit(const VarDecl &D,
   CGM.getCXXABI().EmitGuardedInit(*this, D, DeclPtr, PerformInit);
 }
 
-llvm::Function *
-CodeGenModule::CreateGlobalInitOrDestructFunction(llvm::FunctionType *FTy,
-                                                  const Twine &Name, bool TLS) {
+llvm::Function *CodeGenModule::CreateGlobalInitOrDestructFunction(
+    llvm::FunctionType *FTy, const Twine &Name, SourceLocation Loc, bool TLS) {
   llvm::Function *Fn =
     llvm::Function::Create(FTy, llvm::GlobalValue::InternalLinkage,
                            Name, &getModule());
@@ -236,7 +236,7 @@ CodeGenModule::CreateGlobalInitOrDestructFunction(llvm::FunctionType *FTy,
   if (!getLangOpts().Exceptions)
     Fn->setDoesNotThrow();
 
-  if (!getSanitizerBlacklist().isIn(*Fn)) {
+  if (!isInSanitizerBlacklist(Fn, Loc)) {
     if (getLangOpts().Sanitize.Address)
       Fn->addFnAttr(llvm::Attribute::SanitizeAddress);
     if (getLangOpts().Sanitize.Thread)
@@ -286,7 +286,8 @@ CodeGenModule::EmitCXXGlobalVarDeclInitFunc(const VarDecl *D,
   }
 
   // Create a variable initialization function.
-  llvm::Function *Fn = CreateGlobalInitOrDestructFunction(FTy, FnName.str());
+  llvm::Function *Fn =
+      CreateGlobalInitOrDestructFunction(FTy, FnName.str(), D->getLocation());
 
   auto *ISA = D->getAttr<InitSegAttr>();
   CodeGenFunction(*this).GenerateCXXGlobalVarDeclInitFunc(Fn, D, Addr,
@@ -552,8 +553,8 @@ llvm::Function *CodeGenFunction::generateDestroyHelper(
   const CGFunctionInfo &FI = CGM.getTypes().arrangeFreeFunctionDeclaration(
       getContext().VoidTy, args, FunctionType::ExtInfo(), /*variadic=*/false);
   llvm::FunctionType *FTy = CGM.getTypes().GetFunctionType(FI);
-  llvm::Function *fn =
-      CGM.CreateGlobalInitOrDestructFunction(FTy, "__cxx_global_array_dtor");
+  llvm::Function *fn = CGM.CreateGlobalInitOrDestructFunction(
+      FTy, "__cxx_global_array_dtor", VD->getLocation());
 
   StartFunction(VD, getContext().VoidTy, fn, FI, args);
 

diff --git a/lib/CodeGen/CGExpr.cpp b/lib/CodeGen/CGExpr.cpp
index 927b163d605d00f7be4dadaf481316d346f3de81..32b2e2cc587e5b0675f5f86a8565de1ce66e4cc2 100644 (file)
--- a/lib/CodeGen/CGExpr.cpp
+++ b/lib/CodeGen/CGExpr.cpp
@@ -552,7 +552,8 @@ void CodeGenFunction::EmitTypeCheck(TypeCheckKind TCK, SourceLocation Loc,
                                                      Out);
 
     // Blacklist based on the mangled type.
-    if (!CGM.getSanitizerBlacklist().isBlacklistedType(Out.str())) {
+    if (!CGM.getContext().getSanitizerBlacklist().isBlacklistedType(
+            Out.str())) {
       llvm::hash_code TypeHash = hash_value(Out.str());
 
       // Load the vptr, and compute hash_16_bytes(TypeHash, vptr).

diff --git a/lib/CodeGen/CodeGenFunction.cpp b/lib/CodeGen/CodeGenFunction.cpp
index f9e6c397d93e7f5a26d2bd68ac5e43d5ca7cf2a5..1fc4571aadb634887ef45b6c251fd57d3e67b29b 100644 (file)
--- a/lib/CodeGen/CodeGenFunction.cpp
+++ b/lib/CodeGen/CodeGenFunction.cpp
@@ -579,7 +579,7 @@ void CodeGenFunction::StartFunction(GlobalDecl GD,
   CurFnInfo = &FnInfo;
   assert(CurFn->isDeclaration() && "Function already has body?");
 
-  if (CGM.getSanitizerBlacklist().isIn(*Fn))
+  if (CGM.isInSanitizerBlacklist(Fn, Loc))
     SanOpts = &SanitizerOptions::Disabled;
 
   // Pass inline keyword to optimizer if it appears explicitly on any

diff --git a/lib/CodeGen/CodeGenModule.cpp b/lib/CodeGen/CodeGenModule.cpp
index 78a2631a36613f400ac49c66f84b4ab9b5cac32c..c5afaa19e9be3f80a1789f06838a35778257eab5 100644 (file)
--- a/lib/CodeGen/CodeGenModule.cpp
+++ b/lib/CodeGen/CodeGenModule.cpp
@@ -740,7 +740,7 @@ void CodeGenModule::SetLLVMFunctionAttributesForDefinition(const Decl *D,
     B.addAttribute(llvm::Attribute::StackProtectReq);
 
   // Add sanitizer attributes if function is not blacklisted.
-  if (!getSanitizerBlacklist().isIn(*F)) {
+  if (!isInSanitizerBlacklist(F, D->getLocation())) {
     // When AddressSanitizer is enabled, set SanitizeAddress attribute
     // unless __attribute__((no_sanitize_address)) is used.
     if (LangOpts.Sanitize.Address && !D->hasAttr<NoSanitizeAddressAttr>())
@@ -1168,6 +1168,24 @@ void CodeGenModule::AddGlobalAnnotations(const ValueDecl *D,
     Annotations.push_back(EmitAnnotateAttr(GV, I, D->getLocation()));
 }
 
+bool CodeGenModule::isInSanitizerBlacklist(llvm::Function *Fn,
+                                           SourceLocation Loc) const {
+  const auto &SanitizerBL = getContext().getSanitizerBlacklist();
+  // Blacklist by function name.
+  if (SanitizerBL.isBlacklistedFunction(Fn->getName()))
+    return true;
+  // Blacklist by location.
+  if (!Loc.isInvalid())
+    return SanitizerBL.isBlacklistedLocation(Loc);
+  // If location is unknown, this may be a compiler-generated function. Assume
+  // it's located in the main file.
+  auto &SM = Context.getSourceManager();
+  if (const auto *MainFile = SM.getFileEntryForID(SM.getMainFileID())) {
+    return SanitizerBL.isBlacklistedFile(MainFile->getName());
+  }
+  return false;
+}
+
 bool CodeGenModule::MayDeferGeneration(const ValueDecl *Global) {
   // Never defer when EmitAllDecls is specified.
   if (LangOpts.EmitAllDecls)

diff --git a/lib/CodeGen/CodeGenModule.h b/lib/CodeGen/CodeGenModule.h
index a63b799baa7aa183e02aefd3ab11ae868fcc6b25..546e59309e00b6f0afad27cb851fdbed553d7f48 100644 (file)
--- a/lib/CodeGen/CodeGenModule.h
+++ b/lib/CodeGen/CodeGenModule.h
@@ -686,9 +686,10 @@ public:
   CreateOrReplaceCXXRuntimeVariable(StringRef Name, llvm::Type *Ty,
                                     llvm::GlobalValue::LinkageTypes Linkage);
 
-  llvm::Function *CreateGlobalInitOrDestructFunction(llvm::FunctionType *ty,
-                                                     const Twine &name,
-                                                     bool TLS = false);
+  llvm::Function *
+  CreateGlobalInitOrDestructFunction(llvm::FunctionType *ty, const Twine &name,
+                                     SourceLocation Loc = SourceLocation(),
+                                     bool TLS = false);
 
   /// Return the address space of the underlying global variable for D, as
   /// determined by its declaration. Normally this is the same as the address
@@ -1045,9 +1046,7 @@ public:
   /// annotations are emitted during finalization of the LLVM code.
   void AddGlobalAnnotations(const ValueDecl *D, llvm::GlobalValue *GV);
 
-  const SanitizerBlacklist &getSanitizerBlacklist() const {
-    return Context.getSanitizerBlacklist();
-  }
+  bool isInSanitizerBlacklist(llvm::Function *Fn, SourceLocation Loc) const;
 
   SanitizerMetadata *getSanitizerMetadata() {
     return SanitizerMD.get();

diff --git a/lib/CodeGen/ItaniumCXXABI.cpp b/lib/CodeGen/ItaniumCXXABI.cpp
index 824eba9fdb19ba29f636ebd37d1d8a9de5495d83..eb5fbab4381f62fd8f872c1b7d1073a7c19d02c0 100644 (file)
--- a/lib/CodeGen/ItaniumCXXABI.cpp
+++ b/lib/CodeGen/ItaniumCXXABI.cpp
@@ -1912,6 +1912,7 @@ void ItaniumCXXABI::EmitThreadLocalInitFuncs(
     llvm::FunctionType *FTy =
         llvm::FunctionType::get(CGM.VoidTy, /*isVarArg=*/false);
     InitFunc = CGM.CreateGlobalInitOrDestructFunction(FTy, "__tls_init",
+                                                      SourceLocation(),
                                                       /*TLS=*/true);
     llvm::GlobalVariable *Guard = new llvm::GlobalVariable(
         CGM.getModule(), CGM.Int8Ty, /*isConstant=*/false,

diff --git a/lib/CodeGen/MicrosoftCXXABI.cpp b/lib/CodeGen/MicrosoftCXXABI.cpp
index 9ea26336908af78b61af3d541a81ed37b3153787..dd0846958faa44aba45dc98425ae24b1a720ff62 100644 (file)
--- a/lib/CodeGen/MicrosoftCXXABI.cpp
+++ b/lib/CodeGen/MicrosoftCXXABI.cpp
@@ -1796,9 +1796,9 @@ void MicrosoftCXXABI::EmitThreadLocalInitFuncs(
   if (!NonComdatInits.empty()) {
     llvm::FunctionType *FTy =
         llvm::FunctionType::get(CGM.VoidTy, /*isVarArg=*/false);
-    llvm::Function *InitFunc =
-        CGM.CreateGlobalInitOrDestructFunction(FTy, "__tls_init",
-                                               /*TLS=*/true);
+    llvm::Function *InitFunc = CGM.CreateGlobalInitOrDestructFunction(
+        FTy, "__tls_init", SourceLocation(),
+        /*TLS=*/true);
     CodeGenFunction(CGM).GenerateCXXGlobalInitFunc(InitFunc, NonComdatInits);
 
     AddToXDU(InitFunc);

diff --git a/lib/CodeGen/SanitizerMetadata.cpp b/lib/CodeGen/SanitizerMetadata.cpp
index 2a2b0ed941968c3ff91e084a46d2f1b5d109925e..69c20b80a8e25e42246b17636299947beff3cf8d 100644 (file)
--- a/lib/CodeGen/SanitizerMetadata.cpp
+++ b/lib/CodeGen/SanitizerMetadata.cpp
@@ -25,8 +25,8 @@ void SanitizerMetadata::reportGlobalToASan(llvm::GlobalVariable *GV,
                                            bool IsDynInit, bool IsBlacklisted) {
   if (!CGM.getLangOpts().Sanitize.Address)
     return;
-  IsDynInit &= !CGM.getSanitizerBlacklist().isIn(*GV, "init");
-  IsBlacklisted |= CGM.getSanitizerBlacklist().isIn(*GV);
+  IsDynInit &= !CGM.getContext().getSanitizerBlacklist().isIn(*GV, "init");
+  IsBlacklisted |= CGM.getContext().getSanitizerBlacklist().isIn(*GV);
 
   llvm::Value *LocDescr = nullptr;
   llvm::Value *GlobalName = nullptr;

diff --git a/test/CodeGen/address-safety-attr.cpp b/test/CodeGen/address-safety-attr.cpp
index f94efd62c9c0f38ac9f6f3c67b368b9ba40d767f..ee368f11ebd4769ad4017b709bee26355c05a589 100644 (file)
--- a/test/CodeGen/address-safety-attr.cpp
+++ b/test/CodeGen/address-safety-attr.cpp
@@ -1,9 +1,16 @@
-// RUN: %clang_cc1 -triple x86_64-apple-darwin -emit-llvm -o - %s | FileCheck -check-prefix=WITHOUT %s
-// RUN: %clang_cc1 -triple x86_64-apple-darwin -emit-llvm -o - %s -fsanitize=address | FileCheck -check-prefix=ASAN %s
-// RUN: echo "src:%s" > %t.file.blacklist
+int DefinedInDifferentFile(int *a);
+// RUN: echo "int DefinedInDifferentFile(int *a) { return *a; }" > %t.extra-source.cpp
+// RUN: echo "struct S { S(){} ~S(){} };" >> %t.extra-source.cpp
+// RUN: echo "S glob_array[5];" >> %t.extra-source.cpp
+
+// RUN: %clang_cc1 -triple x86_64-apple-darwin -emit-llvm -o - %s -include %t.extra-source.cpp | FileCheck -check-prefix=WITHOUT %s
+// RUN: %clang_cc1 -triple x86_64-apple-darwin -emit-llvm -o - %s -include %t.extra-source.cpp -fsanitize=address | FileCheck -check-prefix=ASAN %s
+
 // RUN: echo "fun:*BlacklistedFunction*" > %t.func.blacklist
-// RUN: %clang_cc1 -triple x86_64-apple-darwin -emit-llvm -o - %s -fsanitize=address -fsanitize-blacklist=%t.file.blacklist | FileCheck -check-prefix=BLFILE %s
-// RUN: %clang_cc1 -triple x86_64-apple-darwin -emit-llvm -o - %s -fsanitize=address -fsanitize-blacklist=%t.func.blacklist | FileCheck -check-prefix=BLFUNC %s
+// RUN: %clang_cc1 -triple x86_64-apple-darwin -emit-llvm -o - %s -include %t.extra-source.cpp -fsanitize=address -fsanitize-blacklist=%t.func.blacklist | FileCheck -check-prefix=BLFUNC %s
+
+// RUN: echo "src:%s" > %t.file.blacklist
+// RUN: %clang_cc1 -triple x86_64-apple-darwin -emit-llvm -o - %s -include %t.extra-source.cpp -fsanitize=address -fsanitize-blacklist=%t.file.blacklist | FileCheck -check-prefix=BLFILE %s
 
 // FIXME: %t.file.blacklist is like "src:x:\path\to\clang\test\CodeGen\address-safety-attr.cpp"
 // REQUIRES: shell
@@ -12,7 +19,25 @@
 // when AddressSanitizer is enabled, unless no_sanitize_address attribute
 // is present.
 
-// WITHOUT:  NoAddressSafety1{{.*}}) [[NOATTR:#[0-9]+]]
+// Attributes for function defined in different source file:
+// WITHOUT: DefinedInDifferentFile{{.*}} [[NOATTR:#[0-9]+]]
+// BLFILE:  DefinedInDifferentFile{{.*}} [[WITH:#[0-9]+]]
+// BLFUNC:  DefinedInDifferentFile{{.*}} [[WITH:#[0-9]+]]
+// ASAN:    DefinedInDifferentFile{{.*}} [[WITH:#[0-9]+]]
+
+// Check that functions generated for global in different source file are
+// not blacklisted.
+// WITHOUT: @__cxx_global_var_init{{.*}}[[NOATTR_NO_TF:#[0-9]+]]
+// WITHOUT: @__cxx_global_array_dtor{{.*}}[[NOATTR_NO_TF]]
+// BLFILE: @__cxx_global_var_init{{.*}}[[WITH_NO_TF:#[0-9]+]]
+// BLFILE: @__cxx_global_array_dtor{{.*}}[[WITH_NO_TF]]
+// BLFUNC: @__cxx_global_var_init{{.*}}[[WITH_NO_TF:#[0-9]+]]
+// BLFUNC: @__cxx_global_array_dtor{{.*}}[[WITH_NO_TF]]
+// ASAN: @__cxx_global_var_init{{.*}}[[WITH_NO_TF:#[0-9]+]]
+// ASAN: @__cxx_global_array_dtor{{.*}}[[WITH_NO_TF]]
+
+
+// WITHOUT:  NoAddressSafety1{{.*}}) [[NOATTR]]
 // BLFILE:  NoAddressSafety1{{.*}}) [[NOATTR:#[0-9]+]]
 // BLFUNC:  NoAddressSafety1{{.*}}) [[NOATTR:#[0-9]+]]
 // ASAN:  NoAddressSafety1{{.*}}) [[NOATTR:#[0-9]+]]
@@ -29,8 +54,8 @@ int NoAddressSafety2(int *a) { return *a; }
 
 // WITHOUT:  AddressSafetyOk{{.*}}) [[NOATTR]]
 // BLFILE:  AddressSafetyOk{{.*}}) [[NOATTR]]
-// BLFUNC: AddressSafetyOk{{.*}}) [[WITH:#[0-9]+]]
-// ASAN: AddressSafetyOk{{.*}}) [[WITH:#[0-9]+]]
+// BLFUNC: AddressSafetyOk{{.*}}) [[WITH]]
+// ASAN: AddressSafetyOk{{.*}}) [[WITH]]
 int AddressSafetyOk(int *a) { return *a; }
 
 // WITHOUT:  BlacklistedFunction{{.*}}) [[NOATTR]]
@@ -60,21 +85,23 @@ int force_instance = TemplateAddressSafetyOk<42>()
 // Check that __cxx_global_var_init* get the sanitize_address attribute.
 int global1 = 0;
 int global2 = *(int*)((char*)&global1+1);
-// WITHOUT: @__cxx_global_var_init{{.*}}[[NOATTR_NO_TF:#[0-9]+]]
+// WITHOUT: @__cxx_global_var_init{{.*}}[[NOATTR_NO_TF]]
 // BLFILE: @__cxx_global_var_init{{.*}}[[NOATTR_NO_TF:#[0-9]+]]
-// BLFUNC: @__cxx_global_var_init{{.*}}[[WITH_NO_TF:#[0-9]+]]
-// ASAN: @__cxx_global_var_init{{.*}}[[WITH_NO_TF:#[0-9]+]]
+// BLFUNC: @__cxx_global_var_init{{.*}}[[WITH_NO_TF]]
+// ASAN: @__cxx_global_var_init{{.*}}[[WITH_NO_TF]]
 
 // WITHOUT: attributes [[NOATTR]] = { nounwind{{.*}} }
 // WITHOUT: attributes [[NOATTR_NO_TF]] = { nounwind }
 
-// BLFILE: attributes [[NOATTR]] = { nounwind{{.*}} }
+// BLFILE: attributes [[WITH]] = { nounwind sanitize_address{{.*}} }
+// BLFILE: attributes [[WITH_NO_TF]] = { nounwind sanitize_address }
 // BLFILE: attributes [[NOATTR_NO_TF]] = { nounwind }
+// BLFILE: attributes [[NOATTR]] = { nounwind{{.*}} }
 
-// BLFUNC: attributes [[NOATTR]] = { nounwind{{.*}} }
 // BLFUNC: attributes [[WITH]] = { nounwind sanitize_address{{.*}} }
 // BLFUNC: attributes [[WITH_NO_TF]] = { nounwind sanitize_address }
+// BLFUNC: attributes [[NOATTR]] = { nounwind{{.*}} }
 
-// ASAN: attributes [[NOATTR]] = { nounwind{{.*}} }
 // ASAN: attributes [[WITH]] = { nounwind sanitize_address{{.*}} }
 // ASAN: attributes [[WITH_NO_TF]] = { nounwind sanitize_address }
+// ASAN: attributes [[NOATTR]] = { nounwind{{.*}} }