<div dir="ltr">Thank you!<br></div><div class="gmail_extra"><br><br><div class="gmail_quote">On Tue, Apr 30, 2013 at 9:43 AM, Reid Kleckner <span dir="ltr"><<a href="mailto:rnk@google.com" target="_blank">rnk@google.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">That bot went green with r 180770:<br>
<a href="http://lab.llvm.org:8011/builders/clang-X86_64-freebsd/builds/8253" target="_blank">http://lab.llvm.org:8011/builders/clang-X86_64-freebsd/builds/8253</a><br>
<div class="HOEnZb"><div class="h5"><br>
On Mon, Apr 29, 2013 at 2:24 PM, Galina Kistanova <<a href="mailto:gkistanova@gmail.com">gkistanova@gmail.com</a>> wrote:<br>
> Hello Reid,<br>
><br>
> Added ProgramTest always fails on the builder<br>
> <a href="http://lab.llvm.org:8011/builders/clang-X86_64-freebsd" target="_blank">http://lab.llvm.org:8011/builders/clang-X86_64-freebsd</a><br>
><br>
> Please have a look at this.<br>
><br>
> Thanks<br>
><br>
> Galina<br>
><br>
><br>
> On Mon, Apr 22, 2013 at 12:03 PM, Reid Kleckner <<a href="mailto:reid@kleckner.net">reid@kleckner.net</a>> wrote:<br>
>><br>
>> Author: rnk<br>
>> Date: Mon Apr 22 14:03:55 2013<br>
>> New Revision: 180035<br>
>><br>
>> URL: <a href="http://llvm.org/viewvc/llvm-project?rev=180035&view=rev" target="_blank">http://llvm.org/viewvc/llvm-project?rev=180035&view=rev</a><br>
>> Log:<br>
>> [Support] Fix argv string escape bug on Windows<br>
>><br>
>> Summary:<br>
>> This is <a href="http://llvm.org/PR15802" target="_blank">http://llvm.org/PR15802</a>.  Backslashes preceding double quotes in<br>
>> arguments must be escaped.  The interesting bit is that all other<br>
>> backslashes should *not* be escaped, because the un-escaping logic is<br>
>> only triggered by the presence of a double quote character.<br>
>><br>
>> Reviewers: Bigcheese<br>
>><br>
>> CC: llvm-commits<br>
>><br>
>> Differential Revision: <a href="http://llvm-reviews.chandlerc.com/D705" target="_blank">http://llvm-reviews.chandlerc.com/D705</a><br>
>><br>
>> Added:<br>
>>     llvm/trunk/unittests/Support/ProgramTest.cpp   (with props)<br>
>> Modified:<br>
>>     llvm/trunk/lib/Support/Windows/Program.inc<br>
>>     llvm/trunk/unittests/Support/CMakeLists.txt<br>
>><br>
>> Modified: llvm/trunk/lib/Support/Windows/Program.inc<br>
>> URL:<br>
>> <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Support/Windows/Program.inc?rev=180035&r1=180034&r2=180035&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Support/Windows/Program.inc?rev=180035&r1=180034&r2=180035&view=diff</a><br>

