<div dir="ltr"><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div style="font-size:12.8px">After the Windows-specific improvements are committed, you might be able to rewrite the function matching code by using the newly added helper function:</div><div style="font-size:12.8px"><a href="http://reviews.llvm.org/D15921" target="_blank">http://reviews.llvm.org/D15921</a></div></blockquote><div><br></div><div>I'm looking into this, and I have a few questions about this:</div><div><ul><li>Why do we lazily initialize (instead of initializing at construction) the <font face="monospace, monospace">IdentifierInfo</font>*s in the first place? Is the token info only valid at some point after construction?<br></li><li>Is there a better way to check if the <font face="monospace, monospace">IdentifierInfo</font>*s are initialized than a nullptr comparison on every <font face="monospace, monospace">isCalled</font>? i.e.:</li></ul><blockquote style="margin:0px 0px 0px 40px;border:none;padding:0px"><div><pre style="font-family:Consolas;color:gainsboro;background:rgb(30,30,30)"><span style="color:rgb(86,156,214)">if</span> <span style="color:rgb(180,180,180)">(!</span><span style="color:rgb(127,127,127)">CD</span><span style="color:rgb(180,180,180)">.</span><span style="color:rgb(218,218,218)">II</span><span style="color:rgb(180,180,180)">)</span>
<span style="color:rgb(127,127,127)">CD</span><span style="color:rgb(180,180,180)">.</span><span style="color:rgb(218,218,218)">II</span> <span style="color:rgb(180,180,180)">=</span> <span style="color:rgb(180,180,180)">&</span><span style="color:rgb(200,200,200)">getState</span><span style="color:rgb(180,180,180)">()</span><span style="color:rgb(180,180,180)">-></span><span style="color:rgb(200,200,200)">getStateManager</span><span style="color:rgb(180,180,180)">().</span><span style="color:rgb(200,200,200)">getContext</span><span style="color:rgb(180,180,180)">().</span><span style="color:rgb(218,218,218)">Idents</span><span style="color:rgb(180,180,180)">.</span><span style="color:rgb(200,200,200)">get</span><span style="color:rgb(180,180,180)">(</span><span style="color:rgb(127,127,127)">CD</span><span style="color:rgb(180,180,180)">.</span><span style="color:rgb(218,218,218)">FuncName</span><span style="color:rgb(180,180,180)">);</span>
</pre></div></blockquote></div><div><ul><li>How would I:</li><ul><li>Get the original <font face="monospace, monospace">CallEvent</font> from the <font face="monospace, monospace">FunctionDecl</font> passed to <font face="monospace, monospace">MallocChecker::isCMemFunction</font>?</li></ul></ul></div><blockquote style="margin:0px 0px 0px 40px;border:none;padding:0px"><blockquote style="margin:0px 0px 0px 40px;border:none;padding:0px"><div>OR</div></blockquote></blockquote><div><ul><ul><li>Implement <font face="monospace, monospace">CallEvent::isCalled</font> for the <font face="monospace, monospace">FunctionDecl</font>, as passed to <font face="monospace, monospace">MallocChecker::isCMemFunction</font>?</li></ul></ul><div><br></div><div><br></div><div><br></div><div>Perhaps we could create another helper class, that just wraps the comparisons? We could try to use some template magic to wrap groups of allocating functions, so we can replace a bunch of conditions with a single call, e.g.:</div><pre style="font-family:Consolas;color:gainsboro;background:rgb(30,30,30)"><span style="color:rgb(86,156,214)">if</span> <span style="color:rgb(180,180,180)">(</span><span style="color:rgb(127,127,127)">Family</span> <span style="color:rgb(180,180,180)">==</span> <span style="color:rgb(184,215,163)">AF_Malloc</span> <span style="color:rgb(180,180,180)">&&</span> <span style="color:rgb(200,200,200)">CheckAlloc</span><span style="color:rgb(180,180,180)">)</span> <span style="color:rgb(180,180,180)">{</span>
<span style="color:rgb(86,156,214)">if</span> <span style="color:rgb(180,180,180)">(</span><span style="color:rgb(200,200,200)">FunI</span> <span style="color:rgb(180,180,180)">==</span> <span style="color:rgb(218,218,218)">II_malloc</span> <span style="color:rgb(180,180,180)">||</span> <span style="color:rgb(200,200,200)">FunI</span> <span style="color:rgb(180,180,180)">==</span> <span style="color:rgb(218,218,218)">II_realloc</span> <span style="color:rgb(180,180,180)">||</span> <span style="color:rgb(200,200,200)">FunI</span> <span style="color:rgb(180,180,180)">==</span> <span style="color:rgb(218,218,218)">II_reallocf</span> <span style="color:rgb(180,180,180)">||</span>
<span style="color:rgb(200,200,200)">FunI</span> <span style="color:rgb(180,180,180)">==</span> <span style="color:rgb(218,218,218)">II_calloc</span> <span style="color:rgb(180,180,180)">||</span> <span style="color:rgb(200,200,200)">FunI</span> <span style="color:rgb(180,180,180)">==</span> <span style="color:rgb(218,218,218)">II_valloc</span> <span style="color:rgb(180,180,180)">||</span> <span style="color:rgb(200,200,200)">FunI</span> <span style="color:rgb(180,180,180)">==</span> <span style="color:rgb(218,218,218)">II_strdup</span> <span style="color:rgb(180,180,180)">||</span>
<span style="color:rgb(200,200,200)">FunI</span> <span style="color:rgb(180,180,180)">==</span> <span style="color:rgb(218,218,218)">II_win_strdup</span> <span style="color:rgb(180,180,180)">||</span> <span style="color:rgb(200,200,200)">FunI</span> <span style="color:rgb(180,180,180)">==</span> <span style="color:rgb(218,218,218)">II_strndup</span> <span style="color:rgb(180,180,180)">||</span> <span style="color:rgb(200,200,200)">FunI</span> <span style="color:rgb(180,180,180)">==</span> <span style="color:rgb(218,218,218)">II_wcsdup</span> <span style="color:rgb(180,180,180)">||</span>
<span style="color:rgb(200,200,200)">FunI</span> <span style="color:rgb(180,180,180)">==</span> <span style="color:rgb(218,218,218)">II_win_wcsdup</span> <span style="color:rgb(180,180,180)">||</span> <span style="color:rgb(200,200,200)">FunI</span> <span style="color:rgb(180,180,180)">==</span> <span style="color:rgb(218,218,218)">II_kmalloc</span><span style="color:rgb(180,180,180)">)</span>
<span style="color:rgb(86,156,214)">return</span> <span style="color:rgb(86,156,214)">true</span><span style="color:rgb(180,180,180)">;</span>
<span style="color:rgb(180,180,180)">}</span>
</pre></div><div>...could become something like this:<br></div><div><pre style="font-family:Consolas;background:rgb(30,30,30)"><span style="color:rgb(86,156,214)">if</span><font color="#dcdcdc"> </font><span style="color:rgb(180,180,180)">(</span><span style="color:rgb(127,127,127)">Family</span><font color="#dcdcdc"> </font><span style="color:rgb(180,180,180)">==</span><font color="#dcdcdc"> </font><span style="color:rgb(184,215,163)">AF_Malloc</span><font color="#dcdcdc"> </font><span style="color:rgb(180,180,180)">&&</span><font color="#dcdcdc"> </font><span style="color:rgb(200,200,200)">CheckAlloc</span><span style="color:rgb(180,180,180)">)</span><font color="#dcdcdc"> </font><span style="color:rgb(180,180,180)">{</span><font color="#dcdcdc">
</font><span style="color:rgb(86,156,214)">if</span><font color="#dcdcdc"> </font><span style="color:rgb(180,180,180)">(</span><font color="#c8c8c8">MallocFamily.isMallocStyle(FunI)</font><span style="color:rgb(180,180,180)">)</span><font color="#dcdcdc">
</font><span style="color:rgb(86,156,214)">return</span><font color="#dcdcdc"> </font><span style="color:rgb(86,156,214)">true</span><span style="color:rgb(180,180,180)">;</span><font color="#dcdcdc">
</font><span style="color:rgb(180,180,180)">}</span><font color="#dcdcdc">
</font></pre></div><div>Where <font face="monospace, monospace">MallocFamily</font> is something like:</div><div><br></div><font face="monospace, monospace">mutable AllocatorTypeGroup<"malloc", "realloc", "reallocf",<br><br> "calloc", "valloc", "strdup",<br><br> "_strdup", "strndup", "wcsdup",<br><br> "_wcsdup", "kmalloc"> MallocFamily;</font><div>...and <font face="monospace, monospace">AllocatorTypeGroup</font> would be some kind of variadic template, that compares each parameter in the parameter pack with <font face="monospace, monospace">FunI</font>, in pseudocode:</div><div><br></div><div><font face="monospace, monospace">template<typename Last></font></div><div><font face="monospace, monospace">bool AllocatorTypeGroup::isMallocStyle( const IdentifierInfo* FunI,</font></div><div><font face="monospace, monospace"> Last last) {<br></font></div><div><font face="monospace, monospace"> if (FunI == last)</font></div><div><font face="monospace, monospace"> return true;</font></div><div><font face="monospace, monospace">return false;</font></div><div><font face="monospace, monospace">}</font></div><div><font face="monospace, monospace"><br></font></div><div><font face="monospace, monospace">template<typename First, typename... Rest></font></div><div><font face="monospace, monospace">bool AllocatorTypeGroup::isMallocStyle( const IdentifierInfo* FunI,</font></div><div><font face="monospace, monospace"> const First& first,</font></div><div><font face="monospace, monospace"> const Rest&... rest) {</font></div><div><font face="monospace, monospace"> if ((FunI == first) || (isMallocStyle(FunI, rest...))</font></div><div><font face="monospace, monospace"> return true;</font></div><div><font face="monospace, monospace">return false;</font></div><div><font face="monospace, monospace">}</font></div></div><div class="gmail_extra"><br clear="all"><div><div class="gmail_signature"><div dir="ltr"><div><div dir="ltr"><span style="font-size:12.8000001907349px">Sincerely,</span><br style="font-size:12.8000001907349px"><span style="font-size:12.8000001907349px">Alexander Riccio</span><br style="font-size:12.8000001907349px"><span style="font-size:12.8000001907349px">--</span><br style="font-size:12.8000001907349px"><span style="font-size:12.8000001907349px">"Change the world or go home."</span><div style="font-size:12.8000001907349px"><a href="http://about.me/ariccio" target="_blank">about.me/ariccio</a></div><div style="font-size:12.8000001907349px"><a href="http://about.me/ariccio" target="_blank"><br></a></div><div style="font-size:12.8000001907349px">If left to my own devices, I will build more.</div><div style="font-size:12.8000001907349px">⁂</div></div></div></div></div></div>
<br><div class="gmail_quote">On Sun, Feb 28, 2016 at 12:59 AM, Anna Zaks <span dir="ltr"><<a href="mailto:ganna@apple.com" target="_blank">ganna@apple.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div style="word-wrap:break-word"><br><div><blockquote type="cite"><span class=""><div>On Feb 27, 2016, at 8:18 PM, <Alexander G. Riccio> <<a href="mailto:test35965@gmail.com" target="_blank">test35965@gmail.com</a>> wrote:</div><br></span><div><div dir="ltr"><span class="">Hmm. It seems that message got rejected because it was a tiny bit over the size limit. It turns out those scripts were wrong anyways. These are slightly better - Windows uses _strdup instead of strdup, and I don't know how to conditionally <font face="monospace, monospace">#define strdup _strdup</font> in the makefile - but you can modify them to work locally.<div><br></div><div>Because of that strdup/_strdup issue, I've actually found a leak that Clang misses when compiling on Windows. The test is <a href="https://samate.nist.gov/SARD/view_testcase.php?tID=149071" target="_blank">SARD #149071, mem1-bad.c</a>. MallocChecker::isCMemFunction appears to miss it because it doesn't exactly match strdup:</div></span><div><span><strdup_leak_missed_small.PNG></span><br><br></div><span class=""><div>If I understand correctly, to fix this, I just need to:</div><div><ol><li>add a new <font face="monospace, monospace">IdentifierInfo*</font> to MallocChecker</li><li>initialize it in the default constructor</li><li>add a <font face="monospace, monospace">Ctx.Idents.get("_strdup")</font> call in MallocChecker::initIdentifierInfo<br></li><li>add the IdentifierInfo to the check in MallocChecker::isCMemFunction</li><li>add the Identifier info the <font face="monospace, monospace">else if (FunI == II_strdup)</font> in MallocChecker::checkPostStmt</li></ol><div>...and that should be a 5 line patch.</div></div></span></div></div></blockquote><div><br></div>Maybe we should only do that if we are targeting Windows. Also, I am sure there are other functions with the “_” Windows versions we are skipping elsewhere in the analyzer.</div><div><br></div><div>After the Windows-specific improvements are committed, you might be able to rewrite the function matching code by using the newly added helper function:</div><div><a href="http://reviews.llvm.org/D15921" target="_blank">http://reviews.llvm.org/D15921</a></div><div><br></div><div>Thanks,</div><div>Anna.<br><blockquote type="cite"><div><div><div class="h5"><div class="gmail_extra"><br clear="all"><div><div><div dir="ltr"><div><div dir="ltr"><span style="font-size:12.8000001907349px">Sincerely,</span><br style="font-size:12.8000001907349px"><span style="font-size:12.8000001907349px">Alexander Riccio</span><br style="font-size:12.8000001907349px"><span style="font-size:12.8000001907349px">--</span><br style="font-size:12.8000001907349px"><span style="font-size:12.8000001907349px">"Change the world or go home."</span><div style="font-size:12.8000001907349px"><a href="http://about.me/ariccio" target="_blank">about.me/ariccio</a></div><div style="font-size:12.8000001907349px"><a href="http://about.me/ariccio" target="_blank"><br></a></div><div style="font-size:12.8000001907349px">If left to my own devices, I will build more.</div><div style="font-size:12.8000001907349px">⁂</div></div></div></div></div></div>
<br><div class="gmail_quote">On Tue, Feb 23, 2016 at 4:35 PM, <Alexander G. Riccio> <span dir="ltr"><<a href="mailto:test35965@gmail.com" target="_blank">test35965@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div style="font-size:12.8px">I've attached the scripts for the non-buggy SARD tests. Interestingly, I've actually had to append ".attachment" to the ".sh" and ".cmd" filenames, for gmail to let me send them. Gmail was blocking them as executables.</div><div style="font-size:12.8px"><br></div><div style="font-size:12.8px">I've dropped nine tests (of 100). I don't think they'll offer any value (for clang), so we won't be missing anything by dropping them.</div><div style="font-size:12.8px"><br></div><div style="font-size:12.8px">Four of those tests are about SQL injection - which we're quite far from being able to check - and also require third party (MySQL) headers/libraries to run. Anna expressed concern (private email) in non-self-sufficient tests, so those four are not feasible.</div><div style="font-size:12.8px"><br></div><div style="font-size:12.8px"><span style="font-size:12.8px">The other five tests are about cross site scripting - which we're also quite far from being able to check - and require separate compilation. I'm terrible with makefiles, which is why separate compilation is a problem :) ...if anybody with makefile expertise can do this cleanly, please let me know! </span><br></div><div style="font-size:12.8px"><span style="font-size:12.8px"><br></span></div><div style="font-size:12.8px">For the remaining tests, I've had to suppress a few false positives (notably "format string is not a string literal") so that everything builds cleanly.</div><div style="font-size:12.8px"><span style="font-size:12.8px"><br></span></div><div style="font-size:12.8px">I don't have easy access to a proper Linux system, so I can't fully test these scripts (I can only really run SATestBuild.py manually: <font face="monospace, monospace">py -2 "C:\LLVM\llvm\tools\clang\utils\analyzer\SATestBuild.py"</font>), can someone try them for me?</div><div style="font-size:12.8px"></div></div><div class="gmail_extra"><span><br clear="all"><div><div><div dir="ltr"><div><div dir="ltr"><span style="font-size:12.8000001907349px">Sincerely,</span><br style="font-size:12.8000001907349px"><span style="font-size:12.8000001907349px">Alexander Riccio</span><br style="font-size:12.8000001907349px"><span style="font-size:12.8000001907349px">--</span><br style="font-size:12.8000001907349px"><span style="font-size:12.8000001907349px">"Change the world or go home."</span><div style="font-size:12.8000001907349px"><a href="http://about.me/ariccio" target="_blank">about.me/ariccio</a></div><div style="font-size:12.8000001907349px"><a href="http://about.me/ariccio" target="_blank"><br></a></div><div style="font-size:12.8000001907349px">If left to my own devices, I will build more.</div><div style="font-size:12.8000001907349px">⁂</div></div></div></div></div></div>
<br></span><div><div><div class="gmail_quote">On Wed, Feb 17, 2016 at 2:13 AM, <Alexander G. Riccio> <span dir="ltr"><<a href="mailto:test35965@gmail.com" target="_blank">test35965@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr">An update: I've pretty much gotten the non-bug version of the testsuite ready, and now that I know what I'm doing, the buggy version will be soon to follow.</div><div class="gmail_extra"><span><br clear="all"><div><div><div dir="ltr"><div><div dir="ltr"><span style="font-size:12.8000001907349px">Sincerely,</span><br style="font-size:12.8000001907349px"><span style="font-size:12.8000001907349px">Alexander Riccio</span><br style="font-size:12.8000001907349px"><span style="font-size:12.8000001907349px">--</span><br style="font-size:12.8000001907349px"><span style="font-size:12.8000001907349px">"Change the world or go home."</span><div style="font-size:12.8000001907349px"><a href="http://about.me/ariccio" target="_blank">about.me/ariccio</a></div><div style="font-size:12.8000001907349px"><a href="http://about.me/ariccio" target="_blank"><br></a></div><div style="font-size:12.8000001907349px">If left to my own devices, I will build more.</div><div style="font-size:12.8000001907349px">⁂</div></div></div></div></div></div>
<br></span><div><div><div class="gmail_quote">On Thu, Feb 4, 2016 at 6:52 PM, <Alexander G. Riccio> <span dir="ltr"><<a href="mailto:test35965@gmail.com" target="_blank">test35965@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr">Here's an update, I'd been waiting to hear back from NIST/SAMATE about licensing of the SARD testsuite, and it <i>looks</i> like we can use it:<div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><span style="font-size:12.8px">Hi Alexander,</span><br style="font-size:12.8px"><br style="font-size:12.8px"><span style="font-size:12.8px">You are free to use, copy, modify, and distribute (in particular, integrate into Clang's static analysis testing infrastructure) all test cases included in the Software Assurance Reference Dataset (SARD) test suites 100 and 101.</span><br style="font-size:12.8px"><br style="font-size:12.8px"><span style="font-size:12.8px">Most of these test cases were developed by NIST, and so they are in the public domain. The rest of the test cases were contributed by others with permission to use, copy, modify, and distribute them.</span><br style="font-size:12.8px"><br style="font-size:12.8px"><span style="font-size:12.8px">I hope this is helpful.</span><br style="font-size:12.8px"><br style="font-size:12.8px"><span style="font-size:12.8px">Thanks,</span><br style="font-size:12.8px"><br style="font-size:12.8px"><span style="font-size:12.8px">Vadim Okun</span><br style="font-size:12.8px"><span style="font-size:12.8px">SAMATE Project Leader</span><br style="font-size:12.8px"><a href="mailto:vadim.okun@nist.gov" style="font-size:12.8px" target="_blank">vadim.okun@nist.gov</a></blockquote><div><br></div><div>The only bit that's not crystal clear is "<span style="font-size:12.8px">contributed by others with permission to use, copy, modify, and distribute them", which I think fits the UIUC license requirements.</span></div></div><div class="gmail_extra"><span><br clear="all"><div><div><div dir="ltr"><div><div dir="ltr"><span style="font-size:12.8000001907349px">Sincerely,</span><br style="font-size:12.8000001907349px"><span style="font-size:12.8000001907349px">Alexander Riccio</span><br style="font-size:12.8000001907349px"><span style="font-size:12.8000001907349px">--</span><br style="font-size:12.8000001907349px"><span style="font-size:12.8000001907349px">"Change the world or go home."</span><div style="font-size:12.8000001907349px"><a href="http://about.me/ariccio" target="_blank">about.me/ariccio</a></div><div style="font-size:12.8000001907349px"><a href="http://about.me/ariccio" target="_blank"><br></a></div><div style="font-size:12.8000001907349px">If left to my own devices, I will build more.</div><div style="font-size:12.8000001907349px">⁂</div></div></div></div></div></div>
<br></span><div class="gmail_quote"><span>On Wed, Jan 20, 2016 at 1:11 AM, <Alexander G. Riccio> <span dir="ltr"><<a href="mailto:test35965@gmail.com" target="_blank">test35965@gmail.com</a>></span> wrote:<br></span><div><div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr">A quick update on this project:<div><br></div><div>I've been slowed by a technical issue, and I lost ~2 weeks as two family members were in the hospital (sorry!).</div><div><br></div><div>Since I develop on Windows, I quickly hit a testcase that clang didn't detect, as I discussed in "<span style="font-size:inherit;font-weight:inherit">Clang on Windows</span><span style="font-size:inherit;font-weight:inherit"> </span><span style="font-size:inherit;font-weight:inherit">fails</span><span style="font-size:inherit;font-weight:inherit"> </span><span style="font-size:inherit;font-weight:inherit">to</span><span style="font-size:inherit;font-weight:inherit"> </span><span style="font-size:inherit;font-weight:inherit">detect</span><span style="font-size:inherit;font-weight:inherit"> </span><span style="font-size:inherit;font-weight:inherit">trivial double free in static analysis".</span></div><div><br></div><div>That resulted in <a href="http://reviews.llvm.org/D16245" target="_blank">D16245</a>, which (when accepted) fixes that issue. I want to ensure that novice can simply pass "--analyze", and clang to "just work", so I've intentionally put off further testing work. Otherwise, I could hack around it, and subsequently forget about the workaround. Once that's dealt with, then I can resume work at a faster pace.</div><div><br></div><div><br></div></div><div class="gmail_extra"><span><br clear="all"><div><div><div dir="ltr"><div><div dir="ltr"><span style="font-size:12.8000001907349px">Sincerely,</span><br style="font-size:12.8000001907349px"><span style="font-size:12.8000001907349px">Alexander Riccio</span><br style="font-size:12.8000001907349px"><span style="font-size:12.8000001907349px">--</span><br style="font-size:12.8000001907349px"><span style="font-size:12.8000001907349px">"Change the world or go home."</span><div style="font-size:12.8000001907349px"><a href="http://about.me/ariccio" target="_blank">about.me/ariccio</a></div><div style="font-size:12.8000001907349px"><a href="http://about.me/ariccio" target="_blank"><br></a></div><div style="font-size:12.8000001907349px">If left to my own devices, I will build more.</div><div style="font-size:12.8000001907349px">⁂</div></div></div></div></div></div>
<br></span><div><div><div class="gmail_quote">On Mon, Jan 4, 2016 at 3:05 PM, Anna Zaks <span dir="ltr"><<a href="mailto:ganna@apple.com" target="_blank">ganna@apple.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div style="word-wrap:break-word"><br><div><blockquote type="cite"><span><div>On Jan 2, 2016, at 12:45 PM, <Alexander G. Riccio> <<a href="mailto:test35965@gmail.com" target="_blank">test35965@gmail.com</a>> <Alexander G. Riccio> wrote:</div><br></span><span><div><div dir="ltr"><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div style="font-size:12.8px">Devin has started writing scripts for running additional analyzer tests as described in this thread:</div></blockquote><div><br></div><div>A buildbot sounds like the perfect idea! </div><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div style="font-size:12.8px">The idea was to check out the tests/projects from the existing repos instead of copying them. Would it be possible to do the same with these tests?</div></blockquote><div><br></div><div>Eh? What do you mean? Would that stop someone from running them in the clang unit test infrastructure?</div><div><br></div><div>I believe that these tests WILL need to be modified to run in the Clang testing infrastructure. </div><div><br></div></div></div></span></blockquote><br><div>Currently, the analyzer is only tested with the regression tests. However, those need to be fast (since they effect all clang developers) and they have limited coverage. Internally, we’ve been testing the analyzer with the test scripts Devin described in the email I referenced. We use that testing method to analyze whole projects and long running tests. Those tests can and should be executed separately as they take more than an hour to complete. The plan is to set up an external builedbot running those tests. </div><div><br></div><div>How long would it take to analyze the tests you are planning to add? Depending on the answer to that question, adding your tests to the new builedbot might be a better fit than adding them to the regression tests.</div><div><br></div><div>I also prefer not to modify the externally written tests since it would allow us to update them more easily, for example, when a new version of the tests comes out.</div><span><br><blockquote type="cite"><div><div dir="ltr"><div>Is there any way to treat static analyzer warnings as plain old warnings/errors? Dumping them to a plist file from a command line compilation is a bit annoying, and I think is incompatible with the clang unit testing infrastructure?</div><div><br></div></div></div></blockquote><br></span><div>Plist output is one if the outputs that the clang static analyzer supports. It is a much richer format than the textual warning since it contains information about the path on which the error occurred. We did have some lit tests checking plist output as well.</div><div><div><br><blockquote type="cite"><div dir="ltr"><div><br></div></div><div class="gmail_extra"><br clear="all"><div><div><div dir="ltr"><div><div dir="ltr"><span style="font-size:12.8000001907349px">Sincerely,</span><br style="font-size:12.8000001907349px"><span style="font-size:12.8000001907349px">Alexander Riccio</span><br style="font-size:12.8000001907349px"><span style="font-size:12.8000001907349px">--</span><br style="font-size:12.8000001907349px"><span style="font-size:12.8000001907349px">"Change the world or go home."</span><div style="font-size:12.8000001907349px"><a href="http://about.me/ariccio" target="_blank">about.me/ariccio</a></div><div style="font-size:12.8000001907349px"><a href="http://about.me/ariccio" target="_blank"><br></a></div><div style="font-size:12.8000001907349px">If left to my own devices, I will build more.</div><div style="font-size:12.8000001907349px">⁂</div></div></div></div></div></div>
<br><div class="gmail_quote">On Mon, Dec 28, 2015 at 12:23 AM, Anna Zaks <span dir="ltr"><<a href="mailto:ganna@apple.com" target="_blank">ganna@apple.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div style="word-wrap:break-word"><br><div><span><blockquote type="cite"><div>On Dec 17, 2015, at 11:01 AM, <Alexander G. Riccio> <<a href="mailto:alexander@riccio.com" target="_blank">alexander@riccio.com</a>> <Alexander G. Riccio> wrote:</div><br><div><div dir="ltr"><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">However, if the goal is to have the tests<br>because you would like to make efforts to have the compiler diagnose<br>their cases properly, that's far more interesting and a good reason to<br>bring in the tests.</blockquote><div><br></div><div>That's exactly my intention. Improving the static analyzer to detect these cases, that will be interesting.</div><div>placeholder text</div><div><br></div><div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">If the other tests are not clearly licensed, we<br>should try to get NIST to clarify the license of them before<br>inclusion.</blockquote></div><div><br></div><div>That sounds like the best idea, as a government agency, they almost certainly have lawyers.</div><div><br></div><div>I think the next step is to integrate the working (error correctly diagnosed) tests, only those that are obviously in the public domain, and propose them as a big batched patch. This shouldn't itself be controversial. </div><div><br></div><div>How exactly do I submit a patch? I see that the LLVM developer policy says to send it to the mailing list (cfe-commits), but I also see that <a href="http://lists.llvm.org/pipermail/cfe-commits/Week-of-Mon-20151214/145026.html" target="_blank">Phabricator comes into this somewhere</a>?</div></div><div class="gmail_extra"><br clear="all"></div></div></blockquote><br></span><div>Devin has started writing scripts for running additional analyzer tests as described in this thread:</div><div><a href="http://clang-developers.42468.n3.nabble.com/analyzer-Adding-build-bot-for-static-analyzer-reference-results-td4047770.html" target="_blank">http://clang-developers.42468.n3.nabble.com/analyzer-Adding-build-bot-for-static-analyzer-reference-results-td4047770.html</a></div></div><div><div><br></div><div>The idea was to check out the tests/projects from the existing repos instead of copying them. Would it be possible to do the same with these tests?</div><div><br></div><div>Sorry for not replying sooner!</div><span><font color="#888888"><div>Anna.</div></font></span><div><div><blockquote type="cite"><div class="gmail_extra"><div><div><div dir="ltr"><div><div dir="ltr"><div dir="ltr">Sincerely,<br>Alexander Riccio<br>--<br>"Change the world or go home."<div><a href="http://about.me/ariccio" target="_blank">about.me/ariccio</a></div><div><a href="http://about.me/ariccio" target="_blank"><br></a></div><div>If left to my own devices, I will build more.</div><div>⁂<br></div></div></div></div></div></div></div>
<br><div class="gmail_quote">On Thu, Dec 10, 2015 at 4:04 PM, Aaron Ballman <span dir="ltr"><<a href="mailto:aaron@aaronballman.com" target="_blank">aaron@aaronballman.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><span>On Mon, Dec 7, 2015 at 9:50 PM, <Alexander G. Riccio> via cfe-dev<br>
<<a href="mailto:cfe-dev@lists.llvm.org" target="_blank">cfe-dev@lists.llvm.org</a>> wrote:<br>
> First time Clang contributor here,<br>
><br>
> I'd like to add the "C Test Suite for Source Code Analyzer v2", a<br>
> relatively small test suite (102 cases/flaws), some of which Clang<br>
> doesn't yet detect*. See link at bottom.<br>
><br>
> Immediate questions:<br>
> 0. Does the Clang community/project like the idea?<br>
<br>
</span>I've included a few other devs (CCed) to get further opinions.<br>
<br>
I like the idea of being able to diagnose the issues covered by the<br>
test suite, but I don't think including the test suite by itself is<br>
particularly useful without that goal in mind. Also, one question I<br>
would have has to do with the licensing of the tests themselves and<br>
whether we would need to do anything special there.<br>
<span><br>
> 1. What's the procedure for including new tests? (not the technical,<br>
> but the community/project).<br>
<br>
</span>Getting the discussion going about the desired goal (as you are doing)<br>
is the right first step.<br>
<span><br>
> 2. How do I include failing tests without breaking things? Some of<br>
> these tests will fail - that's why I'm proposing their inclusion - but<br>
> they shouldn't yet cause the regression testing system to complain.<br>
<br>
</span>Agreed, any test cases that are failing would have to fail gracefully.<br>
I assume that by failure, you mean "should diagnose in some way, but<br>
currently does not". I would probably split the tests into two types:<br>
one set of tests that properly diagnose the issue (can be checked with<br>
FileCheck or -verify, depending on the kind of tests we're talking<br>
about), and one set of tests where we do not diagnose, but want to see<br>
them someday (which can be tested with expect-no-diagnostics, for<br>
example). This way, we can ensure test cases continue to diagnose when<br>
we want them to, and we can be alerted when new diagnostics start to<br>
catch previously uncaught tests. This is assuming that it makes sense<br>
to include all of the tests at once, which may not make sense in<br>
practice.<br>
<span><br>
> 3. How does Clang handle licensing of third party code? Some of these<br>
> tests are clearly in the public domain (developed at NIST, says "in<br>
> the public domain"), but others are less clearly licensed.<br>
<br>
</span>Oh look, you asked the same question I asked. ;-) If the tests are in<br>
the public domain and clearly state as such, I think we can go ahead<br>
and include them. If the other tests are not clearly licensed, we<br>
should try to get NIST to clarify the license of them before<br>
inclusion. Depending on the license, we may be able to include them<br>
under their original license. If we cannot clarify the license, I<br>
would guess that we simply should not include those tests as part of<br>
our test suite. Note: I could be totally wrong, IANAL. :-)<br>
<span><br>
> Should the community accept that testsuite, and I successfully add<br>
> that test suite, then I'd like to step it up a bit, and include the<br>
> "Juliet Test Suite for C/C++". "Juliet" is a huge test suite by the<br>
> NSA Center for Assured Software & NIST's Software Assurance Metrics<br>
> And Tool Evaluation project, which has 25,477 test cases (!!) for 118<br>
> CWEs. I don't think any other open source compiler could compete with<br>
> Clang after this. There's a ton of literature on the "Juliet" suite,<br>
> and listing it here is not necessary.<br>
><br>
> This project would be my first Clang contribution :)<br>
><br>
> Personally, I'm interested in static analysis, and this is the first<br>
> step in understanding & improving Clang's static analysis<br>
> capabilities.<br>
><br>
> I have some ideas on how to detect the currently undetected bugs, and<br>
> I'm curious to see where things lead.<br>
<br>
</span>Adding the tests by themselves is not necessarily interesting to the<br>
project unless they exercise the compiler in ways it's not currently<br>
being exercised. So just having tests for the sake of having the tests<br>
is not too useful (IMO). However, if the goal is to have the tests<br>
because you would like to make efforts to have the compiler diagnose<br>
their cases properly, that's far more interesting and a good reason to<br>
bring in the tests.<br>
<br>
One possible approach if you are interested in having the compiler<br>
diagnose the cases is to bring the tests in one at a time. Start with<br>
the initial batch of "these are diagnosed properly", then move on to<br>
"this test is diagnosed properly because of this patch." Eventually<br>
we'll get to the stage where all of the tests are diagnosed properly.<br>
<span><br>
> Secondary questions:<br>
> 1. How should I break the new tests up into patches? Should I just<br>
> whack the whole 102 case suite into a single patch, or a bunch of<br>
> smaller ones?<br>
<br>
</span>See comments above.<br>
<span><br>
> 2. How does the Clang/LLVM static analysis testing infrastructure<br>
> work? I'm going to have to figure this out myself anyways, but where<br>
> should I start? Any tips on adding new tests?<br>
<br>
</span><a href="http://clang-analyzer.llvm.org/checker_dev_manual.html" rel="noreferrer" target="_blank">http://clang-analyzer.llvm.org/checker_dev_manual.html</a><br>
<br>
Another good place for some of these checkers may be clang-tidy, or<br>
the compiler frontend itself. It's likely to depend on case-by-case<br>
code patterns.<br>
<br>
<a href="http://clang.llvm.org/extra/clang-tidy/" rel="noreferrer" target="_blank">http://clang.llvm.org/extra/clang-tidy/</a><br>
<br>
Thank you for looking into this!<br>
<br>
~Aaron<br>
<span><br>
><br>
> *If I remember correctly,<br>
> <a href="https://samate.nist.gov/SRD/view_testcase.php?tID=149055" rel="noreferrer" target="_blank">https://samate.nist.gov/SRD/view_testcase.php?tID=149055</a> passes<br>
> analysis without complaint. I manually spot checked a very small<br>
> number of tests.<br>
><br>
> "C Test Suite for Source Code Analyzer v2" (valid code):<br>
> <a href="https://samate.nist.gov/SRD/view.php?tsID=101" rel="noreferrer" target="_blank">https://samate.nist.gov/SRD/view.php?tsID=101</a><br>
> "C Test Suite for Source Code Analyzer v2" (invalid code):<br>
> <a href="https://samate.nist.gov/SRD/view.php?tsID=100" rel="noreferrer" target="_blank">https://samate.nist.gov/SRD/view.php?tsID=100</a><br>
><br>
> "Juliet Test Suite for C/C++" (files):<br>
> <a href="https://samate.nist.gov/SRD/testsuites/juliet/Juliet_Test_Suite_v1.2_for_C_Cpp.zip" rel="noreferrer" target="_blank">https://samate.nist.gov/SRD/testsuites/juliet/Juliet_Test_Suite_v1.2_for_C_Cpp.zip</a><br>
> "Juliet Test Suite for C/C++" (docs):<br>
> <a href="https://samate.nist.gov/SRD/resources/Juliet_Test_Suite_v1.2_for_C_Cpp_-_User_Guide.pdf" rel="noreferrer" target="_blank">https://samate.nist.gov/SRD/resources/Juliet_Test_Suite_v1.2_for_C_Cpp_-_User_Guide.pdf</a><br>
><br>
><br>
> Sincerely,<br>
> Alexander Riccio<br>
</span>> _______________________________________________<br>
> cfe-dev mailing list<br>
> <a href="mailto:cfe-dev@lists.llvm.org" target="_blank">cfe-dev@lists.llvm.org</a><br>
> <a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev" rel="noreferrer" target="_blank">http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev</a><br>
</blockquote></div><br></div>
</blockquote></div></div></div><br></div></blockquote></div><br></div>
</blockquote></div></div></div><br></div></blockquote></div><br></div></div></div>
</blockquote></div></div></div><br></div>
</blockquote></div><br></div></div></div>
</blockquote></div><br></div></div></div>
</blockquote></div><br></div>
</div></div><span><SARD_SCRIPTS_attachment.7z></span></div></blockquote></div><br></div></blockquote></div><br></div>