[cfe-dev] Missing edges in analysis call graph

[Anna Zaks]

> On Nov 18, 2014, at 10:26 AM, Ted Kremenek <kremenek at apple.com> wrote:
> 
> Thanks Daniel.  This looks like a bug.
> 
> Anna: I think this might be your area of the analyzer.  Can you review Daniel's patch?
> 
>> On Nov 18, 2014, at 9:08 AM, Daniel DeFreez <dcdefreez at ucdavis.edu> wrote:
>> 
>> Hi all,
>> 
>> I've been working with the static analyzer and I'm having issues with the call graph that it produces. This is the same call graph that is output by the debug.DumpCallGraph and debug.ViewCallGraph checkers. I'm fairly new to clang, so I would like some feedback on this. Maybe I've missed the mark entirely or there is a better way to fix it. Thanks in advance, the static analyzer is awesome.
>> 
>> With clang version 3.6.0 (trunk 222230), running -analyzer-checker=debug.DumpCallGraph for two equivalent pieces of code produces different results:
>> 
>> First sample
>> ------------------
>> void a() {}
>> void b() { a(); }
>> 
>> --- Call graph Dump --- 
>>  Function: < root > calls: a b 
>>  Function: b calls: a 
>>  Function: a calls: 
>> 
>> Second sample
>> ----------------------
>> void a();
>> void b() { a(); }
>> void a() {}
>> 
>> --- Call graph Dump --- 
>>  Function: < root > calls: b a 
>>  Function: a calls: 
>>  Function: b calls: 
>> 
>> Shouldn't the call graph in the second code sample also have an edge from b to a?

Thanks for finding this! 

>> 
>> I think this happens because the check at /lib/Analysis/CallGraph.cpp:119 throws out Decls based on isThisDeclarationADefinition(). The comment says that this is used to discard template definitions, but the check is too broad.

I suspect the comment talks about the second check (isDependentContext). I think what we wanted is to include all functions for which we have definitions(bodies). The analyzer is currently the only user of this and it is not interested in calls for which we do not have definitions. (This could be changed if we have more users.)

In the case above, the definition for 'a()' is visible in the translation unit, so we should include it. I believe with your patch, the node will get added, but the definition used will not have the body attached to it, so the analyzer will not process the body. 

How about changing the logic in the CallGraph to check if there is a redeclaration with the body and insert that one instead of the one referenced by the call? (This could also be handled on the analyzer side, but I think all the CallGraph users might prefer the declarations that have the bodies attached when those are available.) I would try to use hasBody(), followed by getBody(). What do you think?

  /// getBody - Retrieve the body (definition) of the function. The
  /// function body might be in any of the (re-)declarations of this
  /// function. The variant that accepts a FunctionDecl pointer will
  /// set that function declaration to the actual declaration
  /// containing the body (if there is one).
  /// NOTE: For checking if there is a body, use hasBody() instead, to avoid
  /// unnecessary AST de-serialization of the body.
  Stmt *getBody(const FunctionDecl *&Definition) const;


>> When the b-a CallExpr is visited, DeclRefExpr points at the declaration of the function not the definition, so it is not included in the graph. 
>> 
>> *Feedback please*
>> I fixed it as below. The idea is to filter out template definitions in addition to function definitions that have a previous declaration, but not all declarations.
>> 
>>   if (const FunctionDecl *FD = dyn_cast<FunctionDecl>(D)) {
>> +    // Skip definitions with previous declarations
>> +    if (FD->getPreviousDecl())
>> +      return false;
>> +
>>     // We skip function template definitions, as their semantics is
>>     // only determined when they are instantiated.
>> -    if (!FD->isThisDeclarationADefinition() ||
>> +    if (FD->getDescribedFunctionTemplate() ||
>>         FD->isDependentContext())
>>       return false;
>> 
>> Is this right?
>> 
>> Thanks,
>> 
>> Daniel
>> _______________________________________________
>> cfe-dev mailing list
>> cfe-dev at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev
> 

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-dev/attachments/20141118/1d873b5c/attachment.html>