diff --git a/src/machine.rs b/src/machine.rs index 4ebf7dceab83..943d5570f7b7 100644 --- a/src/machine.rs +++ b/src/machine.rs @@ -418,6 +418,7 @@ impl<'mir, 'tcx> Evaluator<'mir, 'tcx> { ) -> InterpResult<'tcx> { EnvVars::init(this, config)?; Evaluator::init_extern_statics(this)?; + ThreadManager::init(this); Ok(()) } diff --git a/src/shims/time.rs b/src/shims/time.rs index c3eb0161c210..e495f723668d 100644 --- a/src/shims/time.rs +++ b/src/shims/time.rs @@ -233,4 +233,30 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx Ok(0) } + + #[allow(non_snake_case)] + fn Sleep(&mut self, timeout: &OpTy<'tcx, Provenance>) -> InterpResult<'tcx> { + let this = self.eval_context_mut(); + + this.check_no_isolation("`Sleep`")?; + + let timeout_ms = this.read_scalar(timeout)?.to_u32()?; + + let duration = Duration::from_millis(timeout_ms.into()); + let timeout_time = Time::Monotonic(Instant::now().checked_add(duration).unwrap()); + + let active_thread = this.get_active_thread(); + this.block_thread(active_thread); + + this.register_timeout_callback( + active_thread, + timeout_time, + Box::new(move |ecx| { + ecx.unblock_thread(active_thread); + Ok(()) + }), + ); + + Ok(()) + } } diff --git a/src/shims/unix/thread.rs b/src/shims/unix/thread.rs index 094183b3e781..d675df0f53f1 100644 --- a/src/shims/unix/thread.rs +++ b/src/shims/unix/thread.rs @@ -37,7 +37,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx } let thread_id = this.read_scalar(thread)?.to_machine_usize(this)?; - this.join_thread(thread_id.try_into().expect("thread ID should fit in u32"))?; + this.join_thread_exclusive(thread_id.try_into().expect("thread ID should fit in u32"))?; Ok(0) } @@ -46,7 +46,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx let this = self.eval_context_mut(); let thread_id = this.read_scalar(thread)?.to_machine_usize(this)?; - this.detach_thread(thread_id.try_into().expect("thread ID should fit in u32"))?; + this.detach_thread(thread_id.try_into().expect("thread ID should fit in u32"), false)?; Ok(0) } diff --git a/src/shims/windows/dlsym.rs b/src/shims/windows/dlsym.rs index f18e27d38c2b..d64be9cc0527 100644 --- a/src/shims/windows/dlsym.rs +++ b/src/shims/windows/dlsym.rs @@ -5,7 +5,7 @@ use rustc_target::spec::abi::Abi; use log::trace; use crate::helpers::check_arg_count; -use crate::shims::windows::handle::Handle; +use crate::shims::windows::handle::{EvalContextExt as _, Handle}; use crate::*; #[derive(Debug, Copy, Clone)] @@ -118,7 +118,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx match Handle::from_scalar(this.read_scalar(handle)?.check_init()?, this)? { Some(Handle::Thread(thread)) => thread, Some(Handle::CurrentThread) => this.get_active_thread(), - _ => throw_ub_format!("invalid handle"), + _ => this.invalid_handle("SetThreadDescription")?, }; this.set_thread_name_wide(thread, name); diff --git a/src/shims/windows/foreign_items.rs b/src/shims/windows/foreign_items.rs index 00a80f869751..6014281100fc 100644 --- a/src/shims/windows/foreign_items.rs +++ b/src/shims/windows/foreign_items.rs @@ -228,24 +228,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx let [timeout] = this.check_shim(abi, Abi::System { unwind: false }, link_name, args)?; - this.check_no_isolation("`Sleep`")?; - - let timeout_ms = this.read_scalar(timeout)?.to_u32()?; - - let duration = Duration::from_millis(timeout_ms as u64); - let timeout_time = Time::Monotonic(Instant::now().checked_add(duration).unwrap()); - - let active_thread = this.get_active_thread(); - this.block_thread(active_thread); - - this.register_timeout_callback( - active_thread, - timeout_time, - Box::new(move |ecx| { - ecx.unblock_thread(active_thread); - Ok(()) - }), - ); + this.Sleep(timeout)?; } // Synchronization primitives diff --git a/src/shims/windows/handle.rs b/src/shims/windows/handle.rs index ff64a958da3e..e1617ae6a8d9 100644 --- a/src/shims/windows/handle.rs +++ b/src/shims/windows/handle.rs @@ -146,16 +146,19 @@ impl<'mir, 'tcx> EvalContextExt<'mir, 'tcx> for crate::MiriEvalContext<'mir, 'tc #[allow(non_snake_case)] pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx> { + fn invalid_handle(&mut self, function_name: &str) -> InterpResult<'tcx, !> { + throw_machine_stop!(TerminationInfo::Abort(format!( + "invalid handle passed to `{function_name}`" + ))) + } + fn CloseHandle(&mut self, handle_op: &OpTy<'tcx, Provenance>) -> InterpResult<'tcx> { let this = self.eval_context_mut(); match Handle::from_scalar(this.read_scalar(handle_op)?.check_init()?, this)? { - Some(Handle::Thread(thread)) => this.detach_thread(thread)?, - _ => - throw_machine_stop!(TerminationInfo::Abort( - "invalid handle passed to `CloseHandle`".into() - )), - }; + Some(Handle::Thread(thread)) => this.detach_thread(thread, true)?, + _ => this.invalid_handle("CloseHandle")?, + } Ok(()) } diff --git a/src/shims/windows/thread.rs b/src/shims/windows/thread.rs index 66cd9dd0348f..6b6c4916dee2 100644 --- a/src/shims/windows/thread.rs +++ b/src/shims/windows/thread.rs @@ -1,11 +1,8 @@ -use std::time::{Duration, Instant}; - use rustc_middle::ty::layout::LayoutOf; use rustc_target::spec::abi::Abi; -use crate::thread::Time; use crate::*; -use shims::windows::handle::Handle; +use shims::windows::handle::{EvalContextExt as _, Handle}; impl<'mir, 'tcx: 'mir> EvalContextExt<'mir, 'tcx> for crate::MiriEvalContext<'mir, 'tcx> {} @@ -59,25 +56,17 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx let thread = match Handle::from_scalar(this.read_scalar(handle)?.check_init()?, this)? { Some(Handle::Thread(thread)) => thread, - Some(Handle::CurrentThread) => throw_ub_format!("trying to wait on itself"), - _ => throw_ub_format!("invalid handle"), + // Unlike on posix, joining the current thread is not UB on windows. + // It will just deadlock. + Some(Handle::CurrentThread) => this.get_active_thread(), + _ => this.invalid_handle("WaitForSingleObject")?, }; if this.read_scalar(timeout)?.to_u32()? != this.eval_windows("c", "INFINITE")?.to_u32()? { - this.check_no_isolation("`WaitForSingleObject` with non-infinite timeout")?; + throw_unsup_format!("`WaitForSingleObject` with non-infinite timeout"); } - let timeout_ms = this.read_scalar(timeout)?.to_u32()?; - - let timeout_time = if timeout_ms == this.eval_windows("c", "INFINITE")?.to_u32()? { - None - } else { - let duration = Duration::from_millis(timeout_ms as u64); - - Some(Time::Monotonic(Instant::now().checked_add(duration).unwrap())) - }; - - this.wait_on_thread(timeout_time, thread)?; + this.join_thread(thread)?; Ok(()) } diff --git a/src/thread.rs b/src/thread.rs index fa70dcfa2a3d..f7fcdd5822a2 100644 --- a/src/thread.rs +++ b/src/thread.rs @@ -405,6 +405,31 @@ impl<'mir, 'tcx: 'mir> ThreadManager<'mir, 'tcx> { Ok(()) } + /// Mark that the active thread tries to exclusively join the thread with `joined_thread_id`. + /// If the thread is already joined by another thread + fn join_thread_exclusive( + &mut self, + joined_thread_id: ThreadId, + data_race: Option<&mut data_race::GlobalState>, + ) -> InterpResult<'tcx> { + if self.threads[joined_thread_id].join_status == ThreadJoinStatus::Joined { + throw_ub_format!("trying to join an already joined thread"); + } + + if joined_thread_id == self.active_thread { + throw_ub_format!("trying to join itself"); + } + + assert!( + self.threads + .iter() + .all(|thread| thread.state != ThreadState::BlockedOnJoin(joined_thread_id)), + "a joinable thread already has threads waiting for its termination" + ); + + self.join_thread(joined_thread_id, data_race) + } + /// Set the name of the given thread. pub fn set_thread_name(&mut self, thread: ThreadId, new_thread_name: Vec) { self.threads[thread].thread_name = Some(new_thread_name); @@ -700,6 +725,15 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx Ok(()) } + #[inline] + fn join_thread_exclusive(&mut self, joined_thread_id: ThreadId) -> InterpResult<'tcx> { + let this = self.eval_context_mut(); + this.machine + .threads + .join_thread_exclusive(joined_thread_id, this.machine.data_race.as_mut())?; + Ok(()) + } + #[inline] fn set_active_thread(&mut self, thread_id: ThreadId) -> ThreadId { let this = self.eval_context_mut(); diff --git a/tests/fail/concurrency/unwind_top_of_stack.rs b/tests/fail/concurrency/unwind_top_of_stack.rs index 61d3b964e620..4704cfed0393 100644 --- a/tests/fail/concurrency/unwind_top_of_stack.rs +++ b/tests/fail/concurrency/unwind_top_of_stack.rs @@ -1,4 +1,4 @@ -//@ignore-windows: No libc on Windows +//@ignore-target-windows: No libc on Windows //@compile-flags: -Zmiri-disable-abi-check diff --git a/tests/fail/concurrency/windows_join_detached.rs b/tests/fail/concurrency/windows_join_detached.rs new file mode 100644 index 000000000000..548ed63534db --- /dev/null +++ b/tests/fail/concurrency/windows_join_detached.rs @@ -0,0 +1,21 @@ +//@only-target-windows: Uses win32 api functions +//@error-pattern: Undefined Behavior: trying to join a detached thread + +// Joining a detached thread is undefined behavior. + +use std::os::windows::io::{AsRawHandle, RawHandle}; +use std::thread; + +extern "system" { + fn CloseHandle(handle: RawHandle) -> u32; +} + +fn main() { + let thread = thread::spawn(|| ()); + + unsafe { + assert_ne!(CloseHandle(thread.as_raw_handle()), 0); + } + + thread.join().unwrap(); +} diff --git a/tests/fail/concurrency/windows_join_detached.stderr b/tests/fail/concurrency/windows_join_detached.stderr new file mode 100644 index 000000000000..a0e85f6ce5ab --- /dev/null +++ b/tests/fail/concurrency/windows_join_detached.stderr @@ -0,0 +1,22 @@ +error: Undefined Behavior: trying to join a detached thread + --> RUSTLIB/std/src/sys/PLATFORM/thread.rs:LL:CC + | +LL | let rc = unsafe { c::WaitForSingleObject(self.handle.as_raw_handle(), c::INFINITE) }; + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ trying to join a detached thread + | + = help: this indicates a bug in the program: it performed an invalid operation, and caused Undefined Behavior + = help: see https://doc.rust-lang.org/nightly/reference/behavior-considered-undefined.html for further information + = note: backtrace: + = note: inside `std::sys::PLATFORM::thread::Thread::join` at RUSTLIB/std/src/sys/PLATFORM/thread.rs:LL:CC + = note: inside `std::thread::JoinInner::<()>::join` at RUSTLIB/std/src/thread/mod.rs:LL:CC + = note: inside `std::thread::JoinHandle::<()>::join` at RUSTLIB/std/src/thread/mod.rs:LL:CC +note: inside `main` at $DIR/windows_join_detached.rs:LL:CC + --> $DIR/windows_join_detached.rs:LL:CC + | +LL | thread.join().unwrap(); + | ^^^^^^^^^^^^^ + +note: some details are omitted, run with `MIRIFLAGS=-Zmiri-backtrace=full` for a verbose backtrace + +error: aborting due to previous error + diff --git a/tests/fail/concurrency/windows_join_main.rs b/tests/fail/concurrency/windows_join_main.rs new file mode 100644 index 000000000000..ea52220d4499 --- /dev/null +++ b/tests/fail/concurrency/windows_join_main.rs @@ -0,0 +1,28 @@ +//@only-target-windows: Uses win32 api functions +// We are making scheduler assumptions here. +//@compile-flags: -Zmiri-preemption-rate=0 + +// On windows, joining main is not UB, but it will block a thread forever. + +use std::thread; + +extern "system" { + fn WaitForSingleObject(handle: usize, timeout: u32) -> u32; +} + +const INFINITE: u32 = u32::MAX; + +// This is how miri represents the handle for thread 0. +// This value can be "legitimately" obtained by using `GetCurrentThread` with `DuplicateHandle` +// but miri does not implement `DuplicateHandle` yet. +const MAIN_THREAD: usize = 1 << 30; + +fn main() { + thread::spawn(|| { + unsafe { + assert_eq!(WaitForSingleObject(MAIN_THREAD, INFINITE), 0); //~ ERROR: deadlock: the evaluated program deadlocked + } + }) + .join() + .unwrap(); +} diff --git a/tests/fail/concurrency/windows_join_main.stderr b/tests/fail/concurrency/windows_join_main.stderr new file mode 100644 index 000000000000..72b854d354a3 --- /dev/null +++ b/tests/fail/concurrency/windows_join_main.stderr @@ -0,0 +1,12 @@ +error: deadlock: the evaluated program deadlocked + --> $DIR/windows_join_main.rs:LL:CC + | +LL | WaitForSingleObject(MAIN_THREAD, INFINITE); + | ^ the evaluated program deadlocked + | + = note: inside closure at $DIR/windows_join_main.rs:LL:CC + +note: some details are omitted, run with `MIRIFLAGS=-Zmiri-backtrace=full` for a verbose backtrace + +error: aborting due to previous error + diff --git a/tests/fail/concurrency/windows_join_self.rs b/tests/fail/concurrency/windows_join_self.rs new file mode 100644 index 000000000000..d9bbf66a7dca --- /dev/null +++ b/tests/fail/concurrency/windows_join_self.rs @@ -0,0 +1,25 @@ +//@only-target-windows: Uses win32 api functions +// We are making scheduler assumptions here. +//@compile-flags: -Zmiri-preemption-rate=0 + +// On windows, a thread joining itself is not UB, but it will deadlock. + +use std::thread; + +extern "system" { + fn GetCurrentThread() -> usize; + fn WaitForSingleObject(handle: usize, timeout: u32) -> u32; +} + +const INFINITE: u32 = u32::MAX; + +fn main() { + thread::spawn(|| { + unsafe { + let native = GetCurrentThread(); + assert_eq!(WaitForSingleObject(native, INFINITE), 0); //~ ERROR: deadlock: the evaluated program deadlocked + } + }) + .join() + .unwrap(); +} diff --git a/tests/fail/concurrency/windows_join_self.stderr b/tests/fail/concurrency/windows_join_self.stderr new file mode 100644 index 000000000000..bbec3f7257ec --- /dev/null +++ b/tests/fail/concurrency/windows_join_self.stderr @@ -0,0 +1,12 @@ +error: deadlock: the evaluated program deadlocked + --> $DIR/windows_join_self.rs:LL:CC + | +LL | assert_eq!(WaitForSingleObject(native, INFINITE), 0); + | ^ the evaluated program deadlocked + | + = note: inside closure at $DIR/windows_join_self.rs:LL:CC + +note: some details are omitted, run with `MIRIFLAGS=-Zmiri-backtrace=full` for a verbose backtrace + +error: aborting due to previous error + diff --git a/tests/pass/concurrency/windows_detach_terminated.rs b/tests/pass/concurrency/windows_detach_terminated.rs new file mode 100644 index 000000000000..91088ce6aef9 --- /dev/null +++ b/tests/pass/concurrency/windows_detach_terminated.rs @@ -0,0 +1,21 @@ +//@only-target-windows: Uses win32 api functions +// We are making scheduler assumptions here. +//@compile-flags: -Zmiri-preemption-rate=0 + +use std::os::windows::io::IntoRawHandle; +use std::thread; + +extern "system" { + fn CloseHandle(handle: usize) -> i32; +} + +fn main() { + let thread = thread::spawn(|| {}).into_raw_handle() as usize; + + // this yield ensures that `thread` is terminated by this point + thread::yield_now(); + + unsafe { + assert_ne!(CloseHandle(thread), 0); + } +} diff --git a/tests/pass/concurrency/windows_join_multiple.rs b/tests/pass/concurrency/windows_join_multiple.rs new file mode 100644 index 000000000000..986e2b8cc10f --- /dev/null +++ b/tests/pass/concurrency/windows_join_multiple.rs @@ -0,0 +1,41 @@ +//@only-target-windows: Uses win32 api functions +// We are making scheduler assumptions here. +//@compile-flags: -Zmiri-preemption-rate=0 + +use std::os::windows::io::IntoRawHandle; +use std::sync::atomic::{AtomicBool, Ordering}; +use std::thread; + +extern "system" { + fn WaitForSingleObject(handle: usize, timeout: u32) -> u32; +} + +const INFINITE: u32 = u32::MAX; + +fn main() { + static FLAG: AtomicBool = AtomicBool::new(false); + + let blocker = thread::spawn(|| { + while !FLAG.load(Ordering::Relaxed) { + thread::yield_now(); + } + }) + .into_raw_handle() as usize; + + let waiter = move || { + unsafe { + assert_eq!(WaitForSingleObject(blocker, INFINITE), 0); + } + }; + + let waiter1 = thread::spawn(waiter); + let waiter2 = thread::spawn(waiter); + + // this yield ensures `waiter1` & `waiter2` are blocked on `blocker` by this point + thread::yield_now(); + + FLAG.store(true, Ordering::Relaxed); + + waiter1.join().unwrap(); + waiter2.join().unwrap(); +}