Skip to content
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

[llvm-lit] Fix error in compiler-rt tests when using lit internal shell with env #102069

Merged
merged 1 commit into from
Aug 13, 2024

Conversation

Harini0924
Copy link
Contributor

@Harini0924 Harini0924 commented Aug 5, 2024

When running tests in compiler-rt using the lit internal shell with the following command:

LIT_USE_INTERNAL_SHELL=1 ninja check-compiler-rt

one common error that occurs is:

'XRAY_OPTIONS=patch_premain=false:verbosity=1': command not found

This error, along with over 50 similar "environment variable not found" errors, appears across 35 files in complier-rt. These errors happen because the environment variables are not being set correctly when the internal shell is used, leading to commands failing due to unrecognized environment variables.

This patch addresses the issue by using the env command to properly set the environment variables before running the tests. By explicitly setting the environment variables through env, the internal shell can correctly interpret and apply them, allowing the tests to pass.
fixes: #106598
link to RFC

Copy link

github-actions bot commented Aug 5, 2024

Thank you for submitting a Pull Request (PR) to the LLVM Project!

This PR will be automatically labeled and the relevant teams will be
notified.

If you wish to, you can add reviewers by using the "Reviewers" section on this page.

If this is not working for you, it is probably because you do not have write
permissions for the repository. In which case you can instead tag reviewers by
name in a comment by using @ followed by their GitHub username.

If you have received no comments on your PR for a week, you can request a review
by "ping"ing the PR by adding a comment “Ping”. The common courtesy "ping" rate
is once a week. Please remember that you are asking for valuable time from other developers.

If you have further questions, they may be answered by the LLVM GitHub User Guide.

You can also ask questions in a comment on this PR, on the LLVM Discord or on the forums.

@Harini0924
Copy link
Contributor Author

CC: @ilovepi @petrhosek

@llvmbot
Copy link
Member

llvmbot commented Aug 5, 2024

@llvm/pr-subscribers-pgo
@llvm/pr-subscribers-xray

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

Author: None (Harini0924)

Changes

There are several files in compiler-rt that have "command not found" errors. This patch uses the 'env' command to properly set the environment variables correctly, allowing tests to pass.


Patch is 33.95 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/102069.diff

34 Files Affected:

  • (modified) compiler-rt/test/dfsan/custom.cpp (+2-2)
  • (modified) compiler-rt/test/dfsan/flags.c (+3-3)
  • (modified) compiler-rt/test/dfsan/fork.cpp (+1-1)
  • (modified) compiler-rt/test/dfsan/origin_limit.c (+2-2)
  • (modified) compiler-rt/test/msan/Linux/sendmsg.cpp (+4-4)
  • (modified) compiler-rt/test/msan/chained_origin_empty_stack.cpp (+1-1)
  • (modified) compiler-rt/test/msan/chained_origin_limits.cpp (+16-16)
  • (modified) compiler-rt/test/msan/coverage-levels.cpp (+4-4)
  • (modified) compiler-rt/test/msan/dtor-member.cpp (+1-1)
  • (modified) compiler-rt/test/msan/fork.cpp (+1-1)
  • (modified) compiler-rt/test/msan/interception_sigaction_test.cpp (+1-1)
  • (modified) compiler-rt/test/msan/memcmp_test.cpp (+1-1)
  • (modified) compiler-rt/test/msan/msan_check_mem_is_initialized.cpp (+1-1)
  • (modified) compiler-rt/test/msan/poison_in_free.cpp (+1-1)
  • (modified) compiler-rt/test/msan/print_stats.cpp (+4-4)
  • (modified) compiler-rt/test/msan/recover-dso.cpp (+4-4)
  • (modified) compiler-rt/test/msan/recover.cpp (+7-7)
  • (modified) compiler-rt/test/msan/release_origin.c (+3-3)
  • (modified) compiler-rt/test/msan/strcmp.c (+2-2)
  • (modified) compiler-rt/test/msan/strndup.cpp (+1-1)
  • (modified) compiler-rt/test/sanitizer_common/TestCases/Linux/signal_segv_handler.cpp (+1-1)
  • (modified) compiler-rt/test/ubsan/TestCases/Misc/Linux/static-link.cpp (+1-1)
  • (modified) compiler-rt/test/xray/TestCases/Posix/arg1-arg0-logging.cpp (+1-1)
  • (modified) compiler-rt/test/xray/TestCases/Posix/arg1-logger.cpp (+1-1)
  • (modified) compiler-rt/test/xray/TestCases/Posix/arg1-logging-implicit-this.cpp (+1-1)
  • (modified) compiler-rt/test/xray/TestCases/Posix/argv0-log-file-name.cpp (+1-1)
  • (modified) compiler-rt/test/xray/TestCases/Posix/coverage-sample.cpp (+2-2)
  • (modified) compiler-rt/test/xray/TestCases/Posix/fdr-reinit.cpp (+1-1)
  • (modified) compiler-rt/test/xray/TestCases/Posix/fixedsize-logging.cpp (+1-1)
  • (modified) compiler-rt/test/xray/TestCases/Posix/func-id-utils.cpp (+2-2)
  • (modified) compiler-rt/test/xray/TestCases/Posix/optional-inmemory-log.cpp (+1-1)
  • (modified) compiler-rt/test/xray/TestCases/Posix/patching-unpatching.cpp (+2-2)
  • (modified) compiler-rt/test/xray/TestCases/Posix/pic_test.cpp (+1-1)
  • (modified) compiler-rt/test/xray/TestCases/Posix/typed-event-logging.cpp (+1-1)
