[compiler-rt] Fix some tests to work with lit internal shell. #160728

cmtice · 2025-09-25T16:17:11Z

This is part of our work to migrate lit tests to use internal shell by default (performance improvements).

llvmbot · 2025-09-25T16:18:33Z

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

Author: None (cmtice)

Changes

This is part of our work to migrate lit tests to use internal shell by default (performance improvements).

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

6 Files Affected:

(modified) compiler-rt/test/fuzzer/fork-sigusr.test (+3-3)
(modified) compiler-rt/test/fuzzer/sigint.test (+4-3)
(modified) compiler-rt/test/fuzzer/sigusr.test (+3-3)
(modified) compiler-rt/test/msan/allocator_mapping.cpp (+2-1)
(modified) compiler-rt/test/nsan/Posix/allocator_mapping.cpp (+2-1)
(modified) compiler-rt/test/tsan/ignore_lib0.cpp (+4-4)

diff --git a/compiler-rt/test/fuzzer/fork-sigusr.test b/compiler-rt/test/fuzzer/fork-sigusr.test
index 088e63cae4311..54fcb818793b8 100644
--- a/compiler-rt/test/fuzzer/fork-sigusr.test
+++ b/compiler-rt/test/fuzzer/fork-sigusr.test
@@ -1,14 +1,14 @@
 # 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
 RUN: %cpp_compiler %S/SleepOneSecondTest.cpp -o %t/ForkSIGUSR
 
-RUN: %run %t/ForkSIGUSR -fork=3 -ignore_crashes=1 2>%t/log & export PID=$!
+RUN: bash -c "nohup %run %t/ForkSIGUSR -fork=3 -ignore_crashes=1 2>%t/log &" | env
+RUN: ps auxx | grep ForkSIGUSR | grep -v "grep" | awk '{print $2}' | tr -d '\n' > %t2
 RUN: sleep 3
-RUN: kill -SIGUSR2 $PID
+RUN: kill -SIGUSR2 %{readfile:%t2}
 RUN: sleep 6
 RUN: cat %t/log | FileCheck %s --dump-input=fail
 
diff --git a/compiler-rt/test/fuzzer/sigint.test b/compiler-rt/test/fuzzer/sigint.test
index ac482d79b8e28..a64f37db46697 100644
--- a/compiler-rt/test/fuzzer/sigint.test
+++ b/compiler-rt/test/fuzzer/sigint.test
@@ -1,4 +1,4 @@
-REQUIRES: shell, msan
+REQUIRES: msan
 UNSUPPORTED: target=arm{{.*}}
 
 # Check that libFuzzer exits gracefully under SIGINT with MSan.
@@ -6,9 +6,10 @@ RUN: rm -rf %t
 RUN: mkdir -p %t
 RUN: %msan_compiler %S/SleepOneSecondTest.cpp -o %t/LFSIGINT
 
-RUN: %run %t/LFSIGINT 2> %t/log & export PID=$!
+RUN: bash -c "nohup %run %t/LFSIGINT 2> %t/log &" | env
+RUN: ps auxx | grep LFSIGINT | grep -v "grep" | awk '{print $2}' | tr -d '\n' > %t2
 RUN: sleep 2
-RUN: kill -SIGINT $PID
+RUN: kill -SIGINT %{readfile:%t2}
 RUN: sleep 3
 RUN: cat %t/log | FileCheck %s
 
diff --git a/compiler-rt/test/fuzzer/sigusr.test b/compiler-rt/test/fuzzer/sigusr.test
index c8a77ac63a6d7..0db6ce8e8b366 100644
--- a/compiler-rt/test/fuzzer/sigusr.test
+++ b/compiler-rt/test/fuzzer/sigusr.test
@@ -1,15 +1,15 @@
 # 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
 RUN: mkdir -p %t
 RUN: %cpp_compiler %S/SleepOneSecondTest.cpp -o %t/LFSIGUSR
 
-RUN: %run %t/LFSIGUSR 2> %t/log & export PID=$!
+RUN: bash -c "nohup %run %t/LFSIGUSR 2> %t/log &"| env
+RUN: ps auxx | grep LFSIGUSR | grep -v "grep" | awk '{print $2}' | tr -d '\n' > %t2
 RUN: sleep 2
