[llvm] r180035 - [Support] Fix argv string escape bug on Windows
Reid Kleckner
rnk at google.com
Tue Apr 30 09:43:43 PDT 2013
That bot went green with r 180770:
http://lab.llvm.org:8011/builders/clang-X86_64-freebsd/builds/8253
On Mon, Apr 29, 2013 at 2:24 PM, Galina Kistanova <gkistanova at gmail.com> wrote:
> Hello Reid,
>
> Added ProgramTest always fails on the builder
> http://lab.llvm.org:8011/builders/clang-X86_64-freebsd
>
> Please have a look at this.
>
> Thanks
>
> Galina
>
>
> On Mon, Apr 22, 2013 at 12:03 PM, Reid Kleckner <reid at kleckner.net> wrote:
>>
>> 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 = *
>>
>>
>> _______________________________________________
>> llvm-commits mailing list
>> llvm-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>
More information about the llvm-commits
mailing list