Skip to content

Make sleep work with isolation enabled #2506

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

Merged
merged 7 commits into from
Sep 13, 2022
Merged
Show file tree
Hide file tree
Changes from all 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
115 changes: 115 additions & 0 deletions src/clock.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,115 @@
use std::sync::atomic::{AtomicU64, Ordering};
use std::time::{Duration, Instant as StdInstant};

/// When using a virtual clock, this defines how many nanoseconds we pretend are passing for each
/// basic block.
const NANOSECONDS_PER_BASIC_BLOCK: u64 = 10;

#[derive(Debug)]
pub struct Instant {
kind: InstantKind,
}

#[derive(Debug)]
enum InstantKind {
Host(StdInstant),
Virtual { nanoseconds: u64 },
}

impl Instant {
pub fn checked_add(&self, duration: Duration) -> Option<Instant> {
match self.kind {
InstantKind::Host(instant) =>
instant.checked_add(duration).map(|i| Instant { kind: InstantKind::Host(i) }),
InstantKind::Virtual { nanoseconds } =>
u128::from(nanoseconds)
.checked_add(duration.as_nanos())
.and_then(|n| u64::try_from(n).ok())
.map(|nanoseconds| Instant { kind: InstantKind::Virtual { nanoseconds } }),
}
}

pub fn duration_since(&self, earlier: Instant) -> Duration {
match (&self.kind, earlier.kind) {
(InstantKind::Host(instant), InstantKind::Host(earlier)) =>
instant.duration_since(earlier),
(
InstantKind::Virtual { nanoseconds },
InstantKind::Virtual { nanoseconds: earlier },
) => Duration::from_nanos(nanoseconds.saturating_sub(earlier)),
_ => panic!("all `Instant` must be of the same kind"),
}
}
}

/// A monotone clock used for `Instant` simulation.
#[derive(Debug)]
pub struct Clock {
kind: ClockKind,
}

#[derive(Debug)]
enum ClockKind {
Host {
/// The "time anchor" for this machine's monotone clock.
time_anchor: StdInstant,
},
Virtual {
/// The "current virtual time".
nanoseconds: AtomicU64,
},
}

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() }
} else {
ClockKind::Virtual { nanoseconds: 0.into() }
};

Self { kind }
}

/// Let the time pass for a small interval.
pub fn tick(&self) {
match &self.kind {
ClockKind::Host { .. } => {
// Time will pass without us doing anything.
}
ClockKind::Virtual { nanoseconds } => {
nanoseconds.fetch_add(NANOSECONDS_PER_BASIC_BLOCK, Ordering::SeqCst);
}
}
}

/// Sleep for the desired duration.
pub fn sleep(&self, duration: Duration) {
match &self.kind {
ClockKind::Host { .. } => std::thread::sleep(duration),
ClockKind::Virtual { nanoseconds } => {
// Just pretend that we have slept for some time.
nanoseconds.fetch_add(duration.as_nanos().try_into().unwrap(), Ordering::SeqCst);
}
}
}

/// Return the `anchor` instant, to convert between monotone instants and durations relative to the anchor.
pub fn anchor(&self) -> Instant {
match &self.kind {
ClockKind::Host { time_anchor } => Instant { kind: InstantKind::Host(*time_anchor) },
ClockKind::Virtual { .. } => Instant { kind: InstantKind::Virtual { nanoseconds: 0 } },
}
}

pub fn now(&self) -> Instant {
match &self.kind {
ClockKind::Host { .. } => Instant { kind: InstantKind::Host(StdInstant::now()) },
ClockKind::Virtual { nanoseconds } =>
Instant {
kind: InstantKind::Virtual { nanoseconds: nanoseconds.load(Ordering::SeqCst) },
},
}
}
}
50 changes: 29 additions & 21 deletions src/concurrency/thread.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
use std::cell::RefCell;
use std::collections::hash_map::Entry;
use std::num::TryFromIntError;
use std::time::{Duration, Instant, SystemTime};
use std::time::{Duration, SystemTime};

use log::trace;

