From a17eab7beddf87807d9d7fc71b7dfb90b5e2488a Mon Sep 17 00:00:00 2001 From: Ian Jackson Date: Sun, 7 Feb 2021 12:16:11 +0000 Subject: panic ui test: Provide comprehensive test for panic after fork This tests that we can indeed safely panic after fork, both a raw libc::fork and in a Command pre_exec hook. Signed-off-by: Ian Jackson Co-authored-by: Mara Bos --- src/test/ui/panics/abort-on-panic.rs | 48 ++++---- src/test/ui/process/process-panic-after-fork.rs | 150 ++++++++++++++++++++++++ 2 files changed, 176 insertions(+), 22 deletions(-) create mode 100644 src/test/ui/process/process-panic-after-fork.rs (limited to 'src') diff --git a/src/test/ui/panics/abort-on-panic.rs b/src/test/ui/panics/abort-on-panic.rs index f34cf6a9cbf..3ef6d5d1874 100644 --- a/src/test/ui/panics/abort-on-panic.rs +++ b/src/test/ui/panics/abort-on-panic.rs @@ -23,41 +23,45 @@ extern "Rust" fn panic_in_rust_abi() { panic!("TestRust"); } -fn test() { - let _ = panic::catch_unwind(|| { panic_in_ffi(); }); - // The process should have aborted by now. +fn should_have_aborted() { io::stdout().write(b"This should never be printed.\n"); let _ = io::stdout().flush(); } +fn test() { + let _ = panic::catch_unwind(|| { panic_in_ffi(); }); + should_have_aborted(); +} + fn testrust() { let _ = panic::catch_unwind(|| { panic_in_rust_abi(); }); - // The process should have aborted by now. - io::stdout().write(b"This should never be printed.\n"); - let _ = io::stdout().flush(); + should_have_aborted(); } fn main() { + let tests: &[(_, fn())] = &[ + ("test", test), + ("testrust", testrust), + ]; + let args: Vec = env::args().collect(); if args.len() > 1 { // This is inside the self-executed command. - match &*args[1] { - "test" => return test(), - "testrust" => return testrust(), - _ => panic!("bad test"), + for (a,f) in tests { + if &args[1] == a { return f() } } + panic!("bad test"); } - // These end up calling the self-execution branches above. - let mut p = Command::new(&args[0]) - .stdout(Stdio::piped()) - .stdin(Stdio::piped()) - .arg("test").spawn().unwrap(); - assert!(!p.wait().unwrap().success()); - - let mut p = Command::new(&args[0]) - .stdout(Stdio::piped()) - .stdin(Stdio::piped()) - .arg("testrust").spawn().unwrap(); - assert!(!p.wait().unwrap().success()); + let execute_self_expecting_abort = |arg| { + let mut p = Command::new(&args[0]) + .stdout(Stdio::piped()) + .stdin(Stdio::piped()) + .arg(arg).spawn().unwrap(); + assert!(!p.wait().unwrap().success()); + }; + + for (a,_f) in tests { + execute_self_expecting_abort(a); + } } diff --git a/src/test/ui/process/process-panic-after-fork.rs b/src/test/ui/process/process-panic-after-fork.rs new file mode 100644 index 00000000000..6e07a1611c5 --- /dev/null +++ b/src/test/ui/process/process-panic-after-fork.rs @@ -0,0 +1,150 @@ +// run-pass +// no-prefer-dynamic +// ignore-wasm32-bare no libc +// ignore-windows +// ignore-sgx no libc +// ignore-emscripten no processes +// ignore-sgx no processes + +#![feature(bench_black_box)] +#![feature(rustc_private)] +#![feature(never_type)] +#![feature(panic_always_abort)] + +extern crate libc; + +use std::alloc::{GlobalAlloc, Layout}; +use std::fmt; +use std::panic::{self, panic_any}; +use std::os::unix::process::{CommandExt, ExitStatusExt}; +use std::process::{self, Command, ExitStatus}; +use std::sync::atomic::{AtomicU32, Ordering}; + +use libc::c_int; + +/// This stunt allocator allows us to spot heap allocations in the child. +struct PidChecking { + parent: A, + require_pid: AtomicU32, +} + +#[global_allocator] +static ALLOCATOR: PidChecking = PidChecking { + parent: std::alloc::System, + require_pid: AtomicU32::new(0), +}; + +impl PidChecking { + fn engage(&self) { + let parent_pid = process::id(); + eprintln!("engaging allocator trap, parent pid={}", parent_pid); + self.require_pid.store(parent_pid, Ordering::Release); + } + fn check(&self) { + let require_pid = self.require_pid.load(Ordering::Acquire); + if require_pid != 0 { + let actual_pid = process::id(); + if require_pid != actual_pid { + unsafe { + libc::raise(libc::SIGTRAP); + } + } + } + } +} + +unsafe impl GlobalAlloc for PidChecking { + unsafe fn alloc(&self, layout: Layout) -> *mut u8 { + self.check(); + self.parent.alloc(layout) + } + + unsafe fn dealloc(&self, ptr: *mut u8, layout: Layout) { + self.check(); + self.parent.dealloc(ptr, layout) + } + + unsafe fn alloc_zeroed(&self, layout: Layout) -> *mut u8 { + self.check(); + self.parent.alloc_zeroed(layout) + } + + unsafe fn realloc(&self, ptr: *mut u8, layout: Layout, new_size: usize) -> *mut u8 { + self.check(); + self.parent.realloc(ptr, layout, new_size) + } +} + +fn expect_aborted(status: ExitStatus) { + dbg!(status); + let signal = status.signal().expect("expected child process to die of signal"); + assert!(signal == libc::SIGABRT || signal == libc::SIGILL); +} + +fn main() { + ALLOCATOR.engage(); + + fn run(do_panic: &dyn Fn()) -> ExitStatus { + let child = unsafe { libc::fork() }; + assert!(child >= 0); + if child == 0 { + panic::always_abort(); + do_panic(); + process::exit(0); + } + let mut status: c_int = 0; + let got = unsafe { libc::waitpid(child, &mut status, 0) }; + assert_eq!(got, child); + let status = ExitStatus::from_raw(status.into()); + status + } + + fn one(do_panic: &dyn Fn()) { + let status = run(do_panic); + expect_aborted(status); + } + + one(&|| panic!()); + one(&|| panic!("some message")); + one(&|| panic!("message with argument: {}", 42)); + + #[derive(Debug)] + struct Wotsit { } + one(&|| panic_any(Wotsit { })); + + let mut c = Command::new("echo"); + unsafe { + c.pre_exec(|| panic!("{}", "crash now!")); + } + let st = c.status().expect("failed to get command status"); + expect_aborted(st); + + struct DisplayWithHeap; + impl fmt::Display for DisplayWithHeap { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> Result<(), fmt::Error> { + let s = vec![0; 100]; + let s = std::hint::black_box(s); + write!(f, "{:?}", s) + } + } + + // Some panics in the stdlib that we want not to allocate, as + // otherwise these facilities become impossible to use in the + // child after fork, which is really quite awkward. + + one(&|| { None::.unwrap(); }); + one(&|| { None::.expect("unwrapped a none"); }); + one(&|| { std::str::from_utf8(b"\xff").unwrap(); }); + one(&|| { + let x = [0, 1, 2, 3]; + let y = x[std::hint::black_box(4)]; + let _z = std::hint::black_box(y); + }); + + // Finally, check that our stunt allocator can actually catch an allocation after fork. + // ie, that our test is effective. + + let status = run(&|| panic!("allocating to display... {}", DisplayWithHeap)); + dbg!(status); + assert_eq!(status.signal(), Some(libc::SIGTRAP)); +} -- cgit 1.4.1-3-g733a5 From 19429ce132d8cc9526ab4fe46d6287f2ad89ef1c Mon Sep 17 00:00:00 2001 From: Ian Jackson Date: Thu, 22 Apr 2021 15:05:23 +0100 Subject: panic ui test: Improve error handling Previoously, if somehow this program got a wrong argument, it would panic in the re-executed child. But that looks like a "success" for this program! We mustn't panic unless everything is great. Signed-off-by: Ian Jackson --- src/test/ui/panics/abort-on-panic.rs | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) (limited to 'src') diff --git a/src/test/ui/panics/abort-on-panic.rs b/src/test/ui/panics/abort-on-panic.rs index 3ef6d5d1874..7cf60ae9602 100644 --- a/src/test/ui/panics/abort-on-panic.rs +++ b/src/test/ui/panics/abort-on-panic.rs @@ -28,6 +28,11 @@ fn should_have_aborted() { let _ = io::stdout().flush(); } +fn bomb_out_but_not_abort(msg: &str) { + eprintln!("bombing out: {}", msg); + exit(1); +} + fn test() { let _ = panic::catch_unwind(|| { panic_in_ffi(); }); should_have_aborted(); @@ -50,7 +55,7 @@ fn main() { for (a,f) in tests { if &args[1] == a { return f() } } - panic!("bad test"); + bomb_out_but_not_abort("bad test"); } let execute_self_expecting_abort = |arg| { @@ -58,7 +63,11 @@ fn main() { .stdout(Stdio::piped()) .stdin(Stdio::piped()) .arg(arg).spawn().unwrap(); - assert!(!p.wait().unwrap().success()); + let status = p.wait().unwrap(); + assert!(!status.success()); + // Any reasonable platform can distinguish a process which + // called exit(1) from one which panicked. + assert_ne!(status.code(), Some(1)); }; for (a,_f) in tests { -- cgit 1.4.1-3-g733a5 From 12fe50010da70abfca84ae9c0d6e798e987fa882 Mon Sep 17 00:00:00 2001 From: Ian Jackson Date: Thu, 22 Apr 2021 14:22:42 +0100 Subject: panic ui test: Add a test for panic::always_abort Our existing tests are only on Unix. We want a general one too. Signed-off-by: Ian Jackson --- src/test/ui/panics/abort-on-panic.rs | 7 +++++++ 1 file changed, 7 insertions(+) (limited to 'src') diff --git a/src/test/ui/panics/abort-on-panic.rs b/src/test/ui/panics/abort-on-panic.rs index 7cf60ae9602..c02552be519 100644 --- a/src/test/ui/panics/abort-on-panic.rs +++ b/src/test/ui/panics/abort-on-panic.rs @@ -43,10 +43,17 @@ fn testrust() { should_have_aborted(); } +fn test_always_abort() { + panic::always_abort(); + let _ = panic::catch_unwind(|| { panic!(); }); + should_have_aborted(); +} + fn main() { let tests: &[(_, fn())] = &[ ("test", test), ("testrust", testrust), + ("test_always_abort", test_always_abort), ]; let args: Vec = env::args().collect(); -- cgit 1.4.1-3-g733a5 From 756771d54c28c4543d620891e971bf5a95c3ea3e Mon Sep 17 00:00:00 2001 From: Ian Jackson Date: Thu, 22 Apr 2021 15:06:53 +0100 Subject: panic ui test: Test always_abort on one thread, panic on another This test failed on an earlier version of this branch, where this did not work properly, so I know the test works. Signed-off-by: Ian Jackson --- src/test/ui/panics/abort-on-panic.rs | 21 ++++++++++++++++++++- 1 file changed, 20 insertions(+), 1 deletion(-) (limited to 'src') diff --git a/src/test/ui/panics/abort-on-panic.rs b/src/test/ui/panics/abort-on-panic.rs index c02552be519..2aea607e954 100644 --- a/src/test/ui/panics/abort-on-panic.rs +++ b/src/test/ui/panics/abort-on-panic.rs @@ -2,6 +2,7 @@ #![allow(unused_must_use)] #![feature(unwind_attributes)] +#![feature(panic_always_abort)] // Since we mark some ABIs as "nounwind" to LLVM, we must make sure that // we never unwind through them. @@ -11,7 +12,9 @@ use std::{env, panic}; use std::io::prelude::*; use std::io; -use std::process::{Command, Stdio}; +use std::process::{exit, Command, Stdio}; +use std::sync::{Arc, Barrier}; +use std::thread; #[unwind(aborts)] // FIXME(#58794) should work even without the attribute extern "C" fn panic_in_ffi() { @@ -49,11 +52,27 @@ fn test_always_abort() { should_have_aborted(); } +fn test_always_abort_thread() { + let barrier = Arc::new(Barrier::new(2)); + let thr = { + let barrier = barrier.clone(); + thread::spawn(move ||{ + barrier.wait(); + panic!("in thread"); + }) + }; + panic::always_abort(); + barrier.wait(); + let _ = thr.join(); + bomb_out_but_not_abort("joined - but we were supposed to panic!"); +} + fn main() { let tests: &[(_, fn())] = &[ ("test", test), ("testrust", testrust), ("test_always_abort", test_always_abort), + ("test_always_abort_thread", test_always_abort_thread), ]; let args: Vec = env::args().collect(); -- cgit 1.4.1-3-g733a5 From f6a4963cc86bda5f3611f0f59e1ffe53e4b9f3fa Mon Sep 17 00:00:00 2001 From: Ian Jackson Date: Thu, 13 May 2021 18:38:25 +0100 Subject: Use SIGUSR1 rather than SIGTRAP for "allocated after fork" Some platforma (eg ARM64) apparently generate SIGTRAP for panic abort! See eg https://github.com/rust-lang/rust/pull/81858#issuecomment-840702765 This is probably a bug, but (i) we want to avoid that bug rather than trying to fix it now and (ii) it would better to use a signal that is less at risk of strangeness. I grepped the rust-lang/rut codebase for SIGUSR and there were no hits. Signed-off-by: Ian Jackson --- src/test/ui/process/process-panic-after-fork.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'src') diff --git a/src/test/ui/process/process-panic-after-fork.rs b/src/test/ui/process/process-panic-after-fork.rs index 6e07a1611c5..5cc436c335e 100644 --- a/src/test/ui/process/process-panic-after-fork.rs +++ b/src/test/ui/process/process-panic-after-fork.rs @@ -46,7 +46,7 @@ impl PidChecking { let actual_pid = process::id(); if require_pid != actual_pid { unsafe { - libc::raise(libc::SIGTRAP); + libc::raise(libc::SIGUSR1); } } } @@ -146,5 +146,5 @@ fn main() { let status = run(&|| panic!("allocating to display... {}", DisplayWithHeap)); dbg!(status); - assert_eq!(status.signal(), Some(libc::SIGTRAP)); + assert_eq!(status.signal(), Some(libc::SIGUSR1)); } -- cgit 1.4.1-3-g733a5 From 6369637a192bbd0a2fbf8084345ddb7c099aa460 Mon Sep 17 00:00:00 2001 From: Ian Jackson Date: Thu, 13 May 2021 18:41:18 +0100 Subject: Tolerate SIGTRAP for panic abort after panic::always_abort Some platforma (eg ARM64) apparently generate SIGTRAP for panic abort! See eg https://github.com/rust-lang/rust/pull/81858#issuecomment-840702765 This is probably a bug, but we don't want to entangle this MR with it. When it's fixed, this commit should be reverted. Signed-off-by: Ian Jackson --- library/std/src/sys/unix/process/process_unix/tests.rs | 2 +- src/test/ui/process/process-panic-after-fork.rs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) (limited to 'src') diff --git a/library/std/src/sys/unix/process/process_unix/tests.rs b/library/std/src/sys/unix/process/process_unix/tests.rs index d7915d81ebd..157debf2d25 100644 --- a/library/std/src/sys/unix/process/process_unix/tests.rs +++ b/library/std/src/sys/unix/process/process_unix/tests.rs @@ -53,5 +53,5 @@ fn test_command_fork_no_unwind() { let status = got.expect("panic unexpectedly propagated"); dbg!(status); let signal = status.signal().expect("expected child process to die of signal"); - assert!(signal == libc::SIGABRT || signal == libc::SIGILL); + assert!(signal == libc::SIGABRT || signal == libc::SIGILL || signal == libc::SIGTRAP); } diff --git a/src/test/ui/process/process-panic-after-fork.rs b/src/test/ui/process/process-panic-after-fork.rs index 5cc436c335e..f178baefcf6 100644 --- a/src/test/ui/process/process-panic-after-fork.rs +++ b/src/test/ui/process/process-panic-after-fork.rs @@ -78,7 +78,7 @@ unsafe impl GlobalAlloc for PidChecking { fn expect_aborted(status: ExitStatus) { dbg!(status); let signal = status.signal().expect("expected child process to die of signal"); - assert!(signal == libc::SIGABRT || signal == libc::SIGILL); + assert!(signal == libc::SIGABRT || signal == libc::SIGILL || signal == libc::SIGTRAP); } fn main() { -- cgit 1.4.1-3-g733a5 From 88ccaa77f17a74ec8597efa5c86f0f789028d1b4 Mon Sep 17 00:00:00 2001 From: Ian Jackson Date: Fri, 14 May 2021 11:38:25 +0100 Subject: panic abort after fork test: Disable on android And link to the issue. Signed-off-by: Ian Jackson --- src/test/ui/process/process-panic-after-fork.rs | 1 + 1 file changed, 1 insertion(+) (limited to 'src') diff --git a/src/test/ui/process/process-panic-after-fork.rs b/src/test/ui/process/process-panic-after-fork.rs index f178baefcf6..1ccf6bb051c 100644 --- a/src/test/ui/process/process-panic-after-fork.rs +++ b/src/test/ui/process/process-panic-after-fork.rs @@ -5,6 +5,7 @@ // ignore-sgx no libc // ignore-emscripten no processes // ignore-sgx no processes +// ignore-android: FIXME(#85261) #![feature(bench_black_box)] #![feature(rustc_private)] -- cgit 1.4.1-3-g733a5