diff --git a/compiler-rt/test/dfsan/custom.cpp b/compiler-rt/test/dfsan/custom.cpp
index 54bb17cb4a035..873af5cd934e2 100644
--- a/compiler-rt/test/dfsan/custom.cpp
+++ b/compiler-rt/test/dfsan/custom.cpp
@@ -1,7 +1,7 @@
-// RUN: %clang_dfsan %s -o %t && DFSAN_OPTIONS="strict_data_dependencies=0" %run %t
+// RUN: %clang_dfsan %s -o %t && env DFSAN_OPTIONS="strict_data_dependencies=0" %run %t
 // RUN: %clang_dfsan -DSTRICT_DATA_DEPENDENCIES %s -o %t && %run %t
 // RUN: %clang_dfsan -DORIGIN_TRACKING -mllvm -dfsan-track-origins=1 -mllvm -dfsan-combine-pointer-labels-on-load=false -DSTRICT_DATA_DEPENDENCIES %s -o %t && %run %t
-// RUN: %clang_dfsan -DORIGIN_TRACKING -mllvm -dfsan-track-origins=1 -mllvm -dfsan-combine-pointer-labels-on-load=false -no-pie %s -o %t && DFSAN_OPTIONS="strict_data_dependencies=0" %run %t
+// RUN: %clang_dfsan -DORIGIN_TRACKING -mllvm -dfsan-track-origins=1 -mllvm -dfsan-combine-pointer-labels-on-load=false -no-pie %s -o %t && env DFSAN_OPTIONS="strict_data_dependencies=0" %run %t
 //
 // Tests custom implementations of various glibc functions.
 
diff --git a/compiler-rt/test/dfsan/flags.c b/compiler-rt/test/dfsan/flags.c
index 3f68dbc04ad5d..7115f002e9ec8 100644
--- a/compiler-rt/test/dfsan/flags.c
+++ b/compiler-rt/test/dfsan/flags.c
@@ -1,6 +1,6 @@
-// RUN: %clang_dfsan %s -fsanitize-ignorelist=%S/Inputs/flags_abilist.txt -mllvm -dfsan-debug-nonzero-labels -o %t && DFSAN_OPTIONS=warn_unimplemented=1 %run %t 2>&1 | FileCheck %s
-// RUN: %clang_dfsan %s -fsanitize-ignorelist=%S/Inputs/flags_abilist.txt -mllvm -dfsan-debug-nonzero-labels -o %t && DFSAN_OPTIONS=warn_unimplemented=0 %run %t 2>&1 | count 0
-// RUN: %clang_dfsan %s -fsanitize-ignorelist=%S/Inputs/flags_abilist.txt -mllvm -dfsan-debug-nonzero-labels -o %t && DFSAN_OPTIONS=warn_nonzero_labels=1 %run %t 2>&1 | FileCheck --check-prefix=CHECK-NONZERO %s
+// RUN: %clang_dfsan %s -fsanitize-ignorelist=%S/Inputs/flags_abilist.txt -mllvm -dfsan-debug-nonzero-labels -o %t && env DFSAN_OPTIONS=warn_unimplemented=1 %run %t 2>&1 | FileCheck %s
+// RUN: %clang_dfsan %s -fsanitize-ignorelist=%S/Inputs/flags_abilist.txt -mllvm -dfsan-debug-nonzero-labels -o %t && env DFSAN_OPTIONS=warn_unimplemented=0 %run %t 2>&1 | count 0
+// RUN: %clang_dfsan %s -fsanitize-ignorelist=%S/Inputs/flags_abilist.txt -mllvm -dfsan-debug-nonzero-labels -o %t && env DFSAN_OPTIONS=warn_nonzero_labels=1 %run %t 2>&1 | FileCheck --check-prefix=CHECK-NONZERO %s
 
 // Tests that flags work correctly.
 
diff --git a/compiler-rt/test/dfsan/fork.cpp b/compiler-rt/test/dfsan/fork.cpp
index c4c23f3e39b16..ddf35a248e003 100644
--- a/compiler-rt/test/dfsan/fork.cpp
+++ b/compiler-rt/test/dfsan/fork.cpp
@@ -6,7 +6,7 @@
 // RUN: %run %t 2>&1 | FileCheck %s
 //
 // RUN: %clangxx_dfsan -mllvm -dfsan-track-origins=1 %s -o %t
-// RUN: DFSAN_OPTIONS=store_context_size=1000,origin_history_size=0,origin_history_per_stack_limit=0 %run %t 2>&1 | FileCheck %s
+// RUN: env DFSAN_OPTIONS=store_context_size=1000,origin_history_size=0,origin_history_per_stack_limit=0 %run %t 2>&1 | FileCheck %s
 
 #include <assert.h>
 #include <errno.h>
