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

[Anna Zaks]

On Aug 21, 2012, at 9:04 AM, Jordan Rose wrote:

> 
> On Aug 20, 2012, at 18:47 , Jordan Rose <jordan_rose at apple.com> wrote:
> 
>> 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);
> 
> Thinking about this again, this is about whether we're saying "this summary should be applied even when inlining", or "these /effects/ should be applied even when inlining". Even in the latter case I'd consider adding flags to the ArgEffects rather than adding new ArgEffect kinds, but that's an interesting distinction/decision. Obviously the latter is more flexible, but which do you think we /should/ be doing?


There are several reasons why I am not reusing the checkSummary code when processing summaries for inlined function.
 - As you mentioned above, parts/individual effects in a summary are marked as "applied when inlining" not the whole summary. It makes sense to keep it this way. We make decisions on what effect each argument/return value should have separately.
 - I do not perform checking for errors in processSummaryOfInlined, which is a big part of checkSummary. Even in DecRefAndStopTracking, I think it's OK to not check for use-after-free since we did inline the function and know better if it used the object or not. We should have generated the warning while analyzing the function in the former case.
 - Passing an extra flag to checkSummary to act differently in the inlined case is much more messy - the only thing the functions share is the argument visitation logic. I'd rather have the argument iterator and get receiver code duplicated rather than one huge function with a lot of conditions. If you are very concerned about it, we could reduce parameter visitation to a single loop by adding another iteration APIs to EvalCall and RetainSummary, which processes both the arguments and receiver. That would make both functions (checkSummary and processSummaryOfInlined) much smaller. (Not sure that it's worth it though.)

> 

> Jordan

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