<div dir="ltr">I can confirm that unix.MallocSizeof, unix.cstring.BadSizeArg, unix.cstring.NullArg, and unix.API, should also be turned on - they all seem to work correctly.<div><br></div><div>As a matter of fact, should we just enable all of the unix checkers?</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 Tue, Jan 12, 2016 at 8:20 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"><span class="">> My understanding is that this is in the “unix” package for historical reasons from before the analyzer thought about Windows. We have clients that rely on the malloc checker being enabled/disabled with ‘unix.Malloc' so moving it will break compatibility. <div><span style="font-size:12.8px"><br></span></div></span><div>Gahh, ok. Not breaking things is a good reason.</div><span class=""><div><br></div><div>> <span style="font-size:12.8px">I discussed this off-list with Anna Zaks. She recommended changing the driver to enable unix.Malloc explicitly when running in a Windows MSVC environment. This is Clang::ConstructJob() in Tools.cpp. We currently skip enabling the “unix” package there for Windows MSVC.</span></div><div><span style="font-size:12.8px"><br></span></div></span><div><span style="font-size:12.8px">Yup, that'd be a good short term fix.</span></div><span class=""><div><span style="font-size:12.8px"><br></span></div><div>> <span style="font-size:12.8px">Someone who develops on Windows should write a patch and test it. Alexander: would you be willing to do this? It should be very straightforward — there is similar code to disable specific checkers for PS4.</span></div><div><span style="font-size:12.8px"><br></span></div></span><div><span style="font-size:12.8px">Yup! I could very well do that. I think the change would look something like:</span></div><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(200,200,200)">IsWindowsMSVC</span><span style="color:rgb(180,180,180)">)</span>
        <span style="color:rgb(200,200,200)">CmdArgs</span><span style="color:rgb(180,180,180)">.</span><span style="color:rgb(200,200,200)">push_back</span><span style="color:rgb(180,180,180)">(</span><span style="color:rgb(214,157,133)">"-analyzer-checker=unix.Malloc"</span><span style="color:rgb(180,180,180)">);</span></pre></div><span class=""><div><div style="font-size:12.8px">> We should also add a comment Checkers.td to indicate that we should move the Malloc checker to another package when we do remap packages names and break backward compatibility. “core” is probably not the right place for this (since malloc() is in the standard library). Maybe a new “cstdlib” package?</div><div style="font-size:12.8px"></div></div><div><span style="font-size:12.8px"><br></span></div></span><div><span style="font-size:12.8px">(that's what I meant by "short term fix") </span></div><div><span style="font-size:12.8px">"cstdlib" makes more sense - core was just the first thing that came to mind - and we could make cstdlib.malloc/</span><span style="font-size:12.8px">cstdlib.Malloc alias unix.Malloc to avoid breaking users of unix.Malloc.</span></div><div><span style="font-size:12.8px"><br></span></div><div><span style="font-size:12.8px">Sidenote: has anybody ever considered diagnosing incorrectly capitalized checker name arguments? It's not terribly important, but I was quite annoyed to find out that I'd spent a couple hours debugging a lowercase "M".</span></div></div><div class="gmail_extra"><span class=""><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 class="h5"><div class="gmail_quote">On Tue, Jan 12, 2016 at 7:06 PM, Devin Coughlin <span dir="ltr"><<a href="mailto:dcoughlin@apple.com" target="_blank">dcoughlin@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">My understanding is that this is in the “unix” package for historical reasons from before the analyzer thought about Windows. We have clients that rely on the malloc checker being enabled/disabled with ‘unix.Malloc' so moving it will break compatibility. <div><br></div><div>I discussed this off-list with Anna Zaks. She recommended changing the driver to enable unix.Malloc explicitly when running in a Windows MSVC environment. This is Clang::ConstructJob() in Tools.cpp. We currently skip enabling the “unix” package there for Windows MSVC.</div><div><br></div><div>Someone who develops on Windows should write a patch and test it. Alexander: would you be willing to do this? It should be very straightforward — there is similar code to disable specific checkers for PS4.</div><div><br></div><div>We should also add a comment Checkers.td to indicate that we should move the Malloc checker to another package when we do remap packages names and break backward compatibility. “core” is probably not the right place for this (since malloc() is in the standard library). Maybe a new “cstdlib” package?</div><span><font color="#888888"><div><br></div><div>Devin</div></font></span><div><div><div><div><div><br><div><blockquote type="cite"><div>On Jan 12, 2016, at 2:22 PM, Alexander Riccio via cfe-dev <<a href="mailto:cfe-dev@lists.llvm.org" target="_blank">cfe-dev@lists.llvm.org</a>> wrote:</div><br><div><div><div><div style="font-family:Calibri,sans-serif;font-size:11pt">Shoot - I haven't responded to this. I did some debugging the other day and found that if I manually pass the flag to enable the unix.Malloc checker (that's a capital "M", as I discovered the hard way), Clang detects this.<br><br>I was going to suggest something like enabling it by default (obvious), and *maybe* renaming it to something like core.Malloc, because it's not unix-specific.<br><br>The one benefit here of parsing SAL is a more generic mechanism, but that's a different issue.<br><br>sent from my (stupid) windows phone</div></div><div dir="ltr"><hr><span style="font-family:Calibri,sans-serif;font-size:11pt;font-weight:bold">From: </span><span style="font-family:Calibri,sans-serif;font-size:11pt"><a href="mailto:rnk@google.com" target="_blank">Reid Kleckner</a></span><br><span style="font-family:Calibri,sans-serif;font-size:11pt;font-weight:bold">Sent: </span><span style="font-family:Calibri,sans-serif;font-size:11pt">‎1/‎12/‎2016 5:18 PM</span><br><span style="font-family:Calibri,sans-serif;font-size:11pt;font-weight:bold">To: </span><span style="font-family:Calibri,sans-serif;font-size:11pt"><a href="mailto:test35965@gmail.com" target="_blank"><Alexander G. Riccio></a>; <a href="mailto:jordan_rose@apple.com" target="_blank">Jordan Rose</a></span><br><span style="font-family:Calibri,sans-serif;font-size:11pt;font-weight:bold">Cc: </span><span style="font-family:Calibri,sans-serif;font-size:11pt"><a href="mailto:cfe-dev@lists.llvm.org" target="_blank">cfe-dev</a></span><br><span style="font-family:Calibri,sans-serif;font-size:11pt;font-weight:bold">Subject: </span><span style="font-family:Calibri,sans-serif;font-size:11pt">Re: [cfe-dev] Clang on Windows fails to detect trivial double free instatic analysis</span><br><br></div><div dir="ltr">Jordan, how do we enable this checker on Windows?<div><br></div><div>We shouldn't need to be able to parse SAL to do this analysis.</div></div><div class="gmail_extra"><br><div class="gmail_quote">On Sun, Jan 3, 2016 at 10:31 PM, <Alexander G. Riccio> via cfe-dev <span dir="ltr"><<a href="mailto:cfe-dev@lists.llvm.org" target="_blank">cfe-dev@lists.llvm.org</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;padding-left:1ex;border-left-color:rgb(204,204,204);border-left-width:1px;border-left-style:solid"><div dir="ltr">Is it because <a href="http://clang-analyzer.llvm.org/available_checks.html#unix_checkers" target="_blank">the checker is <i><b><u>unix</u></b></i>.malloc</a>? If so, that's actually quite terrible... why only check it on unix??</div><div class="gmail_extra"><span><br clear="all"><div><div><div dir="ltr"><div><div dir="ltr"><span style="font-size:12.8px">Sincerely,</span><br style="font-size:12.8px"><span style="font-size:12.8px">Alexander Riccio</span><br style="font-size:12.8px"><span style="font-size:12.8px">--</span><br style="font-size:12.8px"><span style="font-size:12.8px">"Change the world or go home."</span><div style="font-size:12.8px"><a href="http://about.me/ariccio" target="_blank">about.me/ariccio</a></div><div style="font-size:12.8px"><a href="http://about.me/ariccio" target="_blank"><br></a></div><div style="font-size:12.8px">If left to my own devices, I will build more.</div><div style="font-size:12.8px">⁂</div></div></div></div></div></div>
