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
  • [PATCH] D119367: [HWAS... Mitch Phillips via Phabricator via cfe-commits

Reply via email to