melver updated this revision to Diff 269578.
melver marked 6 inline comments as done.
melver added a comment.
Address comments.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D81390/new/
https://reviews.llvm.org/D81390
Files:
clang/test/CodeGen/asan-globals.cpp
llvm/include/llvm/Transforms/Utils/ModuleUtils.h
llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
llvm/lib/Transforms/Utils/ModuleUtils.cpp
Index: llvm/lib/Transforms/Utils/ModuleUtils.cpp
===================================================================
--- llvm/lib/Transforms/Utils/ModuleUtils.cpp
+++ llvm/lib/Transforms/Utils/ModuleUtils.cpp
@@ -119,6 +119,15 @@
AttributeList());
}
+Function *llvm::createSanitizerCtor(Module &M, StringRef CtorName) {
+ Function *Ctor = Function::Create(
+ FunctionType::get(Type::getVoidTy(M.getContext()), false),
+ GlobalValue::InternalLinkage, CtorName, &M);
+ BasicBlock *CtorBB = BasicBlock::Create(M.getContext(), "", Ctor);
+ ReturnInst::Create(M.getContext(), CtorBB);
+ return Ctor;
+}
+
std::pair<Function *, FunctionCallee> llvm::createSanitizerCtorAndInitFunctions(
Module &M, StringRef CtorName, StringRef InitName,
ArrayRef<Type *> InitArgTypes, ArrayRef<Value *> InitArgs,
@@ -128,11 +137,8 @@
"Sanitizer's init function expects different number of arguments");
FunctionCallee InitFunction =
declareSanitizerInitFunction(M, InitName, InitArgTypes);
- Function *Ctor = Function::Create(
- FunctionType::get(Type::getVoidTy(M.getContext()), false),
- GlobalValue::InternalLinkage, CtorName, &M);
- BasicBlock *CtorBB = BasicBlock::Create(M.getContext(), "", Ctor);
- IRBuilder<> IRB(ReturnInst::Create(M.getContext(), CtorBB));
+ Function *Ctor = createSanitizerCtor(M, CtorName);
+ IRBuilder<> IRB(Ctor->getEntryBlock().getTerminator());
IRB.CreateCall(InitFunction, InitArgs);
if (!VersionCheckName.empty()) {
FunctionCallee VersionCheckFunction = M.getOrInsertFunction(
Index: llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
===================================================================
--- llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
+++ llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
@@ -589,11 +589,10 @@
AddressSanitizer(Module &M, const GlobalsMetadata *GlobalsMD,
bool CompileKernel = false, bool Recover = false,
bool UseAfterScope = false)
- : UseAfterScope(UseAfterScope || ClUseAfterScope), GlobalsMD(*GlobalsMD) {
- this->Recover = ClRecover.getNumOccurrences() > 0 ? ClRecover : Recover;
- this->CompileKernel =
- ClEnableKasan.getNumOccurrences() > 0 ? ClEnableKasan : CompileKernel;
-
+ : CompileKernel(ClEnableKasan.getNumOccurrences() > 0 ? ClEnableKasan
+ : CompileKernel),
+ Recover(ClRecover.getNumOccurrences() > 0 ? ClRecover : Recover),
+ UseAfterScope(UseAfterScope || ClUseAfterScope), GlobalsMD(*GlobalsMD) {
C = &(M.getContext());
LongSize = M.getDataLayout().getPointerSizeInBits();
IntptrTy = Type::getIntNTy(*C, LongSize);
@@ -742,7 +741,11 @@
ModuleAddressSanitizer(Module &M, const GlobalsMetadata *GlobalsMD,
bool CompileKernel = false, bool Recover = false,
bool UseGlobalsGC = true, bool UseOdrIndicator = false)
- : GlobalsMD(*GlobalsMD), UseGlobalsGC(UseGlobalsGC && ClUseGlobalsGC),
+ : GlobalsMD(*GlobalsMD),
+ CompileKernel(ClEnableKasan.getNumOccurrences() > 0 ? ClEnableKasan
+ : CompileKernel),
+ Recover(ClRecover.getNumOccurrences() > 0 ? ClRecover : Recover),
+ UseGlobalsGC(UseGlobalsGC && ClUseGlobalsGC && !this->CompileKernel),
// Enable aliases as they should have no downside with ODR indicators.
UsePrivateAlias(UseOdrIndicator || ClUsePrivateAlias),
UseOdrIndicator(UseOdrIndicator || ClUseOdrIndicator),
@@ -753,11 +756,7 @@
// argument is designed as workaround. Therefore, disable both
// ClWithComdat and ClUseGlobalsGC unless the frontend says it's ok to
// do globals-gc.
- UseCtorComdat(UseGlobalsGC && ClWithComdat) {
- this->Recover = ClRecover.getNumOccurrences() > 0 ? ClRecover : Recover;
- this->CompileKernel =
- ClEnableKasan.getNumOccurrences() > 0 ? ClEnableKasan : CompileKernel;
-
+ UseCtorComdat(UseGlobalsGC && ClWithComdat && !this->CompileKernel) {
C = &(M.getContext());
int LongSize = M.getDataLayout().getPointerSizeInBits();
IntptrTy = Type::getIntNTy(*C, LongSize);
@@ -792,7 +791,8 @@
StringRef InternalSuffix);
Instruction *CreateAsanModuleDtor(Module &M);
- bool ShouldInstrumentGlobal(GlobalVariable *G);
+ bool canInstrumentAliasedGlobal(const GlobalAlias &GA) const;
+ bool shouldInstrumentGlobal(GlobalVariable *G) const;
bool ShouldUseMachOGlobalsSection() const;
StringRef getGlobalMetadataSection() const;
void poisonOneInitializer(Function &GlobalInit, GlobalValue *ModuleName);
@@ -1792,7 +1792,23 @@
}
}
-bool ModuleAddressSanitizer::ShouldInstrumentGlobal(GlobalVariable *G) {
+bool ModuleAddressSanitizer::canInstrumentAliasedGlobal(
+ const GlobalAlias &GA) const {
+ // In case this function should be expanded to include rules that do not just
+ // apply when CompileKernel is true, either guard all existing rules with an
+ // 'if (CompileKernel) { ... }' or be absolutely sure that all these rules
+ // should also apply to user space.
+ assert(CompileKernel && "Only expecting to be called when compiling kernel");
+
+ // When compiling the kernel, globals that are aliased by symbols prefixed
+ // by "__" are special and cannot be padded with a redzone.
+ if (GA.getName().startswith("__"))
+ return false;
+
+ return true;
+}
+
+bool ModuleAddressSanitizer::shouldInstrumentGlobal(GlobalVariable *G) const {
Type *Ty = G->getValueType();
LLVM_DEBUG(dbgs() << "GLOBAL: " << *G << "\n");
@@ -1839,6 +1855,12 @@
}
if (G->hasSection()) {
+ // The kernel uses explicit sections for mostly special global variables
+ // that we should not instrument. E.g. the kernel may rely on their layout
+ // without redzones, or remove them at link time ("discard.*"), etc.
+ if (CompileKernel)
+ return false;
+
StringRef Section = G->getSection();
// Globals from llvm.metadata aren't emitted, do not instrument them.
@@ -1905,6 +1927,13 @@
}
}
+ if (CompileKernel) {
+ // Globals that prefixed by "__" are special and cannot be padded with a
+ // redzone.
+ if (G->getName().startswith("__"))
+ return false;
+ }
+
return true;
}
@@ -2234,10 +2263,22 @@
bool *CtorComdat) {
*CtorComdat = false;
- SmallVector<GlobalVariable *, 16> GlobalsToChange;
+ // Build set of blacklisted globals that are aliased by an alias GA, where
+ // canInstrumentAliasedGlobal(A) returns false.
+ SmallPtrSet<const GlobalVariable *, 16> AliasedGlobalBlacklist;
+ if (CompileKernel) {
+ for (auto &GA : M.aliases()) {
+ if (const auto *GV = dyn_cast<GlobalVariable>(GA.getAliasee())) {
+ if (!canInstrumentAliasedGlobal(GA))
+ AliasedGlobalBlacklist.insert(GV);
+ }
+ }
+ }
+ SmallVector<GlobalVariable *, 16> GlobalsToChange;
for (auto &G : M.globals()) {
- if (ShouldInstrumentGlobal(&G)) GlobalsToChange.push_back(&G);
+ if (!AliasedGlobalBlacklist.count(&G) && shouldInstrumentGlobal(&G))
+ GlobalsToChange.push_back(&G);
}
size_t n = GlobalsToChange.size();
@@ -2454,20 +2495,23 @@
bool ModuleAddressSanitizer::instrumentModule(Module &M) {
initializeCallbacks(M);
- if (CompileKernel)
- return false;
-
// Create a module constructor. A destructor is created lazily because not all
// platforms, and not all modules need it.
- std::string AsanVersion = std::to_string(GetAsanVersion(M));
- std::string VersionCheckName =
- ClInsertVersionCheck ? (kAsanVersionCheckNamePrefix + AsanVersion) : "";
- std::tie(AsanCtorFunction, std::ignore) = createSanitizerCtorAndInitFunctions(
- M, kAsanModuleCtorName, kAsanInitName, /*InitArgTypes=*/{},
- /*InitArgs=*/{}, VersionCheckName);
+ if (CompileKernel) {
+ // The kernel always builds with its own runtime, and therefore does not
+ // need the init and version check calls.
+ AsanCtorFunction = createSanitizerCtor(M, kAsanModuleCtorName);
+ } else {
+ std::string AsanVersion = std::to_string(GetAsanVersion(M));
+ std::string VersionCheckName =
+ ClInsertVersionCheck ? (kAsanVersionCheckNamePrefix + AsanVersion) : "";
+ std::tie(AsanCtorFunction, std::ignore) =
+ createSanitizerCtorAndInitFunctions(M, kAsanModuleCtorName,
+ kAsanInitName, /*InitArgTypes=*/{},
+ /*InitArgs=*/{}, VersionCheckName);
+ }
bool CtorComdat = true;
- // TODO(glider): temporarily disabled globals instrumentation for KASan.
if (ClGlobals) {
IRBuilder<> IRB(AsanCtorFunction->getEntryBlock().getTerminator());
InstrumentGlobals(IRB, M, &CtorComdat);
Index: llvm/include/llvm/Transforms/Utils/ModuleUtils.h
===================================================================
--- llvm/include/llvm/Transforms/Utils/ModuleUtils.h
+++ llvm/include/llvm/Transforms/Utils/ModuleUtils.h
@@ -42,6 +42,10 @@
FunctionCallee declareSanitizerInitFunction(Module &M, StringRef InitName,
ArrayRef<Type *> InitArgTypes);
+/// Creates sanitizer constructor function.
+/// \return Returns pointer to constructor.
+Function *createSanitizerCtor(Module &M, StringRef CtorName);
+
/// Creates sanitizer constructor function, and calls sanitizer's init
/// function from it.
/// \return Returns pair of pointers to constructor, and init functions
Index: clang/test/CodeGen/asan-globals.cpp
===================================================================
--- clang/test/CodeGen/asan-globals.cpp
+++ clang/test/CodeGen/asan-globals.cpp
@@ -1,40 +1,75 @@
// RUN: echo "int extra_global;" > %t.extra-source.cpp
// RUN: echo "global:*blacklisted_global*" > %t.blacklist
-// RUN: %clang_cc1 -include %t.extra-source.cpp -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 --check-prefixes=CHECK,ASAN
+// RUN: %clang_cc1 -include %t.extra-source.cpp -fsanitize=kernel-address -fsanitize-blacklist=%t.blacklist -emit-llvm -o - %s | FileCheck %s --check-prefixes=CHECK,KASAN
// The blacklist file uses regexps, so Windows path backslashes.
// RUN: echo "src:%s" | sed -e 's/\\/\\\\/g' > %t.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
+// RUN: %clang_cc1 -include %t.extra-source.cpp -fsanitize=kernel-address -fsanitize-blacklist=%t.blacklist-src -emit-llvm -o - %s | FileCheck %s --check-prefix=BLACKLIST-SRC
int global;
int dyn_init_global = global;
int __attribute__((no_sanitize("address"))) attributed_global;
int blacklisted_global;
+int __attribute__((section("__DATA, __common"))) sectioned_global; // [KASAN] ignore globals in a section
+int aliased_global; // [KASAN] ignore globals prefixed by aliases with __-prefix [below]
+extern int __attribute__((alias("aliased_global"))) __global_alias; // [KASAN] aliased_global ignored
+int __special_global; // [KASAN]: ignore globals with __-prefix
void func() {
static int static_var = 0;
const char *literal = "Hello, world!";
}
-// CHECK: !llvm.asan.globals = !{![[EXTRA_GLOBAL:[0-9]+]], ![[GLOBAL:[0-9]+]], ![[DYN_INIT_GLOBAL:[0-9]+]], ![[ATTR_GLOBAL:[0-9]+]], ![[BLACKLISTED_GLOBAL:[0-9]+]], ![[STATIC_VAR:[0-9]+]], ![[LITERAL:[0-9]+]]}
+// ASAN: @sectioned_global = global { i32, [60 x i8] }
+// KASAN: @sectioned_global = global i32
+// ASAN: @aliased_global = global { i32, [60 x i8] }
+// KASAN: @aliased_global = global i32
+// ASAN: @__special_global = global { i32, [60 x i8] }
+// KASAN: @__special_global = global i32
+
+// CHECK-LABEL: define internal void @asan.module_ctor
+// ASAN-NEXT: call void @__asan_init
+// ASAN-NEXT: call void @__asan_version_mismatch_check
+// KASAN-NOT: call void @__asan_init
+// KASAN-NOT: call void @__asan_version_mismatch_check
+// ASAN-NEXT: call void @__asan_register_globals({{.*}}, i{{32|64}} 8)
+// KASAN-NEXT: call void @__asan_register_globals({{.*}}, i{{32|64}} 5)
+// CHECK-NEXT: ret void
+
+// CHECK-LABEL: define internal void @asan.module_dtor
+// CHECK-NEXT: call void @__asan_unregister_globals
+// CHECK-NEXT: ret void
+
+// CHECK: !llvm.asan.globals = !{![[EXTRA_GLOBAL:[0-9]+]], ![[GLOBAL:[0-9]+]], ![[DYN_INIT_GLOBAL:[0-9]+]], ![[ATTR_GLOBAL:[0-9]+]], ![[BLACKLISTED_GLOBAL:[0-9]+]], ![[SECTIONED_GLOBAL:[0-9]+]], ![[ALIASED_GLOBAL:[0-9]+]], ![[SPECIAL_GLOBAL:[0-9]+]], ![[STATIC_VAR:[0-9]+]], ![[LITERAL:[0-9]+]]}
// CHECK: ![[EXTRA_GLOBAL]] = !{{{.*}} ![[EXTRA_GLOBAL_LOC:[0-9]+]], !"extra_global", i1 false, i1 false}
// CHECK: ![[EXTRA_GLOBAL_LOC]] = !{!"{{.*}}extra-source.cpp", i32 1, i32 5}
// CHECK: ![[GLOBAL]] = !{{{.*}} ![[GLOBAL_LOC:[0-9]+]], !"global", i1 false, i1 false}
-// CHECK: ![[GLOBAL_LOC]] = !{!"{{.*}}asan-globals.cpp", i32 8, i32 5}
+// CHECK: ![[GLOBAL_LOC]] = !{!"{{.*}}asan-globals.cpp", i32 10, i32 5}
// CHECK: ![[DYN_INIT_GLOBAL]] = !{{{.*}} ![[DYN_INIT_LOC:[0-9]+]], !"dyn_init_global", i1 true, i1 false}
-// CHECK: ![[DYN_INIT_LOC]] = !{!"{{.*}}asan-globals.cpp", i32 9, i32 5}
+// CHECK: ![[DYN_INIT_LOC]] = !{!"{{.*}}asan-globals.cpp", i32 11, i32 5}
// CHECK: ![[ATTR_GLOBAL]] = !{{{.*}}, null, null, i1 false, i1 true}
// CHECK: ![[BLACKLISTED_GLOBAL]] = !{{{.*}}, null, null, i1 false, i1 true}
+// CHECK: ![[SECTIONED_GLOBAL]] = !{{{.*}} ![[SECTIONED_GLOBAL_LOC:[0-9]+]], !"sectioned_global", i1 false, i1 false}
+// CHECK: ![[SECTIONED_GLOBAL_LOC]] = !{!"{{.*}}asan-globals.cpp", i32 14, i32 51}
+// CHECK: ![[ALIASED_GLOBAL]] = !{{{.*}} ![[ALIASED_GLOBAL_LOC:[0-9]+]], !"aliased_global", i1 false, i1 false}
+// CHECK: ![[ALIASED_GLOBAL_LOC]] = !{!"{{.*}}asan-globals.cpp", i32 15, i32 5}
+// CHECK: ![[SPECIAL_GLOBAL]] = !{{{.*}} ![[SPECIAL_GLOBAL_LOC:[0-9]+]], !"__special_global", i1 false, i1 false}
+// CHECK: ![[SPECIAL_GLOBAL_LOC]] = !{!"{{.*}}asan-globals.cpp", i32 17, i32 5}
// CHECK: ![[STATIC_VAR]] = !{{{.*}} ![[STATIC_LOC:[0-9]+]], !"static_var", i1 false, i1 false}
-// CHECK: ![[STATIC_LOC]] = !{!"{{.*}}asan-globals.cpp", i32 14, i32 14}
+// CHECK: ![[STATIC_LOC]] = !{!"{{.*}}asan-globals.cpp", i32 20, i32 14}
// CHECK: ![[LITERAL]] = !{{{.*}} ![[LITERAL_LOC:[0-9]+]], !"<string literal>", i1 false, i1 false}
-// CHECK: ![[LITERAL_LOC]] = !{!"{{.*}}asan-globals.cpp", i32 15, i32 25}
+// CHECK: ![[LITERAL_LOC]] = !{!"{{.*}}asan-globals.cpp", i32 21, i32 25}
-// BLACKLIST-SRC: !llvm.asan.globals = !{![[EXTRA_GLOBAL:[0-9]+]], ![[GLOBAL:[0-9]+]], ![[DYN_INIT_GLOBAL:[0-9]+]], ![[ATTR_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]+]], ![[ATTR_GLOBAL:[0-9]+]], ![[BLACKLISTED_GLOBAL:[0-9]+]], ![[SECTIONED_GLOBAL:[0-9]+]], ![[ALIASED_GLOBAL:[0-9]+]], ![[SPECIAL_GLOBAL:[0-9]+]], ![[STATIC_VAR:[0-9]+]], ![[LITERAL:[0-9]+]]}
// BLACKLIST-SRC: ![[EXTRA_GLOBAL]] = !{{{.*}} ![[EXTRA_GLOBAL_LOC:[0-9]+]], !"extra_global", i1 false, i1 false}
// BLACKLIST-SRC: ![[EXTRA_GLOBAL_LOC]] = !{!"{{.*}}extra-source.cpp", i32 1, i32 5}
// BLACKLIST-SRC: ![[GLOBAL]] = !{{{.*}} null, null, i1 false, i1 true}
// BLACKLIST-SRC: ![[DYN_INIT_GLOBAL]] = !{{{.*}} null, null, i1 true, i1 true}
// BLACKLIST-SRC: ![[ATTR_GLOBAL]] = !{{{.*}}, null, null, i1 false, i1 true}
// BLACKLIST-SRC: ![[BLACKLISTED_GLOBAL]] = !{{{.*}}, null, null, i1 false, i1 true}
+// BLACKLIST-SRC: ![[SECTIONED_GLOBAL]] = !{{{.*}} null, null, i1 false, i1 true}
+// BLACKLIST-SRC: ![[ALIASED_GLOBAL]] = !{{{.*}} null, null, i1 false, i1 true}
+// BLACKLIST-SRC: ![[SPECIAL_GLOBAL]] = !{{{.*}} null, null, i1 false, i1 true}
// BLACKLIST-SRC: ![[STATIC_VAR]] = !{{{.*}} null, null, i1 false, i1 true}
// BLACKLIST-SRC: ![[LITERAL]] = !{{{.*}} null, null, i1 false, i1 true}
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
