Auto merge of #1536 - divergentdave:nanosleep, r=RalfJung

Nanosleep

This PR adds a shim for `libc::nanosleep`, (available under -Zmiri-disable-isolation only) which backs `thread::sleep` on Linux and macOS. I started off by extracting the `timespec` parsing from the `pthread_cond_timedwait` shim into a helper method, and adding checks for invalid values. The second commit adds the new shim and a small test. The shim blocks the current thread, and registers a timeout callback to unblock the thread again, using the same method as `pthread_cond_timedwait` does.

Author: bors

src/helpers.rs

@@ -1,6 +1,7 @@
 use std::convert::{TryFrom, TryInto};
 use std::mem;
 use std::num::NonZeroUsize;
+use std::time::Duration;
 
 use log::trace;
 
@@ -512,6 +513,35 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
         let value_place = op_place.offset(offset, MemPlaceMeta::None, layout, this)?;
         this.write_scalar(value, value_place.into())
     }
+
+    /// Parse a `timespec` struct and return it as a `std::time::Duration`. It returns `None`
+    /// if the value in the `timespec` struct is invalid. Some libc functions will return
+    /// `EINVAL` in this case.
+    fn read_timespec(
+        &mut self,
+        timespec_ptr_op: OpTy<'tcx, Tag>,
+    ) -> InterpResult<'tcx, Option<Duration>> {
+        let this = self.eval_context_mut();
+        let tp = this.deref_operand(timespec_ptr_op)?;
+        let seconds_place = this.mplace_field(tp, 0)?;
+        let seconds_scalar = this.read_scalar(seconds_place.into())?;
+        let seconds = seconds_scalar.to_machine_isize(this)?;
+        let nanoseconds_place = this.mplace_field(tp, 1)?;
+        let nanoseconds_scalar = this.read_scalar(nanoseconds_place.into())?;
+        let nanoseconds = nanoseconds_scalar.to_machine_isize(this)?;
+
+        Ok(try {
+            // tv_sec must be non-negative.
+            let seconds: u64 = seconds.try_into().ok()?;
+            // tv_nsec must be non-negative.
+            let nanoseconds: u32 = nanoseconds.try_into().ok()?;
+            if nanoseconds >= 1_000_000_000 {
+                // tv_nsec must not be greater than 999,999,999.
+                None?
+            }
+            Duration::new(seconds, nanoseconds)
+        })
+    }
 }
 
 /// Check that the number of args is what we expect.

src/lib.rs

@@ -3,6 +3,7 @@
 #![feature(map_first_last)]
 #![feature(never_type)]
 #![feature(or_patterns)]
+#![feature(try_blocks)]
 
 #![warn(rust_2018_idioms)]
 #![allow(clippy::cast_lossless)]

src/shims/posix/foreign_items.rs

@@ -382,6 +382,11 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
                 let result = this.sched_yield()?;
                 this.write_scalar(Scalar::from_i32(result), dest)?;
             }
