Auto merge of #2506 - pvdrz:a-really-bad-clock, r=saethlin

Make `sleep` work with isolation enabled

Implement a virtual monotone clock that can be used to track time while isolation is enabled. This virtual clock keeps an internal nanoseconds counter that will be increased by a fixed amount at the end of every basic block.

When a process sleeps, this clock will return immediately and increase the counter by the interval the process was supposed to sleep. Making miri execution faster than native code .

cc `@RalfJung` `@saethlin` `@JakobDegen`

Author: bors

src/clock.rs

@@ -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) },
+                },
+        }
+    }
+}

src/concurrency/thread.rs

@@ -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;
 
@@ -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)),
         }
@@ -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(_) => {}
@@ -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).
@@ -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);
         }
@@ -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);
@@ -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);
     }
 
@@ -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
@@ -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,

src/eval.rs

@@ -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 => {

src/lib.rs

@@ -51,6 +51,7 @@ extern crate rustc_session;
 extern crate rustc_span;
 extern crate rustc_target;
 
+mod clock;
 mod concurrency;
 mod diagnostics;
 mod eval;
@@ -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,
@@ -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::{

src/machine.rs

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