Skip to content

Add shims for gettid-esque functions #4397

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 11 commits into
base: master
Choose a base branch
from
Open
4 changes: 1 addition & 3 deletions src/helpers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1337,7 +1337,6 @@ where

/// Check that the number of varargs is at least the minimum what we expect.
/// Fixed args should not be included.
/// Use `check_vararg_fixed_arg_count` to extract the varargs slice from full function arguments.
pub fn check_min_vararg_count<'a, 'tcx, const N: usize>(
name: &'a str,
args: &'a [OpTy<'tcx>],
Expand All @@ -1346,9 +1345,8 @@ pub fn check_min_vararg_count<'a, 'tcx, const N: usize>(
return interp_ok(ops);
}
throw_ub_format!(
"not enough variadic arguments for `{name}`: got {}, expected at least {}",
"not enough variadic arguments for `{name}`: got {}, expected at least {N}",
args.len(),
N
)
}

Expand Down
22 changes: 22 additions & 0 deletions src/shims/env.rs
Original file line number Diff line number Diff line change
Expand Up @@ -110,8 +110,30 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
}
}

/// Get the process identifier.
fn get_pid(&self) -> u32 {
let this = self.eval_context_ref();
if this.machine.communicate() { std::process::id() } else { 1000 }
}

/// Get an "OS" thread ID for the current thread.
fn get_current_tid(&self) -> u32 {
let this = self.eval_context_ref();
self.get_tid(this.machine.threads.active_thread())
}

/// Get an "OS" thread ID for any thread.
fn get_tid(&self, thread: ThreadId) -> u32 {
let this = self.eval_context_ref();
let index = thread.to_u32();
let target_os = &this.tcx.sess.target.os;
if target_os == "linux" || target_os == "netbsd" {
// On Linux, the main thread has PID == TID so we uphold this. NetBSD also appears
// to exhibit the same behavior, though I can't find a citation.
this.get_pid().strict_add(index)
} else {
// Other platforms do not display any relationship between PID and TID.
index
}
}
}
2 changes: 1 addition & 1 deletion src/shims/extern_static.rs
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ impl<'tcx> MiriMachine<'tcx> {
ecx,
&["__cxa_thread_atexit_impl", "__clock_gettime64"],
)?;
Self::weak_symbol_extern_statics(ecx, &["getrandom", "statx"])?;
Self::weak_symbol_extern_statics(ecx, &["getrandom", "gettid", "statx"])?;
}
"freebsd" => {
Self::null_ptr_extern_statics(ecx, &["__cxa_thread_atexit_impl"])?;
Expand Down
47 changes: 41 additions & 6 deletions src/shims/unix/env.rs
Original file line number Diff line number Diff line change
Expand Up @@ -274,15 +274,50 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
interp_ok(Scalar::from_u32(this.get_pid()))
}

fn linux_gettid(&mut self) -> InterpResult<'tcx, Scalar> {
/// The `gettid`-like function for Unix platforms that take no parameters and return a 32-bit
/// integer. It is not always named "gettid".
fn unix_gettid(&mut self, link_name: &str) -> InterpResult<'tcx, Scalar> {
let this = self.eval_context_ref();
this.assert_target_os("linux", "gettid");
this.assert_target_os_is_unix(link_name.as_ref());
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
this.assert_target_os_is_unix(link_name.as_ref());
this.assert_target_os_is_unix(link_name);

This should also work, doesn't it?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Indeed, I don't know why I thought it didn't like that...


let index = this.machine.threads.active_thread().to_u32();
// For most platforms the return type is an `i32`, but some are unsigned. The TID
// will always be positive so we don't need to differentiate.
interp_ok(Scalar::from_u32(this.get_current_tid()))
}

/// The Apple-specific `int pthread_threadid_np(pthread_t thread, uint64_t *thread_id)`, which
/// allows querying the ID for arbitrary threads, identified by their pthread_t.
fn apple_pthread_threadip_np(
&mut self,
thread_op: &OpTy<'tcx>,
tid_op: &OpTy<'tcx>,
) -> InterpResult<'tcx, Scalar> {
let this = self.eval_context_mut();
this.assert_target_os("macos", "pthread_threadip_np");

