Skip to content
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.

Commit 2d6c9b4

Browse files
committedFeb 2, 2025
shim Apple's futex primitives
This is necessary to unblock rust-lang#122408. The documentation for these is available [here](https://developer.apple.com/documentation/os/os_sync_wait_on_address?language=objc). Because the futex wait operations (`os_sync_wait_on_address` et al.) return the number of remaining waiters after returning, this required some changes to the common futex infrastructure, which I've changed to take a callback instead of precalculating the return values.
1 parent b82463f commit 2d6c9b4

File tree

7 files changed

+626
-53
lines changed

7 files changed

+626
-53
lines changed
 

‎src/tools/miri/src/concurrency/sync.rs

+34-30
Original file line numberDiff line numberDiff line change
@@ -128,7 +128,7 @@ struct Condvar {
128128
/// The futex state.
129129
#[derive(Default, Debug)]
130130
struct Futex {
131-
waiters: VecDeque<FutexWaiter>,
131+
waiters: Vec<FutexWaiter>,
132132
/// Tracks the happens-before relationship
133133
/// between a futex-wake and a futex-wait
134134
/// during a non-spurious wake event.
@@ -140,6 +140,12 @@ struct Futex {
140140
#[derive(Default, Clone)]
141141
pub struct FutexRef(Rc<RefCell<Futex>>);
142142

143+
impl FutexRef {
144+
pub fn waiters(&self) -> usize {
145+
self.0.borrow().waiters.len()
146+
}
147+
}
148+
143149
impl VisitProvenance for FutexRef {
144150
fn visit_provenance(&self, _visit: &mut VisitWith<'_>) {
145151
// No provenance in `Futex`.
@@ -728,25 +734,21 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
728734
interp_ok(true)
729735
}
730736

731-
/// Wait for the futex to be signaled, or a timeout.
732-
/// On a signal, `retval_succ` is written to `dest`.
733-
/// On a timeout, `retval_timeout` is written to `dest` and `errno_timeout` is set as the last error.
737+
/// Wait for the futex to be signaled, or a timeout. Once the thread is
738+
/// unblocked, `callback` is called with the unblock reason.
734739
fn futex_wait(
735740
&mut self,
736741
futex_ref: FutexRef,
737742
bitset: u32,
738743
timeout: Option<(TimeoutClock, TimeoutAnchor, Duration)>,
739-
retval_succ: Scalar,
740-
retval_timeout: Scalar,
741-
dest: MPlaceTy<'tcx>,
742-
errno_timeout: IoError,
744+
callback: DynUnblockCallback<'tcx>,
743745
) {
744746
let this = self.eval_context_mut();
745747
let thread = this.active_thread();
746748
let mut futex = futex_ref.0.borrow_mut();
747749
let waiters = &mut futex.waiters;
748750
assert!(waiters.iter().all(|waiter| waiter.thread != thread), "thread is already waiting");
749-
waiters.push_back(FutexWaiter { thread, bitset });
751+
waiters.push(FutexWaiter { thread, bitset });
750752
drop(futex);
751753

752754
this.block_thread(
@@ -755,10 +757,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
755757
callback!(
756758
@capture<'tcx> {
757759
futex_ref: FutexRef,
758-
retval_succ: Scalar,
759-
retval_timeout: Scalar,
760-
dest: MPlaceTy<'tcx>,
761-
errno_timeout: IoError,
760+
callback: DynUnblockCallback<'tcx>,
762761
}
763762
|this, unblock: UnblockKind| {
764763
match unblock {
@@ -768,29 +767,29 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
768767
if let Some(data_race) = &this.machine.data_race {
769768
data_race.acquire_clock(&futex.clock, &this.machine.threads);
770769
}
771-
// Write the return value.
772-
this.write_scalar(retval_succ, &dest)?;
773-
interp_ok(())
774770
},
775771
UnblockKind::TimedOut => {
776772
// Remove the waiter from the futex.
777773
let thread = this.active_thread();
778774
let mut futex = futex_ref.0.borrow_mut();
779775
futex.waiters.retain(|waiter| waiter.thread != thread);
780-
// Set errno and write return value.
781-
this.set_last_error(errno_timeout)?;
782-
this.write_scalar(retval_timeout, &dest)?;
783-
interp_ok(())
784776
},
785777
}
778+
779+
callback.call(this, unblock)
786780
}
787781
),
788782
);
789783
}
790784

791-
/// Wake up the first thread in the queue that matches any of the bits in the bitset.
792-
/// Returns whether anything was woken.
793-
fn futex_wake(&mut self, futex_ref: &FutexRef, bitset: u32) -> InterpResult<'tcx, bool> {
785+
/// Wake up `count` of the threads in the queue that match any of the bits
786+
/// in the bitset. Returns how many threads were woken.
787+
fn futex_wake(
788+
&mut self,
789+
futex_ref: &FutexRef,
790+
bitset: u32,
791+
count: usize,
792+
) -> InterpResult<'tcx, usize> {
794793
let this = self.eval_context_mut();
795794
let mut futex = futex_ref.0.borrow_mut();
796795
let data_race = &this.machine.data_race;
@@ -800,13 +799,18 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
800799
data_race.release_clock(&this.machine.threads, |clock| futex.clock.clone_from(clock));
801800
}
802801

803-
// Wake up the first thread in the queue that matches any of the bits in the bitset.
804-
let Some(i) = futex.waiters.iter().position(|w| w.bitset & bitset != 0) else {
805-
return interp_ok(false);
806-
};
807-
let waiter = futex.waiters.remove(i).unwrap();
802+
// Remove `count` of the threads in the queue that match any of the bits in the bitset.
803+
// We collect all of them before unblocking because the unblock callback may access the
804+
// futex state to retrieve the remaining number of waiters on macOS.
805+
let waiters: Vec<_> =
806+
futex.waiters.extract_if(.., |w| w.bitset & bitset != 0).take(count).collect();
808807
drop(futex);
809-
this.unblock_thread(waiter.thread, BlockReason::Futex)?;
810-
interp_ok(true)
808+
809+
let woken = waiters.len();
810+
for waiter in waiters {
811+
this.unblock_thread(waiter.thread, BlockReason::Futex)?;
812+
}
813+
814+
interp_ok(woken)
811815
}
812816
}

