From c8a5d4b6000146db63574196970e9f7d803a386d Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Fri, 28 Oct 2022 16:13:10 +0200 Subject: [PATCH] relative futex and condvar timeouts can work with isolation --- src/tools/miri/src/shims/unix/linux/sync.rs | 8 +- src/tools/miri/src/shims/unix/sync.rs | 3 +- src/tools/miri/src/shims/windows/sync.rs | 2 - .../concurrency/libc_pthread_cond_isolated.rs | 82 +++++++++++++++++++ .../pass/concurrency/thread_park_isolated.rs | 12 +++ 5 files changed, 100 insertions(+), 7 deletions(-) create mode 100644 src/tools/miri/tests/pass-dep/concurrency/libc_pthread_cond_isolated.rs create mode 100644 src/tools/miri/tests/pass/concurrency/thread_park_isolated.rs diff --git a/src/tools/miri/src/shims/unix/linux/sync.rs b/src/tools/miri/src/shims/unix/linux/sync.rs index 91db30e93d8..292b9d2e7a1 100644 --- a/src/tools/miri/src/shims/unix/linux/sync.rs +++ b/src/tools/miri/src/shims/unix/linux/sync.rs @@ -90,9 +90,11 @@ pub fn futex<'tcx>( let timeout_time = if this.ptr_is_null(timeout.ptr)? { None } else { - this.check_no_isolation( - "`futex` syscall with `op=FUTEX_WAIT` and non-null timeout", - )?; + if op & futex_realtime != 0 { + this.check_no_isolation( + "`futex` syscall with `op=FUTEX_WAIT` and non-null timeout with `FUTEX_CLOCK_REALTIME`", + )?; + } let duration = match this.read_timespec(&timeout)? { Some(duration) => duration, None => { diff --git a/src/tools/miri/src/shims/unix/sync.rs b/src/tools/miri/src/shims/unix/sync.rs index 3e1e34c5dbe..fcb00692079 100644 --- a/src/tools/miri/src/shims/unix/sync.rs +++ b/src/tools/miri/src/shims/unix/sync.rs @@ -743,8 +743,6 @@ fn pthread_cond_timedwait( ) -> InterpResult<'tcx> { let this = self.eval_context_mut(); - this.check_no_isolation("`pthread_cond_timedwait`")?; - let id = this.condvar_get_or_create_id(cond_op, CONDVAR_ID_OFFSET)?; let mutex_id = this.mutex_get_or_create_id(mutex_op, MUTEX_ID_OFFSET)?; let active_thread = this.get_active_thread(); @@ -761,6 +759,7 @@ fn pthread_cond_timedwait( }; let timeout_time = if clock_id == this.eval_libc_i32("CLOCK_REALTIME")? { + this.check_no_isolation("`pthread_cond_timedwait` with `CLOCK_REALTIME`")?; Time::RealTime(SystemTime::UNIX_EPOCH.checked_add(duration).unwrap()) } else if clock_id == this.eval_libc_i32("CLOCK_MONOTONIC")? { Time::Monotonic(this.machine.clock.anchor().checked_add(duration).unwrap()) diff --git a/src/tools/miri/src/shims/windows/sync.rs b/src/tools/miri/src/shims/windows/sync.rs index 336ba7db95f..8156ae8af1e 100644 --- a/src/tools/miri/src/shims/windows/sync.rs +++ b/src/tools/miri/src/shims/windows/sync.rs @@ -254,8 +254,6 @@ fn WaitOnAddress( let timeout_time = if timeout_ms == this.eval_windows("c", "INFINITE")?.to_u32()? { None } else { - this.check_no_isolation("`WaitOnAddress` with non-infinite timeout")?; - let duration = Duration::from_millis(timeout_ms.into()); Some(Time::Monotonic(this.machine.clock.now().checked_add(duration).unwrap())) }; diff --git a/src/tools/miri/tests/pass-dep/concurrency/libc_pthread_cond_isolated.rs b/src/tools/miri/tests/pass-dep/concurrency/libc_pthread_cond_isolated.rs new file mode 100644 index 00000000000..103ce44006d --- /dev/null +++ b/src/tools/miri/tests/pass-dep/concurrency/libc_pthread_cond_isolated.rs @@ -0,0 +1,82 @@ +//@ignore-target-windows: No libc on Windows +//@ignore-target-apple: pthread_condattr_setclock is not supported on MacOS. + +/// Test that conditional variable timeouts are working properly +/// with monotonic clocks even under isolation. +use std::mem::MaybeUninit; +use std::time::Instant; + +fn test_timed_wait_timeout(clock_id: i32) { + unsafe { + let mut attr: MaybeUninit = MaybeUninit::uninit(); + assert_eq!(libc::pthread_condattr_init(attr.as_mut_ptr()), 0); + assert_eq!(libc::pthread_condattr_setclock(attr.as_mut_ptr(), clock_id), 0); + + let mut cond: MaybeUninit = MaybeUninit::uninit(); + assert_eq!(libc::pthread_cond_init(cond.as_mut_ptr(), attr.as_ptr()), 0); + assert_eq!(libc::pthread_condattr_destroy(attr.as_mut_ptr()), 0); + + let mut mutex: libc::pthread_mutex_t = libc::PTHREAD_MUTEX_INITIALIZER; + + let mut now_mu: MaybeUninit = MaybeUninit::uninit(); + assert_eq!(libc::clock_gettime(clock_id, now_mu.as_mut_ptr()), 0); + let now = now_mu.assume_init(); + // Waiting for a second... mostly because waiting less requires mich more tricky arithmetic. + // FIXME: wait less. + let timeout = libc::timespec { tv_sec: now.tv_sec + 1, tv_nsec: now.tv_nsec }; + + assert_eq!(libc::pthread_mutex_lock(&mut mutex as *mut _), 0); + let current_time = Instant::now(); + assert_eq!( + libc::pthread_cond_timedwait(cond.as_mut_ptr(), &mut mutex as *mut _, &timeout), + libc::ETIMEDOUT + ); + let elapsed_time = current_time.elapsed().as_millis(); + assert!(900 <= elapsed_time && elapsed_time <= 1300); + + // Test calling `pthread_cond_timedwait` again with an already elapsed timeout. + assert_eq!( + libc::pthread_cond_timedwait(cond.as_mut_ptr(), &mut mutex as *mut _, &timeout), + libc::ETIMEDOUT + ); + + // Test that invalid nanosecond values (above 10^9 or negative) are rejected with the + // correct error code. + let invalid_timeout_1 = libc::timespec { tv_sec: now.tv_sec + 1, tv_nsec: 1_000_000_000 }; + assert_eq!( + libc::pthread_cond_timedwait( + cond.as_mut_ptr(), + &mut mutex as *mut _, + &invalid_timeout_1 + ), + libc::EINVAL + ); + let invalid_timeout_2 = libc::timespec { tv_sec: now.tv_sec + 1, tv_nsec: -1 }; + assert_eq!( + libc::pthread_cond_timedwait( + cond.as_mut_ptr(), + &mut mutex as *mut _, + &invalid_timeout_2 + ), + libc::EINVAL + ); + // Test that invalid second values (negative) are rejected with the correct error code. + let invalid_timeout_3 = libc::timespec { tv_sec: -1, tv_nsec: 0 }; + assert_eq!( + libc::pthread_cond_timedwait( + cond.as_mut_ptr(), + &mut mutex as *mut _, + &invalid_timeout_3 + ), + libc::EINVAL + ); + + assert_eq!(libc::pthread_mutex_unlock(&mut mutex as *mut _), 0); + assert_eq!(libc::pthread_mutex_destroy(&mut mutex as *mut _), 0); + assert_eq!(libc::pthread_cond_destroy(cond.as_mut_ptr()), 0); + } +} + +fn main() { + test_timed_wait_timeout(libc::CLOCK_MONOTONIC); +} diff --git a/src/tools/miri/tests/pass/concurrency/thread_park_isolated.rs b/src/tools/miri/tests/pass/concurrency/thread_park_isolated.rs new file mode 100644 index 00000000000..bf004012e84 --- /dev/null +++ b/src/tools/miri/tests/pass/concurrency/thread_park_isolated.rs @@ -0,0 +1,12 @@ +//@ignore-target-apple: park_timeout on macOS uses the system clock +use std::thread; +use std::time::{Duration, Instant}; + +fn main() { + let start = Instant::now(); + + thread::park_timeout(Duration::from_millis(200)); + + // Thanks to deterministic execution, this will wiat *exactly* 200ms (rounded to 1ms). + assert!((200..201).contains(&start.elapsed().as_millis())); +}