[cfe-dev] Missing edges in analysis call graph

[Ted Kremenek]

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?
> 
> 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.
> 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