<div>I've added a test that shows a condition when my patch helps.</div><div>It fails under valgrind with rev 160170 reverted:</div><blockquote style="margin:0 0 0 40px;border:none;padding:0px"><div><font face="courier new, monospace">$ svn merge -r 160170:160169 <a href="https://llvm.org/svn/llvm-project/llvm/trunk" target="_blank">https://llvm.org/svn/llvm-project/llvm/trunk</a></font></div>
<div><font face="courier new, monospace">$ cd your-build-dir</font></div><div><font face="courier new, monospace">$ make check-all</font></div><div><font face="courier new, monospace">$ valgrind unittests/Support/SupportTests</font></div>
<div><font face="courier new, monospace">...</font></div><div><font face="courier new, monospace">[ RUN ] CommandLineTest.ParseEnvironmentToLocalVar</font></div><div><font face="courier new, monospace">==12673== Conditional jump or move depends on uninitialised value(s)</font></div>
<div><font face="courier new, monospace">==12673== at 0x659A17: GetOptionInfo(llvm::SmallVectorImpl<llvm::cl::Option*>&, llvm::SmallVectorImpl<llvm::cl::Option*>&, llvm::StringMap<llvm::cl::Option*, llvm::MallocAllocator>&) (in /usr/local/google/users/alexfh/cmake-clang-build/unittests/Support/SupportTests)</font></div>
<div><font face="courier new, monospace">==12673== by 0x657E27: llvm::cl::ParseCommandLineOptions(int, char const* const*, char const*, bool) (in /usr/local/google/users/alexfh/cmake-clang-build/unittests/Support/SupportTests)</font></div>
<div><font face="courier new, monospace">==12673== by 0x657B70: llvm::cl::ParseEnvironmentOptions(char const*, char const*, char const*, bool) (in /usr/local/google/users/alexfh/cmake-clang-build/unittests/Support/SupportTests)</font></div>
<div><font face="courier new, monospace">==12673== by 0x495B2F: (anonymous namespace)::CommandLineTest_ParseEnvironmentToLocalVar_Test::TestBody() (in /usr/local/google/users/alexfh/cmake-clang-build/unittests/Support/SupportTests)</font></div>
<div><font face="courier new, monospace">...</font></div>
</blockquote><div>With rev 160170 in place this valgrind message isn't triggered:</div><blockquote style="margin:0 0 0 40px;border:none;padding:0px"><div><div><font face="courier new, monospace">$ svn revert include/llvm/Support/CommandLine.h</font></div>
</div><div><div><font face="courier new, monospace">$ cd your-build-dir</font></div></div><div><div><font face="courier new, monospace">$ make check-all</font></div></div><div><div><font face="courier new, monospace">$ valgrind unittests/Support/SupportTests</font></div>
</div><div><font face="courier new, monospace">...</font></div><div><font face="courier new, monospace"><div>[ RUN ] CommandLineTest.ParseEnvironmentToLocalVar<br></div>
<div>[ OK ] CommandLineTest.ParseEnvironmentToLocalVar (10 ms)</div><div>...</div><div><br></div></font></div></blockquote><font face="arial, helvetica, sans-serif"><div><font face="arial, helvetica, sans-serif">There are also a few tests in tools/clang/tests/Tooling that fail on their own (i.e. </font>without valgrind)<font face="arial, helvetica, sans-serif"> </font>without rev 160170, but these failures highly depend on external conditions and surrounding code layout (however, valgrind consistently finds issues even if the code doesn't break).</div>
</font><font face="arial, helvetica, sans-serif"><div><font face="arial, helvetica, sans-serif"><br></font></div>Is it enough to prove validity of my previous patch? </font>
<div class="gmail_extra"><br><br><div class="gmail_quote">On Mon, Jul 16, 2012 at 3:28 PM, Chandler Carruth <span dir="ltr"><<a href="mailto:chandlerc@google.com" target="_blank">chandlerc@google.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="im">On Mon, Jul 16, 2012 at 6:24 AM, Alexander Kornienko <span dir="ltr"><<a href="mailto:alexfh@google.com" target="_blank">alexfh@google.com</a>></span> wrote:<br>
</div><div class="gmail_extra"><div class="gmail_quote"><div class="im">
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="gmail_extra"><div class="gmail_quote"><div>On Mon, Jul 16, 2012 at 3:10 PM, Chandler Carruth <span dir="ltr"><<a href="mailto:chandlerc@google.com" target="_blank">chandlerc@google.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div>On Mon, Jul 16, 2012 at 6:02 AM, Alexander Kornienko <span dir="ltr"><<a href="mailto:alexfh@google.com" target="_blank">alexfh@google.com</a>></span> wrote:<br></div><div class="gmail_extra"><div class="gmail_quote">
<div>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="gmail_extra"><br><div class="gmail_quote"><div>On Fri, Jul 13, 2012 at 3:06 PM, Duncan Sands <span dir="ltr"><<a href="mailto:baldrick@free.fr" target="_blank">baldrick@free.fr</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
Hi Alexander,<br>
<div>> Initializers for some fields were missing in Option::Option<br>
<br>
</div>did it matter? If so, please add a testcase. If not, why this change?<br></blockquote></div><div>This did matter. It caused random crashes in case cl::opt was a member of non-POD class.</div></div></div></blockquote>
<div><br></div></div><div>Hold on, a *member*? 'cl::opt' objects are supposed to be globals, not members.</div></div></div></blockquote></div><div>We need it as a member to make a class which handles a common subset of command-line options for several clang tools. In a close future we'll need a control on which options should be enabled and which not. So we can't do it with global variables. Why can't we use it as a member?</div>
</div></div></blockquote><div><br></div></div><div>Errr... Ok, this is something we should discuss in more depth.</div><div><br></div><div>Essentially, it's not that you necessarily can't, it's that the library was never designed with that use case in mind. It's completely outside the contract of the library, and so random things are likely to continue breaking. If this the correct design, we need to at least extend the library with unittests to check its behavior in that context. These unittests would then be what you would run under Valgrind in order to detect the failure.<br>
</div></div></div>
</blockquote></div><br><br clear="all"><div><br></div>-- <br><div><div><font color="#666666"><span style="border-top-width:2px;border-right-width:0px;border-bottom-width:0px;border-left-width:0px;border-top-style:solid;border-right-style:solid;border-bottom-style:solid;border-left-style:solid;border-top-color:rgb(213,15,37);border-right-color:rgb(213,15,37);border-bottom-color:rgb(213,15,37);border-left-color:rgb(213,15,37);padding-top:2px;margin-top:2px">Alexander Kornienko |</span><span style="border-top-width:2px;border-right-width:0px;border-bottom-width:0px;border-left-width:0px;border-top-style:solid;border-right-style:solid;border-bottom-style:solid;border-left-style:solid;border-top-color:rgb(51,105,232);border-right-color:rgb(51,105,232);border-bottom-color:rgb(51,105,232);border-left-color:rgb(51,105,232);padding-top:2px;margin-top:2px"> Software Engineer |</span></font><span style="border-top-width:2px;border-right-width:0px;border-bottom-width:0px;border-left-width:0px;border-top-style:solid;border-right-style:solid;border-bottom-style:solid;border-left-style:solid;border-top-color:rgb(0,153,57);border-right-color:rgb(0,153,57);border-bottom-color:rgb(0,153,57);border-left-color:rgb(0,153,57);padding-top:2px;margin-top:2px"><font color="#666666"> </font><a href="mailto:alexfh@google.com" style="color:rgb(17,85,204)" target="_blank">alexfh@google.com</a> |</span><span style="border-top-width:2px;border-right-width:0px;border-bottom-width:0px;border-left-width:0px;border-top-style:solid;border-right-style:solid;border-bottom-style:solid;border-left-style:solid;border-top-color:rgb(238,178,17);border-right-color:rgb(238,178,17);border-bottom-color:rgb(238,178,17);border-left-color:rgb(238,178,17);padding-top:2px;margin-top:2px"> <a value="+35315435283" style="color:rgb(17,85,204)">+49 151 221 77 957</a></span></div>
</div><div><font color="#666666"><span style="background-color:rgb(255,255,255);font-family:Arial,Verdana,sans-serif">Google Germany GmbH | </span><span style="background-color:rgb(255,255,255);font-family:Arial,Verdana,sans-serif">Dienerstr. 12 | </span><span style="background-color:rgb(255,255,255);font-family:Arial,Verdana,sans-serif">80331 München</span></font></div>
<br>
</div>