[llvm] 3793540 - [lit] Always quote arguments containing '[' on windows

[Martin Storsjö via llvm-commits]

Author: Martin Storsjö
Date: 2021-04-14T12:32:48+03:00
New Revision: 37935405efbebc4bd9f1ffac9152571c6a8469dc

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

LOG: [lit] Always quote arguments containing '[' on windows

This avoids breaking clang-tidy/infrastructure/validate-check-names.cpp
if 'not' is evaluated as a lit internal tool (making TestRunner
invoke 'grep' directly in that test, instead of invoking 'not', which
then invokes 'grep').

The quoting of arguments is still brittle if the executable is an
MSYS based tool though, as MSYS based tools incorrectly unescape
backslashes in quoted arguments (contrary to regular win32 argument
parsing rules), see D99406 and
https://github.com/msys2/msys2-runtime/issues/36 for more examples
of the issues.

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

Added: 
    

Modified: 
    llvm/test/Other/lit-quoting.txt
    llvm/utils/lit/lit/TestRunner.py

Removed: 
    


################################################################################
diff  --git a/llvm/test/Other/lit-quoting.txt b/llvm/test/Other/lit-quoting.txt
index cac89147fce5e..c263d8f4a85a0 100644
--- a/llvm/test/Other/lit-quoting.txt
+++ b/llvm/test/Other/lit-quoting.txt
@@ -1,2 +1,20 @@
-RUN: echo "\"" | FileCheck %s
-CHECK: {{^"$}}
+RUN: echo "\"" | FileCheck %s --check-prefix=CHECK1
+RUN: echo '"' | FileCheck %s --check-prefix=CHECK1
+RUN: echo 'a[b\c' | FileCheck %s --check-prefix=CHECK2
+RUN: echo "a[b\\c" | FileCheck %s --check-prefix=CHECK2
+RUN: echo 'a\b\\c\\\\d' | FileCheck %s --check-prefix=CHECK3
+RUN: echo "a\\b\\\\c\\\\\\\\d" | FileCheck %s --check-prefix=CHECK3
+RUN: not not echo "\"" | FileCheck %s --check-prefix=CHECK1
+RUN: not not echo '"' | FileCheck %s --check-prefix=CHECK1
+RUN: not not echo 'a[b\c' | FileCheck %s --check-prefix=CHECK2
+RUN: not not echo "a[b\\c" | FileCheck %s --check-prefix=CHECK2
+RUN: not not echo 'a\b\\c\\\\d' | FileCheck %s --check-prefix=CHECK3
+RUN: not not echo "a\\b\\\\c\\\\\\\\d" | FileCheck %s --check-prefix=CHECK3
+CHECK1: {{^"$}}
+CHECK2: {{^a\[b\\c$}}
+CHECK3: {{^a\\b\\\\c\\\\\\\\d$}}
+
+On Windows, with MSYS based tools, the following commands fail though:
+RUNX: echo 'a[b\c\\d' | FileCheck %s --check-prefix=CHECK4
+RUNX: echo "a[b\\c\\\\d" | FileCheck %s --check-prefix=CHECK4
+CHECK4: {{^a\[b\\c\\\\d$}}

diff  --git a/llvm/utils/lit/lit/TestRunner.py b/llvm/utils/lit/lit/TestRunner.py
index f826bc91fb3e0..70d641e58df9f 100644
--- a/llvm/utils/lit/lit/TestRunner.py
+++ b/llvm/utils/lit/lit/TestRunner.py
@@ -191,7 +191,13 @@ def quote_windows_command(seq):
 
     We use the same algorithm from MSDN as CPython
     (http://msdn.microsoft.com/en-us/library/17w5ykft.aspx), but we treat more
-    characters as needing quoting, such as double quotes themselves.
+    characters as needing quoting, such as double quotes themselves, and square
+    brackets.
+
+    For MSys based tools, this is very brittle though, because quoting an
+    argument makes the MSys based tool unescape backslashes where it shouldn't
+    (e.g. "a\b\\c\\\\d" becomes "a\b\c\\d" where it should stay as it was,
+    according to regular win32 command line parsing rules).
     """
     result = []
     needquote = False
@@ -203,7 +209,7 @@ def quote_windows_command(seq):
             result.append(' ')
 
         # This logic 
diff ers from upstream list2cmdline.
-        needquote = (" " in arg) or ("\t" in arg) or ("\"" in arg) or not arg
+        needquote = (" " in arg) or ("\t" in arg) or ("\"" in arg) or ("[" in arg) or not arg
         if needquote:
             result.append('"')