[PATCH] D133157: Add -fsanitizer-coverage=control-flow

[Navid Emamdoost via Phabricator via cfe-commits]

Navidem added inline comments.


================
Comment at: clang/lib/Driver/SanitizerArgs.cpp:814
         (CoverageTracePC | CoverageTracePCGuard | CoverageInline8bitCounters |
-         CoverageInlineBoolFlag))
+         CoverageInlineBoolFlag | CoverageControlFlow))
       CoverageFeatures |= CoverageEdge;
----------------
vitalybuka wrote:
> why do you need CoverageEdge if enabled CoverageControlFlow?
No essential need, but here I am implicitly enabling `edge` when instrumentation type is not specified. This was the way that I could get `-fsanitizer-coverage=control-flow` working without passing `-fsanitizer-coverage=bb,control-flow`.


================
Comment at: clang/lib/Driver/SanitizerArgs.cpp:807
         << "-fsanitize-coverage=[func|bb|edge]"
-        << "-fsanitize-coverage=[func|bb|edge],[trace-pc-guard|trace-pc]";
+        << "-fsanitize-coverage=[func|bb|edge],[trace-pc-guard|trace-pc|control-flow]";
   }
----------------
Navidem wrote:
> vitalybuka wrote:
> > shouldn't this be:
> > ,[trace-pc-guard|trace-pc],[control-flow]
> > 
> > probably even:
> > [,(trace-pc-guard|trace-pc)][,control-flow]
> > shouldn't this be:
> > ,[trace-pc-guard|trace-pc],[control-flow]
> > 
> > probably even:
> > [,(trace-pc-guard|trace-pc)][,control-flow]
> 
> 
> shouldn't this be:
> ,[trace-pc-guard|trace-pc],[control-flow]

This makes sense, thanks!




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133157