<html><head><meta http-equiv="Content-Type" content="text/html charset=utf-8"></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 3, 2015, at 12:18 AM, Aleksei Sidorin via cfe-dev <<a href="mailto:cfe-dev@lists.llvm.org" class="">cfe-dev@lists.llvm.org</a>> wrote:</div><br class="Apple-interchange-newline"><div class=""><div class="">Hello Scott,<br class=""><br class="">You have touched a very sensitive moment.<br class=""><br class="">TL;DR: The reason warning does not appear is that function is not analyzed out of context if it was inlined before. Functions are analyzed in topological order. In your case, inlining of 'foo' does not touch (len < 10 == true) branch so it will never be analyzed.<br class=""></div></div></blockquote></div><div><blockquote type="cite" class=""><div class=""><div class=""><br class="">But, more thoroughly, CSA has some issues with topological sorting. First, it is not clear if topological sorting is really required since it throws some possible execution paths away (as in Scott's example). </div></div></blockquote><div><br class=""></div><div>The analyzer throws away the execution paths that cannot be triggered by the current translation unit mainly as a performance heuristic. Topological order just ensures that we maximize the effect of this heuristic. If we were to inline leaf functions first, it would not get triggered at all. This can also reduce a number of false positives since the called function might assume some knowledge about its callers.</div><div><br class=""></div><blockquote type="cite" class=""><div class=""><div class="">Second, it does not work as expected because of some call graph issues (I made some attempts to resolve them and hope to contribute these patches).<br class=""></div></div></blockquote><div><br class=""></div>I cannot comment on this since I do not know what they are. </div><div><br class=""><blockquote type="cite" class=""><div class=""><div class=""><br class="">However, the assumption that function is analyzed only in given contexts can reduce amount of false positives.</div></div></blockquote><div><br class=""></div>Yes, that is one of the advantages of the current approach. (And would definitely be a very a good reason for keeping this mode for cross translation unit analysis.)</div><div><br class=""><blockquote type="cite" class=""><div class=""><div class=""><br class="">In our opinion, we should analyze all the functions and skip out-of-context analysis of functions that are:<br class="">1. not externally visible or<br class="">2. are private class members.<br class=""><br class=""></div></div></blockquote><br class=""></div><div>This can definitely be done behind a flag. I expect the performance impact to be significant.</div><div><br class=""><blockquote type="cite" class=""><div class=""><div class="">But this is a subject for discussion.<br class=""><br class=""><br class=""><blockquote type="cite" class="">Hi All,<br class=""><br class="">Given the following code:<br class=""><br class="">// test.cpp<br class="">int foo(int len) {<br class=""> int j = 0;<br class=""> if (len < 10)<br class=""> j = 42 / j;<br class=""> return j;<br class="">}<br class=""><br class="">the command<br class=""><br class="">clang --analyze test.cpp<br class=""><br class="">issues the bug report<br class=""><br class="">tu.cpp:6:10: warning: Division by zero<br class=""> j = 42 / j;<br class=""> ~~~^~~<br class=""><br class="">However, it seems that merely introducing another function which calls<br class="">foo() with an argument that would not trigger a division by zero nullifies<br class="">the bug report. For instance, analyzing<br class=""><br class="">// test.cpp<br class="">int foo(int len) {<br class=""> int j = 0;<br class=""> if (len < 10)<br class=""> j = 42 / j;<br class=""> return j;<br class="">}<br class=""><br class="">void bar() {<br class=""> int m = 12;<br class=""> foo(m);<br class="">}<br class=""><br class="">in the same way will NOT issue a bug report. Isn't this a bug in the static<br class="">analyzer?<br class=""></blockquote></div></div></blockquote><div style="margin: 0px; font-size: 11px; line-height: normal; font-family: Menlo;" class=""><br class=""></div><div style="margin: 0px; font-size: 11px; line-height: normal; font-family: Menlo;" class="">What you see here is the following heuristic being triggered:</div><div style="margin: 0px; font-size: 11px; line-height: normal; font-family: Menlo;" class=""><br class=""></div><div style="margin: 0px; font-size: 11px; line-height: normal; font-family: Menlo;" class=""><span style="font-variant-ligatures: no-common-ligatures; color: #0433ff" class="">void</span> <span style="font-variant-ligatures: no-common-ligatures; color: #3495af" class="">AnalysisConsumer</span>::HandleDeclsCallGraph(<span style="font-variant-ligatures: no-common-ligatures; color: #0433ff" class="">const</span> <span style="font-variant-ligatures: no-common-ligatures; color: #0433ff" class="">unsigned</span> LocalTUDeclsSize) {</div><div style="margin: 0px; font-size: 11px; line-height: normal; font-family: Menlo; color: rgb(0, 143, 0);" class=""><span style="font-variant-ligatures: no-common-ligatures; color: #000000" class=""> </span>// Build the Call Graph by adding all the top level declarations to the graph.</div><div style="margin: 0px; font-size: 11px; line-height: normal; font-family: Menlo; color: rgb(0, 143, 0);" class=""><span style="font-variant-ligatures: no-common-ligatures; color: #000000" class=""> </span>// Note: CallGraph can trigger deserialization of more items from a pch</div><div style="margin: 0px; font-size: 11px; line-height: normal; font-family: Menlo; color: rgb(0, 143, 0);" class=""><span style="font-variant-ligatures: no-common-ligatures; color: #000000" class=""> </span>// (though HandleInterestingDecl); triggering additions to LocalTUDecls.</div><div style="margin: 0px; font-size: 11px; line-height: normal; font-family: Menlo; color: rgb(0, 143, 0);" class=""><span style="font-variant-ligatures: no-common-ligatures; color: #000000" class=""> </span>// We rely on random access to add the initially processed Decls to CG.</div><div style="margin: 0px; font-size: 11px; line-height: normal; font-family: Menlo; color: rgb(52, 149, 175);" class=""><span style="font-variant-ligatures: no-common-ligatures; color: #000000" class=""> </span>CallGraph<span style="font-variant-ligatures: no-common-ligatures; color: #000000" class=""> CG;</span></div><div style="margin: 0px; font-size: 11px; line-height: normal; font-family: Menlo;" class=""> <span style="font-variant-ligatures: no-common-ligatures; color: #0433ff" class="">for</span> (<span style="font-variant-ligatures: no-common-ligatures; color: #0433ff" class="">unsigned</span> i = 0 ; i < LocalTUDeclsSize ; ++i) {</div><div style="margin: 0px; font-size: 11px; line-height: normal; font-family: Menlo;" class=""> CG.addToCallGraph(LocalTUDecls[i]);</div><div style="margin: 0px; font-size: 11px; line-height: normal; font-family: Menlo;" class=""> }</div><div style="margin: 0px; font-size: 11px; line-height: normal; font-family: Menlo; min-height: 13px;" class=""><br class=""></div><div style="margin: 0px; font-size: 11px; line-height: normal; font-family: Menlo; color: rgb(0, 143, 0);" class=""><span style="font-variant-ligatures: no-common-ligatures; color: #000000" class=""> </span>// Walk over all of the call graph nodes in topological order, so that we</div><div style="margin: 0px; font-size: 11px; line-height: normal; font-family: Menlo; color: rgb(0, 143, 0);" class=""><span style="font-variant-ligatures: no-common-ligatures; color: #000000" class=""> </span>// analyze parents before the children. Skip the functions inlined into</div><div style="margin: 0px; font-size: 11px; line-height: normal; font-family: Menlo; color: rgb(0, 143, 0);" class=""><span style="font-variant-ligatures: no-common-ligatures; color: #000000" class=""> </span>// the previously processed functions. Use external Visited set to identify</div><div style="margin: 0px; font-size: 11px; line-height: normal; font-family: Menlo; color: rgb(0, 143, 0);" class=""><span style="font-variant-ligatures: no-common-ligatures; color: #000000" class=""> </span>// inlined functions. The topological order allows the "do not reanalyze</div><div style="margin: 0px; font-size: 11px; line-height: normal; font-family: Menlo; color: rgb(0, 143, 0);" class=""><span style="font-variant-ligatures: no-common-ligatures; color: #000000" class=""> </span>// previously inlined function" performance heuristic to be triggered more</div><div style="margin: 0px; font-size: 11px; line-height: normal; font-family: Menlo; color: rgb(0, 143, 0);" class=""><span style="font-variant-ligatures: no-common-ligatures; color: #000000" class=""> </span>// often.</div><p style="margin: 0px; font-size: 11px; line-height: normal; font-family: Menlo; min-height: 13px;" class=""> …</p><p style="margin: 0px; font-size: 11px; line-height: normal; font-family: Menlo; min-height: 13px;" class=""><br class=""></p><p style="margin: 0px; font-size: 11px; line-height: normal; font-family: Menlo; min-height: 13px;" class="">And also some more details on this here:</p><div class=""><div style="margin: 0px; font-size: 11px; line-height: normal; font-family: Menlo; min-height: 13px;" class=""><br class=""></div><div style="margin: 0px; font-size: 11px; line-height: normal; font-family: Menlo;" class=""><span style="font-variant-ligatures: no-common-ligatures; color: #0433ff" class="">static</span> <span style="font-variant-ligatures: no-common-ligatures; color: #0433ff" class="">bool</span> shouldSkipFunction(<span style="font-variant-ligatures: no-common-ligatures; color: #0433ff" class="">const</span> <span style="font-variant-ligatures: no-common-ligatures; color: #3495af" class="">Decl</span> *D,</div><div style="margin: 0px; font-size: 11px; line-height: normal; font-family: Menlo;" class=""> <span style="font-variant-ligatures: no-common-ligatures; color: #0433ff" class="">const</span> <span style="font-variant-ligatures: no-common-ligatures; color: #3495af" class="">SetOfConstDecls</span> &Visited,</div><div style="margin: 0px; font-size: 11px; line-height: normal; font-family: Menlo;" class=""> <span style="font-variant-ligatures: no-common-ligatures; color: #0433ff" class="">const</span> <span style="font-variant-ligatures: no-common-ligatures; color: #3495af" class="">SetOfConstDecls</span> &VisitedAsTopLevel) {</div><div style="margin: 0px; font-size: 11px; line-height: normal; font-family: Menlo;" class=""> <span style="font-variant-ligatures: no-common-ligatures; color: #0433ff" class="">if</span> (VisitedAsTopLevel.count(D))</div><div style="margin: 0px; font-size: 11px; line-height: normal; font-family: Menlo; color: rgb(4, 51, 255);" class=""><span style="font-variant-ligatures: no-common-ligatures; color: #000000" class=""> </span>return<span style="font-variant-ligatures: no-common-ligatures; color: #000000" class=""> </span>true<span style="font-variant-ligatures: no-common-ligatures; color: #000000" class="">;</span></div><div style="margin: 0px; font-size: 11px; line-height: normal; font-family: Menlo; min-height: 13px;" class=""><br class=""></div><div style="margin: 0px; font-size: 11px; line-height: normal; font-family: Menlo; color: rgb(0, 143, 0);" class=""><span style="font-variant-ligatures: no-common-ligatures; color: #000000" class=""> </span>// We want to re-analyse the functions as top level in the following cases:</div><div style="margin: 0px; font-size: 11px; line-height: normal; font-family: Menlo; color: rgb(0, 143, 0);" class=""><span style="font-variant-ligatures: no-common-ligatures; color: #000000" class=""> </span>// - The 'init' methods should be reanalyzed because</div><div style="margin: 0px; font-size: 11px; line-height: normal; font-family: Menlo; color: rgb(0, 143, 0);" class=""><span style="font-variant-ligatures: no-common-ligatures; color: #000000" class=""> </span>// ObjCNonNilReturnValueChecker assumes that '[super init]' never returns</div><div style="margin: 0px; font-size: 11px; line-height: normal; font-family: Menlo; color: rgb(0, 143, 0);" class=""><span style="font-variant-ligatures: no-common-ligatures; color: #000000" class=""> </span>// 'nil' and unless we analyze the 'init' functions as top level, we will</div><div style="margin: 0px; font-size: 11px; line-height: normal; font-family: Menlo; color: rgb(0, 143, 0);" class=""><span style="font-variant-ligatures: no-common-ligatures; color: #000000" class=""> </span>// not catch errors within defensive code.</div><div style="margin: 0px; font-size: 11px; line-height: normal; font-family: Menlo; color: rgb(0, 143, 0);" class=""><span style="font-variant-ligatures: no-common-ligatures; color: #000000" class=""> </span>// - We want to reanalyze all ObjC methods as top level to report Retain</div><div style="margin: 0px; font-size: 11px; line-height: normal; font-family: Menlo; color: rgb(0, 143, 0);" class=""><span style="font-variant-ligatures: no-common-ligatures; color: #000000" class=""> </span>// Count naming convention errors more aggressively.</div><div style="margin: 0px; font-size: 11px; line-height: normal; font-family: Menlo;" class=""> <span style="font-variant-ligatures: no-common-ligatures; color: #0433ff" class="">if</span> (isa<ObjCMethodDecl>(D))</div><div style="margin: 0px; font-size: 11px; line-height: normal; font-family: Menlo; color: rgb(4, 51, 255);" class=""><span style="font-variant-ligatures: no-common-ligatures; color: #000000" class=""> </span>return<span style="font-variant-ligatures: no-common-ligatures; color: #000000" class=""> </span>false<span style="font-variant-ligatures: no-common-ligatures; color: #000000" class="">;</span></div><div style="margin: 0px; font-size: 11px; line-height: normal; font-family: Menlo; min-height: 13px;" class=""><br class=""></div><div style="margin: 0px; font-size: 11px; line-height: normal; font-family: Menlo; color: rgb(0, 143, 0);" class=""><span style="font-variant-ligatures: no-common-ligatures; color: #000000" class=""> </span>// Otherwise, if we visited the function before, do not reanalyze it.</div><div style="margin: 0px; font-size: 11px; line-height: normal; font-family: Menlo;" class=""> <span style="font-variant-ligatures: no-common-ligatures; color: #0433ff" class="">return</span> Visited.count(D);</div><div style="margin: 0px; font-size: 11px; line-height: normal; font-family: Menlo;" class="">}</div></div><div class=""><br class=""></div><blockquote type="cite" class=""><div class=""><div class=""><blockquote type="cite" class=""><br class="">Note: I tested this with clang 3.7.0 and 3.8.0.<br class=""></blockquote><br class=""><br class="">-- <br class="">Best regards,<br class="">Aleksei Sidorin<br class="">Software Engineer,<br class="">IMSWL-IMCG, SRR, Samsung Electronics<br class=""><br class="">_______________________________________________<br class="">cfe-dev mailing list<br class=""><a href="mailto:cfe-dev@lists.llvm.org" class="">cfe-dev@lists.llvm.org</a><br class="">http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev<br class=""></div></div></blockquote></div><br class=""></body></html>