-RUN: kill -SIGUSR1 $PID
+RUN: kill -SIGUSR1 %{readfile:%t2}
 RUN: sleep 3
 RUN: cat %t/log | FileCheck %s
 
diff --git a/compiler-rt/test/msan/allocator_mapping.cpp b/compiler-rt/test/msan/allocator_mapping.cpp
index e7a12da489152..aa48863861ecb 100644
--- a/compiler-rt/test/msan/allocator_mapping.cpp
+++ b/compiler-rt/test/msan/allocator_mapping.cpp
@@ -3,7 +3,8 @@
 // mapping the heap early, in __msan_init.
 //
 // RUN: %clangxx_msan -O0 %s -o %t_1
-// RUN: %clangxx_msan -O0 -DHEAP_ADDRESS=$(%run %t_1) %s -o %t_2 && %run %t_2
+// RUN: %run %t_1 > %t_3
+// RUN: %clangxx_msan -O0 -DHEAP_ADDRESS=%{readfile:%t_3} %s -o %t_2 && %run %t_2
 //
 // This test only makes sense for the 64-bit allocator. The 32-bit allocator
 // does not have a fixed mapping. Exclude platforms that use the 32-bit
diff --git a/compiler-rt/test/nsan/Posix/allocator_mapping.cpp b/compiler-rt/test/nsan/Posix/allocator_mapping.cpp
index 3a3e655e259d0..8be5008686385 100644
--- a/compiler-rt/test/nsan/Posix/allocator_mapping.cpp
+++ b/compiler-rt/test/nsan/Posix/allocator_mapping.cpp
@@ -2,7 +2,8 @@
 /// Test that a module constructor can not map memory over the NSan heap
 /// (without MAP_FIXED, of course).
 // RUN: %clangxx_nsan -O0 %s -o %t_1
-// RUN: %clangxx_nsan -O0 -DHEAP_ADDRESS=$(%run %t_1) %s -o %t_2 && %run %t_2
+// RUN: %run %t_1 > %t_3
+// RUN: %clangxx_nsan -O0 -DHEAP_ADDRESS=%{readfile:%t_3} %s -o %t_2 && %run %t_2
 
 #include <assert.h>
 #include <stdio.h>
diff --git a/compiler-rt/test/tsan/ignore_lib0.cpp b/compiler-rt/test/tsan/ignore_lib0.cpp
index cba58c6177038..c25439a49fe93 100644
--- a/compiler-rt/test/tsan/ignore_lib0.cpp
+++ b/compiler-rt/test/tsan/ignore_lib0.cpp
@@ -4,11 +4,12 @@
 // RUN: %clangxx_tsan -O1 -fno-builtin %s -DLIB -fPIC -fno-sanitize=thread -shared -o %t-dir/libignore_lib0.so
 // RUN: %clangxx_tsan -O1 %s -L%t-dir -lignore_lib0 %link_libcxx_tsan -o %t
 // RUN: echo running w/o suppressions:
-// RUN: env LD_LIBRARY_PATH=%t-dir${LD_LIBRARY_PATH:+:$LD_LIBRARY_PATH} %deflake %run %t | FileCheck %s --check-prefix=CHECK-NOSUPP
+// RUN: bash -c env | grep LD_LIBRARY_PATH | sed s/.*LD_LIBRARY_PATH=// > %t2
+// RUN: env LD_LIBRARY_PATH=%t-dir:%{readfile:%t2} %deflake %run %t | FileCheck %s --check-prefix=CHECK-NOSUPP
 // RUN: echo running with suppressions:
-// RUN: env LD_LIBRARY_PATH=%t-dir${LD_LIBRARY_PATH:+:$LD_LIBRARY_PATH} %env_tsan_opts=suppressions='%s.supp' %run %t 2>&1 | FileCheck %s --check-prefix=CHECK-WITHSUPP
+// RUN: env LD_LIBRARY_PATH=%t-dir:%{readfile:%t2} %env_tsan_opts=suppressions='%s.supp' %run %t 2>&1 | FileCheck %s --check-prefix=CHECK-WITHSUPP
 // RUN: echo running with generic suppression of noninstrumented code:
