[PATCH] D102772: [SanitizeCoverage] Add support for NoSanitizeCoverage function attribute

[Marco Elver via Phabricator via cfe-commits]

melver added inline comments.


================
Comment at: clang/lib/CodeGen/CodeGenFunction.cpp:757-760
+    bool HaveSanitizeCoverage =
+        CGM.getCodeGenOpts().SanitizeCoverageType ||
+        CGM.getCodeGenOpts().SanitizeCoverageIndirectCalls ||
+        CGM.getCodeGenOpts().SanitizeCoverageTraceCmp;
----------------
vitalybuka wrote:
> consider adding method into CodeGenOptions and replace here and and the rest 
> ideally in a separate patch
> 
https://reviews.llvm.org/D102927


================
Comment at: clang/test/CodeGen/sanitize-coverage.c:23-87
+static inline __attribute__((__always_inline__)) void always_inlined_fn(int n) {
+  if (n)
+    x[n] = 42;
+}
+// CHECK-LABEL: define dso_local void @test_always_inline(
+void test_always_inline(int n) {
+  // CHECK-DAG: call void @__sanitizer_cov_trace_pc
----------------
melver wrote:
> vitalybuka wrote:
> > you are adding tests for unrelated code.
> > Could you please land is as a separate NFC patch first?
> Which bits in particular? Just the 
> 
> 
> ```
> static inline __attribute__((__always_inline__)) void always_inlined_fn(int n) {
>   if (n)
>     x[n] = 42;
> }
> // CHECK-LABEL: define dso_local void @test_always_inline(
> void test_always_inline(int n) {
>   // CHECK-DAG: call void @__sanitizer_cov_trace_pc
>   // CHECK-DAG: call void @__sanitizer_cov_trace_const_cmp
>   always_inlined_fn(n);
> }
> ```
> 
> part?
https://reviews.llvm.org/D102929


================
Comment at: llvm/lib/AsmParser/LLLexer.cpp:674
   KEYWORD(nounwind);
+  KEYWORD(no_sanitize_coverage);
   KEYWORD(null_pointer_is_valid);
----------------
vitalybuka wrote:
> looking at the rest, seems "nosanitize_coverage" follows pattern better :)
I wasn't sure myself, it's a bit inconsistent with "no_sanitize", but I've changed it. ;-)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102772