Skip to content

Commit ff95e52

Browse files
committed
Auto merge of #120326 - tmandry:abort-in-tests, r=cuviper
Actually abort in -Zpanic-abort-tests When a test fails in panic=abort, it can be useful to have a debugger or other tooling hook into the `abort()` call for debugging. Doing this some other way would require it to hard code details of Rust's panic machinery. There's no reason we couldn't have done this in the first place; using a single exit code for "success" or "failed" was just simpler. Now we are aware of the special exit codes for posix and windows platforms, logging a special error if an unrecognized code is used on those platforms, and falling back to just "failure" on other platforms. This continues to account for `#[should_panic]` inside the test process itself, so there's no risk of misinterpreting a random call to `abort()` as an expected panic. Any exit code besides `TR_OK` is logged as a test failure. As an added benefit, this would allow us to support panic=immediate_abort (but not `#[should_panic]`), without noise about unexpected exit codes when a test fails.
2 parents f3b9d47 + 48f664b commit ff95e52

10 files changed

+38
-44
lines changed

Cargo.lock

+1
Original file line numberDiff line numberDiff line change
@@ -5321,6 +5321,7 @@ version = "0.0.0"
53215321
dependencies = [
53225322
"core",
53235323
"getopts",
5324+
"libc",
53245325
"panic_abort",
53255326
"panic_unwind",
53265327
"std",

library/test/Cargo.toml

+1
Original file line numberDiff line numberDiff line change
@@ -9,3 +9,4 @@ std = { path = "../std" }
99
core = { path = "../core" }
1010
panic_unwind = { path = "../panic_unwind" }
1111
panic_abort = { path = "../panic_abort" }
12+
libc = { version = "0.2.150", default-features = false }

library/test/src/helpers/exit_code.rs

-20
This file was deleted.

library/test/src/helpers/mod.rs

-1
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,5 @@
22
//! but used in `libtest`.
33
44
pub mod concurrency;
5-
pub mod exit_code;
65
pub mod metrics;
76
pub mod shuffle;

library/test/src/lib.rs

+2-13
Original file line numberDiff line numberDiff line change
@@ -85,7 +85,6 @@ mod tests;
8585
use core::any::Any;
8686
use event::{CompletedTest, TestEvent};
8787
use helpers::concurrency::get_concurrency;
88-
use helpers::exit_code::get_exit_code;
8988
use helpers::shuffle::{get_shuffle_seed, shuffle_tests};
9089
use options::RunStrategy;
9190
use test_result::*;
@@ -712,17 +711,7 @@ fn spawn_test_subprocess(
712711
formatters::write_stderr_delimiter(&mut test_output, &desc.name);
713712
test_output.extend_from_slice(&stderr);
714713

715-
let result = match (|| -> Result<TestResult, String> {
716-
let exit_code = get_exit_code(status)?;
717-
Ok(get_result_from_exit_code(&desc, exit_code, &time_opts, &exec_time))
718-
})() {
719-
Ok(r) => r,
720-
Err(e) => {
721-
write!(&mut test_output, "Unexpected error: {e}").unwrap();
722-
TrFailed
723-
}
724-
};
725-
714+
let result = get_result_from_exit_code(&desc, status, &time_opts, &exec_time);
726715
(result, test_output, exec_time)
727716
})();
728717

@@ -751,7 +740,7 @@ fn run_test_in_spawned_subprocess(desc: TestDesc, runnable_test: RunnableTest) -
751740
if let TrOk = test_result {
752741
process::exit(test_result::TR_OK);
753742
} else {
754-
process::exit(test_result::TR_FAILED);
743+
process::abort();
755744
}
756745
});
757746
let record_result2 = record_result.clone();

library/test/src/test_result.rs

+29-7
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,8 @@
11
use std::any::Any;
2+
use std::process::ExitStatus;
3+
4+
#[cfg(unix)]
5+
use std::os::unix::process::ExitStatusExt;
26

37
use super::bench::BenchSamples;
48
use super::options::ShouldPanic;
@@ -7,11 +11,15 @@ use super::types::TestDesc;
711

812
pub use self::TestResult::*;
913

