[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 :)


https://reviews.llvm.org/D28330





More information about the cfe-commits mailing list