[PATCH] D126929: Add sanitizer metadata attributes to clang IR gen.

[Mitch Phillips via Phabricator via llvm-commits]

hctim marked 3 inline comments as done.
hctim added inline comments.


================
Comment at: clang/lib/CodeGen/CodeGenModule.cpp:2767-2781
+  if (NoSanitizeL.containsGlobal(LangOpts.Sanitize.Mask, GV->getName(), Category))
     return true;
-  if (NoSanitizeL.containsLocation(EnabledAsanMask, Loc, Category))
+  if (NoSanitizeL.containsLocation(LangOpts.Sanitize.Mask, Loc, Category))
     return true;
   // Check global type.
   if (!Ty.isNull()) {
     // Drill down the array types: if global variable of a fixed type is
----------------
vitalybuka wrote:
> can this lines be landed separately?
sure


================
Comment at: clang/lib/CodeGen/SanitizerMetadata.cpp:44
 
+  if (CGM.isInNoSanitizeList(GV, Loc, Ty))
+    Meta.NoSanitize = true;
----------------
vitalybuka wrote:
> it's incorrect as is
> isInNoSanitizeList is sanitizer specific
> we need to add isInNoSanitizeList version which return relevant SanitizerMask
sure, added the sanitizer-specific parsing as well, including a test (sanitizer-special-case-list-globals.txt)


================
Comment at: clang/lib/CodeGen/SanitizerMetadata.cpp:69
+                                          SanitizerKind::KernelAddress)) {
+    Meta.IsDynInit = IsDynInit && !CGM.isInNoSanitizeList(GV, Loc, Ty, "init");
+  }
----------------
vitalybuka wrote:
> Why don't we care about IsExcluded here?
doesn't matter as the global is disabled anyway, and this preserves previous behaviour which the existing tests assert on.

i'll make it conditional in the follow up change, as all the `llvm.asan.globals` tests need to be deleted. added a note.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126929