-// RUN: env LD_LIBRARY_PATH=%t-dir${LD_LIBRARY_PATH:+:$LD_LIBRARY_PATH} %env_tsan_opts=ignore_noninstrumented_modules=1 %run %t 2>&1 | FileCheck %s --check-prefix=CHECK-WITHSUPP
+// RUN: env LD_LIBRARY_PATH=%t-dir:%{readfile:%t2} %env_tsan_opts=ignore_noninstrumented_modules=1 %run %t 2>&1 | FileCheck %s --check-prefix=CHECK-WITHSUPP
 
 // Tests that interceptors coming from a library specified in called_from_lib
 // suppression are ignored.
@@ -37,4 +38,3 @@ int main() {
 
 // CHECK-WITHSUPP-NOT: WARNING: ThreadSanitizer: data race
 // CHECK-WITHSUPP: OK
-

compiler-rt/test/fuzzer/fork-sigusr.test

boomanaiden154

One minor comment, otherwise LGTM.

compiler-rt/test/fuzzer/fork-sigusr.test

fmayer · 2025-09-25T21:38:12Z

compiler-rt/test/tsan/ignore_lib0.cpp

@@ -4,11 +4,12 @@
 // RUN: %clangxx_tsan -O1 -fno-builtin %s -DLIB -fPIC -fno-sanitize=thread -shared -o %t-dir/libignore_lib0.so
 // RUN: %clangxx_tsan -O1 %s -L%t-dir -lignore_lib0 %link_libcxx_tsan -o %t
 // RUN: echo running w/o suppressions:
-// RUN: env LD_LIBRARY_PATH=%t-dir${LD_LIBRARY_PATH:+:$LD_LIBRARY_PATH} %deflake %run %t | FileCheck %s --check-prefix=CHECK-NOSUPP
+// RUN: bash -c env | grep LD_LIBRARY_PATH | sed s/.*LD_LIBRARY_PATH=// > %t2


don't we have a way to read the environment in the built in shell? This doesn't seem to reproduce what the original code did (not use : when LD_LIBRARY_PATH is unset)

The implementation of 'env' in the internal shell does not allow you to pipe it to other commands such as 'grep'. I was working on an implementation to fix that, but have not been able to get it to work yet.

When working on this test, I threw in some extra 'env' calls (temporarily) and looked at the output, both with the external shell and the internal shell, and made sure the LD_LIBRARY_PATH definitions we got in the internal shell were the same as the ones we got in the external shell.

I don't understand. If unset LD_LIBRARY_PATH, before it would be

env LD_LIBRARY_PATH=%t-dir

after

env LD_LIBRARY_PATH=%t-dir:

no?

Hm...I think the environments in which I ran my tests always had LD_LIBRARY_PATH defined by default. Let me go back and work on this a bit more (thank you for pointing this out).

Just wondering (since fixing this looks difficult): Does having a trailing colon, in the case where LD_LIBRARY_PATH was not previously set, actually cause anything to break?

It causes a difference in that it adds the current directory to the search path:

$ touch libc.so.6 $ LD_LIBRARY_PATH=/tmp /bin/true $ LD_LIBRARY_PATH=/tmp: /bin/true /bin/true: error while loading shared libraries: libc.so.6: file too short

Maybe we can add /dev/null instead if it's empty?

Or wouldn't sed s/.*LD_LIBRARY_PATH=/:/ give us the old behavior?

It ought to, but I'm still having problems with the internal shell. I'm going to revert this test for now, so the rest of the PR can be committed, while I continue debugging this in the internal shell.

dyung · 2025-09-26T19:55:39Z

Hi @cmtice, just in case you were not notified, I suspect your change is causing a failure on one of the build bots. Can you take a look and revert if you need time to investigate?

https://lab.llvm.org/buildbot/#/builders/174/builds/25130

…#160728)" This reverts commit e9185af.

cmtice · 2025-09-26T20:46:13Z

Hi @cmtice, just in case you were not notified, I suspect your change is causing a failure on one of the build bots. Can you take a look and revert if you need time to investigate?

https://lab.llvm.org/buildbot/#/builders/174/builds/25130

I've put out a PR to revert while I investigate this failure.

#160949) Reverts #160728 That appeared to be causing a buildbot failure; reverting this change while I investigate. https://lab.llvm.org/buildbot/#/builders/174/builds/25130

…rnal shell." (#160949) Reverts llvm/llvm-project#160728 That appeared to be causing a buildbot failure; reverting this change while I investigate. https://lab.llvm.org/buildbot/#/builders/174/builds/25130

…60728) This is part of our work to migrate lit tests to use internal shell by default (performance improvements).

