kda added inline comments.
================
Comment at: llvm/include/llvm/Transforms/Instrumentation/MemorySanitizer.h:28
bool Recover;
+ bool EagerChecks;
};
----------------
vitalybuka wrote:
> maybe we should use CheckParamRetVal something here?
>
How about EagerChecksRequested?
================
Comment at: llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp:496
Recover(Options.Recover) {
+ if (Options.EagerChecks) {
+ ClEagerChecks = Options.EagerChecks;
----------------
vitalybuka wrote:
> mllvm is lower level flag, so it should be opposite
> -mllvm flag should override MemorySanitizerOptions
>
> with getOptOrDefault below you don't need anything here
I disagree. The variable ClEagerChecks is referenced in the code. If it is
not configured based on EagerChecksRequested, there is no way to change the
behavior based on the presence of the flag.
I will admit that with the change below, it is a bit circular, because
Options.EagerChecksRequested prefers the -msan-eager-checks flag, but may
default to EagerChecksRequested parameter, and then here the flag variable
(ClEagerChecks) is set based on Options.EagerChecksRequested.
An alternative is to have a member variable of MemorySanitizer that is
consulted in the code. It is configured based on Options.EagerChecksRequested.
The flag (-msan-eager-checks) is only consulted below with the call to
getOptOrDefault. (I coded this up: https://reviews.llvm.org/D116855)
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D116634/new/
https://reviews.llvm.org/D116634
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
