[cfe-commits] r153010 - in /cfe/trunk: lib/StaticAnalyzer/Checkers/MallocChecker.cpp test/Analysis/malloc-plist.c

[Anna Zaks]

Clarification points below...

On Mar 19, 2012, at 7:11 PM, Jordan Rose wrote:

> 
> On Mar 19, 2012, at 18:11, Anna Zaks wrote:
> 
>> 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). 
> 
> Or have a reset() method. Or require that visitors be stateless.
The purpose of my email was to argue against stateless visitors because we want to make it as easy as possible to write checkers.

> I think this is probably overkill for now, though. We can go with Ted's idea and change to redoing the whole path later if it turns out to be useful, but it would probably be more work to go the other way around.
> 
>> 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.
> 
> I don't like this so much -- it seems like unnecessary work. On the one hand, the two don't have much to do with each other. On the other, checkers that need to troll back through the ExplodedGraph to find interesting symbols can do so themselves, like getAllocationSite() already does. The benefit of having the BugReporter manage all this is to have only one traversal for many visitors, but at least for now, all the custom symbols come, directly or indirectly, from a checker.

I just listed the second idea to let you know that we considered it, but decided against it because of the extra strain on the checker writers. Finding interesting symbols might require logic very similar to that of diagnostic generation (like in the case with the symbol you added; I suspect you are relying on the state of the visitor).

> All right, I'll start implementing this, but I'll send a patch before committing.

Sounds good!

Anna.