‎src/tools/miri/src/lib.rs

+2
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,8 @@
1515
#![feature(unqualified_local_imports)]
1616
#![feature(derive_coerce_pointee)]
1717
#![feature(arbitrary_self_types)]
18+
#![feature(unsigned_is_multiple_of)]
19+
#![feature(extract_if)]
1820
// Configure clippy and other lints
1921
#![allow(
2022
clippy::collapsible_else_if,

‎src/tools/miri/src/shims/unix/linux_like/sync.rs

+16-14
Original file line numberDiff line numberDiff line change
@@ -158,14 +158,24 @@ pub fn futex<'tcx>(
158158
.futex
159159
.clone();
160160

161+
let dest = dest.clone();
161162
ecx.futex_wait(
162163
futex_ref,
163164
bitset,
164165
timeout,
165-
Scalar::from_target_isize(0, ecx), // retval_succ
166-
Scalar::from_target_isize(-1, ecx), // retval_timeout
167-
dest.clone(),
168-
LibcError("ETIMEDOUT"), // errno_timeout
166+
callback!(
167+
@capture<'tcx> {
168+
dest: MPlaceTy<'tcx>,
169+
}
170+
|ecx, unblock: UnblockKind| match unblock {
171+
UnblockKind::Ready => {
172+
ecx.write_int(0, &dest)
173+
}
174+
UnblockKind::TimedOut => {
175+
ecx.set_last_error_and_return(LibcError("ETIMEDOUT"), &dest)
176+
}
177+
}
178+
),
169179
);
170180
} else {
171181
// The futex value doesn't match the expected value, so we return failure
@@ -209,16 +219,8 @@ pub fn futex<'tcx>(
209219
// will see the latest value on addr which could be changed by our caller
210220
// before doing the syscall.
211221
ecx.atomic_fence(AtomicFenceOrd::SeqCst)?;
212-
let mut n = 0;
213-
#[expect(clippy::arithmetic_side_effects)]
214-
for _ in 0..val {
215-
if ecx.futex_wake(&futex_ref, bitset)? {
216-
n += 1;
217-
} else {
218-
break;
219-
}
220-
}
221-
ecx.write_scalar(Scalar::from_target_isize(n, ecx), dest)?;
222+
let woken = ecx.futex_wake(&futex_ref, bitset, val.try_into().unwrap())?;
223+
ecx.write_scalar(Scalar::from_target_isize(woken.try_into().unwrap(), ecx), dest)?;
222224
}
223225
op => throw_unsup_format!("Miri does not support `futex` syscall with op={}", op),
224226
}

