This patch fixes problems in multiple leak-like static analyzer checkers, which failed to mark symbols managed by them as possibly-dead, and due to that the leak was not detected even though the symbol was removed from the rest of the program state.
Original discussion in cfe-dev started here: http://lists.llvm.org/pipermail/cfe-dev/2016-March/047922.html
The patch implements the solution proposed by Ted Kremenek and described in cfe-dev here: http://lists.llvm.org/pipermail/cfe-dev/2016-March/048215.html
The patch eliminates the "dead set" from the SymbolReaper as a whole, because we now assume that the set of dead symbols is potentially infinite. Hence,dead_iterator, dead_begin(), dead_end() are removed. The maybeDead() API, which was used for marking symbols dead, is removed in order to avoid confusion. The hasDeadSymbols() function is removed as well. The isDead() is modified to mean !isLive().
In case API break is undesired, there's no problem bringing maybeDead() back as a synonym to isDead(), and define hasDeadSymbols() to always return true.
The patch passes the new test for SimpleStreamChecker described in the mailing lists above. It also fixes a FIXME test in unions.cpp, and throws a new warning in pr22954.c.
The new warning in pr22954.c seems correct with the current ideology, however we should probably eventually fix it. My reasoning for this is as follows. The pointer gets lost upon invalidation of m28[j].s4 after binding to l28->s1[l] which in fact aliases to m28.s3[l]. According to docs/analyzer/RegionStore.txt, such invalidation is the correct behavior: it is possible that j is equal to 1, and l is out of bounds and the binding actually overwrites one of the bytes in m28[j].s4. So it is the valid behavior of the analyzer to scrap all the bindings to the whole m28 variable region after this bind, which is being tested. However, because it is not *certain* that our pointer gets overwritten, we are not conservative enough when we are suspecting that our pointer was overwritten, so the report is not quite valid, and i may easily imagine false positives (eg. l when is constrained to [0, 3] on that path, we won't be able to handle that). So i should probably mark this as FIXME. However, i don't think this issue is the problem of the current patch; it should be solved separately, either by creating a special kind of PointerEscape to catch such cases, or improving invalidation accuracy by supporting offset constraints (or probably both).
There are a couple of shameful hotfixes in my code in the ExprInspection checker - included here because a problem was exposed by this patch in the symbol-reaper.c test, which now tests ExprInspection deeper.
I avoided fixing some FIXMEs in StreamChecker just around the new code, saving for another patch.
The patch was tested on Android open-source platform source code. I didn't test the patch on any large objective-c codebases, even though the patch alters some objective-c checkers. Performance has slightly degraded (~5%) - hmm, i guess the original approach is very performance-oriented. Also, it has found exactly one(!) new positive, which i'm attaching:. Not sure if it is a true bug (depends on external factors about the program which the analyzer could not have guessed anyway), but it is expected from the analyzer to find this positive, so i'm happy with this positive. Still funny to see how issues that seem major in description may actually be very minor when it comes to statistical quality.
So, overally, the patch fixes all the issues and simplifies checker API, but i'm not exactly sure we should go this way; probably going directly for smart data map traits would retain the performance while fixing the issue equally well. Probably we can revert this patch when we allow the core to introspect GDM contents and mark symbols dead automatically.