<br></span><div><div><div class="gmail_quote">On Sat, Jan 2, 2016 at 3:57 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:0px 0px 0px 0.8ex;padding-left:1ex;border-left-color:rgb(204,204,204);border-left-width:1px;border-left-style:solid"><div dir="ltr">When I build the attached C program in windows, using Clang built from a very recent tree version (trunk 256686), Clang fails to detect the trivial double free, as evidenced in the resulting plist file (attached).<div><br></div><div>What's going on here? I have a gut feeling that it has something to do with Clang's ignorance of SAL, which allows MSVC to detect the condition generically:</div><div><br></div><div><pre style="background:rgb(30,30,30);color:gainsboro;font-family:Consolas"><span style="color:rgb(86,156,214)">void</span> <span style="color:rgb(86,156,214)">__cdecl</span> <span style="color:rgb(200,200,200)">free</span><span style="color:rgb(180,180,180)">(</span>
    <span style="color:rgb(189,99,197)">_Pre_maybenull_</span> <span style="color:rgb(189,99,197)">_Post_invalid_</span> <span style="color:rgb(86,156,214)">void</span><span style="color:rgb(180,180,180)">*</span> _Block
    <span style="color:rgb(180,180,180)">);</span>
</pre></div><div>(from C:/Program Files (x86)/Windows Kits/10/Include/10.0.10240.0/ucrt/corecrt_malloc.h)</div><div><br></div><div>I'm also attaching the verbose compilation output.</div><div><div><div><div dir="ltr"><div><div dir="ltr"><span style="font-size:12.8px"><br></span></div><div dir="ltr"><span style="font-size:12.8px">Sincerely,</span><br style="font-size:12.8px"><span style="font-size:12.8px">Alexander Riccio</span><br style="font-size:12.8px"><span style="font-size:12.8px">--</span><br style="font-size:12.8px"><span style="font-size:12.8px">"Change the world or go home."</span><div style="font-size:12.8px"><a href="http://about.me/ariccio" target="_blank">about.me/ariccio</a></div><div style="font-size:12.8px"><a href="http://about.me/ariccio" target="_blank"><br></a></div><div style="font-size:12.8px">If left to my own devices, I will build more.</div><div style="font-size:12.8px">⁂</div></div></div></div></div></div>
</div></div>
</blockquote></div><br></div></div></div>
<br>_______________________________________________<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>
<br></blockquote></div><br></div>
</div>_______________________________________________<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" target="_blank">http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev</a><br></div></blockquote></div><br></div></div></div></div></div></div></blockquote></div><br></div></div></div>
</blockquote></div><br></div>