‎src/tools/miri/src/shims/unix/macos/foreign_items.rs

+63-3
Original file line numberDiff line numberDiff line change
@@ -2,12 +2,20 @@ use rustc_middle::ty::Ty;
22
use rustc_span::Symbol;
33
use rustc_target::callconv::{Conv, FnAbi};
44

5-
use super::sync::EvalContextExt as _;
5+
use super::sync::{EvalContextExt as _, MacOsFutexTimeout};
66
use crate::shims::unix::*;
77
use crate::*;
88

9-
pub fn is_dyn_sym(_name: &str) -> bool {
10-
false
9+
pub fn is_dyn_sym(name: &str) -> bool {
10+
match name {
11+
// These only became available with macOS 11.0, so std looks them up dynamically.
12+
"os_sync_wait_on_address"
13+
| "os_sync_wait_on_address_with_deadline"
14+
| "os_sync_wait_on_address_with_timeout"
15+
| "os_sync_wake_by_address_any"
16+
| "os_sync_wake_by_address_all" => true,
17+
_ => false,
18+
}
1119
}
1220

1321
impl<'tcx> EvalContextExt<'tcx> for crate::MiriInterpCx<'tcx> {}
@@ -214,6 +222,58 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
214222
this.write_scalar(res, dest)?;
215223
}
216224

225+
// Futex primitives
226+
"os_sync_wait_on_address" => {
227+
let [addr_op, value_op, size_op, flags_op] =
228+
this.check_shim(abi, Conv::C, link_name, args)?;
229+
this.os_sync_wait_on_address(
230+
addr_op,
231+
value_op,
232+
size_op,
233+
flags_op,
234+
MacOsFutexTimeout::None,
235+
dest,
236+
)?;
237+
}
238+
"os_sync_wait_on_address_with_deadline" => {
239+
let [addr_op, value_op, size_op, flags_op, clock_op, timeout_op] =
240+
this.check_shim(abi, Conv::C, link_name, args)?;
241+
this.os_sync_wait_on_address(
242+
addr_op,
243+
value_op,
244+
size_op,
245+
flags_op,
246+
MacOsFutexTimeout::Absolute { clock_op, timeout_op },
247+
dest,
248+
)?;
249+
}
250+
"os_sync_wait_on_address_with_timeout" => {
251+
let [addr_op, value_op, size_op, flags_op, clock_op, timeout_op] =
252+
this.check_shim(abi, Conv::C, link_name, args)?;
253+
this.os_sync_wait_on_address(
254+
addr_op,
255+
value_op,
256+
size_op,
257+
flags_op,
258+
MacOsFutexTimeout::Relative { clock_op, timeout_op },
259+
dest,
260+
)?;
261+
}
262+
"os_sync_wake_by_address_any" => {
263+
let [addr_op, size_op, flags_op] =
264+
this.check_shim(abi, Conv::C, link_name, args)?;
265+
this.os_sync_wake_by_address(
266+
addr_op, size_op, flags_op, /* all */ false, dest,
267+
)?;
268+
}
269+
"os_sync_wake_by_address_all" => {
270+
let [addr_op, size_op, flags_op] =
271+
this.check_shim(abi, Conv::C, link_name, args)?;
272+
this.os_sync_wake_by_address(
273+
addr_op, size_op, flags_op, /* all */ true, dest,
274+
)?;
275+
}
276+
217277
"os_unfair_lock_lock" => {
218278
let [lock_op] = this.check_shim(abi, Conv::C, link_name, args)?;
219279
this.os_unfair_lock_lock(lock_op)?;
There was a problem loading the remainder of the diff.

0 commit comments

Comments
 (0)
Failed to load comments.