10-
// Return codes for secondary process.
14+
// Return code for secondary process.
1115
// Start somewhere other than 0 so we know the return code means what we think
1216
// it means.
1317
pub const TR_OK: i32 = 50;
14-
pub const TR_FAILED: i32 = 51;
18+
19+
// On Windows we use __fastfail to abort, which is documented to use this
20+
// exception code.
21+
#[cfg(windows)]
22+
const STATUS_ABORTED: i32 = 0xC0000409u32 as i32;
1523

1624
#[derive(Debug, Clone, PartialEq)]
1725
pub enum TestResult {
@@ -81,14 +89,28 @@ pub fn calc_result<'a>(
8189
/// Creates a `TestResult` depending on the exit code of test subprocess.
8290
pub fn get_result_from_exit_code(
8391
desc: &TestDesc,
84-
code: i32,
92+
status: ExitStatus,
8593
time_opts: &Option<time::TestTimeOptions>,
8694
exec_time: &Option<time::TestExecTime>,
8795
) -> TestResult {
88-
let result = match code {
89-
TR_OK => TestResult::TrOk,
90-
TR_FAILED => TestResult::TrFailed,
91-
_ => TestResult::TrFailedMsg(format!("got unexpected return code {code}")),
96+
let result = match status.code() {
97+
Some(TR_OK) => TestResult::TrOk,
98+
#[cfg(windows)]
99+
Some(STATUS_ABORTED) => TestResult::TrFailed,
100+
#[cfg(unix)]
101+
None => match status.signal() {
102+
Some(libc::SIGABRT) => TestResult::TrFailed,
103+
Some(signal) => {
104+
TestResult::TrFailedMsg(format!("child process exited with signal {signal}"))
105+
}
106+
None => unreachable!("status.code() returned None but status.signal() was None"),
107+
},
108+
#[cfg(not(unix))]
109+
None => TestResult::TrFailedMsg(format!("unknown return code")),
110+
#[cfg(any(windows, unix))]
111+
Some(code) => TestResult::TrFailedMsg(format!("got unexpected return code {code}")),
112+
#[cfg(not(any(windows, unix)))]
113+
Some(_) => TestResult::TrFailed,
92114
};
93115

94116
// If test is already failed (or allowed to fail), do not change the result.

tests/ui/test-attrs/test-panic-abort-nocapture.rs

+1
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
// exec-env:RUST_BACKTRACE=0
77
// normalize-stdout-test "finished in \d+\.\d+s" -> "finished in $$TIME"
88

9+
// ignore-android #120567
910
// ignore-wasm no panic or subprocess support
1011
// ignore-emscripten no panic or subprocess support
1112
// ignore-sgx no subprocess support

tests/ui/test-attrs/test-panic-abort-nocapture.run.stderr

+2-2
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,9 @@
1-
thread 'main' panicked at $DIR/test-panic-abort-nocapture.rs:33:5:
1+
thread 'main' panicked at $DIR/test-panic-abort-nocapture.rs:34:5:
22
assertion `left == right` failed
33
left: 2
44
right: 4
55
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
6-
thread 'main' panicked at $DIR/test-panic-abort-nocapture.rs:27:5:
6+
thread 'main' panicked at $DIR/test-panic-abort-nocapture.rs:28:5:
77
assertion `left == right` failed
88
left: 2
99
right: 4

tests/ui/test-attrs/test-panic-abort.rs

+1
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
// exec-env:RUST_BACKTRACE=0
77
// normalize-stdout-test "finished in \d+\.\d+s" -> "finished in $$TIME"
88

9+
// ignore-android #120567
910
// ignore-wasm no panic or subprocess support
1011
// ignore-emscripten no panic or subprocess support
1112
// ignore-sgx no subprocess support

tests/ui/test-attrs/test-panic-abort.run.stdout

+1-1
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ hello, world
1717
testing123
1818
---- it_fails stderr ----
1919
testing321
20-
thread 'main' panicked at $DIR/test-panic-abort.rs:38:5:
20+
thread 'main' panicked at $DIR/test-panic-abort.rs:39:5:
2121
assertion `left == right` failed
2222
left: 2
2323
right: 5

0 commit comments

Comments
 (0)