-
Notifications
You must be signed in to change notification settings - Fork 15.2k
[ASan] Make duplicate_os_log_reports.cpp work with the internal shell #168656
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
[ASan] Make duplicate_os_log_reports.cpp work with the internal shell #168656
Conversation
Created using spr 1.3.7 [skip ci]
Created using spr 1.3.7
|
@llvm/pr-subscribers-compiler-rt-sanitizer Author: Aiden Grossman (boomanaiden154) ChangesThis test used a for loop to implement retries and also did some trickery with PIDs. Full diff: https://github.com/llvm/llvm-project/pull/168656.diff 2 Files Affected:
diff --git a/compiler-rt/test/asan/TestCases/Darwin/Inputs/check-syslog.sh b/compiler-rt/test/asan/TestCases/Darwin/Inputs/check-syslog.sh
new file mode 100755
index 0000000000000..8939ca7ca1564
--- /dev/null
+++ b/compiler-rt/test/asan/TestCases/Darwin/Inputs/check-syslog.sh
@@ -0,0 +1,6 @@
+#!/bin/sh
+for I in {1..3}; do \
+ log show --debug --last $((SECONDS + 30))s --predicate "processID == $1" --style syslog > $2; \
+ if grep -q "use-after-poison" $2; then break; fi; \
+ sleep 5; \
+done
diff --git a/compiler-rt/test/asan/TestCases/Darwin/duplicate_os_log_reports.cpp b/compiler-rt/test/asan/TestCases/Darwin/duplicate_os_log_reports.cpp
index 5a0353bfb1b31..6adca31745bfd 100644
--- a/compiler-rt/test/asan/TestCases/Darwin/duplicate_os_log_reports.cpp
+++ b/compiler-rt/test/asan/TestCases/Darwin/duplicate_os_log_reports.cpp
@@ -1,8 +1,8 @@
// UNSUPPORTED: ios
// REQUIRES: darwin_log_cmd
// RUN: %clangxx_asan -fsanitize-recover=address %s -o %t
-// RUN: { %env_asan_opts=halt_on_error=0,log_to_syslog=1 %run %t > %t.process_output.txt 2>&1 & } \
-// RUN: ; export TEST_PID=$! ; wait ${TEST_PID}
+// RUN: bash -c "{ %env_asan_opts=halt_on_error=0,log_to_syslog=1 %run %t > %t.process_output.txt 2>&1 & } \
+// RUN: ; export TEST_PID=$! ; wait ${TEST_PID}; echo -n ${TEST_PID} > %t.test_pid"
// Check process output.
// RUN: FileCheck %s --check-prefixes CHECK,CHECK-PROC -input-file=%t.process_output.txt
@@ -10,11 +10,7 @@
// Check syslog output. We filter recent system logs based on PID to avoid
// getting the logs of previous test runs. Make some reattempts in case there
// is a delay.
-// RUN: for I in {1..3}; do \
-// RUN: log show --debug --last $((SECONDS + 30))s --predicate "processID == ${TEST_PID}" --style syslog > %t.process_syslog_output.txt; \
-// RUN: if grep -q "use-after-poison" %t.process_syslog_output.txt; then break; fi; \
-// RUN: sleep 5; \
-// RUN: done
+// RUN: %S/Inputs/check-syslog.sh %{readfile:%t.test_pid} %t.process_syslog_output.txt
// RUN: FileCheck %s -input-file=%t.process_syslog_output.txt
#include <cassert>
#include <cstdio>
|
This test used a for loop to implement retries and also did some trickery with PIDs. For this test, just invoke bash for actually running the test given we need the PID, and move the for loop into a separate shell script file that we can then invoke from within the test. Normally it would make sense to rewrite such a script in Python, but given this test does not have portability concerns only running on Darwin, it is fine to use a shell script here given there is no other convenient alternative. Pull Request: llvm#168656
🐧 Linux x64 Test Results
|
ndrewh
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not convinced the script belongs in Inputs/, but I don't have an alternative I prefer.
| // RUN: { %env_asan_opts=halt_on_error=0,log_to_syslog=1 %run %t > %t.process_output.txt 2>&1 & } \ | ||
| // RUN: ; export TEST_PID=$! ; wait ${TEST_PID} | ||
| // RUN: bash -c "{ %env_asan_opts=halt_on_error=0,log_to_syslog=1 %run %t > %t.process_output.txt 2>&1 & } \ | ||
| // RUN: ; export TEST_PID=$! ; wait ${TEST_PID}; echo -n ${TEST_PID} > %t.test_pid" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
FYI this will no longer work with the default external shell ($! / ${TEST_PID} are expanded by the outer shell), so please land the internal shell as default at the same time as this change.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've updated the test to use single quotes so the variables do not get substituted by the outer shell. Oddly, the test seems to pass either way with the external shell, but we probably lose some fidelity without the singular quotes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If you clear out the temp file, it'll fail iirc
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, yeah, that would probably do it.
Created using spr 1.3.7 [skip ci]
This test used a for loop to implement retries and also did some trickery with PIDs. For this test, just invoke bash for actually running the test given we need the PID, and move the for loop into a separate shell script file that we can then invoke from within the test. Normally it would make sense to rewrite such a script in Python, but given this test does not have portability concerns only running on Darwin, it is fine to use a shell script here given there is no other convenient alternative. Pull Request: llvm#168656
This test used a for loop to implement retries and also did some trickery with PIDs. For this test, just invoke bash for actually running the test given we need the PID, and move the for loop into a separate shell script file that we can then invoke from within the test. Normally it would make sense to rewrite such a script in Python, but given this test does not have portability concerns only running on Darwin, it is fine to use a shell script here given there is no other convenient alternative. Pull Request: llvm#168656
Created using spr 1.3.7 [skip ci]
Created using spr 1.3.7 [skip ci]
…ernal shell This test used a for loop to implement retries and also did some trickery with PIDs. For this test, just invoke bash for actually running the test given we need the PID, and move the for loop into a separate shell script file that we can then invoke from within the test. Normally it would make sense to rewrite such a script in Python, but given this test does not have portability concerns only running on Darwin, it is fine to use a shell script here given there is no other convenient alternative. Reviewers: ndrewh, DanBlackwell, fmayer Reviewed By: ndrewh Pull Request: llvm/llvm-project#168656
This test used a for loop to implement retries and also did some trickery with PIDs.
For this test, just invoke bash for actually running the test given we need the PID,
and move the for loop into a separate shell script file that we can then invoke from
within the test. Normally it would make sense to rewrite such a script in Python, but
given this test does not have portability concerns only running on Darwin, it is fine
to use a shell script here given there is no other convenient alternative.