[HWASan] Allow no_sanitize(..) and change metadata passing.

[hctim]

Summary

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.

[eugenis]

You might want to update Bitcode writer/reader as well, otherwise nosanitize will be lost after a trip through .bc/.ii.

compiler-rt/test/hwasan/TestCases/global-with-reduction.c

50

did you mean to do it twice?

[pcc]

In addition to the .bc support we will also need support for reading/writing .ll files.

llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp

1721

Looks like you'll need to update llvm/test/Instrumentation/HWAddressSanitizer/globals.ll to use the new attribute instead (after adding the .ll support).

[hctim]

Deprecated by D127544.

clang/lib/CodeGen/CGDecl.cpp

void CodeGenFunction::EmitStaticVarDecl(const VarDecl &D,
   // FIXME: It is really dangerous to store this in the map; if anyone
   // RAUW's the GV uses of this constant will be invalid.
   llvm::Constant *castedAddr =
     llvm::ConstantExpr::getPointerBitCastOrAddrSpaceCast(var, expectedType);
   LocalDeclMap.find(&D)->second = Address(castedAddr, elemTy, alignment);
   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();
   if (DI && CGM.getCodeGenOpts().hasReducedDebugInfo()) {
     DI->setLocation(D.getLocation());
     DI->EmitGlobalVariable(var, &D);
   }
 }

clang/lib/CodeGen/CodeGenModule.cpp

if (GV->isDeclaration()) {
     getTargetCodeGenInfo().setTargetAttributes(D, GV, *this);
     // External HIP managed variables needed to be recorded for transformation
     // in both device and host compilations.
     if (getLangOpts().CUDA && D && D->hasAttr<HIPManagedAttr>() &&
         D->hasExternalStorage())
       getCUDARuntime().handleVarRegistration(D, *GV);
   }
 
+  if (D)
+    SanitizerMD->reportGlobalToHwasan(GV, *D);
+
   LangAS ExpectedAS =
       D ? D->getType().getAddressSpace()
         : (LangOpts.OpenCL ? LangAS::opencl_global : LangAS::Default);
   assert(getContext().getTargetAddressSpace(ExpectedAS) == TargetAS);
   if (DAddrSpace != ExpectedAS) {
     return getTargetCodeGenInfo().performAddrSpaceCast(
         *this, GV, DAddrSpace, ExpectedAS, Ty->getPointerTo(TargetAS));
   }

clang/lib/CodeGen/SanitizerMetadata.h

class SanitizerMetadata {
   CodeGenModule &CGM;
 public:
   SanitizerMetadata(CodeGenModule &CGM);
   void reportGlobalToASan(llvm::GlobalVariable *GV, const VarDecl &D,
                           bool IsDynInit = false);
   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:
   llvm::MDNode *getLocationMetadata(SourceLocation Loc);
 };
 }  // end namespace CodeGen
 }  // end namespace clang
 
 #endif

clang/lib/CodeGen/SanitizerMetadata.cpp

 #include "llvm/ADT/StringRef.h"
 #include "llvm/IR/Constants.h"
 
 using namespace clang;
 using namespace CodeGen;
 
 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);
 
   llvm::Metadata *LocDescr = nullptr;
   llvm::Metadata *GlobalName = nullptr;
   llvm::LLVMContext &VMContext = CGM.getLLVMContext();
   if (!IsExcluded) {
void SanitizerMetadata::reportGlobalToASan(llvm::GlobalVariable *GV,
   llvm::MDNode *ThisGlobal = llvm::MDNode::get(VMContext, GlobalMetadata);
   llvm::NamedMDNode *AsanGlobals =
       CGM.getModule().getOrInsertNamedMetadata("llvm.asan.globals");
   AsanGlobals->addOperand(ThisGlobal);
 }
 
 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) {
   I->setMetadata(CGM.getModule().getMDKindID("nosanitize"),
                  llvm::MDNode::get(CGM.getLLVMContext(), None));
 }
 
 llvm::MDNode *SanitizerMetadata::getLocationMetadata(SourceLocation Loc) {

clang/lib/Sema/SemaDeclAttr.cpp

for (unsigned I = 0, E = AL.getNumArgs(); I != E; ++I) {
 
     if (!S.checkStringLiteralArgumentAttr(AL, I, SanitizerName, &LiteralLoc))
       return;
 
     if (parseSanitizerValue(SanitizerName, /*AllowGroups=*/true) ==
             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);
   }
 
   D->addAttr(::new (S.Context) NoSanitizeAttr(S.Context, AL, Sanitizers.data(),
                                               Sanitizers.size()));
 }

compiler-rt/test/hwasan/TestCases/global-with-reduction.c

 // RUN: %clang_hwasan %s -o %t
 // RUN: %run %t 0
 // RUN: not %run %t 1 2>&1 | FileCheck --check-prefixes=CHECK,RSYM %s
 // RUN: not %env_hwasan_opts=symbolize=0 %run %t 1 2>&1 | FileCheck --check-prefixes=CHECK,RNOSYM %s
 // RUN: not %run %t -1 2>&1 | FileCheck --check-prefixes=CHECK,LSYM %s
 // RUN: not %env_hwasan_opts=symbolize=0 %run %t -1 2>&1 | FileCheck --check-prefixes=CHECK,LNOSYM %s
 
 // Test with and without optimizations, with and without PIC, since different
 // backend passes run depending on these flags.
 // RUN: %clang_hwasan -fno-pic %s -o %t
 // RUN: not %run %t 1 2>&1 | FileCheck --check-prefixes=CHECK,RSYM %s
 // RUN: %clang_hwasan -fno-pic -O2 %s -o %t
 // RUN: not %run %t 1 2>&1 | FileCheck --check-prefixes=CHECK,RSYM %s
 // 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;
 }

