Skip to content

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

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.

Sign up for GitHub

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 4 commits into from
Sep 26, 2025
Merged

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

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
ae56b57
[compiler-rt] Fix some tests to work with lit internal shell.
cmtice Sep 25, 2025
a383410
Add comment re ' | env'; remove extra 'x' from 'ps' options.
cmtice Sep 25, 2025
edd2f3d
Use $! inside the bash -c command, to simplify things.
cmtice Sep 25, 2025
8a585b2
Restore original version of tsan/ignore_lib0.cpp.
cmtice Sep 26, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 5 additions & 3 deletions compiler-rt/test/fuzzer/fork-sigusr.test
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,14 +1,16 @@
# 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=$!
# The line below needs the " | env" at the end, in order to make the
# script continue executing, rather than waiting (forever) for the
# 'nohup run...' command to finish.
RUN: bash -c "nohup %run %t/ForkSIGUSR -fork=3 -ignore_crashes=1 2>%t/log & echo -n $! > %t2" | env
RUN: sleep 3
RUN: kill -SIGUSR2 $PID
RUN: kill -SIGUSR2 %{readfile:%t2}
RUN: sleep 6
RUN: cat %t/log | FileCheck %s --dump-input=fail

Expand Down
9 changes: 6 additions & 3 deletions compiler-rt/test/fuzzer/sigint.test
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,14 +1,17 @@
REQUIRES: shell, msan
REQUIRES: msan
UNSUPPORTED: target=arm{{.*}}

# Check that libFuzzer exits gracefully under SIGINT with MSan.
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=$!
# The line below needs the " | env" at the end, in order to make the
# script continue executing, rather than waiting (forever) for the
# 'nohup run...' command to finish.
RUN: bash -c "nohup %run %t/LFSIGINT 2> %t/log & echo -n $! > %t2" | env
RUN: sleep 2
RUN: kill -SIGINT $PID
RUN: kill -SIGINT %{readfile:%t2}
RUN: sleep 3
RUN: cat %t/log | FileCheck %s

Expand Down
8 changes: 5 additions & 3 deletions compiler-rt/test/fuzzer/sigusr.test
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,15 +1,17 @@
# 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=$!
# The line below needs the " | env" at the end, in order to make the
# script continue executing, rather than waiting (forever) for the
# 'nohup run...' command to finish.
RUN: bash -c "nohup %run %t/LFSIGUSR 2> %t/log & echo -n $! > %t2"| env
RUN: sleep 2
RUN: kill -SIGUSR1 $PID
RUN: kill -SIGUSR1 %{readfile:%t2}
RUN: sleep 3
RUN: cat %t/log | FileCheck %s

Expand Down
3 changes: 2 additions & 1 deletion compiler-rt/test/msan/allocator_mapping.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
3 changes: 2 additions & 1 deletion compiler-rt/test/nsan/Posix/allocator_mapping.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down
8 changes: 4 additions & 4 deletions compiler-rt/test/tsan/ignore_lib0.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Copy link
Contributor

@fmayer fmayer Sep 25, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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)

Copy link
Contributor Author

@cmtice cmtice Sep 25, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor

@fmayer fmayer Sep 25, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Copy link
Contributor Author

@cmtice cmtice Sep 25, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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).

Copy link
Contributor Author

@cmtice cmtice Sep 25, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Copy link
Contributor

@fmayer fmayer Sep 25, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor

@fmayer fmayer Sep 25, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor

@fmayer fmayer Sep 26, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor Author

@cmtice cmtice Sep 26, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

// 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.
Expand Down Expand Up @@ -37,4 +38,3 @@ int main() {

// CHECK-WITHSUPP-NOT: WARNING: ThreadSanitizer: data race
// CHECK-WITHSUPP: OK

Loading