let tid_dest = this.read_pointer(tid_op)?;
if this.ptr_is_null(tid_dest)? {
// If NULL is passed, an error is immediately returned
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this documented behavior or just observed?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The possible error codes are documented https://www.manpagez.com/man/3/pthread_threadid_np/, the priority (null check before valid thread check) is observed

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's apparently a BSD manpage, not an Apple one... but it's better than no reference. Would be good to have some sort of link somewhere in the function for what we used as reference.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is from MacOS (see the table at the very bottom). Source for reference https://github.com/apple-oss-distributions/libpthread/blob/1ebf56b3a702df53213c2996e5e128a535d2577e/src/pthread.c#L1118-L1146 (there is also a manpage in that repo)

return interp_ok(this.eval_libc("EINVAL"));
}

let thread = this.read_scalar(thread_op)?.to_int(this.libc_ty_layout("pthread_t").size)?;
let thread = if thread == 0 {
// Null thread ID indicates that we are querying the active thread.
this.machine.threads.active_thread()
} else {
// Our pthread_t is just the raw ThreadId.
let Ok(thread) = this.thread_id_try_from(thread) else {
return interp_ok(this.eval_libc("ESRCH"));
};
thread
};

// Compute a TID for this thread, ensuring that the main thread has PID == TID.
let tid = this.get_pid().strict_add(index);
let tid = this.get_tid(thread);
let tid_dest = this.deref_pointer_as(tid_op, this.machine.layouts.u64)?;
this.write_int(tid, &tid_dest)?;

interp_ok(Scalar::from_u32(tid))
// Never an error if we only ever check the current thread.
interp_ok(Scalar::from_u32(0))
}
}
5 changes: 5 additions & 0 deletions src/shims/unix/freebsd/foreign_items.rs
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,11 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
};
this.write_scalar(res, dest)?;
}
"pthread_getthreadid_np" => {
let [] = this.check_shim(abi, CanonAbi::C, link_name, args)?;
let result = this.unix_gettid(link_name.as_str())?;
this.write_scalar(result, dest)?;
}

"cpuset_getaffinity" => {
// The "same" kind of api as `sched_getaffinity` but more fine grained control for FreeBSD specifically.
Expand Down
4 changes: 2 additions & 2 deletions src/shims/unix/linux/foreign_items.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ use crate::*;
const TASK_COMM_LEN: u64 = 16;

pub fn is_dyn_sym(name: &str) -> bool {
matches!(name, "statx")
matches!(name, "gettid" | "statx")
}

impl<'tcx> EvalContextExt<'tcx> for crate::MiriInterpCx<'tcx> {}
Expand Down Expand Up @@ -117,7 +117,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
}
"gettid" => {
let [] = this.check_shim(abi, CanonAbi::C, link_name, args)?;
let result = this.linux_gettid()?;
let result = this.unix_gettid(link_name.as_str())?;
this.write_scalar(result, dest)?;
}

Expand Down
6 changes: 6 additions & 0 deletions src/shims/unix/linux_like/syscall.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ use rustc_span::Symbol;
use rustc_target::callconv::FnAbi;

use crate::helpers::check_min_vararg_count;
use crate::shims::unix::env::EvalContextExt;
use crate::shims::unix::linux_like::eventfd::EvalContextExt as _;
use crate::shims::unix::linux_like::sync::futex;
use crate::*;
Expand All @@ -24,6 +25,7 @@ pub fn syscall<'tcx>(
let sys_getrandom = ecx.eval_libc("SYS_getrandom").to_target_usize(ecx)?;
let sys_futex = ecx.eval_libc("SYS_futex").to_target_usize(ecx)?;
let sys_eventfd2 = ecx.eval_libc("SYS_eventfd2").to_target_usize(ecx)?;
let sys_gettid = ecx.eval_libc("SYS_gettid").to_target_usize(ecx)?;

