[llvm] d4020ef - [Windows] Fix limit on command line size

[Serge Pavlov via llvm-commits]

Author: Serge Pavlov
Date: 2020-07-21T17:33:22+07:00
New Revision: d4020ef7c474b5e695d77aa100d7f68dc0c66b4e

URL: https://github.com/llvm/llvm-project/commit/d4020ef7c474b5e695d77aa100d7f68dc0c66b4e
DIFF: https://github.com/llvm/llvm-project/commit/d4020ef7c474b5e695d77aa100d7f68dc0c66b4e.diff

LOG: [Windows] Fix limit on command line size

Documentation on CreateProcessW states that maximal size of command line
is 32767 characters including ternimation null character. In the
function llvm::sys::commandLineFitsWithinSystemLimits this limit was set
to 32768. As a result if command line was exactly 32768 characters long,
a response file was not created and CreateProcessW was called with
too long command line.

Differential Revision: https://reviews.llvm.org/D83772

Added: 
    

Modified: 
    llvm/include/llvm/Support/Program.h
    llvm/lib/Support/Windows/Program.inc
    llvm/unittests/Support/CommandLineTest.cpp

Removed: 
    


################################################################################
diff  --git a/llvm/include/llvm/Support/Program.h b/llvm/include/llvm/Support/Program.h
index dbda064cda05..d729d3883650 100644
--- a/llvm/include/llvm/Support/Program.h
+++ b/llvm/include/llvm/Support/Program.h
@@ -218,7 +218,7 @@ namespace sys {
   /// to build a single flat command line appropriate for calling CreateProcess
   /// on
   /// Windows.
-  std::string flattenWindowsCommandLine(ArrayRef<StringRef> Args);
+  ErrorOr<std::wstring> flattenWindowsCommandLine(ArrayRef<StringRef> Args);
 #endif
   }
 }

diff  --git a/llvm/lib/Support/Windows/Program.inc b/llvm/lib/Support/Windows/Program.inc
index 9fe05d24ec2e..c98c78418829 100644
--- a/llvm/lib/Support/Windows/Program.inc
+++ b/llvm/lib/Support/Windows/Program.inc
@@ -189,7 +189,14 @@ static bool Execute(ProcessInfo &PI, StringRef Program,
   // 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).
-  std::string Command = flattenWindowsCommandLine(Args);
+  auto Result = flattenWindowsCommandLine(Args);
+  if (std::error_code ec = Result.getError()) {
+    SetLastError(ec.value());
+    MakeErrMsg(ErrMsg,
+               std::string("Unable to convert command-line to UTF-16"));
+    return false;
+  }
+  std::wstring Command = *Result;
 
   // The pointer to the environment block for the new process.
   std::vector<wchar_t> EnvBlock;
@@ -271,14 +278,8 @@ static bool Execute(ProcessInfo &PI, StringRef Program,
     return false;
   }
 
-  SmallVector<wchar_t, MAX_PATH> CommandUtf16;
-  if (std::error_code ec = windows::UTF8ToUTF16(Command, CommandUtf16)) {
-    SetLastError(ec.value());
-    MakeErrMsg(ErrMsg,
-               std::string("Unable to convert command-line to UTF-16"));
-    return false;
-  }
-
+  std::vector<wchar_t> CommandUtf16(Command.size() + 1, 0);
+  std::copy(Command.begin(), Command.end(), CommandUtf16.begin());
   BOOL rc = CreateProcessW(ProgramUtf16.data(), CommandUtf16.data(), 0, 0,
                            TRUE, CREATE_UNICODE_ENVIRONMENT,
                            EnvBlock.empty() ? 0 : EnvBlock.data(), 0, &si,
@@ -376,7 +377,7 @@ static std::string quoteSingleArg(StringRef Arg) {
 }
 
 namespace llvm {
-std::string sys::flattenWindowsCommandLine(ArrayRef<StringRef> Args) {
+ErrorOr<std::wstring> sys::flattenWindowsCommandLine(ArrayRef<StringRef> Args) {
   std::string Command;
   for (StringRef Arg : Args) {
     if (argNeedsQuotes(Arg))
@@ -387,7 +388,11 @@ std::string sys::flattenWindowsCommandLine(ArrayRef<StringRef> Args) {
     Command.push_back(' ');
   }
 
-  return Command;
+  SmallVector<wchar_t, MAX_PATH> CommandUtf16;
+  if (std::error_code ec = windows::UTF8ToUTF16(Command, CommandUtf16))
+    return ec;
+
+  return std::wstring(CommandUtf16.begin(), CommandUtf16.end());
 }
 
 ProcessInfo sys::Wait(const ProcessInfo &PI, unsigned SecondsToWait,
@@ -532,12 +537,16 @@ llvm::sys::writeFileWithEncoding(StringRef FileName, StringRef Contents,
 
 bool llvm::sys::commandLineFitsWithinSystemLimits(StringRef Program,
                                                   ArrayRef<StringRef> Args) {
-  // The documented max length of the command line passed to CreateProcess.
-  static const size_t MaxCommandStringLength = 32768;
+  // The documentation on CreateProcessW states that the size of the argument
+  // lpCommandLine must not be greater than 32767 characters, including the
+  // Unicode terminating null character. We use smaller value to reduce risk
+  // of getting invalid command line due to unaccounted factors.
+  static const size_t MaxCommandStringLength = 32000;
   SmallVector<StringRef, 8> FullArgs;
   FullArgs.push_back(Program);
   FullArgs.append(Args.begin(), Args.end());
-  std::string Result = flattenWindowsCommandLine(FullArgs);
-  return (Result.size() + 1) <= MaxCommandStringLength;
+  auto Result = flattenWindowsCommandLine(FullArgs);
+  assert(!Result.getError());
+  return (Result->size() + 1) <= MaxCommandStringLength;
 }
 }

diff  --git a/llvm/unittests/Support/CommandLineTest.cpp b/llvm/unittests/Support/CommandLineTest.cpp
index e1b706e2a78e..e8c2cef18e36 100644
--- a/llvm/unittests/Support/CommandLineTest.cpp
+++ b/llvm/unittests/Support/CommandLineTest.cpp
@@ -763,6 +763,18 @@ TEST(CommandLineTest, DefaultOptions) {
 TEST(CommandLineTest, ArgumentLimit) {
   std::string args(32 * 4096, 'a');
   EXPECT_FALSE(llvm::sys::commandLineFitsWithinSystemLimits("cl", args.data()));
+  std::string args2(256, 'a');
+  EXPECT_TRUE(llvm::sys::commandLineFitsWithinSystemLimits("cl", args2.data()));
+  if (Triple(sys::getProcessTriple()).isOSWindows()) {
+    // We use 32000 as a limit for command line length. Program name ('cl'),
+    // separating spaces and termination null character occupy 5 symbols.
+    std::string long_arg(32000 - 5, 'b');
+    EXPECT_TRUE(
+        llvm::sys::commandLineFitsWithinSystemLimits("cl", long_arg.data()));
+    long_arg += 'b';
+    EXPECT_FALSE(
+        llvm::sys::commandLineFitsWithinSystemLimits("cl", long_arg.data()));
+  }
 }
 
 TEST(CommandLineTest, ResponseFileWindows) {