[cfe-commits] [analyzer] Patch to stop tracking symbols based on a retain count summary of inlined function

[Jordan Rose]

Yeah, I don't like this approach as much as I thought I would. My original reason for making a new summary out of an old one was to re-use the existing summary evaluation code. This eliminates that, and processSummaryOfInlined is crippled since it only handles StopTrackingHard and NoRetHard. And there's a lot of duplication elsewhere.

I'd rather see this:

  RetainSummaryManager &Summaries = getSummaryManager(C);
  const RetainSummary *Summ = Summaries.getSummary(Call, C.getState());

  if (C.wasInlined && !Summ->appliesToInlined())
    return;

  checkSummary(*Summ, Call, C);

The only difference between the "hard" and "soft" versions is that NoRetHard means /remove/ RefBindings, while NoRet means don't touch them. This could be renamed to "Untracked" or "UntrackedSymbol".


-  void updateSummaryForCall(const RetainSummary *&Summ,
-                            const CallEvent &Call);
+  void updateForCallbackArgument(const RetainSummary *&Summ,
+                                 const CallEvent &Call);

This is not a core point about the functionality of this patch, but I think this is an inferior interface. The idea behind 'updateSummaryForCall' is that it takes a context-less summary derived from an ObjCMethodDecl, or even just a selector, and adds the context of the arguments and possibly additional information about the receiver besides its type. The summary generation before this point can be cached and re-used if we call the same method later, but updateSummaryForCall is the time to make changes specific to this CallEvent.

Currently, the only such change is the presence or absence of a callback parameter, but there's no reason that can't change in the future. Keeping this as 'updateSummaryForCall' gives a proper opaque effect: given a summary for a given decl, update it for this particular call. getSummary shouldn't be thinking about the various ways in which a CallEvent could affect a base summary.

I have some spot comments, but I'll save those for the next iteration of the patch, or perhaps tomorrow morning.

Jordan


On Aug 20, 2012, at 16:59 , Anna Zaks <ganna at apple.com> wrote:

> This resolves retain count checker false positives that are caused by inlining ObjC and other methods. Essentially, if we are passing an object to a method with "delegate" in the selector or a function pointer as another argument, we should stop tracking the other parameters/return value as far as the retain count checker is concerned.
> 
> Submitting as a patch per Jordan's request.
> 
> Anna.
> <Retain_Count_Stop_Tracking_Even_If_Inlined.diff>