-
Notifications
You must be signed in to change notification settings - Fork 11.6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[compiler-rt][test] Add REQUIRES: shell to tests with &
with lit internal shell
#102988
Conversation
…ionality These tests require features like background processes, signal handling, and session management that are not supported by lit's internal shell. Adding `REQUIRES: shell` ensures they are only run in environments with a full-featured Unix-like shell.
@llvm/pr-subscribers-compiler-rt-sanitizer Author: None (Harini0924) ChangesThis patch adds the Details of the Change:
This change is relevant for enabling the lit internal shell by default, as outlined in [RFC] Enabling the Lit Internal Shell by Default Full diff: https://github.com/llvm/llvm-project/pull/102988.diff 4 Files Affected:
diff --git a/compiler-rt/test/fuzzer/fork-sigusr.test b/compiler-rt/test/fuzzer/fork-sigusr.test
index 4f796171fbd116..088e63cae43118 100644
--- a/compiler-rt/test/fuzzer/fork-sigusr.test
+++ b/compiler-rt/test/fuzzer/fork-sigusr.test
@@ -1,5 +1,6 @@
# Check that libFuzzer honors SIGUSR1/SIGUSR2
# Disabled on Windows which does not have SIGUSR1/SIGUSR2.
+REQUIRES: shell
UNSUPPORTED: darwin, target={{.*windows.*}}, target=aarch64{{.*}}
RUN: rm -rf %t
RUN: mkdir -p %t
diff --git a/compiler-rt/test/fuzzer/merge-sigusr.test b/compiler-rt/test/fuzzer/merge-sigusr.test
index 762ae0d106d289..4e492775400b98 100644
--- a/compiler-rt/test/fuzzer/merge-sigusr.test
+++ b/compiler-rt/test/fuzzer/merge-sigusr.test
@@ -1,6 +1,7 @@
# Check that libFuzzer honors SIGUSR1/SIGUSR2
# FIXME: Disabled on Windows for now because of reliance on posix only features
# (eg: export, "&", pkill).
+REQUIRES: shell
UNSUPPORTED: darwin, target={{.*windows.*}}
RUN: rm -rf %t
RUN: mkdir -p %t
diff --git a/compiler-rt/test/fuzzer/sigint.test b/compiler-rt/test/fuzzer/sigint.test
index 0e239c3ce53859..ac482d79b8e282 100644
--- a/compiler-rt/test/fuzzer/sigint.test
+++ b/compiler-rt/test/fuzzer/sigint.test
@@ -1,4 +1,4 @@
-REQUIRES: msan
+REQUIRES: shell, msan
UNSUPPORTED: target=arm{{.*}}
# Check that libFuzzer exits gracefully under SIGINT with MSan.
diff --git a/compiler-rt/test/fuzzer/sigusr.test b/compiler-rt/test/fuzzer/sigusr.test
index c3d7adf8ea99b3..c8a77ac63a6d7c 100644
--- a/compiler-rt/test/fuzzer/sigusr.test
+++ b/compiler-rt/test/fuzzer/sigusr.test
@@ -1,5 +1,6 @@
# FIXME: Disabled on Windows for now because of reliance on posix only features
# (eg: export, "&", pkill).
+REQUIRES: shell
UNSUPPORTED: darwin, target={{.*windows.*}}
# Check that libFuzzer honors SIGUSR1/SIGUSR2
RUN: rm -rf %t
|
CC: @ilovepi @petrhosek |
…lit Internal Shell (llvm#102988) This patch adds the `REQUIRES: shell` directive to compiler-rt's fuzzer tests that require full shell functionality when using the lit internal shell. These tests depend on features such as background processes, signal handling, and session management, which are not supported by lit's internal shell. The addition of this directive ensures that these tests are only executed in environments that provide the necessary shell capabilities. **Details of the Change:** The following considerations were addressed: - **Background Processes (`&`):** The tests run commands in the background using the `&` operator, which allows the shell to execute commands concurrently without waiting for each one to finish. In a standard Unix-like shell, this is a common feature that facilitates multitasking. However, lit's internal shell does not fully support background job control, which can lead to unpredictable behavior or test failures. Without proper handling of background processes, subsequent commands that depend on the status of these processes may not function correctly. - **Signal Handling (`kill -SIGUSR1`, `kill -SIGUSR2`, `kill -SIGINT`):** These tests involve sending specific signals like `SIGUSR1`, `SIGUSR2`, and `SIGINT` to running processes. These signals are used to trigger certain behaviors in the processes, such as pausing, resuming, or terminating. However, lit's internal shell may not handle these signals properly—it might not send the signal correctly, or the process might not respond as it should. This could lead to the test failing, not because the process is incorrect, but because the shell didn't manage the signals as required. - **Session Management (`setsid`):** The tests use `setsid` to create new sessions, detaching processes from their controlling terminal and isolating them into their own process groups. However, the internal shell in lit does not support session management features like `setsid`, this can't correctly isolate and manage processes as required by these tests. This change is relevant for enabling the lit internal shell by default, as outlined in [[RFC] Enabling the Lit Internal Shell by Default](https://discourse.llvm.org/t/rfc-enabling-the-lit-internal-shell-by-default/80179)
&
with lit internal shell
This patch adds the
REQUIRES: shell
directive to compiler-rt's fuzzer tests that require full shell functionality when using the lit internal shell. These tests depend on features such as background processes, signal handling, and session management, which are not supported by lit's internal shell. The addition of this directive ensures that these tests are only executed in environments that provide the necessary shell capabilities.Details of the Change:
The following considerations were addressed:
&
): The tests run commands in the background using the&
operator, which allows the shell to execute commands concurrently without waiting for each one to finish. In a standard Unix-like shell, this is a common feature that facilitates multitasking. However, lit's internal shell does not fully support background job control, which can lead to unpredictable behavior or test failures. Without proper handling of background processes, subsequent commands that depend on the status of these processes may not function correctly.kill -SIGUSR1
,kill -SIGUSR2
,kill -SIGINT
): These tests involve sending specific signals likeSIGUSR1
,SIGUSR2
, andSIGINT
to running processes. These signals are used to trigger certain behaviors in the processes, such as pausing, resuming, or terminating. However, lit's internal shell may not handle these signals properly—it might not send the signal correctly, or the process might not respond as it should. This could lead to the test failing, not because the process is incorrect, but because the shell didn't manage the signals as required.setsid
): The tests usesetsid
to create new sessions, detaching processes from their controlling terminal and isolating them into their own process groups. However, the internal shell in lit does not support session management features likesetsid
, this can't correctly isolate and manage processes as required by these tests.Previously, this test was causing an
AttributeError
due to incompatibilities with lit's internal shell. By enforcing the use of an external shell with theREQUIRES: shell
directive, this patch addresses the issue temporarily. This approach fixes the problem for now, and when we later revisit and remove theREQUIRES: shell
directive, we can implement a more robust solution to fully support these features in the internal shell.This change is relevant for enabling the lit internal shell by default, as outlined in [RFC] Enabling the Lit Internal Shell by Default
fixes: #102399