+            "nanosleep" => {
+                let &[req, rem] = check_arg_count(args)?;
+                let result = this.nanosleep(req, rem)?;
+                this.write_scalar(Scalar::from_i32(result), dest)?;
+            }
 
             // Miscellaneous
             "isatty" => {

src/shims/posix/sync.rs

@@ -1,11 +1,9 @@
-use std::convert::TryInto;
-use std::time::{Duration, SystemTime};
+use std::time::SystemTime;
 
 use crate::*;
 use stacked_borrows::Tag;
 use thread::Time;
 
-
 // pthread_mutexattr_t is either 4 or 8 bytes, depending on the platform.
 
 // Our chosen memory layout for emulation (does not have to match the platform layout!):
@@ -698,25 +696,15 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
         let mutex_id = mutex_get_or_create_id(this, mutex_op)?;
         let active_thread = this.get_active_thread();
 
-        release_cond_mutex_and_block(this, active_thread, mutex_id)?;
-        this.condvar_wait(id, active_thread, mutex_id);
-
-        // We return success for now and override it in the timeout callback.
-        this.write_scalar(Scalar::from_i32(0), dest)?;
-
         // Extract the timeout.
         let clock_id = cond_get_clock_id(this, cond_op)?.to_i32()?;
-        let duration = {
-            let tp = this.deref_operand(abstime_op)?;
-            let seconds_place = this.mplace_field(tp, 0)?;
-            let seconds = this.read_scalar(seconds_place.into())?;
-            let nanoseconds_place = this.mplace_field(tp, 1)?;
-            let nanoseconds = this.read_scalar(nanoseconds_place.into())?;
-            let (seconds, nanoseconds) = (
-                seconds.to_machine_usize(this)?,
-                nanoseconds.to_machine_usize(this)?.try_into().unwrap(),
-            );
-            Duration::new(seconds, nanoseconds)
+        let duration = match this.read_timespec(abstime_op)? {
+            Some(duration) => duration,
+            None => {
+                let einval = this.eval_libc("EINVAL")?;
+                this.write_scalar(einval, dest)?;
+                return Ok(());
+            }
         };
 
         let timeout_time = if clock_id == this.eval_libc_i32("CLOCK_REALTIME")? {
@@ -727,6 +715,12 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
             throw_unsup_format!("unsupported clock id: {}", clock_id);
         };
 
+        release_cond_mutex_and_block(this, active_thread, mutex_id)?;
+        this.condvar_wait(id, active_thread, mutex_id);
+
+        // We return success for now and override it in the timeout callback.
+        this.write_scalar(Scalar::from_i32(0), dest)?;
+
         // Register the timeout callback.
         this.register_timeout_callback(
             active_thread,
@@ -740,8 +734,8 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
                 ecx.condvar_remove_waiter(id, active_thread);
 
                 // Set the return value: we timed out.
-                let timeout = ecx.eval_libc_i32("ETIMEDOUT")?;
-                ecx.write_scalar(Scalar::from_i32(timeout), dest)?;
+                let etimedout = ecx.eval_libc("ETIMEDOUT")?;
+                ecx.write_scalar(etimedout, dest)?;
 
                 Ok(())
             }),

src/shims/time.rs

@@ -4,6 +4,7 @@ use std::convert::TryFrom;
 use crate::stacked_borrows::Tag;
 use crate::*;
 use helpers::{immty_from_int_checked, immty_from_uint_checked};
+use thread::Time;
 
 /// Returns the time elapsed between the provided time and the unix epoch as a `Duration`.
 pub fn system_time_to_duration<'tcx>(time: &SystemTime) -> InterpResult<'tcx, Duration> {
@@ -177,4 +178,40 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
         this.write_packed_immediates(info, &imms)?;
         Ok(0) // KERN_SUCCESS
     }
+
+    fn nanosleep(
+        &mut self,
+        req_op: OpTy<'tcx, Tag>,
+        _rem: OpTy<'tcx, Tag>,
+    ) -> InterpResult<'tcx, i32> {
+        // Signal handlers are not supported, so rem will never be written to.
+
+        let this = self.eval_context_mut();
+
+        this.check_no_isolation("nanosleep")?;
+
+        let duration = match this.read_timespec(req_op)? {
+            Some(duration) => duration,
+            None => {
+                let einval = this.eval_libc("EINVAL")?;
+                this.set_last_error(einval)?;
+                return Ok(-1);
+            }
+        };
+        let timeout_time = Time::Monotonic(Instant::now().checked_add(duration).unwrap());
+
+        let active_thread = this.get_active_thread();
+        this.block_thread(active_thread);
+
+        this.register_timeout_callback(
+            active_thread,
+            timeout_time,
+            Box::new(move |ecx| {
+                ecx.unblock_thread(active_thread);
+                Ok(())
+            }),
+        );
+
+        Ok(0)
+    }
 }

tests/run-pass/concurrency/libc_pthread_cond.rs

@@ -37,6 +37,38 @@ fn test_timed_wait_timeout(clock_id: i32) {
         );
         let elapsed_time = current_time.elapsed().as_millis();
         assert!(900 <= elapsed_time && elapsed_time <= 1300);
+
+        // 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(
+                &mut cond as *mut _,
+                &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(
+                &mut cond as *mut _,
+                &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(
+                &mut cond as *mut _,
+                &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(&mut cond as *mut _), 0);

tests/run-pass/time.rs

@@ -8,6 +8,15 @@ fn duration_sanity(diff: Duration) {
     assert!(diff.as_millis() < 500);
 }
 
+// Sleeping on Windows is not supported yet.
+#[cfg(unix)]
+fn test_sleep() {
+    let before = Instant::now();
+    std::thread::sleep(Duration::from_millis(100));
+    let after = Instant::now();
+    assert!((after - before).as_millis() >= 100);
+}
+
 fn main() {
     // Check `SystemTime`.
     let now1 = SystemTime::now();
@@ -36,4 +45,7 @@ fn main() {
     assert_eq!(now1 + diff, now2);
     assert_eq!(now2 - diff, now1);
     duration_sanity(diff);
+
+    #[cfg(unix)]
+    test_sleep();
 }