[cfe-dev] CFRefCount Problem #2: Region Invalidation

[Jordy Rose]

On Aug 24, 2011, at 11:21, Ted Kremenek wrote:

> On Aug 23, 2011, at 10:08 PM, Jordy Rose wrote:
> 
>>> Is there a particular reason you were advocating (1)?
>> 
>> (1) is the least intrusive implementation, and it's an implementation that doesn't impose additional semantics on invalidateRegion(s).
> 
> I don't think I quite understand (1).  Could you explain a bit more what you had in mind?  To me, restoring bindings in the store has nothing to do with whether or not a symbol gets invalidated.

Oh! I meant RefBindings (i.e. ref count state), not store bindings. Sorry!

Basic idea was this:
- invalidation (without whitelist) wipes out reference counts
- in postCall, walk backwards in the graph to before the call is evaluated (using the ProgramPoint)
- look up each argument's refcount in the old node and copy it to the new node
- apply the call's ArgEffects to its arguments (as is done now)

This doesn't handle the recursive data structure case, but even if an object has a strong reference to itself it's probably /still/ following retain-count conventions. I think it's okay for RetainReleaseChecker not to handle that case.


>> What I didn't realize is that invalidateRegions is /only/ used for parameters.
> 
> I'm not certain what you mean.  It's a general API, that is meant to be used to invalidate any set of regions, starting with region roots.  invalidateRegions is also used to invalidate globals.  Yes, we use it right now to invalidate parameters, but it could certainly be used in other cases.

Sorry, I meant "only for function/method calls", i.e. running code we can't see. I'm not sure if /that's/ supposed to be in the contract either, but it makes passing around a list of "arguments" not as bad to me, since there /will/ always be arguments.


>> That means that passing the top-level parameters isn't really changing any semantics. So I'm good with (2).
> 
> Ah, you mean by extending the API and making it too specialized?

I don't mind specialization, I just mind having exceptional cases that have to leave fields blank. That makes me think the abstraction's at the wrong level.


>> Is it best to pass a list of SVals? A list of top-level regions? A list of expressions? A list of symbols? A CallOrObjCMessage? The last is clearly most flexible, but I'm not sure that's what we need. I feel like the list of SVals (ArrayRef<SVal>) is probably the way to go.
> 
> Maybe just the list of MemRegion roots and an *optional* ProgramPoint?  The latter could be used to tailor the behavior of the callback.  SVals are really one level too high, since this API really is about regions.

I guess so. What RetainReleaseChecker needs is the symbols, but the API shouldn't be tailored to RetainReleaseChecker, and it's not /that/ hard to extract symbols from MemRegions, some of which are SymbolicRegions.