[PATCH] D150460: [gcov] Add nosanitize metadata to memory access instructions inserted by emitProfileNotes()

[Nick Desaulniers via Phabricator via llvm-commits]

nickdesaulniers added a comment.

Thanks for the patch! Why do we want to skip sanitizing these loads and stores?

Also, `gocv` is misspelled in the commit description, please fix that up.



================
Comment at: llvm/lib/Transforms/Instrumentation/GCOVProfiling.cpp:150-152
+  void setNoSanitizeMetadata(Instruction *I) {
+    I->setMetadata(LLVMContext::MD_nosanitize, MDNode::get(*Ctx, std::nullopt));
+  }
----------------
This looks the same as `SanitizerMetadata::disableSanitizerForInstruction` and `ModuleSanitizerCoverage::SetNoSanitizeMetadata`. Rather than define a third copy, why not declare it as a new method on Instruction and define it in llvm/lib/IR/Metadata.cpp, then use it in all three places?


================
Comment at: llvm/lib/Transforms/Instrumentation/GCOVProfiling.cpp:927-935
+            auto *RMW = Builder.CreateAtomicRMW(
+                AtomicRMWInst::Add, V, Builder.getInt64(1), MaybeAlign(),
+                AtomicOrdering::Monotonic);
+            setNoSanitizeMetadata(RMW);
           } else {
-            Value *Count =
+            auto *OldCount =
                 Builder.CreateLoad(Builder.getInt64Ty(), V, "gcov_ctr");
----------------
Please use `Instruction *` rather than `auto`.
https://llvm.org/docs/CodingStandards.html#use-auto-type-deduction-to-make-code-more-readable


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150460