Skip to content

[compiler-rt][test] Add REQUIRES: shell to compiler-rt tests that use the ulimit command #105339

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

Merged
merged 3 commits into from
Aug 26, 2024

Conversation

Harini0924
Copy link
Contributor

This patch adds the REQUIRES: shell directive to six test files that use the ulimit command, ensuring these tests are only run in environments where a full POSIX-compliant shell is available. The lit internal shell does not use or support the ulimit command, which causes failures when running tests with LIT_USE_INTERNAL_SHELL=1 ninja check-compiler-rt
Specifically, one of the errors encountered is:

# RUN: at line 4
ulimit -s 1000
# executed command: ulimit -s 1000
# .---command stderr------------
# | 'ulimit': command not found
# `-----------------------------
# error: command failed with exit status: 127

Since, the lit internal shell doesn't support ulimit, adding this requirement prevents these tests from failing in the lit internal shell, thereby improving the reliability of the test suite in environments where the full shell is not available.

This change is relevant for [RFC] Enabling the Lit Internal Shell by Default
fixes: #102398

@llvmbot
Copy link
Member

llvmbot commented Aug 20, 2024

@llvm/pr-subscribers-compiler-rt-sanitizer

Author: None (Harini0924)

Changes

This patch adds the REQUIRES: shell directive to six test files that use the ulimit command, ensuring these tests are only run in environments where a full POSIX-compliant shell is available. The lit internal shell does not use or support the ulimit command, which causes failures when running tests with LIT_USE_INTERNAL_SHELL=1 ninja check-compiler-rt
Specifically, one of the errors encountered is:

# RUN: at line 4
ulimit -s 1000
# executed command: ulimit -s 1000
# .---command stderr------------
# | 'ulimit': command not found
# `-----------------------------
# error: command failed with exit status: 127

Since, the lit internal shell doesn't support ulimit, adding this requirement prevents these tests from failing in the lit internal shell, thereby improving the reliability of the test suite in environments where the full shell is not available.

This change is relevant for [RFC] Enabling the Lit Internal Shell by Default
fixes: #102398


Full diff: https://github.com/llvm/llvm-project/pull/105339.diff

5 Files Affected:

  • (modified) compiler-rt/test/asan/TestCases/Linux/allocator_oom_test.cpp (+1)
  • (modified) compiler-rt/test/asan/TestCases/Posix/deep_call_stack.cpp (+1)
  • (modified) compiler-rt/test/fuzzer/ulimit.test (+1)
  • (modified) compiler-rt/test/hwasan/TestCases/print-memory-usage.c (+1)
  • (modified) compiler-rt/test/msan/Linux/reexec_unlimited_stack.cpp (+1)
diff --git a/compiler-rt/test/asan/TestCases/Linux/allocator_oom_test.cpp b/compiler-rt/test/asan/TestCases/Linux/allocator_oom_test.cpp
index b096624a7f95b9..31541dd20ee489 100644
--- a/compiler-rt/test/asan/TestCases/Linux/allocator_oom_test.cpp
+++ b/compiler-rt/test/asan/TestCases/Linux/allocator_oom_test.cpp
@@ -7,6 +7,7 @@
 // Limit this test to Linux since we're relying on allocator internal
 // limits (shadow memory size, allocation limits etc.)
 
+REQUIRES: shell
 // RUN: %clangxx_asan -O0 %s -o %t
 // RUN: ulimit -v 22024290304
 // RUN: not %run %t malloc 2>&1 \
diff --git a/compiler-rt/test/asan/TestCases/Posix/deep_call_stack.cpp b/compiler-rt/test/asan/TestCases/Posix/deep_call_stack.cpp
index e6e82a47572056..02284e8f0cc3e6 100644
--- a/compiler-rt/test/asan/TestCases/Posix/deep_call_stack.cpp
+++ b/compiler-rt/test/asan/TestCases/Posix/deep_call_stack.cpp
@@ -1,3 +1,4 @@
+REQUIRES: shell
 // Check that UAR mode can handle very deep recusrion.
 // RUN: %clangxx_asan -O2 %s -o %t
 // RUN: ulimit -s 4096
