[PATCH] D77877: [llvm][NFC] Inliner.cpp: ensure InlineHistory ID is always initialized;
David Li via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Fri Apr 10 10:25:27 PDT 2020
davidxl added a comment.
Re: "This would still make MSan unable to diagnose the bugs it could diagnose in the original code (If a new use of InlineHistoryID was introduced outside the IsTriviallDead condition - in the current code that could be caught by MSan or the Static Analyzer - now it'll be indistinguishable from an intentional use)"
I think the ultimate goal is to have logically correct code, and making *San happy is the secondary goal which choosing implementations which can not be proved to be correct statically. In the original change, the two choices are 1) one with uninitialized variable use, and 2) one with possible wrong init value. In this case, we should prefer choice 1 to help facilitate finding the bug at runtime or some other choice that can expose the bug in running the Assert build.
For the latest change, we can statically prove it is always correct -- as InlineHistoryID is attribute associated with the callsite. For the case you describe where there is new use in a different path, the right fix will be to eagerly initialize the ID too.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D77877/new/
https://reviews.llvm.org/D77877
More information about the llvm-commits
mailing list