[llvm] [lit] Echo full RUN lines in case of external shells (PR #66408)
Joel E. Denny via llvm-commits
llvm-commits at lists.llvm.org
Tue Sep 19 09:45:53 PDT 2023
https://github.com/jdenny-ornl updated https://github.com/llvm/llvm-project/pull/66408
>From a76ca4cbd2e53d64a616f48947ff5c7671baedd2 Mon Sep 17 00:00:00 2001
From: "Joel E. Denny" <jdenny.ornl at gmail.com>
Date: Tue, 19 Sep 2023 12:43:52 -0400
Subject: [PATCH] [lit] Echo full RUN lines in case of external shells
Before <https://reviews.llvm.org/D154984> and
<https://reviews.llvm.org/D156954>, lit reported full RUN lines in a
`Script:` section. Now, in the case of lit's internal shell, it's the
execution trace that includes them. However, if lit is configured to
use an external shell (e.g., bash, windows `cmd`), they aren't
reported at all.
A fix was requested at the following:
* <https://reviews.llvm.org/D154984#4627605>
* <https://discourse.llvm.org/t/rfc-improving-lits-debug-output/72839/35?u=jdenny-ornl>
This patch does not address the case when the external shell is
windows `cmd`. As discussed at
<https://github.com/llvm/llvm-project/pull/65242>, it's not clear
whether that's a use case that people still care about, and it seems
to be generally broken anyway.
---
llvm/utils/lit/lit/TestRunner.py | 34 +++++++++++++-
.../Inputs/shtest-external-shell-kill/lit.cfg | 5 ++
.../shtest-external-shell-kill/test.txt | 5 ++
.../external-shell/empty-run-line.txt | 3 ++
.../internal-shell/empty-run-line.txt | 3 ++
.../lit/tests/shtest-external-shell-kill.py | 36 ++++++++++++++
llvm/utils/lit/tests/shtest-run-at-line.py | 47 ++++++++++++++++---
7 files changed, 125 insertions(+), 8 deletions(-)
create mode 100644 llvm/utils/lit/tests/Inputs/shtest-external-shell-kill/lit.cfg
create mode 100644 llvm/utils/lit/tests/Inputs/shtest-external-shell-kill/test.txt
create mode 100644 llvm/utils/lit/tests/Inputs/shtest-run-at-line/external-shell/empty-run-line.txt
create mode 100644 llvm/utils/lit/tests/Inputs/shtest-run-at-line/internal-shell/empty-run-line.txt
create mode 100644 llvm/utils/lit/tests/shtest-external-shell-kill.py
diff --git a/llvm/utils/lit/lit/TestRunner.py b/llvm/utils/lit/lit/TestRunner.py
index f85627eaf3de7ce..528694b07f03300 100644
--- a/llvm/utils/lit/lit/TestRunner.py
+++ b/llvm/utils/lit/lit/TestRunner.py
@@ -1170,8 +1170,40 @@ def executeScript(test, litConfig, tmpBase, commands, cwd):
for i, ln in enumerate(commands):
match = re.fullmatch(kPdbgRegex, ln)
if match:
+ dbg = match.group(1)
command = match.group(2)
- commands[i] = match.expand(": '\\1'; \\2" if command else ": '\\1'")
+ # Echo the debugging diagnostic to stderr.
+ #
+ # For that echo command, use 'set' commands to suppress the
+ # shell's execution trace, which would just add noise. Suppress
+ # the shell's execution trace for the 'set' commands by
+ # redirecting their stderr to /dev/null.
+ if command:
+ msg = f"'{dbg}': {shlex.quote(command.lstrip())}"
+ else:
+ msg = f"'{dbg}' has no command after substitutions"
+ commands[i] = (
+ f"{{ set +x; }} 2>/dev/null && "
+ f"echo {msg} >&2 && "
+ f"{{ set -x; }} 2>/dev/null"
+ )
+ # Execute the command, if any.
+ #
+ # 'command' might be something like:
+ #
+ # subcmd & PID=$!
+ #
+ # In that case, we need something like:
+ #
+ # echo_dbg && { subcmd & PID=$!; }
+ #
+ # Without the '{ ...; }' enclosing the original 'command', '&'
+ # would put all of 'echo_dbg && subcmd' in the background. This
+ # would cause 'echo_dbg' to execute at the wrong time, and a
+ # later kill of $PID would target the wrong process. We have
+ # seen the latter manage to terminate the shell running lit.
+ if command:
+ commands[i] += f" && {{ {command}; }}"
if test.config.pipefail:
f.write(b"set -o pipefail;" if mode == "wb" else "set -o pipefail;")
f.write(b"set -x;" if mode == "wb" else "set -x;")
diff --git a/llvm/utils/lit/tests/Inputs/shtest-external-shell-kill/lit.cfg b/llvm/utils/lit/tests/Inputs/shtest-external-shell-kill/lit.cfg
new file mode 100644
index 000000000000000..d10594dc525f568
--- /dev/null
+++ b/llvm/utils/lit/tests/Inputs/shtest-external-shell-kill/lit.cfg
@@ -0,0 +1,5 @@
+import lit.formats
+
+config.test_format = lit.formats.ShTest(execute_external=True)
+config.name = "shtest-external-shell-kill"
+config.suffixes = [".txt"]
diff --git a/llvm/utils/lit/tests/Inputs/shtest-external-shell-kill/test.txt b/llvm/utils/lit/tests/Inputs/shtest-external-shell-kill/test.txt
new file mode 100644
index 000000000000000..dbdf2d689425c5f
--- /dev/null
+++ b/llvm/utils/lit/tests/Inputs/shtest-external-shell-kill/test.txt
@@ -0,0 +1,5 @@
+# RUN: echo start
+# RUN: sleep 300 & PID=$!
+# RUN: sleep 2
+# RUN: kill $PID
+# RUN: echo end
diff --git a/llvm/utils/lit/tests/Inputs/shtest-run-at-line/external-shell/empty-run-line.txt b/llvm/utils/lit/tests/Inputs/shtest-run-at-line/external-shell/empty-run-line.txt
new file mode 100644
index 000000000000000..40a5a7d6e7cce0a
--- /dev/null
+++ b/llvm/utils/lit/tests/Inputs/shtest-run-at-line/external-shell/empty-run-line.txt
@@ -0,0 +1,3 @@
+# DEFINE: %{empty} =
+# RUN: %{empty}
+# RUN: false
diff --git a/llvm/utils/lit/tests/Inputs/shtest-run-at-line/internal-shell/empty-run-line.txt b/llvm/utils/lit/tests/Inputs/shtest-run-at-line/internal-shell/empty-run-line.txt
new file mode 100644
index 000000000000000..40a5a7d6e7cce0a
--- /dev/null
+++ b/llvm/utils/lit/tests/Inputs/shtest-run-at-line/internal-shell/empty-run-line.txt
@@ -0,0 +1,3 @@
+# DEFINE: %{empty} =
+# RUN: %{empty}
+# RUN: false
diff --git a/llvm/utils/lit/tests/shtest-external-shell-kill.py b/llvm/utils/lit/tests/shtest-external-shell-kill.py
new file mode 100644
index 000000000000000..73f8d8601af620b
--- /dev/null
+++ b/llvm/utils/lit/tests/shtest-external-shell-kill.py
@@ -0,0 +1,36 @@
+# This test exercises an external shell use case that, at least at one time,
+# appeared in the following tests:
+#
+# compiler-rt/test/fuzzer/fork-sigusr.test
+# compiler-rt/test/fuzzer/merge-sigusr.test
+# compiler-rt/test/fuzzer/sigint.test
+# compiler-rt/test/fuzzer/sigusr.test
+#
+# That is, a RUN line can be:
+#
+# cmd & PID=$!
+#
+# It is important that '&' only puts 'cmd' in the background and not the
+# debugging commands that lit inserts before 'cmd'. Otherwise:
+#
+# - The debugging commands might execute later than they are supposed to.
+# - A later 'kill $PID' can kill more than just 'cmd'. We've seen it even
+# manage to terminate the shell running lit.
+#
+# The last FileCheck directive below checks that the debugging commands for the
+# above RUN line are not killed and do execute at the right time.
+
+# RUN: %{lit} -a %{inputs}/shtest-external-shell-kill | FileCheck %s
+# END.
+
+# CHECK: Command Output (stdout):
+# CHECK-NEXT: --
+# CHECK-NEXT: start
+# CHECK-NEXT: end
+# CHECK-EMPTY:
+# CHECK-NEXT: --
+# CHECK-NEXT: Command Output (stderr):
+# CHECK-NEXT: --
+# CHECK-NEXT: RUN: at line 1: echo start
+# CHECK-NEXT: echo start
+# CHECK-NEXT: RUN: at line 2: sleep [[#]] & PID=$!
diff --git a/llvm/utils/lit/tests/shtest-run-at-line.py b/llvm/utils/lit/tests/shtest-run-at-line.py
index a0626f872c4c9e0..b92aee97628b25d 100644
--- a/llvm/utils/lit/tests/shtest-run-at-line.py
+++ b/llvm/utils/lit/tests/shtest-run-at-line.py
@@ -6,7 +6,7 @@
# END.
-# CHECK: Testing: 4 tests
+# CHECK: Testing: 6 tests
# In the case of the external shell, we check for only RUN lines in stderr in
@@ -14,15 +14,38 @@
# CHECK-LABEL: FAIL: shtest-run-at-line :: external-shell/basic.txt
-# CHECK: RUN: at line 4
-# CHECK: RUN: at line 5
-# CHECK-NOT: RUN
+# CHECK: Command Output (stderr)
+# CHECK-NEXT: --
+# CHECK-NEXT: {{^}}RUN: at line 4: true{{$}}
+# CHECK-NEXT: true
+# CHECK-NEXT: {{^}}RUN: at line 5: false{{$}}
+# CHECK-NEXT: false
+# CHECK-EMPTY:
+# CHECK-NEXT: --
+
+# CHECK-LABEL: FAIL: shtest-run-at-line :: external-shell/empty-run-line.txt
+
+# CHECK: Command Output (stderr)
+# CHECK-NEXT: --
+# CHECK-NEXT: {{^}}RUN: at line 2 has no command after substitutions{{$}}
+# CHECK-NEXT: {{^}}RUN: at line 3: false{{$}}
+# CHECK-NEXT: false
+# CHECK-EMPTY:
+# CHECK-NEXT: --
# CHECK-LABEL: FAIL: shtest-run-at-line :: external-shell/line-continuation.txt
-# CHECK: RUN: at line 4
-# CHECK: RUN: at line 6
-# CHECK-NOT: RUN
+# The execution trace from an external sh-like shell might print the commands
+# from a pipeline in any order, so this time just check that lit suppresses the
+# trace of the echo command for each 'RUN: at line N: cmd-line'.
+
+# CHECK: Command Output (stderr)
+# CHECK-NEXT: --
+# CHECK-NEXT: {{^}}RUN: at line 4: echo 'foo bar' | FileCheck
+# CHECK-NOT: RUN
+# CHECK: {{^}}RUN: at line 6: echo 'foo baz' | FileCheck
+# CHECK-NOT: RUN
+# CHECK: --
# CHECK-LABEL: FAIL: shtest-run-at-line :: internal-shell/basic.txt
@@ -37,6 +60,16 @@
# CHECK-NEXT: # executed command: false
# CHECK-NOT: RUN
+# CHECK-LABEL: FAIL: shtest-run-at-line :: internal-shell/empty-run-line.txt
+
+# CHECK: Command Output (stdout)
+# CHECK-NEXT: --
+# CHECK-NEXT: # RUN: at line 2 has no command after substitutions
+# CHECK-NEXT: # RUN: at line 3
+# CHECK-NEXT: false
+# CHECK-NEXT: # executed command: false
+# CHECK-NOT: RUN
+
# CHECK-LABEL: FAIL: shtest-run-at-line :: internal-shell/line-continuation.txt
# CHECK: Command Output (stdout)
More information about the llvm-commits
mailing list