diff --git a/compiler-rt/test/dfsan/origin_limit.c b/compiler-rt/test/dfsan/origin_limit.c
index 2cc7c5ff7123b..03e207134f6f5 100644
--- a/compiler-rt/test/dfsan/origin_limit.c
+++ b/compiler-rt/test/dfsan/origin_limit.c
@@ -3,10 +3,10 @@
 // RUN: %run %t >%t.out 2>&1
 // RUN: FileCheck %s < %t.out
 //
-// RUN: DFSAN_OPTIONS=origin_history_size=2 %run %t >%t.out 2>&1
+// RUN: env DFSAN_OPTIONS=origin_history_size=2 %run %t >%t.out 2>&1
 // RUN: FileCheck %s --check-prefix=CHECK2 < %t.out
 //
-// RUN: DFSAN_OPTIONS=origin_history_size=0 %run %t >%t.out 2>&1
+// RUN: env DFSAN_OPTIONS=origin_history_size=0 %run %t >%t.out 2>&1
 // RUN: FileCheck %s --check-prefix=CHECK0 < %t.out
 
 #include <sanitizer/dfsan_interface.h>
diff --git a/compiler-rt/test/msan/Linux/sendmsg.cpp b/compiler-rt/test/msan/Linux/sendmsg.cpp
index 6089f595a6687..6cd33a90cb705 100644
--- a/compiler-rt/test/msan/Linux/sendmsg.cpp
+++ b/compiler-rt/test/msan/Linux/sendmsg.cpp
@@ -9,13 +9,13 @@
 // RUN: %clangxx_msan %s -DSENDMMSG -o %t && %run %t 2>&1 | FileCheck %s --check-prefix=NEGATIVE
 
 // RUN: %clangxx_msan %s -DSEND -DPOISON -o %t && \
-// RUN:   MSAN_OPTIONS=intercept_send=0 %run %t 2>&1 | FileCheck %s --check-prefix=NEGATIVE
+// RUN:   env MSAN_OPTIONS=intercept_send=0 %run %t 2>&1 | FileCheck %s --check-prefix=NEGATIVE
 // RUN: %clangxx_msan %s -DSENDTO -DPOISON -o %t && \
-// RUN:   MSAN_OPTIONS=intercept_send=0 %run %t 2>&1 | FileCheck %s --check-prefix=NEGATIVE
+// RUN:   env MSAN_OPTIONS=intercept_send=0 %run %t 2>&1 | FileCheck %s --check-prefix=NEGATIVE
 // RUN: %clangxx_msan %s -DSENDMSG -DPOISON -o %t && \
-// RUN:   MSAN_OPTIONS=intercept_send=0 %run %t 2>&1 | FileCheck %s --check-prefix=NEGATIVE
+// RUN:   env MSAN_OPTIONS=intercept_send=0 %run %t 2>&1 | FileCheck %s --check-prefix=NEGATIVE
 // RUN: %clangxx_msan %s -DSENDMMSG -DPOISON -o %t && \
-// RUN:   MSAN_OPTIONS=intercept_send=0 %run %t 2>&1 | FileCheck %s --check-prefix=NEGATIVE
+// RUN:   env MSAN_OPTIONS=intercept_send=0 %run %t 2>&1 | FileCheck %s --check-prefix=NEGATIVE
 
 // UNSUPPORTED: android
 
diff --git a/compiler-rt/test/msan/chained_origin_empty_stack.cpp b/compiler-rt/test/msan/chained_origin_empty_stack.cpp
index 101016f2d347c..1666a882a7e2f 100644
--- a/compiler-rt/test/msan/chained_origin_empty_stack.cpp
+++ b/compiler-rt/test/msan/chained_origin_empty_stack.cpp
@@ -1,5 +1,5 @@
 // RUN: %clangxx_msan -fno-sanitize-memory-param-retval -fsanitize-memory-track-origins=2 -O3 %s -o %t && \
-// RUN:     MSAN_OPTIONS=store_context_size=1 not %run %t 2>&1 | FileCheck %s
+// RUN:     env MSAN_OPTIONS=store_context_size=1 not %run %t 2>&1 | FileCheck %s
 
 // Test that stack trace for the intermediate store is not empty.
 
diff --git a/compiler-rt/test/msan/chained_origin_limits.cpp b/compiler-rt/test/msan/chained_origin_limits.cpp
index 9585889eb37a9..3e146b7054f5c 100644
--- a/compiler-rt/test/msan/chained_origin_limits.cpp
+++ b/compiler-rt/test/msan/chained_origin_limits.cpp
@@ -3,63 +3,63 @@
 // Heap origin.
 // RUN: %clangxx_msan -fsanitize-memory-track-origins=2 -O3 %s -o %t
 
-// RUN: MSAN_OPTIONS=origin_history_size=7 not %run %t >%t.out 2>&1
+// RUN: env MSAN_OPTIONS=origin_history_size=7 not %run %t >%t.out 2>&1
 // RUN: FileCheck %s --check-prefix=CHECK7 < %t.out
 
