hctim created this revision. hctim added a reviewer: eugenis. Herald added subscribers: dexonsmith, hiraditya. Herald added a reviewer: aaron.ballman. hctim requested review of this revision. Herald added projects: clang, Sanitizers, LLVM. Herald added subscribers: llvm-commits, Sanitizers, cfe-commits.
Currently HWASan uses the no_sanitize shared with llvm.asan.globals to disable hwasanification. The llvm.asan.globals list contains a whole bunch of fluff (location, name, dynamically-initialized, excluded) in order for dynamic initialization in ASan. We don't do that in HWASan, globals have a static tag that's embedded in the file. Right now, it's not possible for no_sanitize("hwaddress") to be slapped on a global. Add this feature. In addition, add the metadata to the global variable itself, as to whether sanitization is disabled. Passes like GlobalOpt can replace the existing global, and only copies the metadata for the GlobalVariable itself, without replacing the GV pointers in llvm.asan.globals. This can cause bugs, where no_sanitize("hwaddress") (and "address" as well, but that's left for another time) ends up getting ignored if the global is replaced. Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D119367 Files: clang/lib/CodeGen/CGDecl.cpp clang/lib/CodeGen/CodeGenModule.cpp clang/lib/CodeGen/SanitizerMetadata.cpp clang/lib/CodeGen/SanitizerMetadata.h clang/lib/Sema/SemaDeclAttr.cpp compiler-rt/test/hwasan/TestCases/global-with-reduction.c compiler-rt/test/hwasan/TestCases/global.c llvm/include/llvm/IR/GlobalValue.h llvm/lib/IR/Globals.cpp llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp
Index: llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp =================================================================== --- llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp +++ llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp @@ -1717,34 +1717,10 @@ GV->eraseFromParent(); } -static DenseSet<GlobalVariable *> getExcludedGlobals(Module &M) { - NamedMDNode *Globals = M.getNamedMetadata("llvm.asan.globals"); - if (!Globals) - return DenseSet<GlobalVariable *>(); - DenseSet<GlobalVariable *> 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<Constant>(MDN->getOperand(0)); - // The optimizer may optimize away a global entirely. - if (!V) - continue; - auto *StrippedV = V->stripPointerCasts(); - auto *GV = dyn_cast<GlobalVariable>(StrippedV); - if (!GV) - continue; - ConstantInt *IsExcluded = mdconst::extract<ConstantInt>(MDN->getOperand(4)); - if (IsExcluded->isOne()) - Excluded.insert(GV); - } - return Excluded; -} - void HWAddressSanitizer::instrumentGlobals() { std::vector<GlobalVariable *> 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.") || Index: llvm/lib/IR/Globals.cpp =================================================================== --- llvm/lib/IR/Globals.cpp +++ llvm/lib/IR/Globals.cpp @@ -69,6 +69,7 @@ setDLLStorageClass(Src->getDLLStorageClass()); setDSOLocal(Src->isDSOLocal()); setPartition(Src->getPartition()); + setNoSanitize(Src->isNoSanitize()); } void GlobalValue::removeFromParent() { Index: llvm/include/llvm/IR/GlobalValue.h =================================================================== --- llvm/include/llvm/IR/GlobalValue.h +++ 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) { Index: compiler-rt/test/hwasan/TestCases/global.c =================================================================== --- compiler-rt/test/hwasan/TestCases/global.c +++ 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 <stdlib.h> + +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; } Index: compiler-rt/test/hwasan/TestCases/global-with-reduction.c =================================================================== --- compiler-rt/test/hwasan/TestCases/global-with-reduction.c +++ 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 <stdlib.h> + +// 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; } Index: clang/lib/Sema/SemaDeclAttr.cpp =================================================================== --- clang/lib/Sema/SemaDeclAttr.cpp +++ 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); Index: clang/lib/CodeGen/SanitizerMetadata.h =================================================================== --- clang/lib/CodeGen/SanitizerMetadata.h +++ 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: Index: clang/lib/CodeGen/SanitizerMetadata.cpp =================================================================== --- clang/lib/CodeGen/SanitizerMetadata.cpp +++ 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<NoSanitizeAttr>()) - 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<NoSanitizeAttr>()) { + 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) { Index: clang/lib/CodeGen/CodeGenModule.cpp =================================================================== --- clang/lib/CodeGen/CodeGenModule.cpp +++ 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); Index: clang/lib/CodeGen/CGDecl.cpp =================================================================== --- clang/lib/CodeGen/CGDecl.cpp +++ 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();
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits