[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