-// RUN: MSAN_OPTIONS=origin_history_size=2 not %run %t >%t.out 2>&1
+// RUN: env MSAN_OPTIONS=origin_history_size=2 not %run %t >%t.out 2>&1
 // RUN: FileCheck %s --check-prefix=CHECK2 < %t.out
 
-// RUN: MSAN_OPTIONS=origin_history_per_stack_limit=1 not %run %t >%t.out 2>&1
+// RUN: env MSAN_OPTIONS=origin_history_per_stack_limit=1 not %run %t >%t.out 2>&1
 // RUN: FileCheck %s --check-prefix=CHECK-PER-STACK --check-prefix=CHECK-%short-stack < %t.out
 
-// RUN: MSAN_OPTIONS=origin_history_size=7,origin_history_per_stack_limit=0 not %run %t >%t.out 2>&1
+// RUN: env MSAN_OPTIONS=origin_history_size=7,origin_history_per_stack_limit=0 not %run %t >%t.out 2>&1
 // RUN: FileCheck %s --check-prefix=CHECK7 < %t.out
 
 // Stack origin.
 // RUN: %clangxx_msan -DSTACK -fsanitize-memory-track-origins=2 -O3 %s -o %t
 
-// RUN: MSAN_OPTIONS=origin_history_size=7 not %run %t >%t.out 2>&1
+// RUN: env MSAN_OPTIONS=origin_history_size=7 not %run %t >%t.out 2>&1
 // RUN: FileCheck %s --check-prefix=CHECK7 < %t.out
 
-// RUN: MSAN_OPTIONS=origin_history_size=2 not %run %t >%t.out 2>&1
+// RUN: env MSAN_OPTIONS=origin_history_size=2 not %run %t >%t.out 2>&1
 // RUN: FileCheck %s --check-prefix=CHECK2 < %t.out
 
-// RUN: MSAN_OPTIONS=origin_history_per_stack_limit=1 not %run %t >%t.out 2>&1
+// RUN: env MSAN_OPTIONS=origin_history_per_stack_limit=1 not %run %t >%t.out 2>&1
 // RUN: FileCheck %s --check-prefix=CHECK-PER-STACK --check-prefix=CHECK-%short-stack < %t.out
 
-// RUN: MSAN_OPTIONS=origin_history_size=7,origin_history_per_stack_limit=0 not %run %t >%t.out 2>&1
+// RUN: env MSAN_OPTIONS=origin_history_size=7,origin_history_per_stack_limit=0 not %run %t >%t.out 2>&1
 // RUN: FileCheck %s --check-prefix=CHECK7 < %t.out
 
 
 // Heap origin, with calls.
 // RUN: %clangxx_msan -mllvm -msan-instrumentation-with-call-threshold=0 -fsanitize-memory-track-origins=2 -O3 %s -o %t
 
-// RUN: MSAN_OPTIONS=origin_history_size=7 not %run %t >%t.out 2>&1
+// RUN: env MSAN_OPTIONS=origin_history_size=7 not %run %t >%t.out 2>&1
 // RUN: FileCheck %s --check-prefix=CHECK7 < %t.out
 
-// RUN: MSAN_OPTIONS=origin_history_size=2 not %run %t >%t.out 2>&1
+// RUN: env MSAN_OPTIONS=origin_history_size=2 not %run %t >%t.out 2>&1
 // RUN: FileCheck %s --check-prefix=CHECK2 < %t.out
 
-// RUN: MSAN_OPTIONS=origin_history_per_stack_limit=1 not %run %t >%t.out 2>&1
+// RUN: env MSAN_OPTIONS=origin_history_per_stack_limit=1 not %run %t >%t.out 2>&1
 // RUN: FileCheck %s --check-prefix=CHECK-PER-STACK --check-prefix=CHECK-%short-stack < %t.out
 
-// RUN: MSAN_OPTIONS=origin_history_size=7,origin_history_per_stack_limit=0 not %run %t >%t.out 2>&1
+// RUN: env MSAN_OPTIONS=origin_history_size=7,origin_history_per_stack_limit=0 not %run %t >%t.out 2>&1
 // RUN: FileCheck %s --check-prefix=CHECK7 < %t.out
 
 
 // Stack origin, with calls.
 // RUN: %clangxx_msan -DSTACK -mllvm -msan-instrumentation-with-call-threshold=0 -fsanitize-memory-track-origins=2 -O3 %s -o %t
 
-// RUN: MSAN_OPTIONS=origin_history_size=7 not %run %t >%t.out 2>&1
+// RUN: env MSAN_OPTIONS=origin_history_size=7 not %run %t >%t.out 2>&1
 // RUN: FileCheck %s --check-prefix=CHECK7 < %t.out
 
-// RUN: MSAN_OPTIONS=origin_history_size=2 not %run %t >%t.out 2>&1
+// RUN: env MSAN_OPTIONS=origin_history_size=2 not %run %t >%t.out 2>&1
 // RUN: FileCheck %s --check-prefix=CHECK2 < %t.out
 
