[cfe-commits] r153010 - in /cfe/trunk: lib/StaticAnalyzer/Checkers/MallocChecker.cpp test/Analysis/malloc-plist.c
ganna at apple.com
Mon Mar 19 18:11:54 PDT 2012
On Mar 19, 2012, at 5:30 PM, Ted Kremenek wrote:
> On Mar 19, 2012, at 5:22 PM, Jordan Rose <jediknil at belkadan.com> wrote:
>> On the other hand, MallocBugVisitor is currently stateful -- that is, it assumes it's never going to see the same node-pair twice, and that the nodes are always visited end-to-start. I can make it less stateful (i.e. accept the same node-pair twice), but getting it down all the way to no state means a fair amount of rewriting. Is it part of the design that visitors be non-stateful, in case we ever want to traverse the nodes in some other order?
> Hmm. What about make the state functional, and having that state maintained by BugReporter as we walk the path? We could then generate state speculatively, and throw it away when we don't care. Just a half baked idea, that I haven't necessarily thought through.
I just talked to Ted and we agree that whatever solution we come up with, it's best not to place extra requirements (ensuring that the visitors are stateless) on checker writers.
Here is another spin on Ted's idea. This introduces even more performance overhead, but it might not matter anyway because the diagnostic generation is not where the analyzer performance bottleneck is.
(1) Before generating the whole path, set the "hasModifiedSymbols" to false.
(2) During path generation, if any of the BugReporterVisitors modifies the set of interesting symbols, throw away the generated events and go back to (1).
(3) If the interesting symbols don't change (hasModifiedSymbols == false), use the path.
To make this work, visitors would have to implement some sort of clone method (aka virtual copy constructor).
We also considered splitting the visitors/callbacks for generation of interesting symbols and diagnostic pieces. This idea would result in more strain on the checker-writer.
-------------- next part --------------
An HTML attachment was scrubbed...
More information about the cfe-commits