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

[Alexander Potapenko via Phabricator via llvm-commits]

glider 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,
----------------
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 ?)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115236