<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Fri, Dec 26, 2014 at 11:58 AM, Yaron Keren <span dir="ltr"><<a href="mailto:yaron.keren@gmail.com" target="_blank">yaron.keren@gmail.com</a>></span> wrote:<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="rtl"><div dir="ltr">Let's disregard dumb gcc warnings.</div></div></blockquote><div><br>Well that's why I was asking what your motivation for the change was - usually this sort of cleanup is motivated by some tool diagnosing the code as 'bad' (when it might not be/isn't). I'd like to address the root cause there if possible so we set expectations appropriately about what tool suggestions we are willing to accept code changes for, etc.<br> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="rtl"><div dir="ltr"> Initializing pointers only (not value variables) to nullptr will prevent msan detect uninitialized pointer read, but will usually/always? core dump instead. Considering msan works on Linux x64 only, and even then msan isn't used most of the time, how about creating an LLVM_NULLPTR macro which will expand to nothing on msan builds and to nullptr on non-msan builds, used like</div></div></blockquote><div><br>Chandler - you've usually been a pretty big advocate of not initializing things that don't need it (to allow static and dynamic tools to properly detect real issues). Any idea if this is the sort of thing you'd like to see? (modulo name bikeshedding, etc)<br><br>Alexey - any thoughts on how best to write code defensively while also giving sanitizers maximum opportunity to find bugs?<br><br>- David<br> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="rtl"><div dir="ltr"><br></div><div dir="ltr"><span><div dir="ltr" style="font-size:12.7272720336914px">DIBuilder::DIBuilder(Module &m, bool AllowUnresolvedNodes)</div></span><div dir="ltr" style="font-size:12.7272720336914px"> : M(m), VMContext(M.getContext()), TempEnumTypes(<span style="font-size:12.7272720336914px">LLVM_NULLPTR</span>),</div><div dir="ltr" style="font-size:12.7272720336914px"> TempRetainTypes(LLVM_NULLPTR), TempSubprograms(<span style="font-size:12.7272720336914px">LLVM_NULLPTR</span>), TempGVs(<span style="font-size:12.7272720336914px">LLVM_NULLPTR</span>),</div><div dir="ltr" style="font-size:12.7272720336914px"> DeclareFn(<span style="font-size:12.7272720336914px">LLVM_NULLPTR</span>), ValueFn(<span style="font-size:12.7272720336914px">LLVM_NULLPTR</span>),</div><div dir="ltr" style="font-size:12.7272720336914px"> AllowUnresolvedNodes(AllowUnresolvedNodes) {}</div><div><br></div></div><div dir="ltr"><br></div></div><div><div><div class="gmail_extra"><div dir="ltr"><br><div class="gmail_quote">2014-12-26 20:04 GMT+02:00 David Blaikie <span dir="ltr"><<a href="mailto:dblaikie@gmail.com" target="_blank">dblaikie@gmail.com</a>></span>:<br><blockquote class="gmail_quote" style="margin:0 .8ex;border-left:1px #ccc solid;border-right:1px #ccc solid;padding-left:1ex;padding-right:1ex"><br><div class="gmail_extra"><br><div class="gmail_quote"><span>On Thu, Dec 25, 2014 at 9:49 AM, Yaron Keren <span dir="ltr"><<a href="mailto:yaron.keren@gmail.com" target="_blank">yaron.keren@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="rtl"><div dir="ltr">msan uninitialized use diagnostic may be more useful to value variables, as nullptr pointer access will core dump without msan. Initializing pointers to nullptr is done everywhere in llvm and clang (search for '(nullptr)')</div></div></blockquote></span><div><br>There are certainly places where initializing to nullptr is necessary (because the null value is used/tested before any write occurs). And I don't doubt there are many places where it's unnecessary & done out of habit/consistency/forward-aovidance of problems (either due to the code being written before we had MSan, etc - or because the author wasn't aware of the tradeoffs).<br> </div><span><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="rtl"><div dir="ltr"> such as</div><div dir="ltr"><br></div><div dir="ltr"><div dir="ltr"><div dir="ltr">DIBuilder::DIBuilder(Module &m, bool AllowUnresolvedNodes)</div><div dir="ltr"> : M(m), VMContext(M.getContext()), TempEnumTypes(nullptr),</div><div dir="ltr"> TempRetainTypes(nullptr), TempSubprograms(nullptr), TempGVs(nullptr),</div><div dir="ltr"> DeclareFn(nullptr), ValueFn(nullptr),</div><div dir="ltr"> AllowUnresolvedNodes(AllowUnresolvedNodes) {}</div><div><br></div><div>isn't init to nullptr the "right" thing to do?</div></div></div></div></blockquote></span><div><br>Not necessarily. This isn't a hard-and-fast rule, but we've certainly pushed back on initializing variables that don't need initialization when it was motivated by silencing a tool that wasn't smart enough to know that these initializations weren't needed (such as GCC's warnings).<br> </div><div><div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="rtl"><div dir="ltr"><div dir="ltr"><div><br></div></div></div></div><div><div><div class="gmail_extra"><br><div class="gmail_quote"><div dir="ltr">2014-12-25 18:46 GMT+02:00 David Blaikie <span dir="ltr"><<a href="mailto:dblaikie@gmail.com" target="_blank">dblaikie@gmail.com</a>></span>:</div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><p dir="ltr">Any particular reason? It's sometimes handy to leave things uninitialized if they stents intended to be read until after some other write - in that way, msan can catch bugs when that constraint is violated.</p><div><div>
<div class="gmail_quote">On Dec 25, 2014 3:41 AM, "Yaron Keren" <<a href="mailto:yaron.keren@gmail.com" target="_blank">yaron.keren@gmail.com</a>> wrote:<br type="attribution"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Author: yrnkrn<br>
Date: Thu Dec 25 05:38:15 2014<br>
New Revision: 224835<br>
<br>
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=224835&view=rev" target="_blank">http://llvm.org/viewvc/llvm-project?rev=224835&view=rev</a><br>
Log:<br>
Initialize CodeGeneratorImpl::Ctx in constructor.<br>
<br>
<br>
Modified:<br>
cfe/trunk/lib/CodeGen/ModuleBuilder.cpp<br>
<br>
Modified: cfe/trunk/lib/CodeGen/ModuleBuilder.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/ModuleBuilder.cpp?rev=224835&r1=224834&r2=224835&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/ModuleBuilder.cpp?rev=224835&r1=224834&r2=224835&view=diff</a><br>
==============================================================================<br>
--- cfe/trunk/lib/CodeGen/ModuleBuilder.cpp (original)<br>
+++ cfe/trunk/lib/CodeGen/ModuleBuilder.cpp Thu Dec 25 05:38:15 2014<br>
@@ -59,7 +59,7 @@ namespace {<br>
CodeGeneratorImpl(DiagnosticsEngine &diags, const std::string& ModuleName,<br>
const CodeGenOptions &CGO, llvm::LLVMContext& C,<br>
CoverageSourceInfo *CoverageInfo = nullptr)<br>
- : Diags(diags), CodeGenOpts(CGO), HandlingTopLevelDecls(0),<br>
+ : Diags(diags), Ctx(nullptr), CodeGenOpts(CGO), HandlingTopLevelDecls(0),<br>
CoverageInfo(CoverageInfo),<br>
M(new llvm::Module(ModuleName, C)) {}<br>
<br>
<br>
<br>
_______________________________________________<br>
cfe-commits mailing list<br>
<a href="mailto:cfe-commits@cs.uiuc.edu" target="_blank">cfe-commits@cs.uiuc.edu</a><br>
<a href="http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits</a><br>
</blockquote></div>
</div></div></blockquote></div><br></div>
</div></div></blockquote></div></div></div><br></div></blockquote></div></div></div>
</div></div></blockquote></div></div></div>