[libcxx-commits] [libcxx] 5eb8d45 - [libc++] Use proper shell escaping in the executors

[Louis Dionne via libcxx-commits]

Author: Louis Dionne
Date: 2020-04-18T11:22:42-04:00
New Revision: 5eb8d45ab5b85cb3a2edfe995cbf1d4b1beae462

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

LOG: [libc++] Use proper shell escaping in the executors

This was originally committed as f8452ddfcc33 and reverted in 7cb1aa9d9368.
The issue was that shell builtins were being escaped too, and apparently
Bash won't execute a builtin when it is quoted e.g. '!'. Instead, it
thinks it's a command and it can't find it.

Re-committing the change with that issue fixed.

Added: 
    libcxx/test/libcxx/selftest/newformat/remote-substitutions.sh.cpp
    libcxx/test/libcxx/selftest/newformat/shell-escape.sh.cpp

Modified: 
    libcxx/utils/run.py
    libcxx/utils/ssh.py

Removed: 
    libcxx/test/libcxx/selftest/newformat/sh.cpp/remote-substitutions.sh.cpp


################################################################################
diff  --git a/libcxx/test/libcxx/selftest/newformat/sh.cpp/remote-substitutions.sh.cpp b/libcxx/test/libcxx/selftest/newformat/remote-substitutions.sh.cpp
similarity index 100%
rename from libcxx/test/libcxx/selftest/newformat/sh.cpp/remote-substitutions.sh.cpp
rename to libcxx/test/libcxx/selftest/newformat/remote-substitutions.sh.cpp

diff  --git a/libcxx/test/libcxx/selftest/newformat/shell-escape.sh.cpp b/libcxx/test/libcxx/selftest/newformat/shell-escape.sh.cpp
new file mode 100644
index 000000000000..c942df0133b4
--- /dev/null
+++ b/libcxx/test/libcxx/selftest/newformat/shell-escape.sh.cpp
@@ -0,0 +1,18 @@
+//===----------------------------------------------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+// Make sure that arguments of the %{exec} substitution are shell-escaped
+// properly. If that wasn't the case, the command would fail because the
+// shell would look for a matching `"`.
+
+// RUN: %{exec} echo '"'
+
+// Also make sure that we don't escape Shell builtins like `!`, because the
+// shell otherwise thinks it's a command and it can't find it.
+
+// RUN: ! false

diff  --git a/libcxx/utils/run.py b/libcxx/utils/run.py
index 7cdf65264ec0..0453e4a1f018 100644
--- a/libcxx/utils/run.py
+++ b/libcxx/utils/run.py
@@ -30,11 +30,11 @@ def main():
     if len(remaining) < 2:
         sys.stderr.write('Missing actual commands to run')
         exit(1)
-    remaining = remaining[1:] # Skip the '--'
+    commandLine = remaining[1:] # Skip the '--'
 
     # Do any necessary codesigning.
     if args.codesign_identity:
-        exe = remaining[0]
+        exe = commandLine[0]
         rc = subprocess.call(['xcrun', 'codesign', '-f', '-s', args.codesign_identity, exe], env={})
         if rc != 0:
             sys.stderr.write('Failed to codesign: ' + exe)
@@ -57,8 +57,8 @@ def main():
             else:
                 shutil.copy2(dep, args.execdir)
 
-        # Run the executable with the given environment in the execution directory.
-        return subprocess.call(' '.join(remaining), cwd=args.execdir, env=env, shell=True)
+        # Run the command line with the given environment in the execution directory.
+        return subprocess.call(subprocess.list2cmdline(commandLine), cwd=args.execdir, env=env, shell=True)
     finally:
         shutil.rmtree(args.execdir)
 

diff  --git a/libcxx/utils/ssh.py b/libcxx/utils/ssh.py
index c7d8c97a1407..4bb983e5d2cb 100644
--- a/libcxx/utils/ssh.py
+++ b/libcxx/utils/ssh.py
@@ -97,10 +97,11 @@ def main():
         # host by transforming the path of test-executables to their path in the
         # temporary directory, where we know they have been copied when we handled
         # test dependencies above.
+        commandLine = (pathOnRemote(x) if isTestExe(x) else x for x in commandLine)
         remoteCommands += [
             'cd {}'.format(tmp),
             'export {}'.format(' '.join(args.env)),
-            ' '.join(pathOnRemote(x) if isTestExe(x) else x for x in commandLine)
+            subprocess.list2cmdline(commandLine)
         ]
 
         # Finally, SSH to the remote host and execute all the commands.