[llvm] r180035 - [Support] Fix argv string escape bug on Windows
Reid Kleckner
reid at kleckner.net
Mon Apr 22 12:03:55 PDT 2013
Author: rnk
Date: Mon Apr 22 14:03:55 2013
New Revision: 180035
URL: http://llvm.org/viewvc/llvm-project?rev=180035&view=rev
Log:
[Support] Fix argv string escape bug on Windows
Summary:
This is http://llvm.org/PR15802. Backslashes preceding double quotes in
arguments must be escaped. The interesting bit is that all other
backslashes should *not* be escaped, because the un-escaping logic is
only triggered by the presence of a double quote character.
Reviewers: Bigcheese
CC: llvm-commits
Differential Revision: http://llvm-reviews.chandlerc.com/D705
Added:
llvm/trunk/unittests/Support/ProgramTest.cpp (with props)
Modified:
llvm/trunk/lib/Support/Windows/Program.inc
llvm/trunk/unittests/Support/CMakeLists.txt
Modified: llvm/trunk/lib/Support/Windows/Program.inc
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Support/Windows/Program.inc?rev=180035&r1=180034&r2=180035&view=diff
==============================================================================
--- llvm/trunk/lib/Support/Windows/Program.inc (original)
+++ llvm/trunk/lib/Support/Windows/Program.inc Mon Apr 22 14:03:55 2013
@@ -126,15 +126,46 @@ static bool ArgNeedsQuotes(const char *S
return Str[0] == '\0' || strpbrk(Str, "\t \"&\'()*<>\\`^|") != 0;
}
+/// CountPrecedingBackslashes - Returns the number of backslashes preceding Cur
+/// in the C string Start.
+static unsigned int CountPrecedingBackslashes(const char *Start,
+ const char *Cur) {
+ unsigned int Count = 0;
+ --Cur;
+ while (Cur >= Start && *Cur == '\\') {
+ ++Count;
+ --Cur;
+ }
+ return Count;
+}
+
+/// EscapePrecedingEscapes - Append a backslash to Dst for every backslash
+/// preceding Cur in the Start string. Assumes Dst has enough space.
+static char *EscapePrecedingEscapes(char *Dst, const char *Start,
+ const char *Cur) {
+ unsigned PrecedingEscapes = CountPrecedingBackslashes(Start, Cur);
+ while (PrecedingEscapes > 0) {
+ *Dst++ = '\\';
+ --PrecedingEscapes;
+ }
+ return Dst;
+}
/// ArgLenWithQuotes - Check whether argument needs to be quoted when calling
/// CreateProcess and returns length of quoted arg with escaped quotes
static unsigned int ArgLenWithQuotes(const char *Str) {
+ const char *Start = Str;
unsigned int len = ArgNeedsQuotes(Str) ? 2 : 0;
while (*Str != '\0') {
- if (*Str == '\"')
- ++len;
+ if (*Str == '\"') {
+ // We need to add a backslash, but ensure that it isn't escaped.
+ unsigned PrecedingEscapes = CountPrecedingBackslashes(Start, Str);
+ len += PrecedingEscapes + 1;
+ }
+ // Note that we *don't* need to escape runs of backslashes that don't
+ // precede a double quote! See MSDN:
+ // http://msdn.microsoft.com/en-us/library/17w5ykft%28v=vs.85%29.aspx
++len;
++Str;
@@ -180,20 +211,27 @@ Program::Execute(const Path& path,
for (unsigned i = 0; args[i]; i++) {
const char *arg = args[i];
+ const char *start = arg;
bool needsQuoting = ArgNeedsQuotes(arg);
if (needsQuoting)
*p++ = '"';
while (*arg != '\0') {
- if (*arg == '\"')
+ if (*arg == '\"') {
+ // Escape all preceding escapes (if any), and then escape the quote.
+ p = EscapePrecedingEscapes(p, start, arg);
*p++ = '\\';
+ }
*p++ = *arg++;
}
- if (needsQuoting)
+ if (needsQuoting) {
+ // Make sure our quote doesn't get escaped by a trailing backslash.
+ p = EscapePrecedingEscapes(p, start, arg);
*p++ = '"';
+ }
*p++ = ' ';
}
Modified: llvm/trunk/unittests/Support/CMakeLists.txt
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/unittests/Support/CMakeLists.txt?rev=180035&r1=180034&r2=180035&view=diff
==============================================================================
--- llvm/trunk/unittests/Support/CMakeLists.txt (original)
+++ llvm/trunk/unittests/Support/CMakeLists.txt Mon Apr 22 14:03:55 2013
@@ -23,6 +23,7 @@ add_llvm_unittest(SupportTests
MemoryTest.cpp
Path.cpp
ProcessTest.cpp
+ ProgramTest.cpp
RegexTest.cpp
SwapByteOrderTest.cpp
TimeValue.cpp
Added: llvm/trunk/unittests/Support/ProgramTest.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/unittests/Support/ProgramTest.cpp?rev=180035&view=auto
==============================================================================
--- llvm/trunk/unittests/Support/ProgramTest.cpp (added)
+++ llvm/trunk/unittests/Support/ProgramTest.cpp Mon Apr 22 14:03:55 2013
@@ -0,0 +1,63 @@
+//===- unittest/Support/ProgramTest.cpp -----------------------------------===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+#include "llvm/Support/CommandLine.h"
+#include "llvm/Support/Path.h"
+#include "llvm/Support/Program.h"
+#include "gtest/gtest.h"
+
+#include <stdlib.h>
+
+namespace {
+
+using namespace llvm;
+using namespace sys;
+
+static cl::opt<std::string>
+ProgramTestStringArg1("program-test-string-arg1");
+static cl::opt<std::string>
+ProgramTestStringArg2("program-test-string-arg2");
+
+TEST(ProgramTest, CreateProcessTrailingSlash) {
+ if (getenv("LLVM_PROGRAM_TEST_CHILD")) {
+ if (ProgramTestStringArg1 == "has\\\\ trailing\\" &&
+ ProgramTestStringArg2 == "has\\\\ trailing\\") {
+ exit(0); // Success! The arguments were passed and parsed.
+ }
+ exit(1);
+ }
+
+ // FIXME: Hardcoding argv0 here since I don't know a good cross-platform way
+ // to get it. Maybe ParseCommandLineOptions() should save it?
+ Path my_exe = Path::GetMainExecutable("SupportTests", &ProgramTestStringArg1);
+ const char *argv[] = {
+ my_exe.c_str(),
+ "--gtest_filter=ProgramTest.CreateProcessTrailingSlashChild",
+ "-program-test-string-arg1", "has\\\\ trailing\\",
+ "-program-test-string-arg2", "has\\\\ trailing\\",
+ 0
+ };
+ const char *envp[] = { "LLVM_PROGRAM_TEST_CHILD=1", 0 };
+ std::string error;
+ bool ExecutionFailed;
+ // Redirect stdout and stdin to NUL, but let stderr through.
+#ifdef LLVM_ON_WIN32
+ Path nul("NUL");
+#else
+ Path nul("/dev/null");
+#endif
+ const Path *redirects[] = { &nul, &nul, 0 };
+ int rc = Program::ExecuteAndWait(my_exe, argv, envp, redirects,
+ /*secondsToWait=*/10, /*memoryLimit=*/0,
+ &error, &ExecutionFailed);
+ EXPECT_FALSE(ExecutionFailed) << error;
+ EXPECT_EQ(0, rc);
+}
+
+} // end anonymous namespace
Propchange: llvm/trunk/unittests/Support/ProgramTest.cpp
------------------------------------------------------------------------------
svn:executable = *
More information about the llvm-commits
mailing list