kda added inline comments.
================ Comment at: clang/lib/CodeGen/CGCall.cpp:2384 + if ((CodeGenOpts.EnableNoundefAttrs || + CodeGenOpts.SanitizeMemoryParamRetval) && + ArgNoUndef) ---------------- eugenis wrote: > vitalybuka wrote: > > vitalybuka wrote: > > > @eugenis Would be better to force CodeGenOpts.EnableNoundefAttrs if > > > SanitizeMemoryParamRetval is provided? > > > @eugenis Would be better to force CodeGenOpts.EnableNoundefAttrs if > > > SanitizeMemoryParamRetval is provided? > > > > To clarify, checking SanitizeMemoryParamRetval here as-is is LGTM, unless > > @eugenis or someone else thinks EnableNoundefAttrs reset is better. > I don't think this is right at all. An argument being noundef has nothing to > do at all with memory sanitization. It affects optimization, too. > SanitizeMemoryParamRetval should only affect what MemorySanitizerPass does > with noundef attributes. Is the problem the form of the new code? - instead of introducing a single new flag to flip 4 others, should there be 1 flag to pick up the CodeGen changes, and another for the Sanitizer? (Is 2 flags better than 4?) - another option is have the changes propagate in the other direction: use the flag (-fsanitize-memory-param-retval), to passes along '-mllvm -enable_noundef_analysis=1' to CodeGen (via SanitizerArgs.addArgs). OR is there a problem forcing EnableNoundefAttrs based on an "unrelated" flag? - then leave existing code, just don't do anything fancy to change EnableNoundefAttrs. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D116633/new/ https://reviews.llvm.org/D116633 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
