[cfe-dev] Removing "TransferFuncs" from the analyzer

[Ted Kremenek]

Hi Jordy,

I'm still on paternity leave, but I hope to respond to you soon with more detail.  The upshot is that I have wanted to remove TransferFuncs for a long time, and I agree with you that the Checker interface has grown enough that we can do this.  I think most of the logic not specific to retain release checker can just be moved into ExprEngine (i.e., ivar invalidation).  Most of the rest of the retain release checker can be done as pre- and post- conditions instead of literally evaluating the function call/methods (which we can do well with the Checker interface).  The only thing we cannot model now with post conditions alone are methods that return aliases (e.g., -retain) since we do not support unification of two distinct symbols yet (i.e., saying that two symbols are actually the same).  That's a really big piece to add, but it is needed in several contexts.

Ted

Sent from my iPad

On Jun 27, 2011, at 7:35 PM, Jordy Rose <jediknil at belkadan.com> wrote:

> Hi Ted and cfe-dev. In my free time I've been looking into the issue of "transfer functions" in the analyzer — a supposedly swappable implementation of logic for evaluating function calls and Objective-C messages, among other things. As the Checker infrastructure has grown, almost all of the once-unique transfer function logic can now be implemented using essentially-pluggable Checkers.
> 
> That wouldn't be such a problem, except that in reality the implementation of TransferFuncs in the analyzer is CFRefCount, which mixes in a whole bunch of logic for tracking retain counts and Cocoa conventions. We shouldn't need to run that with every run of the analyzer (though most of the code will early-exit when it realizes it's not relevant). Some of the code has already been extracted to a class called RetainReleaseChecker, but at this point we can do a lot more.
> 
> This e-mail is recording my thoughts on this and how far I got; it's not intended as an immediate call to action.
> 
> In fact, the behavior of a hypothetical SimpleTransferFuncs only includes this:
> - Invalidating ivars of ObjC message receiver
> - Invalidating ivars of 'this' for C++ method call
> - Invalidating function/method args
> - Invalidating globals if necessary
> - Conjuring return values
> 
> And these effects only happen if no checker claims "evalCall" for a given function call.
> 
> 
> On the other hand, a fully-fledged RetainReleaseChecker with all existing functionality isn't quite possible because of four things:
> 
> - CFBugReports use function call summaries to show the retain/release history of a leaked object. Function summaries are currently associated with ExplodedNodes in a DenseMap. In addition to being somewhat inefficient (summaries for calls that have nothing to do with ref-counting are also saved), Checkers are const, and so can't store summaries in a mutable data structure. (Marking the map 'mutable' doesn't feel like the right solution.) I don't have a solution for this.
> 
> - There's no evalObjCMessage, for the very sensible reason that overriding a method can make its behavior totally different. Nevertheless, there are a few messages we /do/ want to model in a RetainReleaseChecker: -[NSAutoreleasePool init], -[NSAutoreleasePool addObject:], -retain, -release, and -autorelease. Fortunately, pretty much everything else would fit in a PostObjCMessage implementation.
> 
> - When function/method arguments are invalidated, their reference counts persist. But any other sort of invalidation does destroy reference count records. Currently this uses a whitelist built before invalidating the various regions referenced in a call, but with no special consideration given to RetainReleaseChecker, that'd be a little strange. I can think of two ways around this: reconstruct the whitelist on every region invalidation if we're in a function call (probably not a good idea) and reconstruct the reference bindings in a post-call check (by walking backwards through ExplodedNodes to the pre-call state).
> 
> - CFRefCount currently registers a GRState::Printer for printing the retain count info in a state. Checkers currently have no way to register printers for custom state info, and that should probably be fixed anyway.
> 
> Everything else that CFRefCount currently does can be done using the current Checker architecture. (For more detailed but less structured notes on this, see attached
> <CFRefCount.txt>
> .)
> 
> 
> As for what I /did/ accomplish, evalBind and evalAssume can be migrated to RetainReleaseChecker almost without changes:
> <CFRefCount.1.patch>
> 
> 
> Everything /other/ than evalCall, evalObjCMessage, and evalSummary can be moved to RetainReleaseChecker, but that leaves plenty of hackery behind:
> <CFRefCount.2.patch>
> 
> 
> 
> I'm going to stop this work here...it's where it crosses the line from "simple migration" to "design decisions". Once the four issues above are resolved, it should actually be pretty straightforward to eliminate TransferFuncs and CFRefCount from the analyzer and make RetainReleaseChecker optional. Which would be great, right?
> 
> Jordy