diff --git a/src/tools/miri/src/clock.rs b/src/tools/miri/src/clock.rs index 3f86767277a..c9bffc449f7 100644 --- a/src/tools/miri/src/clock.rs +++ b/src/tools/miri/src/clock.rs @@ -20,14 +20,20 @@ enum InstantKind { } impl Instant { - pub fn checked_add(&self, duration: Duration) -> Option { + /// Will try to add `duration`, but if that overflows it may add less. + pub fn add_lossy(&self, duration: Duration) -> Instant { match self.kind { - InstantKind::Host(instant) => - instant.checked_add(duration).map(|i| Instant { kind: InstantKind::Host(i) }), - InstantKind::Virtual { nanoseconds } => - nanoseconds - .checked_add(duration.as_nanos()) - .map(|nanoseconds| Instant { kind: InstantKind::Virtual { nanoseconds } }), + InstantKind::Host(instant) => { + // If this overflows, try adding just 1h and assume that will not overflow. + let i = instant + .checked_add(duration) + .unwrap_or_else(|| instant.checked_add(Duration::from_secs(3600)).unwrap()); + Instant { kind: InstantKind::Host(i) } + } + InstantKind::Virtual { nanoseconds } => { + let n = nanoseconds.saturating_add(duration.as_nanos()); + Instant { kind: InstantKind::Virtual { nanoseconds: n } } + } } } @@ -63,8 +69,9 @@ pub struct Clock { #[derive(Debug)] enum ClockKind { Host { - /// The "time anchor" for this machine's monotone clock. - time_anchor: StdInstant, + /// The "epoch" for this machine's monotone clock: + /// the moment we consider to be time = 0. + epoch: StdInstant, }, Virtual { /// The "current virtual time". @@ -76,7 +83,7 @@ impl Clock { /// Create a new clock based on the availability of communication with the host. pub fn new(communicate: bool) -> Self { let kind = if communicate { - ClockKind::Host { time_anchor: StdInstant::now() } + ClockKind::Host { epoch: StdInstant::now() } } else { ClockKind::Virtual { nanoseconds: 0.into() } }; @@ -111,10 +118,10 @@ impl Clock { } } - /// Return the `anchor` instant, to convert between monotone instants and durations relative to the anchor. - pub fn anchor(&self) -> Instant { + /// Return the `epoch` instant (time = 0), to convert between monotone instants and absolute durations. + pub fn epoch(&self) -> Instant { match &self.kind { - ClockKind::Host { time_anchor } => Instant { kind: InstantKind::Host(*time_anchor) }, + ClockKind::Host { epoch } => Instant { kind: InstantKind::Host(*epoch) }, ClockKind::Virtual { .. } => Instant { kind: InstantKind::Virtual { nanoseconds: 0 } }, } } diff --git a/src/tools/miri/src/concurrency/sync.rs b/src/tools/miri/src/concurrency/sync.rs index 030546b7cb5..76a045c4dc1 100644 --- a/src/tools/miri/src/concurrency/sync.rs +++ b/src/tools/miri/src/concurrency/sync.rs @@ -1,5 +1,6 @@ use std::collections::{hash_map::Entry, VecDeque}; use std::ops::Not; +use std::time::Duration; use rustc_data_structures::fx::FxHashMap; use rustc_index::{Idx, IndexVec}; @@ -623,7 +624,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { &mut self, condvar: CondvarId, mutex: MutexId, - timeout: Option, + timeout: Option<(TimeoutClock, TimeoutAnchor, Duration)>, retval_succ: Scalar, retval_timeout: Scalar, dest: MPlaceTy<'tcx>, @@ -704,7 +705,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { &mut self, addr: u64, bitset: u32, - timeout: Option, + timeout: Option<(TimeoutClock, TimeoutAnchor, Duration)>, retval_succ: Scalar, retval_timeout: Scalar, dest: MPlaceTy<'tcx>, diff --git a/src/tools/miri/src/concurrency/thread.rs b/src/tools/miri/src/concurrency/thread.rs index e76c16fb830..6a2b99825ad 100644 --- a/src/tools/miri/src/concurrency/thread.rs +++ b/src/tools/miri/src/concurrency/thread.rs @@ -407,7 +407,7 @@ impl VisitProvenance for Frame<'_, Provenance, FrameExtra<'_>> { /// The moment in time when a blocked thread should be woken up. #[derive(Debug)] -pub enum Timeout { +enum Timeout { Monotonic(Instant), RealTime(SystemTime), } @@ -421,6 +421,34 @@ impl Timeout { time.duration_since(SystemTime::now()).unwrap_or(Duration::ZERO), } } + + /// Will try to add `duration`, but if that overflows it may add less. + fn add_lossy(&self, duration: Duration) -> Self { + match self { + Timeout::Monotonic(i) => Timeout::Monotonic(i.add_lossy(duration)), + Timeout::RealTime(s) => { + // If this overflows, try adding just 1h and assume that will not overflow. + Timeout::RealTime( + s.checked_add(duration) + .unwrap_or_else(|| s.checked_add(Duration::from_secs(3600)).unwrap()), + ) + } + } + } +} + +/// The clock to use for the timeout you are asking for. +#[derive(Debug, Copy, Clone)] +pub enum TimeoutClock { + Monotonic, + RealTime, +} + +/// Whether the timeout is relative or absolute. +#[derive(Debug, Copy, Clone)] +pub enum TimeoutAnchor { + Relative, + Absolute, } /// A set of threads. @@ -995,13 +1023,30 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { fn block_thread( &mut self, reason: BlockReason, - timeout: Option, + timeout: Option<(TimeoutClock, TimeoutAnchor, Duration)>, callback: impl UnblockCallback<'tcx> + 'tcx, ) { let this = self.eval_context_mut(); - if !this.machine.communicate() && matches!(timeout, Some(Timeout::RealTime(..))) { - panic!("cannot have `RealTime` callback with isolation enabled!") - } + let timeout = timeout.map(|(clock, anchor, duration)| { + let anchor = match clock { + TimeoutClock::RealTime => { + assert!( + this.machine.communicate(), + "cannot have `RealTime` timeout with isolation enabled!" + ); + Timeout::RealTime(match anchor { + TimeoutAnchor::Absolute => SystemTime::UNIX_EPOCH, + TimeoutAnchor::Relative => SystemTime::now(), + }) + } + TimeoutClock::Monotonic => + Timeout::Monotonic(match anchor { + TimeoutAnchor::Absolute => this.machine.clock.epoch(), + TimeoutAnchor::Relative => this.machine.clock.now(), + }), + }; + anchor.add_lossy(duration) + }); this.machine.threads.block_thread(reason, timeout, callback); } diff --git a/src/tools/miri/src/lib.rs b/src/tools/miri/src/lib.rs index 94aed0645fa..11cbbde06aa 100644 --- a/src/tools/miri/src/lib.rs +++ b/src/tools/miri/src/lib.rs @@ -132,8 +132,8 @@ pub use crate::concurrency::{ init_once::{EvalContextExt as _, InitOnceId}, sync::{CondvarId, EvalContextExt as _, MutexId, RwLockId, SynchronizationObjects}, thread::{ - BlockReason, EvalContextExt as _, StackEmptyCallback, ThreadId, ThreadManager, Timeout, - UnblockCallback, + BlockReason, EvalContextExt as _, StackEmptyCallback, ThreadId, ThreadManager, + TimeoutAnchor, TimeoutClock, UnblockCallback, }, }; pub use crate::diagnostics::{ diff --git a/src/tools/miri/src/shims/time.rs b/src/tools/miri/src/shims/time.rs index 8206b15d0a0..ae17196f0b7 100644 --- a/src/tools/miri/src/shims/time.rs +++ b/src/tools/miri/src/shims/time.rs @@ -78,7 +78,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { this.check_no_isolation("`clock_gettime` with `REALTIME` clocks")?; system_time_to_duration(&SystemTime::now())? } else if relative_clocks.contains(&clk_id) { - this.machine.clock.now().duration_since(this.machine.clock.anchor()) + this.machine.clock.now().duration_since(this.machine.clock.epoch()) } else { let einval = this.eval_libc("EINVAL"); this.set_last_error(einval)?; @@ -246,7 +246,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { // QueryPerformanceCounter uses a hardware counter as its basis. // Miri will emulate a counter with a resolution of 1 nanosecond. - let duration = this.machine.clock.now().duration_since(this.machine.clock.anchor()); + let duration = this.machine.clock.now().duration_since(this.machine.clock.epoch()); let qpc = i64::try_from(duration.as_nanos()).map_err(|_| { err_unsup_format!("programs running longer than 2^63 nanoseconds are not supported") })?; @@ -282,7 +282,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { // This returns a u64, with time units determined dynamically by `mach_timebase_info`. // We return plain nanoseconds. - let duration = this.machine.clock.now().duration_since(this.machine.clock.anchor()); + let duration = this.machine.clock.now().duration_since(this.machine.clock.epoch()); let res = u64::try_from(duration.as_nanos()).map_err(|_| { err_unsup_format!("programs running longer than 2^64 nanoseconds are not supported") })?; @@ -323,16 +323,10 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { return Ok(-1); } }; - // If adding the duration overflows, let's just sleep for an hour. Waking up early is always acceptable. - let now = this.machine.clock.now(); - let timeout_time = now - .checked_add(duration) - .unwrap_or_else(|| now.checked_add(Duration::from_secs(3600)).unwrap()); - let timeout_time = Timeout::Monotonic(timeout_time); this.block_thread( BlockReason::Sleep, - Some(timeout_time), + Some((TimeoutClock::Monotonic, TimeoutAnchor::Relative, duration)), callback!( @capture<'tcx> {} @unblock = |_this| { panic!("sleeping thread unblocked before time is up") } @@ -351,12 +345,10 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { let timeout_ms = this.read_scalar(timeout)?.to_u32()?; let duration = Duration::from_millis(timeout_ms.into()); - let timeout_time = this.machine.clock.now().checked_add(duration).unwrap(); - let timeout_time = Timeout::Monotonic(timeout_time); this.block_thread( BlockReason::Sleep, - Some(timeout_time), + Some((TimeoutClock::Monotonic, TimeoutAnchor::Relative, duration)), callback!( @capture<'tcx> {} @unblock = |_this| { panic!("sleeping thread unblocked before time is up") } diff --git a/src/tools/miri/src/shims/unix/linux/sync.rs b/src/tools/miri/src/shims/unix/linux/sync.rs index 3e0d5e58d4c..bd212039074 100644 --- a/src/tools/miri/src/shims/unix/linux/sync.rs +++ b/src/tools/miri/src/shims/unix/linux/sync.rs @@ -1,5 +1,3 @@ -use std::time::SystemTime; - use crate::*; /// Implementation of the SYS_futex syscall. @@ -84,15 +82,9 @@ pub fn futex<'tcx>( } let timeout = this.deref_pointer_as(&args[3], this.libc_ty_layout("timespec"))?; - let timeout_time = if this.ptr_is_null(timeout.ptr())? { + let timeout = if this.ptr_is_null(timeout.ptr())? { None } else { - let realtime = op & futex_realtime == futex_realtime; - if realtime { - 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 => { @@ -102,23 +94,22 @@ pub fn futex<'tcx>( return Ok(()); } }; - Some(if wait_bitset { + let timeout_clock = if op & futex_realtime == futex_realtime { + this.check_no_isolation( + "`futex` syscall with `op=FUTEX_WAIT` and non-null timeout with `FUTEX_CLOCK_REALTIME`", + )?; + TimeoutClock::RealTime + } else { + TimeoutClock::Monotonic + }; + let timeout_anchor = if wait_bitset { // FUTEX_WAIT_BITSET uses an absolute timestamp. - if realtime { - Timeout::RealTime(SystemTime::UNIX_EPOCH.checked_add(duration).unwrap()) - } else { - Timeout::Monotonic( - this.machine.clock.anchor().checked_add(duration).unwrap(), - ) - } + TimeoutAnchor::Absolute } else { // FUTEX_WAIT uses a relative timestamp. - if realtime { - Timeout::RealTime(SystemTime::now().checked_add(duration).unwrap()) - } else { - Timeout::Monotonic(this.machine.clock.now().checked_add(duration).unwrap()) - } - }) + TimeoutAnchor::Relative + }; + Some((timeout_clock, timeout_anchor, duration)) }; // There may be a concurrent thread changing the value of addr // and then invoking the FUTEX_WAKE syscall. It is critical that the @@ -172,7 +163,7 @@ pub fn futex<'tcx>( this.futex_wait( addr_usize, bitset, - timeout_time, + timeout, Scalar::from_target_isize(0, this), // retval_succ Scalar::from_target_isize(-1, this), // retval_timeout dest.clone(), diff --git a/src/tools/miri/src/shims/unix/sync.rs b/src/tools/miri/src/shims/unix/sync.rs index 7d4c32bc87e..be6732b1b67 100644 --- a/src/tools/miri/src/shims/unix/sync.rs +++ b/src/tools/miri/src/shims/unix/sync.rs @@ -1,5 +1,4 @@ use std::sync::atomic::{AtomicBool, Ordering}; -use std::time::SystemTime; use rustc_target::abi::Size; @@ -849,11 +848,11 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { return Ok(()); } }; - let timeout_time = if is_cond_clock_realtime(this, clock_id) { + let timeout_clock = if is_cond_clock_realtime(this, clock_id) { this.check_no_isolation("`pthread_cond_timedwait` with `CLOCK_REALTIME`")?; - Timeout::RealTime(SystemTime::UNIX_EPOCH.checked_add(duration).unwrap()) + TimeoutClock::RealTime } else if clock_id == this.eval_libc_i32("CLOCK_MONOTONIC") { - Timeout::Monotonic(this.machine.clock.anchor().checked_add(duration).unwrap()) + TimeoutClock::Monotonic } else { throw_unsup_format!("unsupported clock id: {}", clock_id); }; @@ -861,7 +860,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { this.condvar_wait( id, mutex_id, - Some(timeout_time), + Some((timeout_clock, TimeoutAnchor::Absolute, duration)), Scalar::from_i32(0), this.eval_libc("ETIMEDOUT"), // retval_timeout dest.clone(), diff --git a/src/tools/miri/src/shims/windows/sync.rs b/src/tools/miri/src/shims/windows/sync.rs index e77986dd5fe..e1fbb77037c 100644 --- a/src/tools/miri/src/shims/windows/sync.rs +++ b/src/tools/miri/src/shims/windows/sync.rs @@ -157,11 +157,11 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { }; let size = Size::from_bytes(size); - let timeout_time = if timeout_ms == this.eval_windows_u32("c", "INFINITE") { + let timeout = if timeout_ms == this.eval_windows_u32("c", "INFINITE") { None } else { let duration = Duration::from_millis(timeout_ms.into()); - Some(Timeout::Monotonic(this.machine.clock.now().checked_add(duration).unwrap())) + Some((TimeoutClock::Monotonic, TimeoutAnchor::Relative, duration)) }; // See the Linux futex implementation for why this fence exists. @@ -177,7 +177,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { this.futex_wait( addr, u32::MAX, // bitset - timeout_time, + timeout, Scalar::from_i32(1), // retval_succ Scalar::from_i32(0), // retval_timeout dest.clone(),