diff --git a/clang/lib/CodeGen/CGDecl.cpp b/clang/lib/CodeGen/CGDecl.cpp --- a/clang/lib/CodeGen/CGDecl.cpp +++ b/clang/lib/CodeGen/CGDecl.cpp @@ -463,6 +463,7 @@ CGM.setStaticLocalDeclAddress(&D, castedAddr); CGM.getSanitizerMetadata()->reportGlobalToASan(var, D); + CGM.getSanitizerMetadata()->reportGlobalToHwasan(var, D); // Emit global variable debug descriptor for static vars. CGDebugInfo *DI = getDebugInfo(); diff --git a/clang/lib/CodeGen/CodeGenModule.cpp b/clang/lib/CodeGen/CodeGenModule.cpp --- a/clang/lib/CodeGen/CodeGenModule.cpp +++ b/clang/lib/CodeGen/CodeGenModule.cpp @@ -4195,6 +4195,9 @@ getCUDARuntime().handleVarRegistration(D, *GV); } + if (D) + SanitizerMD->reportGlobalToHwasan(GV, *D); + LangAS ExpectedAS = D ? D->getType().getAddressSpace() : (LangOpts.OpenCL ? LangAS::opencl_global : LangAS::Default); diff --git a/clang/lib/CodeGen/SanitizerMetadata.h b/clang/lib/CodeGen/SanitizerMetadata.h --- a/clang/lib/CodeGen/SanitizerMetadata.h +++ b/clang/lib/CodeGen/SanitizerMetadata.h @@ -41,6 +41,8 @@ void reportGlobalToASan(llvm::GlobalVariable *GV, SourceLocation Loc, StringRef Name, QualType Ty, bool IsDynInit = false, bool IsExcluded = false); + void reportDisabledGlobalToHwasan(llvm::GlobalVariable *GV); + void reportGlobalToHwasan(llvm::GlobalVariable *GV, const VarDecl &D); void disableSanitizerForGlobal(llvm::GlobalVariable *GV); void disableSanitizerForInstruction(llvm::Instruction *I); private: diff --git a/clang/lib/CodeGen/SanitizerMetadata.cpp b/clang/lib/CodeGen/SanitizerMetadata.cpp --- a/clang/lib/CodeGen/SanitizerMetadata.cpp +++ b/clang/lib/CodeGen/SanitizerMetadata.cpp @@ -22,17 +22,19 @@ SanitizerMetadata::SanitizerMetadata(CodeGenModule &CGM) : CGM(CGM) {} -static bool isAsanHwasanOrMemTag(const SanitizerSet& SS) { - return SS.hasOneOf(SanitizerKind::Address | SanitizerKind::KernelAddress | - SanitizerKind::HWAddress | SanitizerKind::KernelHWAddress | - SanitizerKind::MemTag); +static bool isAsan(const SanitizerSet &SS) { + return SS.hasOneOf(SanitizerKind::Address | SanitizerKind::KernelAddress); +} + +static bool isHwasan(const SanitizerSet &SS) { + return SS.hasOneOf(SanitizerKind::HWAddress | SanitizerKind::KernelHWAddress); } void SanitizerMetadata::reportGlobalToASan(llvm::GlobalVariable *GV, SourceLocation Loc, StringRef Name, QualType Ty, bool IsDynInit, bool IsExcluded) { - if (!isAsanHwasanOrMemTag(CGM.getLangOpts().Sanitize)) + if (!isAsan(CGM.getLangOpts().Sanitize)) return; IsDynInit &= !CGM.isInNoSanitizeList(GV, Loc, Ty, "init"); IsExcluded |= CGM.isInNoSanitizeList(GV, Loc, Ty); @@ -63,25 +65,52 @@ void SanitizerMetadata::reportGlobalToASan(llvm::GlobalVariable *GV, const VarDecl &D, bool IsDynInit) { - if (!isAsanHwasanOrMemTag(CGM.getLangOpts().Sanitize)) + if (!isAsan(CGM.getLangOpts().Sanitize)) return; + std::string QualName; llvm::raw_string_ostream OS(QualName); D.printQualifiedName(OS); bool IsExcluded = false; for (auto Attr : D.specific_attrs()) - if (Attr->getMask() & SanitizerKind::Address) + if (Attr->getMask() & (SanitizerKind::Address)) IsExcluded = true; reportGlobalToASan(GV, D.getLocation(), OS.str(), D.getType(), IsDynInit, IsExcluded); } +void SanitizerMetadata::reportDisabledGlobalToHwasan(llvm::GlobalVariable *GV) { + if (!isHwasan(CGM.getLangOpts().Sanitize)) + return; + + GV->setNoSanitize(true); +} + +void SanitizerMetadata::reportGlobalToHwasan(llvm::GlobalVariable *GV, + const VarDecl &D) { + if (!isHwasan(CGM.getLangOpts().Sanitize)) + return; + + bool IsExcluded = false; + for (auto Attr : D.specific_attrs()) { + if (Attr->getMask() & SanitizerKind::HWAddress) + IsExcluded = true; + } + + if (!IsExcluded) + return; + + reportDisabledGlobalToHwasan(GV); +} + void SanitizerMetadata::disableSanitizerForGlobal(llvm::GlobalVariable *GV) { // For now, just make sure the global is not modified by the ASan // instrumentation. - if (isAsanHwasanOrMemTag(CGM.getLangOpts().Sanitize)) + if (isAsan(CGM.getLangOpts().Sanitize)) reportGlobalToASan(GV, SourceLocation(), "", QualType(), false, true); + else if (isHwasan(CGM.getLangOpts().Sanitize)) + reportDisabledGlobalToHwasan(GV); } void SanitizerMetadata::disableSanitizerForInstruction(llvm::Instruction *I) { diff --git a/clang/lib/Sema/SemaDeclAttr.cpp b/clang/lib/Sema/SemaDeclAttr.cpp --- a/clang/lib/Sema/SemaDeclAttr.cpp +++ b/clang/lib/Sema/SemaDeclAttr.cpp @@ -7734,7 +7734,8 @@ SanitizerMask() && SanitizerName != "coverage") S.Diag(LiteralLoc, diag::warn_unknown_sanitizer_ignored) << SanitizerName; - else if (isGlobalVar(D) && SanitizerName != "address") + else if (isGlobalVar(D) && SanitizerName != "address" && + SanitizerName != "hwaddress") S.Diag(D->getLocation(), diag::err_attribute_wrong_decl_type) << AL << ExpectedFunctionOrMethod; Sanitizers.push_back(SanitizerName); diff --git a/compiler-rt/test/hwasan/TestCases/global.c b/compiler-rt/test/hwasan/TestCases/global-with-reduction.c copy from compiler-rt/test/hwasan/TestCases/global.c copy to compiler-rt/test/hwasan/TestCases/global-with-reduction.c --- a/compiler-rt/test/hwasan/TestCases/global.c +++ b/compiler-rt/test/hwasan/TestCases/global-with-reduction.c @@ -14,18 +14,39 @@ // RUN: %clang_hwasan -O2 %s -o %t // RUN: not %run %t 1 2>&1 | FileCheck --check-prefixes=CHECK,RSYM %s +// RUN: %clang_hwasan -DUSE_NOSANITIZE %s -o %t && %run %t 0 +// RUN: %clang_hwasan -DUSE_NOSANITIZE %s -o %t && %run %t 1 +// RUN: %clang_hwasan -DUSE_NOSANITIZE %s -o %t -fno-pic && %run %t 1 +// RUN: %clang_hwasan -DUSE_NOSANITIZE %s -o %t -O2 && %run %t 1 +// RUN: %clang_hwasan -DUSE_NOSANITIZE %s -o %t -fno-pic -O2 && %run %t 1 + // REQUIRES: pointer-tagging -int x = 1; +#include + +// GlobalOpt may replace the current GV with a new boolean-typed GV. Previously, +// this resulted in the "nosanitize" getting dropped because while the data/code +// references to the GV were updated, the old metadata references weren't. +int* f() { +#ifdef USE_NOSANITIZE +__attribute__((no_sanitize("hwaddress"))) static int x = 1; +#else // USE_NOSANITIZE + static int x = 1; +#endif // USE_NOSANITIZE + if (x == 1) x = 0; + return &x; +} int main(int argc, char **argv) { // CHECK: Cause: global-overflow - // RSYM: is located 0 bytes to the right of 4-byte global variable x {{.*}} in {{.*}}global.c.tmp + // RSYM: is located 0 bytes to the right of 4-byte global variable f.x {{.*}} in {{.*}}global-with-reduction.c.tmp // RNOSYM: is located to the right of a 4-byte global variable in - // RNOSYM-NEXT: #0 0x{{.*}} ({{.*}}global.c.tmp+{{.*}}) - // LSYM: is located 4 bytes to the left of 4-byte global variable x {{.*}} in {{.*}}global.c.tmp + // RNOSYM-NEXT: #0 0x{{.*}} ({{.*}}global-with-reduction.c.tmp+{{.*}}) + // LSYM: is located 4 bytes to the left of 4-byte global variable f.x {{.*}} in {{.*}}global-with-reduction.c.tmp // LNOSYM: is located to the left of a 4-byte global variable in - // LNOSYM-NEXT: #0 0x{{.*}} ({{.*}}global.c.tmp+{{.*}}) + // LNOSYM-NEXT: #0 0x{{.*}} ({{.*}}global-with-reduction.c.tmp+{{.*}}) // CHECK-NOT: can not describe - (&x)[atoi(argv[1])] = 1; + f()[atoi(argv[1])] = 1; + f()[atoi(argv[1])] = 1; + return 0; } diff --git a/compiler-rt/test/hwasan/TestCases/global.c b/compiler-rt/test/hwasan/TestCases/global.c --- a/compiler-rt/test/hwasan/TestCases/global.c +++ b/compiler-rt/test/hwasan/TestCases/global.c @@ -14,9 +14,23 @@ // RUN: %clang_hwasan -O2 %s -o %t // RUN: not %run %t 1 2>&1 | FileCheck --check-prefixes=CHECK,RSYM %s +// RUN: %clang_hwasan -DUSE_NOSANITIZE %s -o %t && %run %t 0 +// RUN: %clang_hwasan -DUSE_NOSANITIZE %s -o %t && %run %t 1 +// RUN: %clang_hwasan -DUSE_NOSANITIZE %s -o %t -fno-pic && %run %t 1 +// RUN: %clang_hwasan -DUSE_NOSANITIZE %s -o %t -O2 && %run %t 1 +// RUN: %clang_hwasan -DUSE_NOSANITIZE %s -o %t -fno-pic -O2 && %run %t 1 + // REQUIRES: pointer-tagging +#include + +int a = 1; +#ifdef USE_NOSANITIZE +__attribute__((no_sanitize("hwaddress"))) int x = 1; +#else // USE_NOSANITIZE int x = 1; +#endif // USE_NOSANITIZE +int b = 1; int main(int argc, char **argv) { // CHECK: Cause: global-overflow @@ -28,4 +42,5 @@ // LNOSYM-NEXT: #0 0x{{.*}} ({{.*}}global.c.tmp+{{.*}}) // CHECK-NOT: can not describe (&x)[atoi(argv[1])] = 1; + return 0; } diff --git a/llvm/include/llvm/IR/GlobalValue.h b/llvm/include/llvm/IR/GlobalValue.h --- a/llvm/include/llvm/IR/GlobalValue.h +++ b/llvm/include/llvm/IR/GlobalValue.h @@ -80,14 +80,14 @@ UnnamedAddrVal(unsigned(UnnamedAddr::None)), DllStorageClass(DefaultStorageClass), ThreadLocal(NotThreadLocal), HasLLVMReservedName(false), IsDSOLocal(false), HasPartition(false), - IntID((Intrinsic::ID)0U), Parent(nullptr) { + NoSanitize(false), IntID((Intrinsic::ID)0U), Parent(nullptr) { setLinkage(Linkage); setName(Name); } Type *ValueType; - static const unsigned GlobalValueSubClassDataBits = 16; + static const unsigned GlobalValueSubClassDataBits = 15; // All bitfields use unsigned as the underlying type so that MSVC will pack // them. @@ -112,9 +112,15 @@ /// https://lld.llvm.org/Partitions.html). unsigned HasPartition : 1; + /// Should this variable be excluded from sanitization. Used for HWASan, so + /// that global variables can be marked as + /// __attribute__((no_sanitize("hwaddress"))), as well as ensuring that + /// markers for other sanitizers (e.g. ubsan) don't get sanitized. + unsigned NoSanitize : 1; + private: // Give subclasses access to what otherwise would be wasted padding. - // (16 + 4 + 2 + 2 + 2 + 3 + 1 + 1 + 1) == 32. + // (15 + 4 + 2 + 2 + 2 + 3 + 1 + 1 + 1 + 1) == 32. unsigned SubClassData : GlobalValueSubClassDataBits; friend class Constant; @@ -240,6 +246,9 @@ setDSOLocal(true); } + bool isNoSanitize() const { return NoSanitize; } + void setNoSanitize(bool S) { NoSanitize = S; } + /// If the value is "Thread Local", its value isn't shared by the threads. bool isThreadLocal() const { return getThreadLocalMode() != NotThreadLocal; } void setThreadLocal(bool Val) { diff --git a/llvm/lib/IR/Globals.cpp b/llvm/lib/IR/Globals.cpp --- a/llvm/lib/IR/Globals.cpp +++ b/llvm/lib/IR/Globals.cpp @@ -69,6 +69,7 @@ setDLLStorageClass(Src->getDLLStorageClass()); setDSOLocal(Src->isDSOLocal()); setPartition(Src->getPartition()); + setNoSanitize(Src->isNoSanitize()); } void GlobalValue::removeFromParent() { diff --git a/llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp b/llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp --- a/llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp +++ b/llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp @@ -1717,34 +1717,10 @@ GV->eraseFromParent(); } -static DenseSet getExcludedGlobals(Module &M) { - NamedMDNode *Globals = M.getNamedMetadata("llvm.asan.globals"); - if (!Globals) - return DenseSet(); - DenseSet Excluded(Globals->getNumOperands()); - for (auto MDN : Globals->operands()) { - // Metadata node contains the global and the fields of "Entry". - assert(MDN->getNumOperands() == 5); - auto *V = mdconst::extract_or_null(MDN->getOperand(0)); - // The optimizer may optimize away a global entirely. - if (!V) - continue; - auto *StrippedV = V->stripPointerCasts(); - auto *GV = dyn_cast(StrippedV); - if (!GV) - continue; - ConstantInt *IsExcluded = mdconst::extract(MDN->getOperand(4)); - if (IsExcluded->isOne()) - Excluded.insert(GV); - } - return Excluded; -} - void HWAddressSanitizer::instrumentGlobals() { std::vector Globals; - auto ExcludedGlobals = getExcludedGlobals(M); for (GlobalVariable &GV : M.globals()) { - if (ExcludedGlobals.count(&GV)) + if (GV.isNoSanitize()) continue; if (GV.isDeclarationForLinker() || GV.getName().startswith("llvm.") ||