From a0ac3c2bf049eb28e77a19dd70cbd570585f4747 Mon Sep 17 00:00:00 2001 From: Alexey Samsonov Date: Fri, 17 Oct 2014 22:37:33 +0000 Subject: [PATCH] [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 | 4 ++-- clang/include/clang/Basic/SanitizerBlacklist.h | 8 ++----- clang/lib/Basic/SanitizerBlacklist.cpp | 21 +++-------------- clang/lib/CodeGen/CodeGenModule.cpp | 31 +++++++++++++++++++++++++- clang/lib/CodeGen/CodeGenModule.h | 4 ++++ clang/lib/CodeGen/SanitizerMetadata.cpp | 12 +++++----- clang/lib/CodeGen/SanitizerMetadata.h | 3 ++- clang/test/CodeGen/asan-globals.cpp | 21 ++++++++++------- clang/test/CodeGen/sanitize-init-order.cpp | 18 ++++++++++++--- 9 files changed, 78 insertions(+), 44 deletions(-) diff --git a/clang/docs/AddressSanitizer.rst b/clang/docs/AddressSanitizer.rst index 93a6a0e..122b31a 100644 --- a/clang/docs/AddressSanitizer.rst +++ b/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 diff --git a/clang/include/clang/Basic/SanitizerBlacklist.h b/clang/include/clang/Basic/SanitizerBlacklist.h index 79bb75a..2ce268a 100644 --- a/clang/include/clang/Basic/SanitizerBlacklist.h +++ b/clang/include/clang/Basic/SanitizerBlacklist.h @@ -21,10 +21,6 @@ #include "llvm/Support/SpecialCaseList.h" #include -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, - StringRef Category = StringRef()) const; + bool isBlacklistedGlobal(StringRef GlobalName, + StringRef Category = StringRef()) const; bool isBlacklistedType(StringRef MangledTypeName, StringRef Category = StringRef()) const; bool isBlacklistedFunction(StringRef FunctionName) const; diff --git a/clang/lib/Basic/SanitizerBlacklist.cpp b/clang/lib/Basic/SanitizerBlacklist.cpp index 2f4bdb5..9b2cdfd 100644 --- a/clang/lib/Basic/SanitizerBlacklist.cpp +++ b/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(GType)) { - if (!SGType->isLiteral()) - return SGType->getName(); - } - return ""; -} - SanitizerBlacklist::SanitizerBlacklist(StringRef BlacklistPath, SourceManager &SM) : SCL(llvm::SpecialCaseList::createOrDie(BlacklistPath)), SM(SM) {} -bool SanitizerBlacklist::isIn(const llvm::GlobalVariable &G, - StringRef Category) const { - return isBlacklistedFile(G.getParent()->getModuleIdentifier(), Category) || - SCL->inSection("global", G.getName(), Category) || - SCL->inSection("type", GetGlobalTypeString(G), Category); +bool SanitizerBlacklist::isBlacklistedGlobal(StringRef GlobalName, + StringRef Category) const { + return SCL->inSection("global", GlobalName, Category); } bool SanitizerBlacklist::isBlacklistedType(StringRef MangledTypeName, diff --git a/clang/lib/CodeGen/CodeGenModule.cpp b/clang/lib/CodeGen/CodeGenModule.cpp index c5afaa1..a9c5fb2 100644 --- a/clang/lib/CodeGen/CodeGenModule.cpp +++ b/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(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), ""); + SanitizerMD->reportGlobalToASan(GV, S->getStrTokenLoc(0), "", + QualType()); return GV; } diff --git a/clang/lib/CodeGen/CodeGenModule.h b/clang/lib/CodeGen/CodeGenModule.h index 546e593..a3bbada 100644 --- a/clang/lib/CodeGen/CodeGenModule.h +++ b/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(); } diff --git a/clang/lib/CodeGen/SanitizerMetadata.cpp b/clang/lib/CodeGen/SanitizerMetadata.cpp index 69c20b8..7516dd0 100644 --- a/clang/lib/CodeGen/SanitizerMetadata.cpp +++ b/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"); - IsBlacklisted |= CGM.getContext().getSanitizerBlacklist().isIn(*GV); + IsDynInit &= !CGM.isInSanitizerBlacklist(GV, Loc, Ty, "init"); + 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) { diff --git a/clang/lib/CodeGen/SanitizerMetadata.h b/clang/lib/CodeGen/SanitizerMetadata.h index 4d63aef..f3c700a 100644 --- a/clang/lib/CodeGen/SanitizerMetadata.h +++ b/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); diff --git a/clang/test/CodeGen/asan-globals.cpp b/clang/test/CodeGen/asan-globals.cpp index a9b9b5f..4fae9a4 100644 --- a/clang/test/CodeGen/asan-globals.cpp +++ b/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 !"", 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} diff --git a/clang/test/CodeGen/sanitize-init-order.cpp b/clang/test/CodeGen/sanitize-init-order.cpp index 8c662db..d8279a9 100644 --- a/clang/test/CodeGen/sanitize-init-order.cpp +++ b/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} -- 2.7.4