match ecx.read_target_usize(op)? {
// `libc::syscall(NR_GETRANDOM, buf.as_mut_ptr(), buf.len(), GRND_NONBLOCK)`
Expand Down Expand Up @@ -53,6 +55,10 @@ pub fn syscall<'tcx>(
let result = ecx.eventfd(initval, flags)?;
ecx.write_int(result.to_i32()?, dest)?;
}
num if num == sys_gettid => {
let result = ecx.unix_gettid("SYS_gettid")?;
ecx.write_int(result.to_u32()?, dest)?;
}
num => {
throw_unsup_format!("syscall: unsupported syscall number {num}");
}
Expand Down
5 changes: 5 additions & 0 deletions src/shims/unix/macos/foreign_items.rs
Original file line number Diff line number Diff line change
Expand Up @@ -222,6 +222,11 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
};
this.write_scalar(res, dest)?;
}
"pthread_threadid_np" => {
let [thread, tid_ptr] = this.check_shim(abi, CanonAbi::C, link_name, args)?;
let res = this.apple_pthread_threadip_np(thread, tid_ptr)?;
this.write_scalar(res, dest)?;
}

// Synchronization primitives
"os_sync_wait_on_address" => {
Expand Down
24 changes: 24 additions & 0 deletions src/shims/windows/foreign_items.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ use rustc_target::callconv::FnAbi;

use self::shims::windows::handle::{Handle, PseudoHandle};
use crate::shims::os_str::bytes_to_os_str;
use crate::shims::windows::handle::HandleError;
use crate::shims::windows::*;
use crate::*;

Expand Down Expand Up @@ -629,6 +630,29 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
this.write_scalar(name, &name_ptr)?;
this.write_scalar(res, dest)?;
}
"GetThreadId" => {
let [handle] = this.check_shim(abi, sys_conv, link_name, args)?;

let handle = this.read_scalar(handle)?;
let tid = match Handle::try_from_scalar(handle, this)? {
Comment on lines +636 to +637
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The expectation is that you'll usually call read_handle instead (so try_from_scalar can remain private). That's what you used to do and now you changed it... why? At the very least this needs a long comment explaining why the consistent behavior read_handle is intended to enforce everywhere is somehow not right here.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is to match the documented behavior of returning 0 on failure https://learn.microsoft.com/en-us/windows/win32/api/processthreadsapi/nf-processthreadsapi-getthreadid. But with #4397 (comment), this can be dropped.

Ok(Handle::Thread(thread)) => this.get_tid(thread),
Ok(Handle::Pseudo(PseudoHandle::CurrentThread)) => {
let thread = this.active_thread();
this.get_tid(thread)
}
// Unknown threads return 0
Ok(_) => 0,
Err(HandleError::InvalidHandle | HandleError::ThreadNotFound(_)) => 0,
};

this.write_scalar(Scalar::from_u32(tid), dest)?;
}
"GetCurrentThreadId" => {
let [] = this.check_shim(abi, sys_conv, link_name, args)?;
let thread = this.active_thread();
let tid = this.get_tid(thread);
this.write_scalar(Scalar::from_u32(tid), dest)?;
}

// Miscellaneous
"ExitProcess" => {
Expand Down
2 changes: 1 addition & 1 deletion src/shims/windows/handle.rs
Original file line number Diff line number Diff line change
Expand Up @@ -166,7 +166,7 @@ impl Handle {
/// Structurally invalid handles return [`HandleError::InvalidHandle`].
/// If the handle is structurally valid but semantically invalid, e.g. a for non-existent thread
/// ID, returns [`HandleError::ThreadNotFound`].
fn try_from_scalar<'tcx>(
pub fn try_from_scalar<'tcx>(
handle: Scalar,
cx: &MiriInterpCx<'tcx>,
) -> InterpResult<'tcx, Result<Self, HandleError>> {
Expand Down
Loading