[llvm] r180035 - [Support] Fix argv string escape bug on Windows
Galina Kistanova
gkistanova at gmail.com
Mon Apr 29 14:24:43 PDT 2013
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
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20130429/eb7cfac9/attachment.html>
More information about the llvm-commits
mailing list