diff --git a/src/machine.rs b/src/machine.rs index b76159694d82..ebe3c509ad87 100644 --- a/src/machine.rs +++ b/src/machine.rs @@ -252,11 +252,6 @@ pub struct Evaluator<'mir, 'tcx> { pub(crate) file_handler: shims::posix::FileHandler, pub(crate) dir_handler: shims::posix::DirHandler, - /// The temporary used for storing the argument of - /// the call to `miri_start_panic` (the panic payload) when unwinding. - /// This is pointer-sized, and matches the `Payload` type in `src/libpanic_unwind/miri.rs`. - pub(crate) panic_payload: Option>, - /// The "time anchor" for this machine's monotone clock (for `Instant` simulation). pub(crate) time_anchor: Instant, @@ -291,7 +286,6 @@ impl<'mir, 'tcx> Evaluator<'mir, 'tcx> { validate, file_handler: Default::default(), dir_handler: Default::default(), - panic_payload: None, time_anchor: Instant::now(), layouts, threads: ThreadManager::default(), diff --git a/src/shims/panic.rs b/src/shims/panic.rs index 45a41b9b7be0..b9d8ceb1dfb5 100644 --- a/src/shims/panic.rs +++ b/src/shims/panic.rs @@ -48,11 +48,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx // Get the raw pointer stored in arg[0] (the panic payload). let &[payload] = check_arg_count(args)?; let payload = this.read_scalar(payload)?.check_init()?; - assert!( - this.machine.panic_payload.is_none(), - "the panic runtime should avoid double-panics" - ); - this.machine.panic_payload = Some(payload); + this.set_panic_payload(payload); // Jump to the unwind block to begin unwinding. this.unwind_to_block(unwind); @@ -132,9 +128,9 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx // We set the return value of `try` to 1, since there was a panic. this.write_scalar(Scalar::from_i32(1), catch_unwind.dest)?; - // `panic_payload` holds what was passed to `miri_start_panic`. + // The Thread's `panic_payload` holds what was passed to `miri_start_panic`. // This is exactly the second argument we need to pass to `catch_fn`. - let payload = this.machine.panic_payload.take().unwrap(); + let payload = this.take_panic_payload(); // Push the `catch_fn` stackframe. let f_instance = this.memory.get_fn(catch_unwind.catch_fn)?.as_instance()?; diff --git a/src/thread.rs b/src/thread.rs index cffbec93c5ca..dd5358bfb5e0 100644 --- a/src/thread.rs +++ b/src/thread.rs @@ -106,12 +106,21 @@ enum ThreadJoinStatus { /// A thread. pub struct Thread<'mir, 'tcx> { state: ThreadState, + /// Name of the thread. thread_name: Option>, + /// The virtual call stack. stack: Vec>>, + /// The join status. join_status: ThreadJoinStatus, + + /// The temporary used for storing the argument of + /// the call to `miri_start_panic` (the panic payload) when unwinding. + /// This is pointer-sized, and matches the `Payload` type in `src/libpanic_unwind/miri.rs`. + panic_payload: Option>, + } impl<'mir, 'tcx> Thread<'mir, 'tcx> { @@ -150,6 +159,7 @@ impl<'mir, 'tcx> Default for Thread<'mir, 'tcx> { thread_name: None, stack: Vec::new(), join_status: ThreadJoinStatus::Joinable, + panic_payload: None, } } } @@ -509,6 +519,21 @@ impl<'mir, 'tcx: 'mir> ThreadManager<'mir, 'tcx> { throw_machine_stop!(TerminationInfo::Deadlock); } } + + /// Store the panic payload when beginning unwinding. + fn set_panic_payload(&mut self, payload: Scalar) { + let thread = self.active_thread_mut(); + assert!( + thread.panic_payload.is_none(), + "the panic runtime should avoid double-panics" + ); + thread.panic_payload = Some(payload); + } + + /// Retrieve the panic payload, for use in `catch_unwind`. + fn take_panic_payload(&mut self) -> Scalar { + self.active_thread_mut().panic_payload.take().unwrap() + } } // Public interface to thread management. @@ -686,4 +711,16 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx } Ok(()) } + + /// Store the panic payload when beginning unwinding. + fn set_panic_payload(&mut self, payload: Scalar) { + let this = self.eval_context_mut(); + this.machine.threads.set_panic_payload(payload); + } + + /// Retrieve the panic payload, for use in `catch_unwind`. + fn take_panic_payload(&mut self) -> Scalar { + let this = self.eval_context_mut(); + this.machine.threads.take_panic_payload() + } } diff --git a/tests/run-pass/panic/concurrent-panic.rs b/tests/run-pass/panic/concurrent-panic.rs new file mode 100644 index 000000000000..e798f514711f --- /dev/null +++ b/tests/run-pass/panic/concurrent-panic.rs @@ -0,0 +1,80 @@ +// ignore-windows: Concurrency on Windows is not supported yet. +use std::sync::{Arc, Condvar, Mutex}; +use std::thread::{spawn, JoinHandle}; + +struct BlockOnDrop(Option>); + +impl BlockOnDrop { + fn new(handle: JoinHandle<()>) -> BlockOnDrop { + BlockOnDrop(Some(handle)) + } +} + +impl Drop for BlockOnDrop { + fn drop(&mut self) { + let _ = self.0.take().unwrap().join(); + } +} + +/// Cause a panic in one thread while another thread is unwinding. +fn main() { + let t1_started_pair = Arc::new((Mutex::new(false), Condvar::new())); + let t2_started_pair = Arc::new((Mutex::new(false), Condvar::new())); + + let t1_continue_mutex = Arc::new(Mutex::new(())); + let t1_continue_guard = t1_continue_mutex.lock(); + + let t1 = { + let t1_started_pair = t1_started_pair.clone(); + let t1_continue_mutex = t1_continue_mutex.clone(); + spawn(move || { + let (mutex, condvar) = &*t1_started_pair; + *mutex.lock().unwrap() = true; + condvar.notify_one(); + + drop(t1_continue_mutex.lock()); + panic!("panic in thread 1"); + }) + }; + let t2 = { + let t2_started_pair = t2_started_pair.clone(); + let block_on_drop = BlockOnDrop::new(t1); + spawn(move || { + let _ = block_on_drop; + + let (mutex, condvar) = &*t2_started_pair; + *mutex.lock().unwrap() = true; + condvar.notify_one(); + + panic!("panic in thread 2"); + }) + }; + + // Wait for thread 1 to signal it has started. + let (t1_started_mutex, t1_started_condvar) = &*t1_started_pair; + let mut t1_started_guard = t1_started_mutex.lock().unwrap(); + while !*t1_started_guard { + t1_started_guard = t1_started_condvar.wait(t1_started_guard).unwrap(); + } + // Thread 1 should now be blocked waiting on t1_continue_mutex. + + // Wait for thread 2 to signal it has started. + let (t2_started_mutex, t2_started_condvar) = &*t2_started_pair; + let mut t2_started_guard = t2_started_mutex.lock().unwrap(); + while !*t2_started_guard { + t2_started_guard = t2_started_condvar.wait(t2_started_guard).unwrap(); + } + // Thread 2 should now have already panicked and be in the middle of + // unwinding. It should now be blocked on joining thread 1. + + // Unlock t1_continue_mutex, and allow thread 1 to proceed. + drop(t1_continue_guard); + // Thread 1 will panic the next time it is scheduled. This will test the + // behavior of interest to this test, whether Miri properly handles + // concurrent panics in two different threads. + + // Block the main thread on waiting to join thread 2. Thread 2 should + // already be blocked on joining thread 1, so thread 1 will be scheduled + // to run next, as it is the only ready thread. + assert!(t2.join().is_err()); +} diff --git a/tests/run-pass/panic/concurrent-panic.stderr b/tests/run-pass/panic/concurrent-panic.stderr new file mode 100644 index 000000000000..6652137c9659 --- /dev/null +++ b/tests/run-pass/panic/concurrent-panic.stderr @@ -0,0 +1,4 @@ +warning: thread support is experimental. For example, Miri does not detect data races yet. + +thread '' panicked at 'panic in thread 2', $DIR/concurrent-panic.rs:49:13 +thread '' panicked at 'panic in thread 1', $DIR/concurrent-panic.rs:36:13