[PATCH] D126100: Add sanitizer-specific GlobalValue attributes.

[Vitaly Buka via Phabricator via cfe-commits]

vitalybuka added inline comments.


================
Comment at: llvm/include/llvm/IR/GlobalValue.h:329
+    }
+    void deserialize(unsigned V) {
+      if (V & (1 << 0)) NoSanitize = true;
----------------
vitalybuka wrote:
> serialization should not be in this header, this bit stuff is /Bitcode/ specific, both should be there
> serialization should not be in this header, this bit stuff is /Bitcode/ specific, both should be there

I don't see it's done


================
Comment at: llvm/lib/IR/AsmWriter.cpp:3543-3552
+    if (MD.NoSanitize) {
+      Out << ", no_sanitize";
+    } else {
+      if (MD.NoAddress)
+        Out << ", no_sanitize_address";
+      if (MD.NoHWAddress)
+        Out << ", no_sanitize_hwaddress";
----------------
even if it looks redundant I would not recommend to optimize here,
AsmWriter should not care about semantics. if you let SanitizerMetadata assign this way, then AsmWriter should store it as is.


================
Comment at: llvm/lib/IR/Globals.cpp:70
   setPartition(Src->getPartition());
+  if (Src->hasSanitizerMetadata())
+    setSanitizerMetadata(Src->getSanitizerMetadata());
----------------
It should copy unconditionally.
E.g. if "this"  had Metadata, but src does not , it must be reset here.


================
Comment at: llvm/lib/IR/Globals.cpp:231
+  getContext().pImpl->GlobalValueSanitizerMetadata[this] = Meta;
+  HasSanitizerMetadata = true;
+}
----------------



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126100