Auto merge of #81858 - ijackson:fork-no-unwind, r=m-ou-se

Do not allocate or unwind after fork

### Objective scenarios

 * Make (simple) panics safe in `Command::pre_exec_hook`, including most `panic!` calls, `Option::unwrap`, and array bounds check failures.
 * Make it possible to `libc::fork` and then safely panic in the child (needed for the above, but this requirement means exposing the new raw hook API which the `Command` implementation needs).
 * In singlethreaded programs, where panic in `pre_exec_hook` is already memory-safe, prevent the double-unwinding malfunction #79740.

I think we want to make panic after fork safe even though the post-fork child environment is only experienced by users of `unsafe`, beause the subset of Rust in which any panic is UB is really far too hazardous and unnatural.

#### Approach

 * Provide a way for a program to, at runtime, switch to having panics abort.  This makes it possible to panic without making *any* heap allocations, which is needed because on some platforms malloc is UB in a child forked from a multithreaded program (see https://github.com/rust-lang/rust/pull/80263#issuecomment-774272370, and maybe also the SuS [spec](https://pubs.opengroup.org/onlinepubs/9699919799/functions/fork.html)).
 * Make that change in the child spawned by `Command`.
 * Document the rules comprehensively enough that a programmer has a fighting chance of writing correct code.
 * Test that this all works as expected (and in particular, that there aren't any heap allocations we missed)

Fixes #79740

#### Rejected (or previously attempted) approaches

 * Change the panic machinery to be able to unwind without allocating, at least when the payload and message are both `'static`.  This seems like it would be even more subtle.  Also that is a potentially-hot path which I don't want to mess with.
 * Change the existing panic hook mechanism to not convert the message to a `String` before calling the hook.  This would be a surprising change for existing code and would not be detected by the type system.
 * Provide a `raw_panic_hook` function to intercept panics in a way that doesn't allocate.  (That was an earlier version of this MR.)

### History

This MR could be considered a v2 of #80263.  Thanks to everyone who commented there.  In particular, thanks to `@m-ou-se,` `@Mark-Simulacrum` and `@hyd-dev.`  (Tagging you since I think you might be interested in this new MR.)  Compared to #80263, this MR has very substantial changes and additions.

Additionally, I have recently (2021-04-20) completely revised this series following very helpful comments from `@m-ou-se.`

r? `@m-ou-se`
This commit is contained in:
bors 2021-05-15 22:27:09 +00:00
commit d565c74887
7 changed files with 333 additions and 39 deletions

View file

@ -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() {
@ -23,41 +26,77 @@ 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 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();
}
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 test_always_abort() {
panic::always_abort();
let _ = panic::catch_unwind(|| { panic!(); });
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<String> = 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() }
}
bomb_out_but_not_abort("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 execute_self_expecting_abort = |arg| {
let mut p = Command::new(&args[0])
.stdout(Stdio::piped())
.stdin(Stdio::piped())
.arg(arg).spawn().unwrap();
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));
};
let mut p = Command::new(&args[0])
.stdout(Stdio::piped())
.stdin(Stdio::piped())
.arg("testrust").spawn().unwrap();
assert!(!p.wait().unwrap().success());
for (a,_f) in tests {
execute_self_expecting_abort(a);
}
}

View file

@ -0,0 +1,151 @@
// run-pass
// no-prefer-dynamic
// ignore-wasm32-bare no libc
// ignore-windows
// ignore-sgx no libc
// ignore-emscripten no processes
// ignore-sgx no processes
// ignore-android: FIXME(#85261)
#![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<A> {
parent: A,
require_pid: AtomicU32,
}
#[global_allocator]
static ALLOCATOR: PidChecking<std::alloc::System> = PidChecking {
parent: std::alloc::System,
require_pid: AtomicU32::new(0),
};
impl<A> PidChecking<A> {
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::SIGUSR1);
}
}
}
}
}
unsafe impl<A:GlobalAlloc> GlobalAlloc for PidChecking<A> {
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 || signal == libc::SIGTRAP);
}
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::<DisplayWithHeap>.unwrap(); });
one(&|| { None::<DisplayWithHeap>.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::SIGUSR1));
}