[PATCH] D33092: [analyzer] Add checker to model builtin functions

[Artem Dergachev via Phabricator via cfe-commits]

NoQ accepted this revision.
NoQ added a comment.
This revision is now accepted and ready to land.

Great, thanks!



================
Comment at: lib/StaticAnalyzer/Checkers/BuiltinFunctionChecker.cpp:51
+    state = state->assume(ArgSVal.castAs<DefinedOrUnknownSVal>(), true);
+    // FIXME: do we want to warn here?
+    if (!state)
----------------
That's a good question, because `assume(false)` seems much more broken than `assert(false)`.

But because we are overaggressive on branching as compared to only relying on presumption of code liveness (eg. splitting `void foo(x, y) { if (x) ...; if (y) ...; }` into 4 paths is unsafe - all code may be live without all paths being feasible - but we do that anyway, same with loops and eager assumptions and our aliasing approach and the whole inlining thing), i think we'd rather not warn.

I mean, due to the above, it is much more likely that the assumption is valid and the branch on which it fails is infeasible due to some function contract we dont' see, than that the assumption is actually violated. We may do well about null dereferences and such, but generally the static analyzer should try to avoid parts of code that have been carefully thought out :) We may probably want to catch cases when the assumption fails on all paths reaching it, but we shouldn't use symbolic execution for all-paths warnings.


https://reviews.llvm.org/D33092