[llvm] r334356 - Revert "Resubmit "[Support] Expose flattenWindowsCommandLine.""
Zachary Turner via llvm-commits
llvm-commits at lists.llvm.org
Sat Jun 9 20:16:25 PDT 2018
Author: zturner
Date: Sat Jun 9 20:16:25 2018
New Revision: 334356
URL: http://llvm.org/viewvc/llvm-project?rev=334356&view=rev
Log:
Revert "Resubmit "[Support] Expose flattenWindowsCommandLine.""
This reverts commit 65243b6d19143cb7a03f68df0169dcb63e8b4632.
Seems like it's not a flake. It might have something to do with
the '*' character being in a command line.
Modified:
llvm/trunk/include/llvm/Support/Program.h
llvm/trunk/lib/Support/Program.cpp
llvm/trunk/lib/Support/Unix/Program.inc
llvm/trunk/lib/Support/Windows/Path.inc
llvm/trunk/lib/Support/Windows/Program.inc
Modified: llvm/trunk/include/llvm/Support/Program.h
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/Support/Program.h?rev=334356&r1=334355&r2=334356&view=diff
==============================================================================
--- llvm/trunk/include/llvm/Support/Program.h (original)
+++ llvm/trunk/include/llvm/Support/Program.h Sat Jun 9 20:16:25 2018
@@ -133,11 +133,6 @@ namespace sys {
/// Return true if the given arguments fit within system-specific
/// argument length limits.
bool commandLineFitsWithinSystemLimits(StringRef Program,
- ArrayRef<StringRef> Args);
-
- /// Return true if the given arguments fit within system-specific
- /// argument length limits.
- bool commandLineFitsWithinSystemLimits(StringRef Program,
ArrayRef<const char *> Args);
/// File encoding options when writing contents that a non-UTF8 tool will
@@ -194,14 +189,6 @@ namespace sys {
///< string is non-empty upon return an error occurred while invoking the
///< program.
);
-
-#if defined(_WIN32)
- /// Given a list of command line arguments, quote and escape them as necessary
- /// to build a single flat command line appropriate for calling CreateProcess
- /// on
- /// Windows.
- std::string flattenWindowsCommandLine(ArrayRef<StringRef> Args);
-#endif
}
}
Modified: llvm/trunk/lib/Support/Program.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Support/Program.cpp?rev=334356&r1=334355&r2=334356&view=diff
==============================================================================
--- llvm/trunk/lib/Support/Program.cpp (original)
+++ llvm/trunk/lib/Support/Program.cpp Sat Jun 9 20:16:25 2018
@@ -63,15 +63,6 @@ ProcessInfo sys::ExecuteNoWait(StringRef
return PI;
}
-bool sys::commandLineFitsWithinSystemLimits(StringRef Program,
- ArrayRef<const char *> Args) {
- SmallVector<StringRef, 8> StringRefArgs;
- StringRefArgs.reserve(Args.size());
- for (const char *A : Args)
- StringRefArgs.emplace_back(A);
- return commandLineFitsWithinSystemLimits(Program, StringRefArgs);
-}
-
// Include the platform-specific parts of this class.
#ifdef LLVM_ON_UNIX
#include "Unix/Program.inc"
Modified: llvm/trunk/lib/Support/Unix/Program.inc
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Support/Unix/Program.inc?rev=334356&r1=334355&r2=334356&view=diff
==============================================================================
--- llvm/trunk/lib/Support/Unix/Program.inc (original)
+++ llvm/trunk/lib/Support/Unix/Program.inc Sat Jun 9 20:16:25 2018
@@ -434,7 +434,7 @@ llvm::sys::writeFileWithEncoding(StringR
}
bool llvm::sys::commandLineFitsWithinSystemLimits(StringRef Program,
- ArrayRef<StringRef> Args) {
+ ArrayRef<const char *> Args) {
static long ArgMax = sysconf(_SC_ARG_MAX);
// POSIX requires that _POSIX_ARG_MAX is 4096, which is the lowest possible
// value for ARG_MAX on a POSIX compliant system.
@@ -456,16 +456,18 @@ bool llvm::sys::commandLineFitsWithinSys
long HalfArgMax = EffectiveArgMax / 2;
size_t ArgLength = Program.size() + 1;
- for (StringRef Arg : Args) {
+ for (const char* Arg : Args) {
+ size_t length = strlen(Arg);
+
// Ensure that we do not exceed the MAX_ARG_STRLEN constant on Linux, which
// does not have a constant unlike what the man pages would have you
// believe. Since this limit is pretty high, perform the check
// unconditionally rather than trying to be aggressive and limiting it to
// Linux only.
- if (Arg.size() >= (32 * 4096))
+ if (length >= (32 * 4096))
return false;
- ArgLength += Arg.size() + 1;
+ ArgLength += length + 1;
if (ArgLength > size_t(HalfArgMax)) {
return false;
}
Modified: llvm/trunk/lib/Support/Windows/Path.inc
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Support/Windows/Path.inc?rev=334356&r1=334355&r2=334356&view=diff
==============================================================================
--- llvm/trunk/lib/Support/Windows/Path.inc (original)
+++ llvm/trunk/lib/Support/Windows/Path.inc Sat Jun 9 20:16:25 2018
@@ -1197,7 +1197,7 @@ Expected<file_t> openNativeFileForRead(c
if (Result && RealPath)
realPathFromHandle(*Result, *RealPath);
- return Result;
+ return std::move(Result);
}
void closeFile(file_t &F) {
Modified: llvm/trunk/lib/Support/Windows/Program.inc
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Support/Windows/Program.inc?rev=334356&r1=334355&r2=334356&view=diff
==============================================================================
--- llvm/trunk/lib/Support/Windows/Program.inc (original)
+++ llvm/trunk/lib/Support/Windows/Program.inc Sat Jun 9 20:16:25 2018
@@ -23,7 +23,6 @@
#include <fcntl.h>
#include <io.h>
#include <malloc.h>
-#include <numeric>
//===----------------------------------------------------------------------===//
//=== WARNING: Implementation here must contain only Win32 specific code
@@ -32,7 +31,7 @@
namespace llvm {
-ProcessInfo::ProcessInfo() : Pid(0), Process(0), ReturnCode(0) {}
+ProcessInfo::ProcessInfo() : Process(0), Pid(0), ReturnCode(0) {}
ErrorOr<std::string> sys::findProgramByName(StringRef Name,
ArrayRef<StringRef> Paths) {
@@ -147,13 +146,108 @@ static HANDLE RedirectIO(Optional<String
return h;
}
+/// ArgNeedsQuotes - Check whether argument needs to be quoted when calling
+/// CreateProcess.
+static bool ArgNeedsQuotes(const char *Str) {
+ 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;
+ bool Quoted = ArgNeedsQuotes(Str);
+ unsigned int len = Quoted ? 2 : 0;
+
+ while (*Str != '\0') {
+ 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;
+ }
+
+ if (Quoted) {
+ // Make sure the closing quote doesn't get escaped by a trailing backslash.
+ unsigned PrecedingEscapes = CountPrecedingBackslashes(Start, Str);
+ len += PrecedingEscapes + 1;
+ }
+
+ return len;
+}
+
}
-static SmallVector<StringRef, 8> buildArgVector(const char **Args) {
- SmallVector<StringRef, 8> Result;
- for (unsigned I = 0; Args[I]; ++I)
- Result.push_back(StringRef(Args[I]));
- return Result;
+static std::unique_ptr<char[]> flattenArgs(const char **Args) {
+ // First, determine the length of the command line.
+ unsigned len = 0;
+ for (unsigned i = 0; Args[i]; i++) {
+ len += ArgLenWithQuotes(Args[i]) + 1;
+ }
+
+ // Now build the command line.
+ std::unique_ptr<char[]> command(new char[len+1]);
+ char *p = command.get();
+
+ 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 == '\"') {
+ // Escape all preceding escapes (if any), and then escape the quote.
+ p = EscapePrecedingEscapes(p, start, arg);
+ *p++ = '\\';
+ }
+
+ *p++ = *arg++;
+ }
+
+ if (needsQuoting) {
+ // Make sure our quote doesn't get escaped by a trailing backslash.
+ p = EscapePrecedingEscapes(p, start, arg);
+ *p++ = '"';
+ }
+ *p++ = ' ';
+ }
+
+ *p = 0;
+ return command;
}
static bool Execute(ProcessInfo &PI, StringRef Program, const char **Args,
@@ -176,8 +270,7 @@ static bool Execute(ProcessInfo &PI, Str
// Windows wants a command line, not an array of args, to pass to the new
// process. We have to concatenate them all, while quoting the args that
// have embedded spaces (or are empty).
- auto ArgVector = buildArgVector(Args);
- std::string Command = flattenWindowsCommandLine(ArgVector);
+ std::unique_ptr<char[]> command = flattenArgs(Args);
// The pointer to the environment block for the new process.
std::vector<wchar_t> EnvBlock;
@@ -260,7 +353,7 @@ static bool Execute(ProcessInfo &PI, Str
}
SmallVector<wchar_t, MAX_PATH> CommandUtf16;
- if (std::error_code ec = windows::UTF8ToUTF16(Command, CommandUtf16)) {
+ if (std::error_code ec = windows::UTF8ToUTF16(command.get(), CommandUtf16)) {
SetLastError(ec.value());
MakeErrMsg(ErrMsg,
std::string("Unable to convert command-line to UTF-16"));
@@ -321,63 +414,7 @@ static bool Execute(ProcessInfo &PI, Str
return true;
}
-static bool argNeedsQuotes(StringRef Arg) {
- if (Arg.empty())
- return true;
- return StringRef::npos != Arg.find_first_of(" \t\n\v\"");
-}
-
-static std::string quoteSingleArg(StringRef Arg) {
- std::string Result;
- Result.push_back('"');
-
- while (!Arg.empty()) {
- size_t FirstNonBackslash = Arg.find_first_not_of('\\');
- size_t BackslashCount = FirstNonBackslash;
- if (FirstNonBackslash == StringRef::npos) {
- // The entire remainder of the argument is backslashes. Escape all of
- // them and just early out.
- BackslashCount = Arg.size();
- Result.append(BackslashCount * 2, '\\');
- break;
- }
-
- if (Arg[FirstNonBackslash] == '\"') {
- // This is an embedded quote. Escape all preceding backslashes, then
- // add one additional backslash to escape the quote.
- Result.append(BackslashCount * 2 + 1, '\\');
- Result.push_back('\"');
- } else {
- // This is just a normal character. Don't escape any of the preceding
- // backslashes, just append them as they are and then append the
- // character.
- Result.append(BackslashCount, '\\');
- Result.push_back(Arg[FirstNonBackslash]);
- }
-
- // Drop all the backslashes, plus the following character.
- Arg = Arg.drop_front(FirstNonBackslash + 1);
- }
-
- Result.push_back('"');
- return Result;
-}
-
namespace llvm {
-std::string sys::flattenWindowsCommandLine(ArrayRef<StringRef> Args) {
- std::string Command;
- for (StringRef Arg : Args) {
- if (argNeedsQuotes(Arg))
- Command += quoteSingleArg(Arg);
- else
- Command += Arg;
-
- Command.push_back(' ');
- }
-
- return Command;
-}
-
ProcessInfo sys::Wait(const ProcessInfo &PI, unsigned SecondsToWait,
bool WaitUntilChildTerminates, std::string *ErrMsg) {
assert(PI.Pid && "invalid pid to wait on, process not started?");
@@ -500,13 +537,19 @@ llvm::sys::writeFileWithEncoding(StringR
}
bool llvm::sys::commandLineFitsWithinSystemLimits(StringRef Program,
- ArrayRef<StringRef> Args) {
+ ArrayRef<const char *> Args) {
// The documented max length of the command line passed to CreateProcess.
static const size_t MaxCommandStringLength = 32768;
- SmallVector<StringRef, 8> FullArgs;
- FullArgs.push_back(Program);
- FullArgs.append(Args.begin(), Args.end());
- std::string Result = flattenWindowsCommandLine(FullArgs);
- return (Result.size() + 1) <= MaxCommandStringLength;
+ // Account for the trailing space for the program path and the
+ // trailing NULL of the last argument.
+ size_t ArgLength = ArgLenWithQuotes(Program.str().c_str()) + 2;
+ for (const char* Arg : Args) {
+ // Account for the trailing space for every arg
+ ArgLength += ArgLenWithQuotes(Arg) + 1;
+ if (ArgLength > MaxCommandStringLength) {
+ return false;
+ }
+ }
+ return true;
}
}
More information about the llvm-commits
mailing list