From 1254d8e75ea4aa5fa06030ee55959c0d40737ec5 Mon Sep 17 00:00:00 2001 From: Yoh Deadfall Date: Thu, 31 Oct 2024 02:30:44 +0300 Subject: [PATCH] Get/set thread name shims return errors for invalid handles --- src/tools/miri/src/concurrency/thread.rs | 44 +++++++------ .../miri/src/shims/unix/android/thread.rs | 6 +- .../miri/src/shims/unix/foreign_items.rs | 8 +-- .../src/shims/unix/linux/foreign_items.rs | 20 ++++-- .../src/shims/unix/macos/foreign_items.rs | 22 +++---- src/tools/miri/src/shims/unix/mod.rs | 2 +- .../src/shims/unix/solarish/foreign_items.rs | 21 ++++--- src/tools/miri/src/shims/unix/thread.rs | 62 +++++++++++++------ .../miri/src/shims/windows/foreign_items.rs | 56 ++++++++++------- src/tools/miri/src/shims/windows/handle.rs | 25 +++++--- src/tools/miri/src/shims/windows/thread.rs | 10 ++- .../tests/pass-dep/libc/pthread-threadname.rs | 25 ++++++++ 12 files changed, 192 insertions(+), 109 deletions(-) diff --git a/src/tools/miri/src/concurrency/thread.rs b/src/tools/miri/src/concurrency/thread.rs index 281242bf373..7477494281d 100644 --- a/src/tools/miri/src/concurrency/thread.rs +++ b/src/tools/miri/src/concurrency/thread.rs @@ -1,7 +1,6 @@ //! Implements threads. use std::mem; -use std::num::TryFromIntError; use std::sync::atomic::Ordering::Relaxed; use std::task::Poll; use std::time::{Duration, SystemTime}; @@ -127,26 +126,6 @@ fn index(self) -> usize { } } -impl TryFrom for ThreadId { - type Error = TryFromIntError; - fn try_from(id: u64) -> Result { - u32::try_from(id).map(Self) - } -} - -impl TryFrom for ThreadId { - type Error = TryFromIntError; - fn try_from(id: i128) -> Result { - u32::try_from(id).map(Self) - } -} - -impl From for ThreadId { - fn from(id: u32) -> Self { - Self(id) - } -} - impl From for u64 { fn from(t: ThreadId) -> Self { t.0.into() @@ -448,6 +427,10 @@ pub enum TimeoutAnchor { Absolute, } +/// An error signaling that the requested thread doesn't exist. +#[derive(Debug, Copy, Clone)] +pub struct ThreadNotFound; + /// A set of threads. #[derive(Debug)] pub struct ThreadManager<'tcx> { @@ -509,6 +492,16 @@ pub(crate) fn init( } } + pub fn thread_id_try_from(&self, id: impl TryInto) -> Result { + if let Ok(id) = id.try_into() + && usize::try_from(id).is_ok_and(|id| id < self.threads.len()) + { + Ok(ThreadId(id)) + } else { + Err(ThreadNotFound) + } + } + /// Check if we have an allocation for the given thread local static for the /// active thread. fn get_thread_local_alloc_id(&self, def_id: DefId) -> Option { @@ -534,6 +527,7 @@ pub fn active_thread_stack_mut( ) -> &mut Vec>> { &mut self.threads[self.active_thread].stack } + pub fn all_stacks( &self, ) -> impl Iterator>])> { @@ -868,6 +862,11 @@ fn run_on_stack_empty(&mut self) -> InterpResult<'tcx, Poll<()>> { // Public interface to thread management. impl<'tcx> EvalContextExt<'tcx> for crate::MiriInterpCx<'tcx> {} pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { + #[inline] + fn thread_id_try_from(&self, id: impl TryInto) -> Result { + self.eval_context_ref().machine.threads.thread_id_try_from(id) + } + /// Get a thread-specific allocation id for the given thread-local static. /// If needed, allocate a new one. fn get_or_create_thread_local_alloc( @@ -1160,8 +1159,7 @@ fn active_thread_stack_mut<'a>( /// Set the name of the current thread. The buffer must not include the null terminator. #[inline] fn set_thread_name(&mut self, thread: ThreadId, new_thread_name: Vec) { - let this = self.eval_context_mut(); - this.machine.threads.set_thread_name(thread, new_thread_name); + self.eval_context_mut().machine.threads.set_thread_name(thread, new_thread_name); } #[inline] diff --git a/src/tools/miri/src/shims/unix/android/thread.rs b/src/tools/miri/src/shims/unix/android/thread.rs index 1da13d48252..093b7405ccd 100644 --- a/src/tools/miri/src/shims/unix/android/thread.rs +++ b/src/tools/miri/src/shims/unix/android/thread.rs @@ -2,7 +2,7 @@ use rustc_span::Symbol; use crate::helpers::check_min_arg_count; -use crate::shims::unix::thread::EvalContextExt as _; +use crate::shims::unix::thread::{EvalContextExt as _, ThreadNameResult}; use crate::*; const TASK_COMM_LEN: usize = 16; @@ -32,7 +32,7 @@ pub fn prctl<'tcx>( // https://www.man7.org/linux/man-pages/man2/PR_SET_NAME.2const.html let res = this.pthread_setname_np(thread, name, TASK_COMM_LEN, /* truncate */ true)?; - assert!(res); + assert_eq!(res, ThreadNameResult::Ok); Scalar::from_u32(0) } op if op == pr_get_name => { @@ -46,7 +46,7 @@ pub fn prctl<'tcx>( CheckInAllocMsg::MemoryAccessTest, )?; let res = this.pthread_getname_np(thread, name, len, /* truncate*/ false)?; - assert!(res); + assert_eq!(res, ThreadNameResult::Ok); Scalar::from_u32(0) } op => throw_unsup_format!("Miri does not support `prctl` syscall with op={}", op), diff --git a/src/tools/miri/src/shims/unix/foreign_items.rs b/src/tools/miri/src/shims/unix/foreign_items.rs index d59d6712c4f..55202a08149 100644 --- a/src/tools/miri/src/shims/unix/foreign_items.rs +++ b/src/tools/miri/src/shims/unix/foreign_items.rs @@ -603,13 +603,13 @@ fn emulate_foreign_item_inner( } "pthread_join" => { let [thread, retval] = this.check_shim(abi, ExternAbi::C { unwind: false }, link_name, args)?; - this.pthread_join(thread, retval)?; - this.write_null(dest)?; + let res = this.pthread_join(thread, retval)?; + this.write_scalar(res, dest)?; } "pthread_detach" => { let [thread] = this.check_shim(abi, ExternAbi::C { unwind: false }, link_name, args)?; - this.pthread_detach(thread)?; - this.write_null(dest)?; + let res = this.pthread_detach(thread)?; + this.write_scalar(res, dest)?; } "pthread_self" => { let [] = this.check_shim(abi, ExternAbi::C { unwind: false }, link_name, args)?; diff --git a/src/tools/miri/src/shims/unix/linux/foreign_items.rs b/src/tools/miri/src/shims/unix/linux/foreign_items.rs index 35658ebc2f2..85f0d6e1330 100644 --- a/src/tools/miri/src/shims/unix/linux/foreign_items.rs +++ b/src/tools/miri/src/shims/unix/linux/foreign_items.rs @@ -81,13 +81,17 @@ fn emulate_foreign_item_inner( "pthread_setname_np" => { let [thread, name] = this.check_shim(abi, ExternAbi::C { unwind: false }, link_name, args)?; - let res = this.pthread_setname_np( + let res = match this.pthread_setname_np( this.read_scalar(thread)?, this.read_scalar(name)?, TASK_COMM_LEN, /* truncate */ false, - )?; - let res = if res { Scalar::from_u32(0) } else { this.eval_libc("ERANGE") }; + )? { + ThreadNameResult::Ok => Scalar::from_u32(0), + ThreadNameResult::NameTooLong => this.eval_libc("ERANGE"), + // Act like we faild to open `/proc/self/task/$tid/comm`. + ThreadNameResult::ThreadNotFound => this.eval_libc("ENOENT"), + }; this.write_scalar(res, dest)?; } "pthread_getname_np" => { @@ -97,14 +101,18 @@ fn emulate_foreign_item_inner( // In case of glibc, the length of the output buffer must // be not shorter than TASK_COMM_LEN. let len = this.read_scalar(len)?; - let res = if len.to_target_usize(this)? >= TASK_COMM_LEN as u64 - && this.pthread_getname_np( + let res = if len.to_target_usize(this)? >= TASK_COMM_LEN as u64 { + match this.pthread_getname_np( this.read_scalar(thread)?, this.read_scalar(name)?, len, /* truncate*/ false, )? { - Scalar::from_u32(0) + ThreadNameResult::Ok => Scalar::from_u32(0), + ThreadNameResult::NameTooLong => unreachable!(), + // Act like we faild to open `/proc/self/task/$tid/comm`. + ThreadNameResult::ThreadNotFound => this.eval_libc("ENOENT"), + } } else { this.eval_libc("ERANGE") }; diff --git a/src/tools/miri/src/shims/unix/macos/foreign_items.rs b/src/tools/miri/src/shims/unix/macos/foreign_items.rs index b77b46e325d..003025916cd 100644 --- a/src/tools/miri/src/shims/unix/macos/foreign_items.rs +++ b/src/tools/miri/src/shims/unix/macos/foreign_items.rs @@ -181,18 +181,16 @@ fn emulate_foreign_item_inner( // are met, then the name is set and 0 is returned. Otherwise, if // the specified name is lomnger than MAXTHREADNAMESIZE, then // ENAMETOOLONG is returned. - // - // FIXME: the real implementation maybe returns ESRCH if the thread ID is invalid. let thread = this.pthread_self()?; - let res = if this.pthread_setname_np( + let res = match this.pthread_setname_np( thread, this.read_scalar(name)?, this.eval_libc("MAXTHREADNAMESIZE").to_target_usize(this)?.try_into().unwrap(), /* truncate */ false, )? { - Scalar::from_u32(0) - } else { - this.eval_libc("ENAMETOOLONG") + ThreadNameResult::Ok => Scalar::from_u32(0), + ThreadNameResult::NameTooLong => this.eval_libc("ENAMETOOLONG"), + ThreadNameResult::ThreadNotFound => unreachable!(), }; // Contrary to the manpage, `pthread_setname_np` on macOS still // returns an integer indicating success. @@ -210,15 +208,17 @@ fn emulate_foreign_item_inner( // https://github.com/apple-oss-distributions/libpthread/blob/c032e0b076700a0a47db75528a282b8d3a06531a/src/pthread.c#L1160-L1175. // The key part is the strlcpy, which truncates the resulting value, // but always null terminates (except for zero sized buffers). - // - // FIXME: the real implementation returns ESRCH if the thread ID is invalid. - let res = Scalar::from_u32(0); - this.pthread_getname_np( + let res = match this.pthread_getname_np( this.read_scalar(thread)?, this.read_scalar(name)?, this.read_scalar(len)?, /* truncate */ true, - )?; + )? { + ThreadNameResult::Ok => Scalar::from_u32(0), + // `NameTooLong` is possible when the buffer is zero sized, + ThreadNameResult::NameTooLong => Scalar::from_u32(0), + ThreadNameResult::ThreadNotFound => this.eval_libc("ESRCH"), + }; this.write_scalar(res, dest)?; } diff --git a/src/tools/miri/src/shims/unix/mod.rs b/src/tools/miri/src/shims/unix/mod.rs index 9bc310e8d0a..c8c25c636ee 100644 --- a/src/tools/miri/src/shims/unix/mod.rs +++ b/src/tools/miri/src/shims/unix/mod.rs @@ -21,7 +21,7 @@ pub use self::linux::epoll::EpollInterestTable; pub use self::mem::EvalContextExt as _; pub use self::sync::EvalContextExt as _; -pub use self::thread::EvalContextExt as _; +pub use self::thread::{EvalContextExt as _, ThreadNameResult}; pub use self::unnamed_socket::EvalContextExt as _; // Make up some constants. diff --git a/src/tools/miri/src/shims/unix/solarish/foreign_items.rs b/src/tools/miri/src/shims/unix/solarish/foreign_items.rs index efdc64f45fc..526b64cff69 100644 --- a/src/tools/miri/src/shims/unix/solarish/foreign_items.rs +++ b/src/tools/miri/src/shims/unix/solarish/foreign_items.rs @@ -26,26 +26,33 @@ fn emulate_foreign_item_inner( // THREAD_NAME_MAX allows a thread name of 31+1 length // https://github.com/illumos/illumos-gate/blob/7671517e13b8123748eda4ef1ee165c6d9dba7fe/usr/src/uts/common/sys/thread.h#L613 let max_len = 32; - let res = this.pthread_setname_np( + // See https://illumos.org/man/3C/pthread_setname_np for the error codes. + let res = match this.pthread_setname_np( this.read_scalar(thread)?, this.read_scalar(name)?, max_len, /* truncate */ false, - )?; - let res = if res { Scalar::from_u32(0) } else { this.eval_libc("ERANGE") }; + )? { + ThreadNameResult::Ok => Scalar::from_u32(0), + ThreadNameResult::NameTooLong => this.eval_libc("ERANGE"), + ThreadNameResult::ThreadNotFound => this.eval_libc("ESRCH"), + }; this.write_scalar(res, dest)?; } "pthread_getname_np" => { let [thread, name, len] = this.check_shim(abi, ExternAbi::C { unwind: false }, link_name, args)?; - // https://github.com/illumos/illumos-gate/blob/c56822be04b6c157c8b6f2281e47214c3b86f657/usr/src/lib/libc/port/threads/thr.c#L2449-L2480 - let res = this.pthread_getname_np( + // See https://illumos.org/man/3C/pthread_getname_np for the error codes. + let res = match this.pthread_getname_np( this.read_scalar(thread)?, this.read_scalar(name)?, this.read_scalar(len)?, /* truncate */ false, - )?; - let res = if res { Scalar::from_u32(0) } else { this.eval_libc("ERANGE") }; + )? { + ThreadNameResult::Ok => Scalar::from_u32(0), + ThreadNameResult::NameTooLong => this.eval_libc("ERANGE"), + ThreadNameResult::ThreadNotFound => this.eval_libc("ESRCH"), + }; this.write_scalar(res, dest)?; } diff --git a/src/tools/miri/src/shims/unix/thread.rs b/src/tools/miri/src/shims/unix/thread.rs index 52c135f8540..3d990a1a042 100644 --- a/src/tools/miri/src/shims/unix/thread.rs +++ b/src/tools/miri/src/shims/unix/thread.rs @@ -2,6 +2,13 @@ use crate::*; +#[derive(Debug, Clone, PartialEq, Eq)] +pub enum ThreadNameResult { + Ok, + NameTooLong, + ThreadNotFound, +} + impl<'tcx> EvalContextExt<'tcx> for crate::MiriInterpCx<'tcx> {} pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { fn pthread_create( @@ -30,7 +37,11 @@ fn pthread_create( interp_ok(()) } - fn pthread_join(&mut self, thread: &OpTy<'tcx>, retval: &OpTy<'tcx>) -> InterpResult<'tcx, ()> { + fn pthread_join( + &mut self, + thread: &OpTy<'tcx>, + retval: &OpTy<'tcx>, + ) -> InterpResult<'tcx, Scalar> { let this = self.eval_context_mut(); if !this.ptr_is_null(this.read_pointer(retval)?)? { @@ -38,22 +49,26 @@ fn pthread_join(&mut self, thread: &OpTy<'tcx>, retval: &OpTy<'tcx>) -> InterpRe throw_unsup_format!("Miri supports pthread_join only with retval==NULL"); } - let thread_id = this.read_scalar(thread)?.to_int(this.libc_ty_layout("pthread_t").size)?; - this.join_thread_exclusive(thread_id.try_into().expect("thread ID should fit in u32"))?; + let thread = this.read_scalar(thread)?.to_int(this.libc_ty_layout("pthread_t").size)?; + let Ok(thread) = this.thread_id_try_from(thread) else { + return interp_ok(this.eval_libc("ESRCH")); + }; - interp_ok(()) + this.join_thread_exclusive(thread)?; + + interp_ok(Scalar::from_u32(0)) } - fn pthread_detach(&mut self, thread: &OpTy<'tcx>) -> InterpResult<'tcx, ()> { + fn pthread_detach(&mut self, thread: &OpTy<'tcx>) -> InterpResult<'tcx, Scalar> { let this = self.eval_context_mut(); - let thread_id = this.read_scalar(thread)?.to_int(this.libc_ty_layout("pthread_t").size)?; - this.detach_thread( - thread_id.try_into().expect("thread ID should fit in u32"), - /*allow_terminated_joined*/ false, - )?; + let thread = this.read_scalar(thread)?.to_int(this.libc_ty_layout("pthread_t").size)?; + let Ok(thread) = this.thread_id_try_from(thread) else { + return interp_ok(this.eval_libc("ESRCH")); + }; + this.detach_thread(thread, /*allow_terminated_joined*/ false)?; - interp_ok(()) + interp_ok(Scalar::from_u32(0)) } fn pthread_self(&mut self) -> InterpResult<'tcx, Scalar> { @@ -65,18 +80,21 @@ fn pthread_self(&mut self) -> InterpResult<'tcx, Scalar> { /// Set the name of the specified thread. If the name including the null terminator /// is longer or equals to `name_max_len`, then if `truncate` is set the truncated name - /// is used as the thread name, otherwise `false` is returned. + /// is used as the thread name, otherwise [`ThreadNameResult::NameTooLong`] is returned. + /// If the specified thread wasn't found, [`ThreadNameResult::ThreadNotFound`] is returned. fn pthread_setname_np( &mut self, thread: Scalar, name: Scalar, name_max_len: usize, truncate: bool, - ) -> InterpResult<'tcx, bool> { + ) -> InterpResult<'tcx, ThreadNameResult> { let this = self.eval_context_mut(); let thread = thread.to_int(this.libc_ty_layout("pthread_t").size)?; - let thread = ThreadId::try_from(thread).unwrap(); + let Ok(thread) = this.thread_id_try_from(thread) else { + return interp_ok(ThreadNameResult::ThreadNotFound); + }; let name = name.to_pointer(this)?; let mut name = this.read_c_str(name)?.to_owned(); @@ -85,29 +103,32 @@ fn pthread_setname_np( if truncate { name.truncate(name_max_len.saturating_sub(1)); } else { - return interp_ok(false); + return interp_ok(ThreadNameResult::NameTooLong); } } this.set_thread_name(thread, name); - interp_ok(true) + interp_ok(ThreadNameResult::Ok) } /// Get the name of the specified thread. If the thread name doesn't fit /// the buffer, then if `truncate` is set the truncated name is written out, - /// otherwise `false` is returned. + /// otherwise [`ThreadNameResult::NameTooLong`] is returned. If the specified + /// thread wasn't found, [`ThreadNameResult::ThreadNotFound`] is returned. fn pthread_getname_np( &mut self, thread: Scalar, name_out: Scalar, len: Scalar, truncate: bool, - ) -> InterpResult<'tcx, bool> { + ) -> InterpResult<'tcx, ThreadNameResult> { let this = self.eval_context_mut(); let thread = thread.to_int(this.libc_ty_layout("pthread_t").size)?; - let thread = ThreadId::try_from(thread).unwrap(); + let Ok(thread) = this.thread_id_try_from(thread) else { + return interp_ok(ThreadNameResult::ThreadNotFound); + }; let name_out = name_out.to_pointer(this)?; let len = len.to_target_usize(this)?; @@ -119,8 +140,9 @@ fn pthread_getname_np( }; let (success, _written) = this.write_c_str(name, name_out, len)?; + let res = if success { ThreadNameResult::Ok } else { ThreadNameResult::NameTooLong }; - interp_ok(success) + interp_ok(res) } fn sched_yield(&mut self) -> InterpResult<'tcx, ()> { diff --git a/src/tools/miri/src/shims/windows/foreign_items.rs b/src/tools/miri/src/shims/windows/foreign_items.rs index fe11aa8da4a..504efed3cfd 100644 --- a/src/tools/miri/src/shims/windows/foreign_items.rs +++ b/src/tools/miri/src/shims/windows/foreign_items.rs @@ -10,6 +10,10 @@ use crate::shims::windows::*; use crate::*; +// The NTSTATUS STATUS_INVALID_HANDLE (0xC0000008) encoded as a HRESULT by setting the N bit. +// (https://learn.microsoft.com/en-us/openspecs/windows_protocols/ms-erref/0642cb2f-2075-4469-918c-4441e69c548a) +const STATUS_INVALID_HANDLE: u32 = 0xD0000008; + pub fn is_dyn_sym(name: &str) -> bool { // std does dynamic detection for these symbols matches!( @@ -484,14 +488,14 @@ fn emulate_foreign_item_inner( let thread_id = this.CreateThread(security, stacksize, start, arg, flags, thread)?; - this.write_scalar(Handle::Thread(thread_id).to_scalar(this), dest)?; + this.write_scalar(Handle::Thread(thread_id.to_u32()).to_scalar(this), dest)?; } "WaitForSingleObject" => { let [handle, timeout] = this.check_shim(abi, ExternAbi::System { unwind: false }, link_name, args)?; let ret = this.WaitForSingleObject(handle, timeout)?; - this.write_scalar(Scalar::from_u32(ret), dest)?; + this.write_scalar(ret, dest)?; } "GetCurrentThread" => { let [] = @@ -510,15 +514,20 @@ fn emulate_foreign_item_inner( let name = this.read_wide_str(this.read_pointer(name)?)?; let thread = match Handle::from_scalar(handle, this)? { - Some(Handle::Thread(thread)) => thread, - Some(Handle::Pseudo(PseudoHandle::CurrentThread)) => this.active_thread(), + Some(Handle::Thread(thread)) => this.thread_id_try_from(thread), + Some(Handle::Pseudo(PseudoHandle::CurrentThread)) => Ok(this.active_thread()), _ => this.invalid_handle("SetThreadDescription")?, }; + let res = match thread { + Ok(thread) => { + // FIXME: use non-lossy conversion + this.set_thread_name(thread, String::from_utf16_lossy(&name).into_bytes()); + Scalar::from_u32(0) + } + Err(_) => Scalar::from_u32(STATUS_INVALID_HANDLE), + }; - // FIXME: use non-lossy conversion - this.set_thread_name(thread, String::from_utf16_lossy(&name).into_bytes()); - - this.write_null(dest)?; + this.write_scalar(res, dest)?; } "GetThreadDescription" => { let [handle, name_ptr] = @@ -528,20 +537,25 @@ fn emulate_foreign_item_inner( let name_ptr = this.deref_pointer(name_ptr)?; // the pointer where we should store the ptr to the name let thread = match Handle::from_scalar(handle, this)? { - Some(Handle::Thread(thread)) => thread, - Some(Handle::Pseudo(PseudoHandle::CurrentThread)) => this.active_thread(), + Some(Handle::Thread(thread)) => this.thread_id_try_from(thread), + Some(Handle::Pseudo(PseudoHandle::CurrentThread)) => Ok(this.active_thread()), _ => this.invalid_handle("GetThreadDescription")?, }; - // Looks like the default thread name is empty. - let name = this.get_thread_name(thread).unwrap_or(b"").to_owned(); - let name = this.alloc_os_str_as_wide_str( - bytes_to_os_str(&name)?, - MiriMemoryKind::WinLocal.into(), - )?; + let (name, res) = match thread { + Ok(thread) => { + // Looks like the default thread name is empty. + let name = this.get_thread_name(thread).unwrap_or(b"").to_owned(); + let name = this.alloc_os_str_as_wide_str( + bytes_to_os_str(&name)?, + MiriMemoryKind::WinLocal.into(), + )?; + (Scalar::from_maybe_pointer(name, this), Scalar::from_u32(0)) + } + Err(_) => (Scalar::null_ptr(this), Scalar::from_u32(STATUS_INVALID_HANDLE)), + }; - this.write_scalar(Scalar::from_maybe_pointer(name, this), &name_ptr)?; - - this.write_null(dest)?; + this.write_scalar(name, &name_ptr)?; + this.write_scalar(res, dest)?; } // Miscellaneous @@ -630,9 +644,9 @@ fn emulate_foreign_item_inner( let [handle] = this.check_shim(abi, ExternAbi::System { unwind: false }, link_name, args)?; - this.CloseHandle(handle)?; + let ret = this.CloseHandle(handle)?; - this.write_int(1, dest)?; + this.write_scalar(ret, dest)?; } "GetModuleFileNameW" => { let [handle, filename, size] = diff --git a/src/tools/miri/src/shims/windows/handle.rs b/src/tools/miri/src/shims/windows/handle.rs index 437a21534c9..b40c00efedd 100644 --- a/src/tools/miri/src/shims/windows/handle.rs +++ b/src/tools/miri/src/shims/windows/handle.rs @@ -14,7 +14,7 @@ pub enum PseudoHandle { pub enum Handle { Null, Pseudo(PseudoHandle), - Thread(ThreadId), + Thread(u32), } impl PseudoHandle { @@ -51,7 +51,7 @@ fn data(self) -> u32 { match self { Self::Null => 0, Self::Pseudo(pseudo_handle) => pseudo_handle.value(), - Self::Thread(thread) => thread.to_u32(), + Self::Thread(thread) => thread, } } @@ -95,7 +95,7 @@ fn new(discriminant: u32, data: u32) -> Option { match discriminant { Self::NULL_DISCRIMINANT if data == 0 => Some(Self::Null), Self::PSEUDO_DISCRIMINANT => Some(Self::Pseudo(PseudoHandle::from_value(data)?)), - Self::THREAD_DISCRIMINANT => Some(Self::Thread(data.into())), + Self::THREAD_DISCRIMINANT => Some(Self::Thread(data)), _ => None, } } @@ -154,17 +154,22 @@ fn invalid_handle(&mut self, function_name: &str) -> InterpResult<'tcx, !> { ))) } - fn CloseHandle(&mut self, handle_op: &OpTy<'tcx>) -> InterpResult<'tcx> { + fn CloseHandle(&mut self, handle_op: &OpTy<'tcx>) -> InterpResult<'tcx, Scalar> { let this = self.eval_context_mut(); let handle = this.read_scalar(handle_op)?; - - match Handle::from_scalar(handle, this)? { - Some(Handle::Thread(thread)) => - this.detach_thread(thread, /*allow_terminated_joined*/ true)?, + let ret = match Handle::from_scalar(handle, this)? { + Some(Handle::Thread(thread)) => { + if let Ok(thread) = this.thread_id_try_from(thread) { + this.detach_thread(thread, /*allow_terminated_joined*/ true)?; + this.eval_windows("c", "TRUE") + } else { + this.invalid_handle("CloseHandle")? + } + } _ => this.invalid_handle("CloseHandle")?, - } + }; - interp_ok(()) + interp_ok(ret) } } diff --git a/src/tools/miri/src/shims/windows/thread.rs b/src/tools/miri/src/shims/windows/thread.rs index fd3ef1413ed..7af15fc647c 100644 --- a/src/tools/miri/src/shims/windows/thread.rs +++ b/src/tools/miri/src/shims/windows/thread.rs @@ -59,14 +59,18 @@ fn WaitForSingleObject( &mut self, handle_op: &OpTy<'tcx>, timeout_op: &OpTy<'tcx>, - ) -> InterpResult<'tcx, u32> { + ) -> InterpResult<'tcx, Scalar> { let this = self.eval_context_mut(); let handle = this.read_scalar(handle_op)?; let timeout = this.read_scalar(timeout_op)?.to_u32()?; let thread = match Handle::from_scalar(handle, this)? { - Some(Handle::Thread(thread)) => thread, + Some(Handle::Thread(thread)) => + match this.thread_id_try_from(thread) { + Ok(thread) => thread, + Err(_) => this.invalid_handle("WaitForSingleObject")?, + }, // Unlike on posix, the outcome of joining the current thread is not documented. // On current Windows, it just deadlocks. Some(Handle::Pseudo(PseudoHandle::CurrentThread)) => this.active_thread(), @@ -79,6 +83,6 @@ fn WaitForSingleObject( this.join_thread(thread)?; - interp_ok(0) + interp_ok(this.eval_windows("c", "WAIT_OBJECT_0")) } } diff --git a/src/tools/miri/tests/pass-dep/libc/pthread-threadname.rs b/src/tools/miri/tests/pass-dep/libc/pthread-threadname.rs index 0e5b501bbcc..cf634bc6890 100644 --- a/src/tools/miri/tests/pass-dep/libc/pthread-threadname.rs +++ b/src/tools/miri/tests/pass-dep/libc/pthread-threadname.rs @@ -199,4 +199,29 @@ fn get_thread_name(name: &mut [u8]) -> i32 { .unwrap() .join() .unwrap(); + + // Now set the name for a non-existing thread and verify error codes. + // (FreeBSD doesn't return an error code.) + #[cfg(not(target_os = "freebsd"))] + { + let invalid_thread = 0xdeadbeef; + let error = { + cfg_if::cfg_if! { + if #[cfg(target_os = "linux")] { + libc::ENOENT + } else { + libc::ESRCH + } + } + }; + #[cfg(not(target_os = "macos"))] + { + // macOS has no `setname` function accepting a thread id as the first argument. + let res = unsafe { libc::pthread_setname_np(invalid_thread, [0].as_ptr()) }; + assert_eq!(res, error); + } + let mut buf = [0; 64]; + let res = unsafe { libc::pthread_getname_np(invalid_thread, buf.as_mut_ptr(), buf.len()) }; + assert_eq!(res, error); + } }