From f622e832d41171578b6987d0b25b3a84d6d1c81c Mon Sep 17 00:00:00 2001 From: Tyler Mandry Date: Wed, 24 Jan 2024 15:57:43 -0800 Subject: [PATCH] Actually abort in panic-abort-tests --- Cargo.lock | 1 + library/test/Cargo.toml | 1 + library/test/src/helpers/exit_code.rs | 20 --------------- library/test/src/helpers/mod.rs | 1 - library/test/src/lib.rs | 15 ++--------- library/test/src/test_result.rs | 36 +++++++++++++++++++++------ 6 files changed, 33 insertions(+), 41 deletions(-) delete mode 100644 library/test/src/helpers/exit_code.rs diff --git a/Cargo.lock b/Cargo.lock index 74f96983b31..24054e8592e 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -5332,6 +5332,7 @@ version = "0.0.0" dependencies = [ "core", "getopts", + "libc", "panic_abort", "panic_unwind", "std", diff --git a/library/test/Cargo.toml b/library/test/Cargo.toml index 92c535501bf..4c42e3bae56 100644 --- a/library/test/Cargo.toml +++ b/library/test/Cargo.toml @@ -9,3 +9,4 @@ std = { path = "../std" } core = { path = "../core" } panic_unwind = { path = "../panic_unwind" } panic_abort = { path = "../panic_abort" } +libc = { version = "0.2.150", default-features = false } diff --git a/library/test/src/helpers/exit_code.rs b/library/test/src/helpers/exit_code.rs deleted file mode 100644 index f762f88819d..00000000000 --- a/library/test/src/helpers/exit_code.rs +++ /dev/null @@ -1,20 +0,0 @@ -//! Helper module to detect subprocess exit code. - -use std::process::ExitStatus; - -#[cfg(not(unix))] -pub fn get_exit_code(status: ExitStatus) -> Result { - status.code().ok_or_else(|| "received no exit code from child process".into()) -} - -#[cfg(unix)] -pub fn get_exit_code(status: ExitStatus) -> Result { - use std::os::unix::process::ExitStatusExt; - match status.code() { - Some(code) => Ok(code), - None => match status.signal() { - Some(signal) => Err(format!("child process exited with signal {signal}")), - None => Err("child process exited with unknown signal".into()), - }, - } -} diff --git a/library/test/src/helpers/mod.rs b/library/test/src/helpers/mod.rs index 6f366a911e8..3c79b90b167 100644 --- a/library/test/src/helpers/mod.rs +++ b/library/test/src/helpers/mod.rs @@ -2,6 +2,5 @@ //! but used in `libtest`. pub mod concurrency; -pub mod exit_code; pub mod metrics; pub mod shuffle; diff --git a/library/test/src/lib.rs b/library/test/src/lib.rs index 5969d6b772c..da110f99248 100644 --- a/library/test/src/lib.rs +++ b/library/test/src/lib.rs @@ -85,7 +85,6 @@ pub mod test { use core::any::Any; use event::{CompletedTest, TestEvent}; use helpers::concurrency::get_concurrency; -use helpers::exit_code::get_exit_code; use helpers::shuffle::{get_shuffle_seed, shuffle_tests}; use options::RunStrategy; use test_result::*; @@ -712,17 +711,7 @@ fn spawn_test_subprocess( formatters::write_stderr_delimiter(&mut test_output, &desc.name); test_output.extend_from_slice(&stderr); - let result = match (|| -> Result { - let exit_code = get_exit_code(status)?; - Ok(get_result_from_exit_code(&desc, exit_code, &time_opts, &exec_time)) - })() { - Ok(r) => r, - Err(e) => { - write!(&mut test_output, "Unexpected error: {e}").unwrap(); - TrFailed - } - }; - + let result = get_result_from_exit_code(&desc, status, &time_opts, &exec_time); (result, test_output, exec_time) })(); @@ -751,7 +740,7 @@ fn run_test_in_spawned_subprocess(desc: TestDesc, runnable_test: RunnableTest) - if let TrOk = test_result { process::exit(test_result::TR_OK); } else { - process::exit(test_result::TR_FAILED); + process::abort(); } }); let record_result2 = record_result.clone(); diff --git a/library/test/src/test_result.rs b/library/test/src/test_result.rs index 1da238e3e8c..bb32c70d663 100644 --- a/library/test/src/test_result.rs +++ b/library/test/src/test_result.rs @@ -1,4 +1,8 @@ use std::any::Any; +use std::process::ExitStatus; + +#[cfg(unix)] +use std::os::unix::process::ExitStatusExt; use super::bench::BenchSamples; use super::options::ShouldPanic; @@ -7,11 +11,15 @@ pub use self::TestResult::*; -// Return codes for secondary process. +// Return code for secondary process. // Start somewhere other than 0 so we know the return code means what we think // it means. pub const TR_OK: i32 = 50; -pub const TR_FAILED: i32 = 51; + +// On Windows we use __fastfail to abort, which is documented to use this +// exception code. +#[cfg(windows)] +const STATUS_ABORTED: i32 = 0xC0000409u32 as i32; #[derive(Debug, Clone, PartialEq)] pub enum TestResult { @@ -81,14 +89,28 @@ pub fn calc_result<'a>( /// Creates a `TestResult` depending on the exit code of test subprocess. pub fn get_result_from_exit_code( desc: &TestDesc, - code: i32, + status: ExitStatus, time_opts: &Option, exec_time: &Option, ) -> TestResult { - let result = match code { - TR_OK => TestResult::TrOk, - TR_FAILED => TestResult::TrFailed, - _ => TestResult::TrFailedMsg(format!("got unexpected return code {code}")), + let result = match status.code() { + Some(TR_OK) => TestResult::TrOk, + #[cfg(windows)] + Some(STATUS_ABORTED) => TestResult::TrFailed, + #[cfg(unix)] + None => match status.signal() { + Some(libc::SIGABRT) => TestResult::TrFailed, + Some(signal) => { + TestResult::TrFailedMsg(format!("child process exited with signal {signal}")) + } + None => unreachable!("status.code() returned None but status.signal() was None"), + }, + #[cfg(not(unix))] + None => TestResult::TrFailedMsg(format!("unknown return code")), + #[cfg(any(windows, unix))] + Some(code) => TestResult::TrFailedMsg(format!("got unexpected return code {code}")), + #[cfg(not(any(windows, unix)))] + Some(_) => TestResult::TrFailed, }; // If test is already failed (or allowed to fail), do not change the result.