[cfe-dev] [Patch] Update to Checker Development Manual

[Anna Zaks]

On May 12, 2013, at 7:04 PM, Sam Handler <samuel.handler+cfedev at gmail.com> wrote:

> 
> 
> 
> On Mon, May 6, 2013 at 8:43 PM, Jordan Rose <jordan_rose at apple.com> wrote:
>> +If needed, the <tt>addTransition</tt> function can be called with no arguments to return the current
>> +<tt>ExplodedNode</tt> without changing the program's state.
> 
> This isn't exactly true; addTransition returns a new node without changing the current node's state, but it does return a new node, either for emitting a bug report or just putting your stamp on whatever you are checking. If you use addTransition just to get the current node, it's all to you end up splitting the path.
> 
> Calling addTransition without arguments does not in fact appear to return a new node.
> 
> ExplodedNode *addTransition(ProgramStateRef State = 0,
>                             const ProgramPointTag *Tag = 0) {
>   return addTransitionImpl(State ? State : getState(), false, 0, Tag);
> }
> 
> ExplodedNode *addTransitionImpl(ProgramStateRef State,
>                                bool MarkAsSink,
>                                ExplodedNode *P = 0,
>                                const ProgramPointTag *Tag = 0) {
>   if (!State || (State == Pred->getState() && !Tag && !MarkAsSink))
>     return Pred;
>   ...
> 
> 
> Since getState() returns Pred->getState(), calling addTransition() is the same as calling addTransitionImpl(Pred->getState(), false, 0, 0), which will simply return Pred.
> 

Sam,

Thanks for drawing the attention to this. I think Jordan is right, the checker should tag the node when using it for a BugReport. Otherwise, the analyzer engine's internal node reclamation might remove the node altogether (at least in theory). I vaguely recall adding the if statement as a micro optimization. If it was not there, addTransition() would create a new tagged node.

I'll try taking out the if statement and see if it results in any performance regressions.

Thanks,
Anna.

> I'm not sure if this behavior is intentional, although there are many existing checkers that call addTransition with no arguments to get the ExplodedNode for a bug report.
> 
> In any case, I've changed the text to recommend getPredecessor(), as this seems like the more logical choice. I've also made it clearer that this can only be used if no state transition has been performed.
> 
> All other comments should be addressed.
> 
> --Sam
> 
> 
> -- 
> ===============================================================================
> All opinions expressed in posts from this account are entirely my own, and not
> those of any present or former employer. Furthermore, I assert that all works
> contributed to the Clang project (1) were developed using no equipment,
> supplies, facility or trade secrets of any such employer, (2) were developed
> entirely on my own time, and (3) do not result from any work performed for any
> such employer.
> 
> <checker_dev_manual.diff>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20130512/ace1e8be/attachment.html>