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

Add support for FUTEX_{WAIT,WAKE}_BITSET #2054

Merged
merged 7 commits into from
Apr 6, 2022
Merged
Show file tree
Hide file tree
Changes from 2 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
49 changes: 40 additions & 9 deletions src/shims/posix/linux/sync.rs
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ pub fn futex<'tcx>(

let futex_private = this.eval_libc_i32("FUTEX_PRIVATE_FLAG")?;
let futex_wait = this.eval_libc_i32("FUTEX_WAIT")?;
let futex_wait_bitset = this.eval_libc_i32("FUTEX_WAIT_BITSET")?;
let futex_wake = this.eval_libc_i32("FUTEX_WAKE")?;
let futex_realtime = this.eval_libc_i32("FUTEX_CLOCK_REALTIME")?;

Expand All @@ -45,12 +46,32 @@ pub fn futex<'tcx>(
// FUTEX_WAIT: (int *addr, int op = FUTEX_WAIT, int val, const timespec *timeout)
// Blocks the thread if *addr still equals val. Wakes up when FUTEX_WAKE is called on the same address,
// or *timeout expires. `timeout == null` for an infinite timeout.
op if op & !futex_realtime == futex_wait => {
if args.len() < 5 {
throw_ub_format!(
"incorrect number of arguments for `futex` syscall with `op=FUTEX_WAIT`: got {}, expected at least 5",
args.len()
);
//
// FUTEX_WAIT_BITSET: (int *addr, int op = FUTEX_WAIT_BITSET, int val, const timespec *timeout, int *_ignored, unsigned int bitset)
// When bitset is u32::MAX, this is identical to FUTEX_WAIT, except the timeout is absolute rather than relative.
op if op & !futex_realtime == futex_wait || op & !futex_realtime == futex_wait_bitset => {
let wait_bitset = op & !futex_realtime == futex_wait_bitset;

if wait_bitset {
if args.len() != 7 {
throw_ub_format!(
"incorrect number of arguments for `futex` syscall with `op=FUTEX_WAIT_BITSET`: got {}, expected 7",
args.len()
);
}

let bitset = this.read_scalar(&args[6])?.to_u32()?;

if bitset != u32::MAX {
throw_unsup_format!("Miri does not support `futex` syscall with `op=FUTEX_WAIT_BITSET` with a bitset other than UINT_MAX");
}
} else {
if args.len() < 5 {
throw_ub_format!(
"incorrect number of arguments for `futex` syscall with `op=FUTEX_WAIT`: got {}, expected at least 5",
args.len()
);
}
}

// `deref_operand` but not actually dereferencing the ptr yet (it might be NULL!).
Expand All @@ -70,10 +91,20 @@ pub fn futex<'tcx>(
return Ok(());
}
};
Some(if op & futex_realtime != 0 {
Time::RealTime(SystemTime::now().checked_add(duration).unwrap())
Some(if wait_bitset {
// FUTEX_WAIT_BITSET uses an absolute timestamp.
if op & futex_realtime != 0 {
Time::RealTime(SystemTime::UNIX_EPOCH.checked_add(duration).unwrap())
} else {
Time::Monotonic(this.machine.time_anchor.checked_add(duration).unwrap())
}
} else {
Time::Monotonic(Instant::now().checked_add(duration).unwrap())
// FUTEX_WAIT uses a relative timestamp.
if op & futex_realtime != 0 {
Time::RealTime(SystemTime::now().checked_add(duration).unwrap())
} else {
Time::Monotonic(Instant::now().checked_add(duration).unwrap())
}
})
};
// Check the pointer for alignment and validity.
Expand Down
38 changes: 38 additions & 0 deletions tests/run-pass/concurrency/linux-futex.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
#![feature(rustc_private)]
extern crate libc;

use std::mem::MaybeUninit;
use std::ptr;
use std::thread;
use std::time::{Duration, Instant};
Expand Down Expand Up @@ -93,6 +94,42 @@ fn wait_timeout() {
assert!((200..1000).contains(&start.elapsed().as_millis()));
}

fn wait_absolute_timeout() {
let start = Instant::now();

// Get the current monotonic timestamp as timespec.
let mut timeout = unsafe {
let mut now: MaybeUninit<libc::timespec> = MaybeUninit::uninit();
assert_eq!(libc::clock_gettime(libc::CLOCK_MONOTONIC, now.as_mut_ptr()), 0);
now.assume_init()
};

// Add 200ms.
timeout.tv_nsec += 200_000_000;
if timeout.tv_nsec > 1_000_000_000 {
timeout.tv_nsec -= 1_000_000_000;
timeout.tv_sec += 1;
}

let futex: i32 = 123;

// Wait for 200ms from now, with nobody waking us up early.
unsafe {
assert_eq!(libc::syscall(
libc::SYS_futex,
&futex as *const i32,
libc::FUTEX_WAIT_BITSET,
Copy link
Member Author

Choose a reason for hiding this comment

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

This test also passes if I put FUTEX_WAIT here, as it seems that the monotonic clock starts at zero in the tests. It'd be a better test if it was run with a monotonic clock that does not start at zero.

Copy link
Member

Choose a reason for hiding this comment

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

Specifically, our monotone clock is relative to the time_anchor which is initialized via Instant::now() when the interpreter starts.

I guess we could add an arbitrary offset to that? It also should make a difference if the interpreter has already been running for a bit (e.g. because of a previous timeout test).

123,
&timeout,
0,
Copy link
Member

Choose a reason for hiding this comment

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

This turns out to be a subtle bug, since 0 becomes 0i32 but this argument should be pointer-sized. #2058 makes Miri detect that problem.

(It'd probably still work on x86 due to details of the calling convention, but e.g. if arguments are passed on the stack then va_arg would probably do the wrong thing here.)

u32::MAX,
), -1);
assert_eq!(*libc::__errno_location(), libc::ETIMEDOUT);
}

assert!((200..1000).contains(&start.elapsed().as_millis()));
}

fn wait_wake() {
let start = Instant::now();

Expand Down Expand Up @@ -128,5 +165,6 @@ fn main() {
wake_dangling();
wait_wrong_val();
wait_timeout();
wait_absolute_timeout();
wait_wake();
}