diff --git a/compiler-rt/test/fuzzer/ulimit.test b/compiler-rt/test/fuzzer/ulimit.test
index 223f2ac9bb6e2e..3e1c9ceedbb3c3 100644
--- a/compiler-rt/test/fuzzer/ulimit.test
+++ b/compiler-rt/test/fuzzer/ulimit.test
@@ -1,4 +1,5 @@
 # FIXME: Disabled on Windows for now because Windows has no ulimit command.
+REQUIRES: shell
 UNSUPPORTED: target={{.*windows.*}}
 RUN: %cpp_compiler %S/SimpleTest.cpp -o %t-SimpleTest
 RUN: ulimit -s 1000
diff --git a/compiler-rt/test/hwasan/TestCases/print-memory-usage.c b/compiler-rt/test/hwasan/TestCases/print-memory-usage.c
index 2c89d4e70ebc74..7c095e03125de5 100644
--- a/compiler-rt/test/hwasan/TestCases/print-memory-usage.c
+++ b/compiler-rt/test/hwasan/TestCases/print-memory-usage.c
@@ -1,3 +1,4 @@
+REQUIRES: shell
 // Tests __hwasan_print_memory_usage.
 // RUN: %clang_hwasan %s -o %t
 // RUN: ulimit -s 1000
diff --git a/compiler-rt/test/msan/Linux/reexec_unlimited_stack.cpp b/compiler-rt/test/msan/Linux/reexec_unlimited_stack.cpp
index 61492ec34533fe..30c47abe02936a 100644
--- a/compiler-rt/test/msan/Linux/reexec_unlimited_stack.cpp
+++ b/compiler-rt/test/msan/Linux/reexec_unlimited_stack.cpp
@@ -1,3 +1,4 @@
+REQUIRES: shell
 // MSAN re-execs on unlimited stacks. We use that to verify ReExec() uses the
 // right path.
 // RUN: %clangxx_msan -O0 %s -o %t && ulimit -s unlimited && %run %t | FileCheck %s

@Harini0924 Harini0924 changed the title [llvm-lit] Add REQUIRES: shell in compiler-rt tests that use the ulimit command [llvm-lit] Add REQUIRES: shell in compiler-rt tests that use the ulimit command in lit internal shell Aug 20, 2024
@Harini0924
Copy link
Contributor Author

CC: @ilovepi @petrhosek @jh7370

Copy link

github-actions bot commented Aug 20, 2024

✅ With the latest revision this PR passed the C/C++ code formatter.

Added REQUIRES: shell in 6 files that use the ulimit command because lit
internal shell doesn't support this command.
@Harini0924 Harini0924 force-pushed the not-supported-ulimit branch from ed84ad2 to ce8cbb0 Compare August 21, 2024 17:24
@Harini0924
Copy link
Contributor Author

After discussing with @ilovepi we decided to rebase all the commits to fix the code formatter issues.

@Harini0924
Copy link
Contributor Author

@MaskRay I removed windows and FIXME. Should I change anything else in this patch?

@ilovepi
Copy link
Contributor

ilovepi commented Aug 23, 2024

Probably worth updating the title to [compiler-rt][test] Add REQUIRES: shell in tests that use the ulimit, since you're not changing anything in llvm-lit.

@Harini0924 Harini0924 changed the title [llvm-lit] Add REQUIRES: shell in compiler-rt tests that use the ulimit command in lit internal shell [compiler-rt][test] Add REQUIRES: shell in tests that use the ulimit command Aug 23, 2024
@Harini0924 Harini0924 merged commit 2d37e48 into llvm:main Aug 26, 2024
7 checks passed
@Harini0924 Harini0924 changed the title [compiler-rt][test] Add REQUIRES: shell in tests that use the ulimit command [compiler-rt][test] Add REQUIRES: shell to compiler-rt tests that use the ulimit command Aug 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[llvm-lit] Ulimit command not found in lit internal shell
5 participants