llvm#160949) Reverts llvm#160728 That appeared to be causing a buildbot failure; reverting this change while I investigate. https://lab.llvm.org/buildbot/#/builders/174/builds/25130

…60728) This is part of our work to migrate lit tests to use internal shell by default (performance improvements).

llvm#160949) Reverts llvm#160728 That appeared to be causing a buildbot failure; reverting this change while I investigate. https://lab.llvm.org/buildbot/#/builders/174/builds/25130

[compiler-rt] Fix some tests to work with lit internal shell.

ae56b57

This is part of our work to migrate lit tests to use internal shell by default (performance improvements).

cmtice marked this pull request as ready for review September 25, 2025 16:18

llvmbot added compiler-rt compiler-rt:fuzzer compiler-rt:tsan Thread sanitizer compiler-rt:msan Memory sanitizer compiler-rt:sanitizer labels Sep 25, 2025

cmtice requested review from boomanaiden154, fmayer and lnihlen September 25, 2025 16:18

vitalybuka approved these changes Sep 25, 2025

View reviewed changes

fmayer reviewed Sep 25, 2025

View reviewed changes

compiler-rt/test/fuzzer/fork-sigusr.test Outdated Show resolved Hide resolved

boomanaiden154 approved these changes Sep 25, 2025

View reviewed changes

compiler-rt/test/fuzzer/fork-sigusr.test Outdated Show resolved Hide resolved

cmtice added 2 commits September 25, 2025 13:07

Add comment re ' | env'; remove extra 'x' from 'ps' options.

a383410

Use $! inside the bash -c command, to simplify things.

edd2f3d

fmayer reviewed Sep 25, 2025

View reviewed changes

Restore original version of tsan/ignore_lib0.cpp.

8a585b2

fmayer approved these changes Sep 26, 2025

View reviewed changes

cmtice merged commit e9185af into llvm:main Sep 26, 2025
9 checks passed

cmtice added a commit that referenced this pull request Sep 26, 2025

Revert "[compiler-rt] Fix some tests to work with lit internal shell. (…

82903ac

…#160728)" This reverts commit e9185af.

cmtice mentioned this pull request Sep 26, 2025

Revert "[compiler-rt] Fix some tests to work with lit internal shell." #160949

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[compiler-rt] Fix some tests to work with lit internal shell. #160728

[compiler-rt] Fix some tests to work with lit internal shell. #160728

Uh oh!

cmtice commented Sep 25, 2025

Uh oh!

llvmbot commented Sep 25, 2025

Uh oh!

Uh oh!

boomanaiden154 left a comment

Uh oh!

Uh oh!

fmayer Sep 25, 2025

Uh oh!

cmtice Sep 25, 2025

Uh oh!

fmayer Sep 25, 2025

Uh oh!

cmtice Sep 25, 2025

Uh oh!

cmtice Sep 25, 2025

Uh oh!

fmayer Sep 25, 2025

Uh oh!

fmayer Sep 25, 2025

Uh oh!

fmayer Sep 26, 2025

Uh oh!

cmtice Sep 26, 2025

Uh oh!

Uh oh!

dyung commented Sep 26, 2025

Uh oh!

cmtice commented Sep 26, 2025

Uh oh!

Uh oh!

[compiler-rt] Fix some tests to work with lit internal shell. #160728

[compiler-rt] Fix some tests to work with lit internal shell. #160728

Uh oh!

Conversation

cmtice commented Sep 25, 2025

Uh oh!

llvmbot commented Sep 25, 2025

Uh oh!

Uh oh!

boomanaiden154 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

dyung commented Sep 26, 2025

Uh oh!

cmtice commented Sep 26, 2025

Uh oh!

Uh oh!