-// RUN: MSAN_OPTIONS=origin_history_per_stack_limit=1 not %run %t >%t.out 2>&1
+// RUN: env MSAN_OPTIONS=origin_history_per_stack_limit=1 not %run %t >%t.out 2>&1
 // RUN: FileCheck %s --check-prefix=CHECK-PER-STACK --check-prefix=CHECK-%short-stack < %t.out
 
-// RUN: MSAN_OPTIONS=origin_history_size=7,origin_history_per_stack_limit=0 not %run %t >%t.out 2>&1
+// RUN: env MSAN_OPTIONS=origin_history_size=7,origin_history_per_stack_limit=0 not %run %t >%t.out 2>&1
 // RUN: FileCheck %s --check-prefix=CHECK7 < %t.out
 
 #include <stdio.h>
diff --git a/compiler-rt/test/msan/coverage-levels.cpp b/compiler-rt/test/msan/coverage-levels.cpp
index 1b7778e9d7aa8..0133381580853 100644
--- a/compiler-rt/test/msan/coverage-levels.cpp
+++ b/compiler-rt/test/msan/coverage-levels.cpp
@@ -2,13 +2,13 @@
 //
 // RUN: %clangxx_msan -DINIT_VAR=1 -O1 -fsanitize-coverage=func,trace-pc-guard  %s -o %t
 // RUN: mkdir -p %t-dir
-// RUN: MSAN_OPTIONS=coverage=1:verbosity=1:coverage_dir=%t-dir %run %t 2>&1 | FileCheck %s --check-prefix=CHECK1 --check-prefix=CHECK_NOWARN
+// RUN: env MSAN_OPTIONS=coverage=1:verbosity=1:coverage_dir=%t-dir %run %t 2>&1 | FileCheck %s --check-prefix=CHECK1 --check-prefix=CHECK_NOWARN
 // RUN: %clangxx_msan -O1 -fsanitize-coverage=func,trace-pc-guard  %s -o %t
-// RUN: MSAN_OPTIONS=coverage=1:verbosity=1:coverage_dir=%t-dir not %run %t 2>&1 | FileCheck %s --check-prefix=CHECK1 --check-prefix=CHECK_WARN
+// RUN: env MSAN_OPTIONS=coverage=1:verbosity=1:coverage_dir=%t-dir not %run %t 2>&1 | FileCheck %s --check-prefix=CHECK1 --check-prefix=CHECK_WARN
 // RUN: %clangxx_msan -O1 -fsanitize-coverage=bb,trace-pc-guard  %s -o %t
-// RUN: MSAN_OPTIONS=coverage=1:verbosity=1:coverage_dir=%t-dir not %run %t 2>&1 | FileCheck %s --check-prefix=CHECK2 --check-prefix=CHECK_WARN
+// RUN: env MSAN_OPTIONS=coverage=1:verbosity=1:coverage_dir=%t-dir not %run %t 2>&1 | FileCheck %s --check-prefix=CHECK2 --check-prefix=CHECK_WARN
 // RUN: %clangxx_msan -O1 -fsanitize-coverage=edge,trace-pc-guard  %s -o %t
