diff --git a/src/shims/panic.rs b/src/shims/panic.rs index b9d8ceb1dfb..f907b76b679 100644 --- a/src/shims/panic.rs +++ b/src/shims/panic.rs @@ -48,7 +48,12 @@ 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()?; - this.set_panic_payload(payload); + let thread = this.active_thread_mut(); + assert!( + thread.panic_payload.is_none(), + "the panic runtime should avoid double-panics" + ); + thread.panic_payload = Some(payload); // Jump to the unwind block to begin unwinding. this.unwind_to_block(unwind); @@ -130,7 +135,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx // 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.take_panic_payload(); + let payload = this.active_thread_mut().panic_payload.take().unwrap(); // 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 dd5358bfb5e..a542d0895b2 100644 --- a/src/thread.rs +++ b/src/thread.rs @@ -119,8 +119,7 @@ pub struct Thread<'mir, 'tcx> { /// 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>, - + pub(crate) panic_payload: Option>, } impl<'mir, 'tcx> Thread<'mir, 'tcx> { @@ -519,21 +518,6 @@ 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. @@ -593,6 +577,12 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx this.machine.threads.get_active_thread_id() } + #[inline] + fn active_thread_mut(&mut self) -> &mut Thread<'mir, 'tcx> { + let this = self.eval_context_mut(); + this.machine.threads.active_thread_mut() + } + #[inline] fn get_total_thread_count(&self) -> usize { let this = self.eval_context_ref(); @@ -711,16 +701,4 @@ 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 index e798f514711..0ff5788e204 100644 --- a/tests/run-pass/panic/concurrent-panic.rs +++ b/tests/run-pass/panic/concurrent-panic.rs @@ -1,4 +1,8 @@ // ignore-windows: Concurrency on Windows is not supported yet. + +//! Cause a panic in one thread while another thread is unwinding. This checks +//! that separate threads have their own panicking state. + use std::sync::{Arc, Condvar, Mutex}; use std::thread::{spawn, JoinHandle}; @@ -12,11 +16,12 @@ impl BlockOnDrop { impl Drop for BlockOnDrop { fn drop(&mut self) { + eprintln!("Thread 2 blocking on thread 1"); let _ = self.0.take().unwrap().join(); + eprintln!("Thread 1 has exited"); } } -/// 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())); @@ -28,6 +33,7 @@ fn main() { let t1_started_pair = t1_started_pair.clone(); let t1_continue_mutex = t1_continue_mutex.clone(); spawn(move || { + eprintln!("Thread 1 starting, will block on mutex"); let (mutex, condvar) = &*t1_started_pair; *mutex.lock().unwrap() = true; condvar.notify_one(); @@ -36,6 +42,16 @@ fn main() { panic!("panic in thread 1"); }) }; + + // 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(); + } + eprintln!("Thread 1 reported it has started"); + // Thread 1 should now be blocked waiting on t1_continue_mutex. + let t2 = { let t2_started_pair = t2_started_pair.clone(); let block_on_drop = BlockOnDrop::new(t1); @@ -50,24 +66,18 @@ fn main() { }) }; - // 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(); } + eprintln!("Thread 2 reported it has started"); // 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. + eprintln!("Unlocking mutex"); 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 @@ -77,4 +87,5 @@ fn main() { // 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()); + eprintln!("Thread 2 has exited"); } diff --git a/tests/run-pass/panic/concurrent-panic.stderr b/tests/run-pass/panic/concurrent-panic.stderr index 6652137c965..d538efdb0e8 100644 --- a/tests/run-pass/panic/concurrent-panic.stderr +++ b/tests/run-pass/panic/concurrent-panic.stderr @@ -1,4 +1,11 @@ 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 +Thread 1 starting, will block on mutex +Thread 1 reported it has started +thread '' panicked at 'panic in thread 2', $DIR/concurrent-panic.rs:65:13 +Thread 2 blocking on thread 1 +Thread 2 reported it has started +Unlocking mutex +thread '' panicked at 'panic in thread 1', $DIR/concurrent-panic.rs:42:13 +Thread 1 has exited +Thread 2 has exited