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

[Jordy Rose]

Looking at this some more, it seems the only information the function summaries provide that aren't also available through RefVals are:

1. Treating [object dealloc] differently from [object release]. This could be replaced by examining the current ExplodedNode's Stmt every time we hit a Released RefVal, or by adding a ReleasedByDeallocation RefVal::Kind. Or we could ignore it since it is so rare in Cocoa code. (I suppose non-Cocoa code doesn't have to be ref-counted for the RetainReleaseChecker to catch alloc/dealloc balances.)

2. Noting that (NS|CF)MakeCollectable is ignored in non-GC mode. This is definitely nice QoI-wise but isn't (IMHO) a critical part of the bug report's usefulness.

3. Noting that -autorelease, -retain, and -release are ignored in GC mode. Same as above.

Noting that X is ignored is something that's a little annoying to do while trying to keep GRState explosion down. My initial thought was to add a counter to RefVal for these messages, but that would mean that something like this would create two states after the branch is done (i.e. no uniquing the states).

id object = getObject();
if (arbitraryCondition()) {
  [object retain];
  doSomethingWith(object);
  [object release];
}

Another possibility is a simple single-bit toggle; that way any ignored messages will still show up as a change in the state of the referred variable, and only branches with an odd number of ignored messages will result in distinct GRStates. This feels like more of a hack, but it costs less in memory (it could even be combined in a bitfield with the object kind).

With either way of forcing a change, we'd then examine the current ExplodedNode's statement to see what was wrong.

I'm going to start working on this soon, using the bit toggle for the ignored messages, and try to come up with orthogonal patches that chip away at TransferFuncs and CFRefCount!

Jordy


On Jun 27, 2011, at 19:35, Jordy Rose 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