-// RUN: MSAN_OPTIONS=coverage=1:verbosity=1:coverage_dir=%t-dir not %run %t 2>&1 | FileCheck %s --check-prefix=CHECK3 --check-prefix=CHECK_WARN
+// RUN: env MSAN_OPTIONS=coverage=1:verbosity=1:coverage_dir=%t-dir not %run %t 2>&1 | FileCheck %s --check-prefix=CHECK3 --check-prefix=CHECK_WARN
 
 volatile int sink;
 int main(int argc, char **argv) {
diff --git a/compiler-rt/test/msan/dtor-member.cpp b/compiler-rt/test/msan/dtor-member.cpp
index 6092c8a3a3fdd..39533dbaa1c1b 100644
--- a/compiler-rt/test/msan/dtor-member.cpp
+++ b/compiler-rt/test/msan/dtor-member.cpp
@@ -10,7 +10,7 @@
 // RUN: %clangxx_msan %s -fsanitize=memory -fno-sanitize-memory-use-after-dtor -o %t &&  %run %t >%t.out 2>&1
 // RUN: FileCheck %s --check-prefix=CHECK-NO-FLAG < %t.out
 
-// RUN: %clangxx_msan -fsanitize=memory -fsanitize-memory-use-after-dtor %s -o %t && MSAN_OPTIONS=poison_in_dtor=0 %run %t >%t.out 2>&1
+// RUN: %clangxx_msan -fsanitize=memory -fsanitize-memory-use-after-dtor %s -o %t && env MSAN_OPTIONS=poison_in_dtor=0 %run %t >%t.out 2>&1
 // RUN: FileCheck %s --check-prefix=CHECK-NO-FLAG < %t.out
 
 #include <sanitizer/msan_interface.h>
diff --git a/compiler-rt/test/msan/fork.cpp b/compiler-rt/test/msan/fork.cpp
index c0d6cb7f02426..438766ab4d99c 100644
--- a/compiler-rt/test/msan/fork.cpp
+++ b/compiler-rt/test/msan/fork.cpp
@@ -3,7 +3,7 @@
 // and verify that origin reads do not deadlock in the child process.
 
 // RUN: %clangxx_msan -std=c++11 -fsanitize-memory-track-origins=2 -g -O3 %s -o %t
-// RUN: MSAN_OPTIONS=store_context_size=1000,origin_history_size=0,origin_history_per_stack_limit=0 %run %t 2>&1 | FileCheck %s
+// RUN: env MSAN_OPTIONS=store_context_size=1000,origin_history_size=0,origin_history_per_stack_limit=0 %run %t 2>&1 | FileCheck %s
 
 // Fun fact: if test output is redirected to a file (as opposed to
 // being piped directly to FileCheck), we may lose some "done"s due to
diff --git a/compiler-rt/test/msan/interception_sigaction_test.cpp b/compiler-rt/test/msan/interception_sigaction_test.cpp
index 282771923960e..657ce1a6db0c7 100644
--- a/compiler-rt/test/msan/interception_sigaction_test.cpp
+++ b/compiler-rt/test/msan/interception_sigaction_test.cpp
@@ -1,5 +1,5 @@
 // RUN: %clangxx_msan -O0 -g %s -o %t
-// RUN: MSAN_OPTIONS=handle_segv=2 %t 2>&1 | FileCheck %s
+// RUN: env MSAN_OPTIONS=handle_segv=2 %t 2>&1 | FileCheck %s
 #include <stdlib.h>
 #include <stdio.h>
 #include <unistd.h>
diff --git a/compiler-rt/test/msan/memcmp_test.cpp b/compiler-rt/test/msan/memcmp_test.cpp
index 42230cc4d0c54..4ef7dc4d501ba 100644
--- a/compiler-rt/test/msan/memcmp_test.cpp
+++ b/compiler-rt/test/msan/memcmp_test.cpp
@@ -1,6 +1,6 @@
 // RUN: %clangxx_msan -O0 -g %s -o %t
 // RUN: not %run %t 2>&1 | FileCheck %s
-// RUN: MSAN_OPTIONS=intercept_memcmp=0 %run %t
+// RUN: env MSAN_OPTIONS=intercept_memcmp=0 %run %t
 
 #include <string.h>
 #include <stdio.h>
diff --git a/compiler-rt/test/msan/msan_check_mem_is_initialized.cpp b/compiler-rt/test/msan/msan_check_mem_is_initialized.cpp
index 81752291c7c5c..aaf5737ebe236 100644
--- a/compiler-rt/test/msan/msan_check_mem_is_initialized.cpp
+++ b/compiler-rt/test/msan/msan_check_mem_is_initialized.cpp
@@ -1,6 +1,6 @@
 // RUN: %clangxx_msan -O0 -g -DPOSITIVE %s -o %t
 // RUN: not %run %t 2>&1 | FileCheck %s
-// RUN: MSAN_OPTIONS=verbosity=1 not %run %t 2>&1 | \
+// RUN: env MSAN_OPTIONS=verbosity=1 not %run %t 2>&1 | \
 // RUN:     FileCheck %s --check-prefix=CHECK --check-prefix=CHECK-VERBOSE
 
 // RUN: %clangxx_msan -O0 -g %s -o %t && %run %t
diff --git a/compiler-rt/test/msan/poison_in_free.cpp b/compiler-rt/test/msan/poison_in_free.cpp
index e144c147a3251..4c90df25cfa44 100644
--- a/compiler-rt/test/msan/poison_in_free.cpp
+++ b/compiler-rt/test/msan/poison_in_free.cpp
@@ -1,6 +1,6 @@
 // RUN: %clangxx_msan -O0 %s -o %t && not %run %t >%t.out 2>&1
 // FileCheck %s <%t.out
-// RUN: %clangxx_msan -O0 %s -o %t && MSAN_OPTIONS=poison_in_free=0 %run %t >%t.out 2>&1
+// RUN: %clangxx_msan -O0 %s -o %t && env env MSAN_OPTIONS=poison_in_free=0 %run %t >%t.out 2>&1
 
 #include <stdio.h>
 #include <stdlib.h>
diff --git a/compiler-rt/test/msan/print_stats.cpp b/compiler-rt/test/msan/print_stats.cpp
index 5b46d4ebbff24..d9a78a5329473 100644
--- a/compiler-rt/test/msan/print_stats.cpp
+++ b/compiler-rt/test/msan/print_stats.cpp
@@ -1,21 +1,21 @@
 // RUN: %clangxx_msan -fsanitize-memory-track-origins=2 -g %s -o %t 
 // RUN: %run %t 2>&1 | \
 // RUN:   FileCheck --check-prefixes=CHECK,CHECK-NOSTATS %s
-// RUN: MSAN_OPTIONS=print_stats=1 %run %t 2>&1 | \
+// RUN: env MSAN_OPTIONS=print_stats=1 %run %t 2>&1 | \
 // RUN:   FileCheck --check-prefixes=CHECK,CHECK-NOSTATS %s
-// RUN: MSAN_OPTIONS=print_stats=1,atexit=1 %run %t 2>&1 | \
+// RUN: env MSAN_OPTIONS=print_stats=1,atexit=1 %run %t 2>&1 | \
 // RUN:   FileCheck --check-prefixes=CHECK,CHECK-STATS %s
 
 // RUN: %clangxx_msan -fsanitize-memory-track-origins=2 -g -DPOSITIVE=1 %s -o %t 
 // RUN: not %run %t 2>&1 | \
 // RUN:   FileCheck --check-prefixes=CHECK,CHECK-NOSTATS %s
-// RUN: MSAN_OPTIONS=print_stats=1 not %run %t 2>&1 | \
+// RUN: env MSAN_OPTIONS=print_stats=1 not %run %t 2>&1 | \
 // RUN:   FileCheck --check-prefixes=CHECK,CHECK-STATS %s
 
 // RUN: %clangxx_msan -fsanitize-memory-track-origins=2 -fsanitize-recover=memory -g -DPOSITIVE=1 %s -o %t
 // RUN: not %run %t 2>&1 | \
 // RUN:  FileCheck --check-prefixes=CHECK,CHECK-NOSTATS,CHECK-RECOVER %s
-// RUN: MSAN_OPTIONS=print_stats=1 not %run %t 2>&1 | \
+// RUN: env MSAN_OPTIONS=print_stats=1 not %run %t 2>&1 | \
 // RUN:   FileCheck --check-prefixes=CHECK,CHECK-STATS,CHECK-RECOVER %s
 
 #include <stdio.h>
diff --git a/compiler-rt/test/msan/recover-dso.cpp b/compiler-rt/test/msan/recover-dso.cpp
index 2f4225659dd43..f24758aee9332 100644
--- a/compiler-rt/test/msan/recover-dso.cpp
+++ b/compiler-rt/test/msan/recover-dso.cpp
@@ -1,8 +1,8 @@
 // RUN: %clangxx_msan -O0 %s -o %t && not %run %t >%t.out 2>&1
 // FileCheck --check-prefix=CHECK-RECOVER %s <%t.out
-// RUN: %clangxx_msan -O0 %s -o %t && MSAN_OPTIONS=keep_going=0 not %run %t >%t.out 2>&1
+// RUN: %clangxx_msan -O0 %s -o %t && env MSAN_OPTIONS=keep_going=0 not %run %t >%t.out 2>&1
 // FileCheck %s <%t.out
-// RUN: %clangxx_msan -O0 %s -o %t && MSAN_OPTIONS=keep_going=1 not %run %t >%t.out 2>&1
+// RUN: %clangxx_msan -O0 %s -o %t && env MSAN_OPTIONS=keep_going=1 not %run %t >%t.out 2>&1
 // FileCheck --check-prefix=CHECK-RECOVER %s <%t.out
 
 // Test how -fsanitize-recover=memory and MSAN_OPTIONS=keep_going affect reports
@@ -12,9 +12,9 @@
 
 // RUN: %clangxx_msan -fsanitize-recover=memory -O0 %s -o %t && not %run %t >%t.out 2>&1
 // FileCheck --check-prefix=CHECK-RECOVER %s <%t.out
-// RUN: %clangxx_msan -fsanitize-recover=memory -O0 %s -o %t && MSAN_OPTIONS=keep_going=0 not %run %t >%t.out 2>&1
+// RUN: %clangxx_msan -fsanitize-recover=memory -O0 %s -o %t && env MSAN_OPTIONS=keep_going=0 not %run %t >%t.out 2>&1
 // FileCheck %s <%t.out
-// RUN: %clangxx_msan -fsanitize-recover=memory -O0 %s -o %t && MSAN_OPTIONS=keep_going=1 not %run %t >%t.out 2>&1
+// RUN: %clangxx_msan -fsanitize-recover=memory -O0 %s -o %t && env MSAN_OPTIONS=keep_going=1 not %run %t >%t.out 2>&1
 // FileCheck --check-prefix=CHECK-RECOVER %s <%t.out
 
 // Test how legacy -mllvm -msan-keep-going and MSAN_OPTIONS=keep_going affect
diff --git a/compiler-rt/test/msan/recover.cpp b/compiler-rt/test/msan/recover.cpp
index cb9916ef4755b..b265fec56d172 100644
--- a/compiler-rt/test/msan/recover.cpp
+++ b/compiler-rt/test/msan/recover.cpp
@@ -1,8 +1,8 @@
 // RUN: %clangxx_msan -O0 %s -o %t && not %run %t >%t.out 2>&1
 // FileCheck %s <%t.out
-// RUN: %clangxx_msan -O0 %s -o %t && MSAN_OPTIONS=keep_going=0 not %run %t >%t.out 2>&1
+// RUN: %clangxx_msan -O0 %s -o %t && env MSAN_OPTIONS=keep_going=0 not %run %t >%t.out 2>&1
 // FileCheck %s <%t.out
-// RUN: %clangxx_msan -O0 %s -o %t && MSAN_OPTIONS=keep_going=1 not %run %t >%t.out 2>&1
+// RUN: %clangxx_msan -O0 %s -o %t && env MSAN_OPTIONS=keep_going=1 not %run %t >%t.out 2>&1
 // FileCheck %s <%t.out
 
 // Test behavior of -fsanitize-recover=memory and MSAN_OPTIONS=keep_going.
@@ -11,20 +11,20 @@
 
 // RUN: %clangxx_msan -fsanitize-recover=memory -O0 %s -o %t && not %run %t >%t.out 2>&1
 // FileCheck --check-prefix=CHECK-RECOVER %s <%t.out
-// RUN: %clangxx_msan -fsanitize-recover=memory -O0 %s -o %t && MSAN_OPTIONS=keep_going=0 not %run %t >%t.out 2>&1
+// RUN: %clangxx_msan -fsanitize-recover=memory -O0 %s -o %t && env MSAN_OPTIONS=keep_going=0 not %run %t >%t.out 2>&1
 // FileCheck %s <%t.out
-// RUN: %clangxx_msan -fsanitize-recover=memory -O0 %s -o %t && MSAN_OPTIONS=keep_going=1 not %run %t >%t.out 2>&1
+// RUN...
[truncated]

Copy link
Contributor

@ilovepi ilovepi left a comment

Choose a reason for hiding this comment

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

LGTM, modulo a typo, but lets let compiler-rt code owners weigh in.

Also, please update the title and description to note that this occurs when using llvm-lit's internal shell. Referencing the RFC and any tracking bugs wouldn't be amiss either.

@ilovepi ilovepi requested a review from vitalybuka August 5, 2024 22:54
@Harini0924 Harini0924 changed the title [llvm-lit] Fixing Command not found errors in Compiler-rt [llvm-lit] Fix error in compiler-rt tests when using lit internal shell Aug 5, 2024
Copy link

github-actions bot commented Aug 7, 2024

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

@Harini0924 Harini0924 changed the title [llvm-lit] Fix error in compiler-rt tests when using lit internal shell [llvm-lit] Fix error in compiler-rt tests when using lit internal shell add env Aug 7, 2024
@Harini0924 Harini0924 changed the title [llvm-lit] Fix error in compiler-rt tests when using lit internal shell add env [llvm-lit] Fix error in compiler-rt tests when using lit internal shell add env Aug 7, 2024
@vitalybuka
Copy link
Collaborator

How did you get to the issue?
We run these test on bots and locally, and see no issue.

Please add that into description.

Also please address clang-format complains.

There are several files in compiler-rt that have command not found
errors. This patch uses the env command to properly set the environment
variables correctly when using the internal shell.
@llvmbot llvmbot added the PGO Profile Guided Optimizations label Aug 8, 2024
@MaskRay
Copy link
Member

MaskRay commented Aug 9, 2024

VAR=a command is so commonly used that perhaps the internal lit shell should be taught this syntax.
Then we can avoid these changes.

@Harini0924 Harini0924 changed the title [llvm-lit] Fix error in compiler-rt tests when using lit internal shell add env [llvm-lit] Fix error in compiler-rt tests when using lit internal shell with env Aug 9, 2024
@vitalybuka
Copy link
Collaborator

VAR=a command is so commonly used that perhaps the internal lit shell should be taught this syntax. Then we can avoid these changes.

Are you asking to block the patch?
It's looks good to me regardless of lit improvement.

@ilovepi
Copy link
Contributor

ilovepi commented Aug 10, 2024

VAR=a command is so commonly used that perhaps the internal lit shell should be taught this syntax. Then we can avoid these changes.

While I think it would be nice if llvm-lit handled this more gracefully, its already has a standard way to handle this via env, which I think (and forgive me if I'm mistaken here) works better w/ older shells, like sh, csh, ksh, etc. Plus, env is already used pretty pervasively across LLVM, so I'm not seeing how this change isn't desirable on its own, especially given that it allows the tests to run in more environments.

You noted in https://discourse.llvm.org/t/rfc-enabling-the-lit-internal-shell-by-default/80179/20 that some improvements to llvm-lit aren't essential to change the default. This type of test fixing would seem to fall under that. If you feel strongly about your POV, then we should probably move the discussion to the RFC thread on discourse to solicit more feedback from the community.

@vitalybuka vitalybuka merged commit 643a208 into llvm:main Aug 13, 2024
8 checks passed
Copy link

@Harini0924 Congratulations on having your first Pull Request (PR) merged into the LLVM Project!

Your changes will be combined with recent changes from other authors, then tested
by our build bots. If there is a problem with a build, you may receive a report in an email or a comment on this PR.

Please check whether problems have been caused by your change specifically, as
the builds can include changes from many authors. It is not uncommon for your
change to be included in a build that fails due to someone else's changes, or
infrastructure issues.

How to do this, and the rest of the post-merge process, is covered in detail here.

If your change does cause a problem, it may be reverted, or you can revert it yourself.
This is a normal part of LLVM development. You can fix your changes and open a new PR to merge them again.

If you don't get any reports, no action is required from you. Your changes are working as expected, well done!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
compiler-rt:msan Memory sanitizer compiler-rt:sanitizer compiler-rt:ubsan Undefined behavior sanitizer compiler-rt PGO Profile Guided Optimizations xray
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[llvm-lit] Environment variable not found in compiler-rt subproject with lit internal shell
5 participants