compiler-rt/test/hwasan/TestCases/global.c

 // RUN: %clang_hwasan %s -o %t
 // RUN: %run %t 0
 // RUN: not %run %t 1 2>&1 | FileCheck --check-prefixes=CHECK,RSYM %s
 // RUN: not %env_hwasan_opts=symbolize=0 %run %t 1 2>&1 | FileCheck --check-prefixes=CHECK,RNOSYM %s
 // RUN: not %run %t -1 2>&1 | FileCheck --check-prefixes=CHECK,LSYM %s
 // RUN: not %env_hwasan_opts=symbolize=0 %run %t -1 2>&1 | FileCheck --check-prefixes=CHECK,LNOSYM %s
 
 // Test with and without optimizations, with and without PIC, since different
 // backend passes run depending on these flags.
 // RUN: %clang_hwasan -fno-pic %s -o %t
 // RUN: not %run %t 1 2>&1 | FileCheck --check-prefixes=CHECK,RSYM %s
 // RUN: %clang_hwasan -fno-pic -O2 %s -o %t
 // RUN: not %run %t 1 2>&1 | FileCheck --check-prefixes=CHECK,RSYM %s
 // 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
   // RSYM: is located 0 bytes to the right of 4-byte global variable x {{.*}} in {{.*}}global.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
   // LNOSYM: is located to the left of a 4-byte global variable in
   // LNOSYM-NEXT: #0 0x{{.*}} ({{.*}}global.c.tmp+{{.*}})
   // CHECK-NOT: can not describe
   (&x)[atoi(argv[1])] = 1;
+  return 0;
 }

llvm/include/llvm/IR/GlobalValue.h

 protected:
   GlobalValue(Type *Ty, ValueTy VTy, Use *Ops, unsigned NumOps,
               LinkageTypes Linkage, const Twine &Name, unsigned AddressSpace)
       : Constant(PointerType::get(Ty, AddressSpace), VTy, Ops, NumOps),
         ValueType(Ty), Visibility(DefaultVisibility),
         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.
   unsigned Linkage : 4;       // The linkage of this global
   unsigned Visibility : 2;    // The visibility style of this global
   unsigned UnnamedAddrVal : 2; // This value's address is not significant
   unsigned DllStorageClass : 2; // DLL storage class
 
   unsigned ThreadLocal : 3; // Is this symbol "Thread Local", if so, what is
                             // the desired model?
 
   /// True if the function's name starts with "llvm.".  This corresponds to the
   /// value of Function::isIntrinsic(), which may be true even if
   /// Function::intrinsicID() returns Intrinsic::not_intrinsic.
   unsigned HasLLVMReservedName : 1;
 
   /// If true then there is a definition within the same linkage unit and that
   /// definition cannot be runtime preempted.
   unsigned IsDSOLocal : 1;
 
   /// True if this symbol has a partition name assigned (see
   /// 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;
 
   void destroyConstantImpl();
   Value *handleOperandChangeImpl(Value *From, Value *To);
 
   /// Returns true if the definition of this global may be replaced by a
public:
   void setVisibility(VisibilityTypes V) {
     assert((!hasLocalLinkage() || V == DefaultVisibility) &&
            "local linkage requires default visibility");
     Visibility = V;
     if (isImplicitDSOLocal())
       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) {
     setThreadLocalMode(Val ? GeneralDynamicTLSModel : NotThreadLocal);
   }
   void setThreadLocalMode(ThreadLocalMode Val) {
     assert(Val == NotThreadLocal || getValueID() != Value::FunctionVal);
     ThreadLocal = Val;

llvm/lib/IR/Globals.cpp

 /// create a GlobalValue) from the GlobalValue Src to this one.
 void GlobalValue::copyAttributesFrom(const GlobalValue *Src) {
   setVisibility(Src->getVisibility());
   setUnnamedAddr(Src->getUnnamedAddr());
   setThreadLocalMode(Src->getThreadLocalMode());
   setDLLStorageClass(Src->getDLLStorageClass());
   setDSOLocal(Src->isDSOLocal());
   setPartition(Src->getPartition());
+  setNoSanitize(Src->isNoSanitize());
 }
 
 void GlobalValue::removeFromParent() {
   switch (getValueID()) {
 #define HANDLE_GLOBAL_VALUE(NAME)                                              \
   case Value::NAME##Val:                                                       \
     return static_cast<NAME *>(this)->removeFromParent();
 #include "llvm/IR/Value.def"

llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp

void HWAddressSanitizer::instrumentGlobal(GlobalVariable *GV, uint8_t Tag) {
   auto *Alias = GlobalAlias::create(GV->getValueType(), GV->getAddressSpace(),
                                     GV->getLinkage(), "", Aliasee, &M);
   Alias->setVisibility(GV->getVisibility());
   Alias->takeName(GV);
   GV->replaceAllUsesWith(Alias);
   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.") ||
         GV.isThreadLocal())
       continue;
 
     // Common symbols can't have aliases point to them, so they can't be tagged.
     if (GV.hasCommonLinkage())