<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Wed, Mar 9, 2016 at 11:58 AM, Alexander Riccio via cfe-commits <span dir="ltr"><<a href="mailto:cfe-commits@lists.llvm.org" target="_blank">cfe-commits@lists.llvm.org</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">ariccio added a comment.<br>
<span class=""><br>
In <a href="http://reviews.llvm.org/D17983#370717" rel="noreferrer" target="_blank">http://reviews.llvm.org/D17983#370717</a>, @dblaikie wrote:<br>
<br>
> Initializations we never expect to use (eg because we have a covered switch<br>
> that initializes in all cases, or some slightly complex control flow the<br>
> compiler can't see through) hinder our ability to find uses of those with<br>
> tools like msan.<br>
<br>
<br>
</span>Too bad... Maybe msan should have a way to specify that you're default initializing a variable, and treat it as if you're not initializing it at all?<br></blockquote><div><br></div><div>Maybe - could be worth discussing (I'd start a more general thread for that if I were you, with some msan devs and llvm-dev)</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">That said, I'm not sure that I agree with keeping `Potentially uninitialized local pointer variable 'name' used` off (//that's C4703 <<a href="https://msdn.microsoft.com/en-us/library/jj851030.aspx" rel="noreferrer" target="_blank">https://msdn.microsoft.com/en-us/library/jj851030.aspx</a>>, not C4701 <<a href="https://msdn.microsoft.com/en-us/library/1wea5zwe.aspx" rel="noreferrer" target="_blank">https://msdn.microsoft.com/en-us/library/1wea5zwe.aspx</a>>//), because uninitialized local pointer bugs are more dangerous (I'm thinking <<a href="http://googleprojectzero.blogspot.com/2015/08/one-font-vulnerability-to-rule-them-all_21.html" rel="noreferrer" target="_blank">http://googleprojectzero.blogspot.com/2015/08/one-font-vulnerability-to-rule-them-all_21.html</a>> of write-what-where <<a href="http://thesprawl.org/research/ost-introduction-software-exploits-uninit-overflow/" rel="noreferrer" target="_blank">http://thesprawl.org/research/ost-introduction-software-exploits-uninit-overflow/</a>> uninitialized pointer vulnerabilities <<a href="http://ioctl.ir/index.php/2016/02/13/cve-2016-0040-story-of-uninitialized-pointer/" rel="noreferrer" target="_blank">http://ioctl.ir/index.php/2016/02/13/cve-2016-0040-story-of-uninitialized-pointer/</a>>), and I'd rather crash on a nullptr deref than some undefined behavior.<br></blockquote><div><br></div><div>That being the problem - UB is what allows us to check these things and have confidence we're finding bugs. (& not all the code, of course, would deref into null - some places might use null as a valid state that is only established after initialization (eg: int *x; if (a) { x = nullptr; } - so we could still catch use of uninitialized in addition to null pointer handling)</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">I will drop the `default` cases as @dsanders noted, but should I drop all the local variable inits too? In several cases (which I didn't specifically note) I chose default initialization values that would trigger pre-existing `assert`s if nobody assigned anything else to them.<br></blockquote><div><br></div><div>I'm not sure how valuable that is - again, we have msan to catch these and broader issues.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Regarding the 33 warnings I noted, i couldn't tell whether they're obviously false positives, which I think warrants someone other than me carefully looking through it. </blockquote><div><br></div><div>These are 33 warnings you don't have fixes for? Sorry, I haven't looked closely at the warning list compared to the proposed patch.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Some of the warnings are caused by assigning to a variable inside of a loop with a dynamically sized bound; for those, I'd much rather (a) `assert` that the bound is nonzero/loop will be executed at least once, or (b) assign the variable some sentinel value, and then assert it before the variable is actually used. Thoughts?<br></blockquote><div><br></div><div>I'd be happy to see an assertion for a loop like that, generally. (though I haven't looked at the specific cases you're referring to)<br><br>- Dave</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div class="HOEnZb"><div class="h5"><br>
<br>
<a href="http://reviews.llvm.org/D17983" rel="noreferrer" target="_blank">http://reviews.llvm.org/D17983</a><br>
<br>
<br>
<br>
_______________________________________________<br>
cfe-commits mailing list<br>
<a href="mailto:cfe-commits@lists.llvm.org">cfe-commits@lists.llvm.org</a><br>
<a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits" rel="noreferrer" target="_blank">http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits</a><br>
</div></div></blockquote></div><br></div></div>