Skip to content
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

shim Apple's futex primitives #4142

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 14 additions & 17 deletions src/concurrency/sync.rs
Original file line number Diff line number Diff line change
Expand Up @@ -140,6 +140,12 @@ struct Futex {
#[derive(Default, Clone)]
pub struct FutexRef(Rc<RefCell<Futex>>);

impl FutexRef {
pub fn waiters(&self) -> usize {
self.0.borrow().waiters.len()
}
}

impl VisitProvenance for FutexRef {
fn visit_provenance(&self, _visit: &mut VisitWith<'_>) {
// No provenance in `Futex`.
Expand Down Expand Up @@ -729,17 +735,16 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
}

/// Wait for the futex to be signaled, or a timeout.
/// On a signal, `retval_succ` is written to `dest`.
/// On a timeout, `retval_timeout` is written to `dest` and `errno_timeout` is set as the last error.
/// * On a signal, `retval_succ` is called with the number of waiters on the
/// futex and its result is written to `dest`.
/// * On a timeout, `retval_timeout` is written to `dest` and `errno_timeout`
/// is set as the last error.
Comment on lines +738 to +741
Copy link
Member

Choose a reason for hiding this comment

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

Most of the variables referenced here do not exist any more.

fn futex_wait(
&mut self,
futex_ref: FutexRef,
bitset: u32,
timeout: Option<(TimeoutClock, TimeoutAnchor, Duration)>,
retval_succ: Scalar,
retval_timeout: Scalar,
dest: MPlaceTy<'tcx>,
errno_timeout: IoError,
callback: DynUnblockCallback<'tcx>,
) {
let this = self.eval_context_mut();
let thread = this.active_thread();
Expand All @@ -755,10 +760,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
callback!(
@capture<'tcx> {
futex_ref: FutexRef,
retval_succ: Scalar,
retval_timeout: Scalar,
dest: MPlaceTy<'tcx>,
errno_timeout: IoError,
callback: DynUnblockCallback<'tcx>,
}
|this, unblock: UnblockKind| {
match unblock {
Expand All @@ -768,21 +770,16 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
if let Some(data_race) = &this.machine.data_race {
data_race.acquire_clock(&futex.clock, &this.machine.threads);
}
// Write the return value.
this.write_scalar(retval_succ, &dest)?;
interp_ok(())
},
UnblockKind::TimedOut => {
// Remove the waiter from the futex.
let thread = this.active_thread();
let mut futex = futex_ref.0.borrow_mut();
futex.waiters.retain(|waiter| waiter.thread != thread);
// Set errno and write return value.
this.set_last_error(errno_timeout)?;
this.write_scalar(retval_timeout, &dest)?;
interp_ok(())
},
}

callback.call(this, unblock)
}
),
);
Expand Down
18 changes: 14 additions & 4 deletions src/shims/unix/linux_like/sync.rs
Original file line number Diff line number Diff line change
Expand Up @@ -156,14 +156,24 @@ pub fn futex<'tcx>(
.futex
.clone();

let dest = dest.clone();
ecx.futex_wait(
futex_ref,
bitset,
timeout,
Scalar::from_target_isize(0, ecx), // retval_succ
Scalar::from_target_isize(-1, ecx), // retval_timeout
dest.clone(),
LibcError("ETIMEDOUT"), // errno_timeout
callback!(
@capture<'tcx> {
dest: MPlaceTy<'tcx>,
}
|ecx, unblock: UnblockKind| match unblock {
UnblockKind::Ready => {
ecx.write_scalar(Scalar::from_target_isize(0, ecx), &dest)
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
ecx.write_scalar(Scalar::from_target_isize(0, ecx), &dest)
ecx.write_int(0, &dest)

}
UnblockKind::TimedOut => {
ecx.set_last_error_and_return(LibcError("ETIMEDOUT"), &dest)
}
}
),
);
} else {
// The futex value doesn't match the expected value, so we return failure
Expand Down
61 changes: 58 additions & 3 deletions src/shims/unix/macos/foreign_items.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,21 @@ use rustc_middle::ty::Ty;
use rustc_span::Symbol;
use rustc_target::callconv::{Conv, FnAbi};

use super::sync::EvalContextExt as _;
use super::sync::{EvalContextExt as _, MacOsFutexTimeout};
use crate::helpers::check_min_arg_count;
use crate::shims::unix::*;
use crate::*;

pub fn is_dyn_sym(_name: &str) -> bool {
false
pub fn is_dyn_sym(name: &str) -> bool {
match name {
// These only became available with macOS 11.0, so std looks them up dynamically.
"os_sync_wait_on_address"
| "os_sync_wait_on_address_with_deadline"
| "os_sync_wait_on_address_with_timeout"
| "os_sync_wake_by_address_any"
| "os_sync_wake_by_address_all" => true,
_ => false,
}
}

impl<'tcx> EvalContextExt<'tcx> for crate::MiriInterpCx<'tcx> {}
Expand Down Expand Up @@ -216,6 +224,53 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
this.write_scalar(res, dest)?;
}

"os_sync_wait_on_address" => {
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
"os_sync_wait_on_address" => {
// Futex primitives
"os_sync_wait_on_address" => {

let [addr_op, value_op, size_op, flags_op] =
this.check_shim(abi, Conv::C, link_name, args)?;
this.os_sync_wait_on_address(
addr_op,
value_op,
size_op,
flags_op,
MacOsFutexTimeout::None,
dest,
)?;
}
"os_sync_wait_on_address_with_deadline" => {
let [addr_op, value_op, size_op, flags_op, clock_op, timeout_op] =
this.check_shim(abi, Conv::C, link_name, args)?;
this.os_sync_wait_on_address(
addr_op,
value_op,
size_op,
flags_op,
MacOsFutexTimeout::Absolute { clock_op, timeout_op },
dest,
)?;
}
"os_sync_wait_on_address_with_timeout" => {
let [addr_op, value_op, size_op, flags_op, clock_op, timeout_op] =
this.check_shim(abi, Conv::C, link_name, args)?;
this.os_sync_wait_on_address(
addr_op,
value_op,
size_op,
flags_op,
MacOsFutexTimeout::Relative { clock_op, timeout_op },
dest,
)?;
}
"os_sync_wake_by_address_any" => {
let [addr_op, size_op, flags_op] =
this.check_shim(abi, Conv::C, link_name, args)?;
this.os_sync_wake_by_address(addr_op, size_op, flags_op, false, dest)?;
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.os_sync_wake_by_address(addr_op, size_op, flags_op, false, dest)?;
this.os_sync_wake_by_address(addr_op, size_op, flags_op, /* all */ false, dest)?;

same thing a few lines below

}
"os_sync_wake_by_address_all" => {
let [addr_op, size_op, flags_op] =
this.check_shim(abi, Conv::C, link_name, args)?;
this.os_sync_wake_by_address(addr_op, size_op, flags_op, true, dest)?;
}

"os_unfair_lock_lock" => {
let [lock_op] = this.check_shim(abi, Conv::C, link_name, args)?;
this.os_unfair_lock_lock(lock_op)?;
Expand Down
Loading
Loading