[PATCH] D28330: [analyzer] Fix false positives in Keychain API checker
Artem Dergachev via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Mon Jan 9 02:16:08 PST 2017
NoQ accepted this revision.
NoQ added a reviewer: NoQ.
NoQ added a comment.
In https://reviews.llvm.org/D28330#637075, @zaks.anna wrote:
> I did not think of solution #1! It's definitely better than the pattern matching I've added here. However, this checker fires so infrequently, that I do not think it's worth investing more time into perfecting it.
Well, the thing i like about your solution is that it resists arbitrary state splits. Eg., if the reason for the state split is some body farm function or a checker state split, which doesn't constitute a programmer's intent to check for error, then there's no reason to clean state. However, we won't be able to find this out by looking at range constraints retrospectively - we'd only know it during `evalAssume`/`checkBranchCondition`. However, currently we're doing a great job in other places around the analyzer to avoid unnecessary state splits, so range constraints are pretty reliable.
In fact, a check in an inlined function should also not be considered a valid reason for state cleanup. Or probably even for a state split. But that's another story. Anyway, i approve your approach here :)
More information about the cfe-commits