[PATCH] D115236: [msan] Implement -msan-no-sanitize-all.

[Marco Elver via Phabricator via llvm-commits]

melver added inline comments.


================
Comment at: llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp:311
+static cl::opt<bool>
+    ClNoSanitizeAll("msan-no-sanitize-all",
+                    cl::desc("Apply no_sanitize to the whole file"), cl::Hidden,
----------------
glider wrote:
> vitalybuka wrote:
> > I don't see value in -all suffix on the flag applied to compilation unit. It's obvious that it's apply to "all"
> > "msan-no-sanitize-all" -> "msan-no-sanitize"
> > 
> > Maybe, but I am not sure: it's also a boolean flag why not just 
> > ```
> > static cl::opt<bool>
> > ClSanitize("msan-sanitize",
> > ...
> > cl::init(true));
> > ```
> > 
> > Are you going to add fronted flag? Usually they have -fsanitize-memory- prefix:
> > -fsanitize-memory-???? Also not sure what this could be.
> > 
> > -fsanitize-memory-ignore?
> > I don't see value in -all suffix on the flag applied to compilation unit. It's obvious that it's apply to "all"
> > "msan-no-sanitize-all" -> "msan-no-sanitize"
> 
> I imagine this being read as "apply no_sanitize to all functions", and "msan-no-sanitize" actually sounds very much like -fnosanitize=memory. Perhaps we need a better name...
> 
> > Maybe, but I am not sure: it's also a boolean flag why not just 
> > ```
> > static cl::opt<bool>
> > ClSanitize("msan-sanitize",
> > ...
> > cl::init(true));
> > ```
> > 
> > Are you going to add fronted flag? Usually they have -fsanitize-memory- prefix:
> > -fsanitize-memory-???? Also not sure what this could be.
> Good question, it didn't come to my mind. Not necessarily needed for KMSAN, but as long as we have the backend flag, might be a good idea.
> 
> > -fsanitize-memory-ignore?
> 
> -fsanitize-memory-ignore-all-checks, maybe?
> 
> (and -mllvm -msan-ignore-all-checks ?)
Probably, like Vitaly pointed out, the "all" might be redundant.

The other question is, are checks "ignored", or "disabled".

Perhaps one of the following:
```
-fsanitize-memory-ignore-checks / -mllvm -msan-ignore-checks
-fsanitize-memory-disable-checks / -mllvm -msan-disable-checks
```

whichever you find to be clearest.



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115236