[llvm] r180035 - [Support] Fix argv string escape bug on Windows
Galina Kistanova
gkistanova at gmail.com
Tue Apr 30 10:33:58 PDT 2013
Thank you!
On Tue, Apr 30, 2013 at 9:43 AM, Reid Kleckner <rnk at google.com> wrote:
> 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
> >
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20130430/4492010a/attachment.html>
More information about the llvm-commits
mailing list