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

[Mitch Phillips via Phabricator via cfe-commits]

hctim abandoned this revision.
hctim added a comment.
Herald added a subscriber: Enna1.
Herald added a project: All.

Deprecated by D127544 .


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D119367/new/

https://reviews.llvm.org/D119367

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Peter Collingbourne via Phabricator via cfe-commits]

pcc added a comment.

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




Comment at: llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp:1721
-static DenseSet getExcludedGlobals(Module &M) {
-  NamedMDNode *Globals = M.getNamedMetadata("llvm.asan.globals");
-  if (!Globals)

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


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D119367/new/

https://reviews.llvm.org/D119367

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Evgenii Stepanov via Phabricator via cfe-commits]

eugenis added a comment.

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




Comment at: compiler-rt/test/hwasan/TestCases/global-with-reduction.c:50
+  f()[atoi(argv[1])] = 1;
+  f()[atoi(argv[1])] = 1;
+  return 0;

did you mean to do it twice?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D119367/new/

https://reviews.llvm.org/D119367

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Mitch Phillips via Phabricator via cfe-commits]

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 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.") ||
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 @@
   s