>><br>
>> ==============================================================================<br>
>> --- llvm/trunk/lib/Support/Windows/Program.inc (original)<br>
>> +++ llvm/trunk/lib/Support/Windows/Program.inc Mon Apr 22 14:03:55 2013<br>
>> @@ -126,15 +126,46 @@ static bool ArgNeedsQuotes(const char *S<br>
>>    return Str[0] == '\0' || strpbrk(Str, "\t \"&\'()*<>\\`^|") != 0;<br>
>>  }<br>
>><br>
>> +/// CountPrecedingBackslashes - Returns the number of backslashes<br>
>> preceding Cur<br>
>> +/// in the C string Start.<br>
>> +static unsigned int CountPrecedingBackslashes(const char *Start,<br>
>> +                                              const char *Cur) {<br>
>> +  unsigned int Count = 0;<br>
>> +  --Cur;<br>
>> +  while (Cur >= Start && *Cur == '\\') {<br>
>> +    ++Count;<br>
>> +    --Cur;<br>
>> +  }<br>
>> +  return Count;<br>
>> +}<br>
>> +<br>
>> +/// EscapePrecedingEscapes - Append a backslash to Dst for every<br>
>> backslash<br>
>> +/// preceding Cur in the Start string.  Assumes Dst has enough space.<br>
>> +static char *EscapePrecedingEscapes(char *Dst, const char *Start,<br>
>> +                                    const char *Cur) {<br>
>> +  unsigned PrecedingEscapes = CountPrecedingBackslashes(Start, Cur);<br>
>> +  while (PrecedingEscapes > 0) {<br>
>> +    *Dst++ = '\\';<br>
>> +    --PrecedingEscapes;<br>
>> +  }<br>
>> +  return Dst;<br>
>> +}<br>
>><br>
>>  /// ArgLenWithQuotes - Check whether argument needs to be quoted when<br>
>> calling<br>
>>  /// CreateProcess and returns length of quoted arg with escaped quotes<br>
>>  static unsigned int ArgLenWithQuotes(const char *Str) {<br>
>> +  const char *Start = Str;<br>
>>    unsigned int len = ArgNeedsQuotes(Str) ? 2 : 0;<br>
>><br>
>>    while (*Str != '\0') {<br>
>> -    if (*Str == '\"')<br>
>> -      ++len;<br>
>> +    if (*Str == '\"') {<br>
>> +      // We need to add a backslash, but ensure that it isn't escaped.<br>
>> +      unsigned PrecedingEscapes = CountPrecedingBackslashes(Start, Str);<br>
>> +      len += PrecedingEscapes + 1;<br>
>> +    }<br>
>> +    // Note that we *don't* need to escape runs of backslashes that don't<br>
>> +    // precede a double quote!  See MSDN:<br>
>> +    // <a href="http://msdn.microsoft.com/en-us/library/17w5ykft%28v=vs.85%29.aspx" target="_blank">http://msdn.microsoft.com/en-us/library/17w5ykft%28v=vs.85%29.aspx</a><br>
>><br>
>>      ++len;<br>
>>      ++Str;<br>
>> @@ -180,20 +211,27 @@ Program::Execute(const Path& path,<br>
>><br>
>>    for (unsigned i = 0; args[i]; i++) {<br>
>>      const char *arg = args[i];<br>
>> +    const char *start = arg;<br>
>><br>
>>      bool needsQuoting = ArgNeedsQuotes(arg);<br>
>>      if (needsQuoting)<br>
>>        *p++ = '"';<br>
>><br>
>>      while (*arg != '\0') {<br>
>> -      if (*arg == '\"')<br>
>> +      if (*arg == '\"') {<br>
>> +        // Escape all preceding escapes (if any), and then escape the<br>
>> quote.<br>
>> +        p = EscapePrecedingEscapes(p, start, arg);<br>
>>          *p++ = '\\';<br>
>> +      }<br>
>><br>
>>        *p++ = *arg++;<br>
>>      }<br>
>><br>
>> -    if (needsQuoting)<br>
>> +    if (needsQuoting) {<br>
>> +      // Make sure our quote doesn't get escaped by a trailing backslash.<br>
>> +      p = EscapePrecedingEscapes(p, start, arg);<br>
>>        *p++ = '"';<br>
>> +    }<br>
>>      *p++ = ' ';<br>
>>    }<br>
>><br>
>><br>
>> Modified: llvm/trunk/unittests/Support/CMakeLists.txt<br>
>> URL:<br>
>> <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/unittests/Support/CMakeLists.txt?rev=180035&r1=180034&r2=180035&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/unittests/Support/CMakeLists.txt?rev=180035&r1=180034&r2=180035&view=diff</a><br>

>><br>
>> ==============================================================================<br>
>> --- llvm/trunk/unittests/Support/CMakeLists.txt (original)<br>
>> +++ llvm/trunk/unittests/Support/CMakeLists.txt Mon Apr 22 14:03:55 2013<br>
>> @@ -23,6 +23,7 @@ add_llvm_unittest(SupportTests<br>
>>    MemoryTest.cpp<br>
>>    Path.cpp<br>
>>    ProcessTest.cpp<br>
>> +  ProgramTest.cpp<br>
>>    RegexTest.cpp<br>
>>    SwapByteOrderTest.cpp<br>
>>    TimeValue.cpp<br>
>><br>
>> Added: llvm/trunk/unittests/Support/ProgramTest.cpp<br>
>> URL:<br>
>> <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/unittests/Support/ProgramTest.cpp?rev=180035&view=auto" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/unittests/Support/ProgramTest.cpp?rev=180035&view=auto</a><br>

>><br>
>> ==============================================================================<br>
>> --- llvm/trunk/unittests/Support/ProgramTest.cpp (added)<br>
>> +++ llvm/trunk/unittests/Support/ProgramTest.cpp Mon Apr 22 14:03:55 2013<br>
>> @@ -0,0 +1,63 @@<br>
>> +//===- unittest/Support/ProgramTest.cpp<br>
>> -----------------------------------===//<br>
>> +//<br>
>> +//                     The LLVM Compiler Infrastructure<br>
>> +//<br>
>> +// This file is distributed under the University of Illinois Open Source<br>
>> +// License. See LICENSE.TXT for details.<br>
>> +//<br>
>><br>
>> +//===----------------------------------------------------------------------===//<br>
>> +<br>
>> +#include "llvm/Support/CommandLine.h"<br>
>> +#include "llvm/Support/Path.h"<br>
>> +#include "llvm/Support/Program.h"<br>
>> +#include "gtest/gtest.h"<br>
>> +<br>
>> +#include <stdlib.h><br>
>> +<br>
>> +namespace {<br>
>> +<br>
>> +using namespace llvm;<br>
>> +using namespace sys;<br>
>> +<br>
>> +static cl::opt<std::string><br>
>> +ProgramTestStringArg1("program-test-string-arg1");<br>
>> +static cl::opt<std::string><br>
>> +ProgramTestStringArg2("program-test-string-arg2");<br>
>> +<br>
>> +TEST(ProgramTest, CreateProcessTrailingSlash) {<br>
>> +  if (getenv("LLVM_PROGRAM_TEST_CHILD")) {<br>
>> +    if (ProgramTestStringArg1 == "has\\\\ trailing\\" &&<br>
>> +        ProgramTestStringArg2 == "has\\\\ trailing\\") {<br>
>> +      exit(0);  // Success!  The arguments were passed and parsed.<br>
>> +    }<br>
>> +    exit(1);<br>
>> +  }<br>
>> +<br>
>> +  // FIXME: Hardcoding argv0 here since I don't know a good<br>
>> cross-platform way<br>
>> +  // to get it.  Maybe ParseCommandLineOptions() should save it?<br>
>> +  Path my_exe = Path::GetMainExecutable("SupportTests",<br>
>> &ProgramTestStringArg1);<br>
>> +  const char *argv[] = {<br>
>> +    my_exe.c_str(),<br>
>> +    "--gtest_filter=ProgramTest.CreateProcessTrailingSlashChild",<br>
>> +    "-program-test-string-arg1", "has\\\\ trailing\\",<br>
>> +    "-program-test-string-arg2", "has\\\\ trailing\\",<br>
>> +    0<br>
>> +  };<br>
>> +  const char *envp[] = { "LLVM_PROGRAM_TEST_CHILD=1", 0 };<br>
>> +  std::string error;<br>
>> +  bool ExecutionFailed;<br>
>> +  // Redirect stdout and stdin to NUL, but let stderr through.<br>
>> +#ifdef LLVM_ON_WIN32<br>
>> +  Path nul("NUL");<br>
>> +#else<br>
>> +  Path nul("/dev/null");<br>
>> +#endif<br>
>> +  const Path *redirects[] = { &nul, &nul, 0 };<br>
>> +  int rc = Program::ExecuteAndWait(my_exe, argv, envp, redirects,<br>
>> +                                   /*secondsToWait=*/10,<br>
>> /*memoryLimit=*/0,<br>
>> +                                   &error, &ExecutionFailed);<br>
>> +  EXPECT_FALSE(ExecutionFailed) << error;<br>
>> +  EXPECT_EQ(0, rc);<br>
>> +}<br>
>> +<br>
>> +} // end anonymous namespace<br>
>><br>
>> Propchange: llvm/trunk/unittests/Support/ProgramTest.cpp<br>
>><br>
>> ------------------------------------------------------------------------------<br>
>>     svn:executable = *<br>
>><br>
>><br>
>> _______________________________________________<br>
>> llvm-commits mailing list<br>
>> <a href="mailto:llvm-commits@cs.uiuc.edu">llvm-commits@cs.uiuc.edu</a><br>
>> <a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits</a><br>
><br>
><br>
><br>
> _______________________________________________<br>
> llvm-commits mailing list<br>
> <a href="mailto:llvm-commits@cs.uiuc.edu">llvm-commits@cs.uiuc.edu</a><br>
> <a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits</a><br>
><br>
</div></div></blockquote></div><br></div>