mikros/rust
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Auto merge of #3663 - RalfJung:timeouts, r=RalfJung

Browse Source 
don't panic if time computaton overflows

Let the thread blocking system handle timeout computation, and on overflows we just set the timeout to 1h.
This commit is contained in:
bors 2024-06-09 11:11:26 +00:00
commit b5ae8bdb01
8 changed files with 102 additions and 67 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

33
src/tools/miri/src/clock.rs
View File

@ -20,14 +20,20 @@ enum InstantKind {
}
impl Instant {
pub fn checked_add(&self, duration: Duration) -> Option<Instant> {
/// 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 @@ pub fn sleep(&self, duration: Duration) {
}
}
/// 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 } },
}
}

5
src/tools/miri/src/concurrency/sync.rs
View File

@ -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 @@ fn condvar_wait(
&mut self,
condvar: CondvarId,
mutex: MutexId,
timeout: Option<Timeout>,
timeout: Option<(TimeoutClock, TimeoutAnchor, Duration)>,
retval_succ: Scalar,
retval_timeout: Scalar,
dest: MPlaceTy<'tcx>,
@ -704,7 +705,7 @@ fn futex_wait(
&mut self,
addr: u64,
bitset: u32,
timeout: Option<Timeout>,
timeout: Option<(TimeoutClock, TimeoutAnchor, Duration)>,
retval_succ: Scalar,
retval_timeout: Scalar,
dest: MPlaceTy<'tcx>,

55
src/tools/miri/src/concurrency/thread.rs
View File

@ -407,7 +407,7 @@ fn visit_provenance(&self, visit: &mut VisitWith<'_>) {
/// 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 @@ fn get_wait_time(&self, clock: &Clock) -> Duration {
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 @@ fn terminate_active_thread(&mut self, tls_alloc_action: TlsAllocAction) -> Inter
fn block_thread(
&mut self,
reason: BlockReason,
timeout: Option<Timeout>,
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);
}

4
src/tools/miri/src/lib.rs
View File

@ -132,8 +132,8 @@
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::{

18
src/tools/miri/src/shims/time.rs
View File

@ -78,7 +78,7 @@ fn clock_gettime(
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 @@ fn QueryPerformanceCounter(
// 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 @@ fn mach_absolute_time(&self) -> InterpResult<'tcx, Scalar> {
// 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 @@ fn nanosleep(
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 @@ fn Sleep(&mut self, timeout: &OpTy<'tcx>) -> InterpResult<'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") }

39
src/tools/miri/src/shims/unix/linux/sync.rs
View File

@ -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(),

9
src/tools/miri/src/shims/unix/sync.rs
View File

@ -1,5 +1,4 @@
use std::sync::atomic::{AtomicBool, Ordering};
use std::time::SystemTime;
use rustc_target::abi::Size;
@ -849,11 +848,11 @@ fn pthread_cond_timedwait(
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 @@ fn pthread_cond_timedwait(
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(),

6
src/tools/miri/src/shims/windows/sync.rs
View File

@ -157,11 +157,11 @@ fn WaitOnAddress(
};
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 @@ fn WaitOnAddress(
this.futex_wait(
addr,
u32::MAX, // bitset
timeout_time,
timeout,
Scalar::from_i32(1), // retval_succ
Scalar::from_i32(0), // retval_timeout
dest.clone(),