<div dir="ltr">Hello Reid,<br><br>Added ProgramTest always fails on the builder<br><a href="http://lab.llvm.org:8011/builders/clang-X86_64-freebsd">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></div><div class="gmail_extra"><br><br><div class="gmail_quote">On Mon, Apr 22, 2013 at 12:03 PM, Reid Kleckner <span dir="ltr"><<a href="mailto:reid@kleckner.net" target="_blank">reid@kleckner.net</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">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: <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>
--- 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 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 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 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 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: <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>
--- 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: <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>
--- 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>
+// 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>
+#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 cross-platform way<br>
+ // to get it. Maybe ParseCommandLineOptions() should save it?<br>
+ Path my_exe = Path::GetMainExecutable("SupportTests", &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, /*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>
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>
</blockquote></div><br></div>