Expand Down Expand Up @@ -189,9 +189,9 @@ pub enum Time {

impl Time {
/// How long do we have to wait from now until the specified time?
fn get_wait_time(&self) -> Duration {
fn get_wait_time(&self, clock: &Clock) -> Duration {
match self {
Time::Monotonic(instant) => instant.saturating_duration_since(Instant::now()),
Time::Monotonic(instant) => instant.duration_since(clock.now()),
Time::RealTime(time) =>
time.duration_since(SystemTime::now()).unwrap_or(Duration::new(0, 0)),
}
Expand Down Expand Up @@ -490,13 +490,16 @@ impl<'mir, 'tcx: 'mir> ThreadManager<'mir, 'tcx> {
}

/// Get a callback that is ready to be called.
fn get_ready_callback(&mut self) -> Option<(ThreadId, TimeoutCallback<'mir, 'tcx>)> {
fn get_ready_callback(
&mut self,
clock: &Clock,
) -> Option<(ThreadId, TimeoutCallback<'mir, 'tcx>)> {
// We iterate over all threads in the order of their indices because
// this allows us to have a deterministic scheduler.
for thread in self.threads.indices() {
match self.timeout_callbacks.entry(thread) {
Entry::Occupied(entry) =>
if entry.get().call_time.get_wait_time() == Duration::new(0, 0) {
if entry.get().call_time.get_wait_time(clock) == Duration::new(0, 0) {
return Some((thread, entry.remove().callback));
},
Entry::Vacant(_) => {}
Expand Down Expand Up @@ -553,7 +556,7 @@ impl<'mir, 'tcx: 'mir> ThreadManager<'mir, 'tcx> {
/// used in stateless model checkers such as Loom: run the active thread as
/// long as we can and switch only when we have to (the active thread was
/// blocked, terminated, or has explicitly asked to be preempted).
fn schedule(&mut self) -> InterpResult<'tcx, SchedulingAction> {
fn schedule(&mut self, clock: &Clock) -> InterpResult<'tcx, SchedulingAction> {
// Check whether the thread has **just** terminated (`check_terminated`
// checks whether the thread has popped all its stack and if yes, sets
// the thread state to terminated).
Expand All @@ -580,7 +583,7 @@ impl<'mir, 'tcx: 'mir> ThreadManager<'mir, 'tcx> {
// at the time of the call".
// <https://pubs.opengroup.org/onlinepubs/9699919799/functions/pthread_cond_timedwait.html>
let potential_sleep_time =
self.timeout_callbacks.values().map(|info| info.call_time.get_wait_time()).min();
self.timeout_callbacks.values().map(|info| info.call_time.get_wait_time(clock)).min();
if potential_sleep_time == Some(Duration::new(0, 0)) {
return Ok(SchedulingAction::ExecuteTimeoutCallback);
}
Expand Down Expand Up @@ -615,7 +618,8 @@ impl<'mir, 'tcx: 'mir> ThreadManager<'mir, 'tcx> {
// All threads are currently blocked, but we have unexecuted
// timeout_callbacks, which may unblock some of the threads. Hence,
// sleep until the first callback.
std::thread::sleep(sleep_time);

clock.sleep(sleep_time);
Ok(SchedulingAction::ExecuteTimeoutCallback)
} else {
throw_machine_stop!(TerminationInfo::Deadlock);
Expand Down Expand Up @@ -865,6 +869,9 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
callback: TimeoutCallback<'mir, 'tcx>,
) {
let this = self.eval_context_mut();
if !this.machine.communicate() && matches!(call_time, Time::RealTime(..)) {
panic!("cannot have `RealTime` callback with isolation enabled!")
}
this.machine.threads.register_timeout_callback(thread, call_time, callback);
}

Expand All @@ -878,18 +885,19 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
#[inline]
fn run_timeout_callback(&mut self) -> InterpResult<'tcx> {
let this = self.eval_context_mut();
let (thread, callback) =
if let Some((thread, callback)) = this.machine.threads.get_ready_callback() {
(thread, callback)
} else {
// get_ready_callback can return None if the computer's clock
// was shifted after calling the scheduler and before the call
// to get_ready_callback (see issue
// https://github.com/rust-lang/miri/issues/1763). In this case,
// just do nothing, which effectively just returns to the
// scheduler.
return Ok(());
};
let (thread, callback) = if let Some((thread, callback)) =
this.machine.threads.get_ready_callback(&this.machine.clock)
{
(thread, callback)
} else {
// get_ready_callback can return None if the computer's clock
// was shifted after calling the scheduler and before the call
// to get_ready_callback (see issue
// https://github.com/rust-lang/miri/issues/1763). In this case,
// just do nothing, which effectively just returns to the
// scheduler.
return Ok(());
};
// This back-and-forth with `set_active_thread` is here because of two
// design decisions:
// 1. Make the caller and not the callback responsible for changing
Expand All @@ -906,7 +914,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
#[inline]
fn schedule(&mut self) -> InterpResult<'tcx, SchedulingAction> {
let this = self.eval_context_mut();
this.machine.threads.schedule()
this.machine.threads.schedule(&this.machine.clock)
}

/// Handles thread termination of the active thread: wakes up threads joining on this one,
Expand Down
5 changes: 0 additions & 5 deletions src/eval.rs
Original file line number Diff line number Diff line change
Expand Up @@ -359,11 +359,6 @@ pub fn eval_entry<'tcx>(
assert!(ecx.step()?, "a terminated thread was scheduled for execution");
}
SchedulingAction::ExecuteTimeoutCallback => {
assert!(
ecx.machine.communicate(),
"scheduler callbacks require disabled isolation, but the code \
that created the callback did not check it"
);
ecx.run_timeout_callback()?;
}
SchedulingAction::ExecuteDtors => {
Expand Down
4 changes: 3 additions & 1 deletion src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ extern crate rustc_session;
extern crate rustc_span;
extern crate rustc_target;

mod clock;
mod concurrency;
mod diagnostics;
mod eval;
Expand Down Expand Up @@ -81,6 +82,7 @@ pub use crate::shims::time::EvalContextExt as _;
pub use crate::shims::tls::{EvalContextExt as _, TlsData};
pub use crate::shims::EvalContextExt as _;

pub use crate::clock::{Clock, Instant};
pub use crate::concurrency::{
data_race::{
AtomicFenceOrd, AtomicReadOrd, AtomicRwOrd, AtomicWriteOrd,
Expand All @@ -89,7 +91,7 @@ pub use crate::concurrency::{
sync::{CondvarId, EvalContextExt as SyncEvalContextExt, MutexId, RwLockId},
thread::{
EvalContextExt as ThreadsEvalContextExt, SchedulingAction, ThreadId, ThreadManager,
ThreadState,
ThreadState, Time,
},
};
pub use crate::diagnostics::{
Expand Down
11 changes: 7 additions & 4 deletions src/machine.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@
use std::borrow::Cow;
use std::cell::RefCell;
use std::fmt;
use std::time::Instant;

use rand::rngs::StdRng;
use rand::SeedableRng;
Expand Down Expand Up @@ -327,8 +326,8 @@ pub struct Evaluator<'mir, 'tcx> {
/// The table of directory descriptors.
pub(crate) dir_handler: shims::unix::DirHandler,

/// The "time anchor" for this machine's monotone clock (for `Instant` simulation).
pub(crate) time_anchor: Instant,
/// This machine's monotone clock.
pub(crate) clock: Clock,

/// The set of threads.
pub(crate) threads: ThreadManager<'mir, 'tcx>,
Expand Down Expand Up @@ -434,7 +433,6 @@ impl<'mir, 'tcx> Evaluator<'mir, 'tcx> {
enforce_abi: config.check_abi,
file_handler: FileHandler::new(config.mute_stdout_stderr),
dir_handler: Default::default(),
time_anchor: Instant::now(),
layouts,
threads: ThreadManager::default(),
static_roots: Vec::new(),
Expand All @@ -454,6 +452,7 @@ impl<'mir, 'tcx> Evaluator<'mir, 'tcx> {
preemption_rate: config.preemption_rate,
report_progress: config.report_progress,
basic_block_count: 0,
clock: Clock::new(config.isolated_op == IsolatedOp::Allow),
external_so_lib: config.external_so_file.as_ref().map(|lib_file_path| {
// Check if host target == the session target.
if env!("TARGET") != target_triple {
Expand Down Expand Up @@ -1036,6 +1035,10 @@ impl<'mir, 'tcx> Machine<'mir, 'tcx> for Evaluator<'mir, 'tcx> {

// These are our preemption points.
ecx.maybe_preempt_active_thread();

// Make sure some time passes.
ecx.machine.clock.tick();

Ok(())
}

Expand Down
Loading