On Wed, Mar 9, 2016 at 11:58 AM, Alexander Riccio via cfe-commits < cfe-commits@lists.llvm.org> wrote:
> ariccio added a comment. > > In http://reviews.llvm.org/D17983#370717, @dblaikie wrote: > > > Initializations we never expect to use (eg because we have a covered > switch > > that initializes in all cases, or some slightly complex control flow the > > compiler can't see through) hinder our ability to find uses of those > with > > tools like msan. > > > 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? > 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) > That said, I'm not sure that I agree with keeping `Potentially > uninitialized local pointer variable 'name' used` off (//that's C4703 < > https://msdn.microsoft.com/en-us/library/jj851030.aspx>, not C4701 < > https://msdn.microsoft.com/en-us/library/1wea5zwe.aspx>//), because > uninitialized local pointer bugs are more dangerous (I'm thinking < > http://googleprojectzero.blogspot.com/2015/08/one-font-vulnerability-to-rule-them-all_21.html> > of write-what-where < > http://thesprawl.org/research/ost-introduction-software-exploits-uninit-overflow/> > uninitialized pointer vulnerabilities < > http://ioctl.ir/index.php/2016/02/13/cve-2016-0040-story-of-uninitialized-pointer/>), > and I'd rather crash on a nullptr deref than some undefined behavior. > 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) > 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. > I'm not sure how valuable that is - again, we have msan to catch these and broader issues. > 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. 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. > 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? > 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) - Dave > > > http://reviews.llvm.org/D17983 > > > > _______________________________________________ > cfe-commits mailing list > cfe-commits@lists.llvm.org > http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits >
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
