[PATCH] D41258: [analyzer] trackNullOrUndefValue: deduplicate path pieces for each node.
Artem Dergachev via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Thu Dec 14 13:42:49 PST 2017
NoQ created this revision.
NoQ added reviewers: dcoughlin, xazax.hun, a.sidorin, george.karpenkov, szepet.
Herald added subscribers: cfe-commits, rnkovacs.
This addresses the regression in https://reviews.llvm.org/D41254 in `inlining/path-notes.cpp` by adding a new straightforward mechanism that deduplicates path diagnostic pieces constructed by visitors for the same exploded graph node.
It would be incorrect to remove duplicate pieces globally for the whole report, because pieces are attached to source locations without node or even location context information, and source locations can easily coincide within a loop or a recursive function. It would also not be correct to deduplicate subsequent duplicate pieces, because the order in which visitors fire is not reliable, even if deterministic, so there may easily appear an unrelated diagnostic between duplicate diagnostics attached to the same node.
The regression in https://reviews.llvm.org/D41254 appeared because `FindLastStoreBRVisitor` was added twice - directly by `DivZeroChecker` for the divisor and indirectly by `ReturnVisitor` for the function call return value to which the divisor was tracked. However, the `EnableNullFPSuppression` flag for the two copies of the visitor had different value: `true` for the former, `false` for the latter. It is false for the latter because we want to suppress reports on "return null" paths from functions (even if it wasn't an inlined defensive check, just plain null) only when "null" is a pointer. So we get two different visitors finding last store to the visitor.
For now, because we are completely unprotected from such issues, i thought that simply deduplicating nodes should make the whole thing safer without obvious downsides. In general, just turning this whole machinery into a single big visitor might be a better idea.
-------------- next part --------------
A non-text attachment was scrubbed...
Size: 91252 bytes
Desc: not available
More information about the cfe-commits