[llvm] r334295 - commandLineFitsWithinSystemLimits Overestimates System Limits

[Alexander Kornienko via llvm-commits]

Author: alexfh
Date: Fri Jun  8 08:19:16 2018
New Revision: 334295

URL: http://llvm.org/viewvc/llvm-project?rev=334295&view=rev
Log:
commandLineFitsWithinSystemLimits Overestimates System Limits

Summary:
The function `llvm::sys::commandLineFitsWithinSystemLimits` appears to be overestimating the system limits. This issue was discovered while attempting to enable response files in the Swift compiler. When the compiler submits its frontend jobs, those jobs are subjected to the system limits on command line length. `commandLineFitsWithinSystemLimits` is used to determine if the job's arguments need to be wrapped in a response file. There are some cases where the argument size for the job passes `commandLineFitsWithinSystemLimits`, but actually exceeds the real system limit, and the job fails.

`clang` also uses this function to decide whether or not to wrap it's job arguments in response files. See: https://github.com/llvm-mirror/clang/blob/master/lib/Driver/Driver.cpp#L1341. Clang will also fail for response files who's size falls within a certain range. I wrote a script that should find a failure point for `clang++`. All that is needed to run it is Python 2.7, and a simple "hello world" program for `test.cc`. It should run on Linux and on macOS. The script is available here: https://gist.github.com/dabelknap/71bd083cd06b91c5b3cef6a7f4d3d427. When it hits a failure point, you should see a `clang: error: unable to execute command: posix_spawn failed: Argument list too long`.

The proposed solution is to mirror the behavior of `xargs` in `commandLinefitsWithinSystemLimits`. `xargs` defaults to 128k for the command line length size (See: https://fossies.org/dox/findutils-4.6.0/buildcmd_8c_source.html#l00551). It adjusts this depending on the value of `ARG_MAX`.

Reviewers: alexfh

Reviewed By: alexfh

Subscribers: llvm-commits

Tags: #clang

Patch by Austin Belknap!

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

Modified:
    llvm/trunk/lib/Support/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=334295&r1=334294&r2=334295&view=diff
==============================================================================
--- llvm/trunk/lib/Support/Unix/Program.inc (original)
+++ llvm/trunk/lib/Support/Unix/Program.inc Fri Jun  8 08:19:16 2018
@@ -436,13 +436,24 @@ llvm::sys::writeFileWithEncoding(StringR
 bool llvm::sys::commandLineFitsWithinSystemLimits(StringRef Program,
                                                   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.
+  static long ArgMin = _POSIX_ARG_MAX;
+
+  // This the same baseline used by xargs.
+  long EffectiveArgMax = 128 * 1024;
+
+  if (EffectiveArgMax > ArgMax)
+    EffectiveArgMax = ArgMax;
+  else if (EffectiveArgMax < ArgMin)
+    EffectiveArgMax = ArgMin;
 
   // System says no practical limit.
   if (ArgMax == -1)
     return true;
 
   // Conservatively account for space required by environment variables.
-  long HalfArgMax = ArgMax / 2;
+  long HalfArgMax = EffectiveArgMax / 2;
 
   size_t ArgLength = Program.size() + 1;
   for (const char* Arg : Args) {