<html><head><meta http-equiv="Content-Type" content="text/html charset=us-ascii"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;" class=""><br class=""><div><blockquote type="cite" class=""><div class="">On Nov 18, 2014, at 10:26 AM, Ted Kremenek <<a href="mailto:kremenek@apple.com" class="">kremenek@apple.com</a>> wrote:</div><br class="Apple-interchange-newline"><div class="">Thanks Daniel. This looks like a bug.</div></blockquote><blockquote type="cite" class=""><div class=""><br class="">Anna: I think this might be your area of the analyzer. Can you review Daniel's patch?<br class=""><br class=""><blockquote type="cite" class="">On Nov 18, 2014, at 9:08 AM, Daniel DeFreez <<a href="mailto:dcdefreez@ucdavis.edu" class="">dcdefreez@ucdavis.edu</a>> wrote:<br class=""><br class="">Hi all,<br class=""><br class="">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.<br class=""><br class="">With clang version 3.6.0 (trunk 222230), running -analyzer-checker=debug.DumpCallGraph for two equivalent pieces of code produces different results:<br class=""><br class="">First sample<br class="">------------------<br class="">void a() {}<br class="">void b() { a(); }<br class=""><br class=""> --- Call graph Dump --- <br class=""> Function: < root > calls: a b <br class=""> Function: b calls: a <br class=""> Function: a calls: <br class=""><br class="">Second sample<br class="">----------------------<br class="">void a();<br class="">void b() { a(); }<br class="">void a() {}<br class=""><br class=""> --- Call graph Dump --- <br class=""> Function: < root > calls: b a <br class=""> Function: a calls: <br class=""> Function: b calls: <br class=""><br class="">Shouldn't the call graph in the second code sample also have an edge from b to a?<br class=""></blockquote></div></blockquote><div><br class=""></div><div>Thanks for finding this! </div><div><br class=""></div><blockquote type="cite" class=""><div class=""><blockquote type="cite" class=""><br class="">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.<br class=""></blockquote></div></blockquote><div><br class=""></div><div>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.)</div><div><br class=""></div>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. </div><div><br class=""></div><div>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?</div><div><br class=""></div><div><div style="margin: 0px; font-size: 14px; font-family: Menlo; color: rgb(0, 132, 0);" class=""><span style="font-variant-ligatures: no-common-ligatures; color: #000000" class=""> </span>/// getBody - Retrieve the body (definition) of the function. The</div><div style="margin: 0px; font-size: 14px; font-family: Menlo; color: rgb(0, 132, 0);" class=""><span style="font-variant-ligatures: no-common-ligatures; color: #000000" class=""> </span>/// function body might be in any of the (re-)declarations of this</div><div style="margin: 0px; font-size: 14px; font-family: Menlo; color: rgb(0, 132, 0);" class=""><span style="font-variant-ligatures: no-common-ligatures; color: #000000" class=""> </span>/// function. The variant that accepts a FunctionDecl pointer will</div><div style="margin: 0px; font-size: 14px; font-family: Menlo; color: rgb(0, 132, 0);" class=""><span style="font-variant-ligatures: no-common-ligatures; color: #000000" class=""> </span>/// set that function declaration to the actual declaration</div><div style="margin: 0px; font-size: 14px; font-family: Menlo; color: rgb(0, 132, 0);" class=""><span style="font-variant-ligatures: no-common-ligatures; color: #000000" class=""> </span>/// containing the body (if there is one).</div><div style="margin: 0px; font-size: 14px; font-family: Menlo; color: rgb(0, 132, 0);" class=""><span style="font-variant-ligatures: no-common-ligatures; color: #000000" class=""> </span>/// NOTE: For checking if there is a body, use hasBody() instead, to avoid</div><div style="margin: 0px; font-size: 14px; font-family: Menlo; color: rgb(0, 132, 0);" class=""><span style="font-variant-ligatures: no-common-ligatures; color: #000000" class=""> </span>/// unnecessary AST de-serialization of the body.</div><div style="margin: 0px; font-size: 14px; font-family: Menlo;" class=""> <span style="font-variant-ligatures: no-common-ligatures; color: #4f8187" class="">Stmt</span> *getBody(<span style="font-variant-ligatures: no-common-ligatures; color: #bb2ca2" class="">const</span> <span style="font-variant-ligatures: no-common-ligatures; color: #4f8187" class="">FunctionDecl</span> *&Definition) <span style="font-variant-ligatures: no-common-ligatures; color: #bb2ca2" class="">const</span>;</div><div style="margin: 0px; font-size: 14px; font-family: Menlo;" class=""><br class=""></div><div style="margin: 0px; font-size: 14px; font-family: Menlo;" class=""><br class=""></div></div><div><blockquote type="cite" class=""><div class=""><blockquote type="cite" class="">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. <br class=""><br class="">*Feedback please*<br class="">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.<br class=""><br class=""></blockquote></div></blockquote><blockquote type="cite" class=""><div class=""><blockquote type="cite" class=""> if (const FunctionDecl *FD = dyn_cast<FunctionDecl>(D)) {<br class="">+ // Skip definitions with previous declarations<br class="">+ if (FD->getPreviousDecl())<br class="">+ return false;<br class="">+</blockquote></div></blockquote><blockquote type="cite" class=""><div class=""><blockquote type="cite" class=""> // We skip function template definitions, as their semantics is<br class=""> // only determined when they are instantiated.<br class="">- if (!FD->isThisDeclarationADefinition() ||<br class="">+ if (FD->getDescribedFunctionTemplate() ||<br class=""> FD->isDependentContext())<br class=""> return false;<br class=""><br class="">Is this right?<br class=""><br class="">Thanks,<br class=""><br class="">Daniel<br class="">_______________________________________________<br class="">cfe-dev mailing list<br class=""><a href="mailto:cfe-dev@cs.uiuc.edu" class="">cfe-dev@cs.uiuc.edu</a><br class="">http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev<br class=""></blockquote><br class=""></div></blockquote></div><br class=""></body></html>