detect when pthread_cond_t is moved

Closes rust-lang#3749

Author: Mandragorian

src/concurrency/sync.rs

@@ -134,6 +134,9 @@ struct Condvar {
     /// Contains the clock of the last thread to
     /// perform a condvar-signal.
     clock: VClock,
+
+    /// Additional data that can be set by shim implementations.
+    data: Option<Box<dyn Any>>,
 }
 
 /// The futex state.
@@ -344,21 +347,49 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
         this.machine.sync.rwlocks[id].data.as_deref().and_then(|p| p.downcast_ref::<T>())
     }
 
+    /// Eagerly create and initialize a new condvar.
+    fn condvar_create(
+        &mut self,
+        condvar: &MPlaceTy<'tcx>,
+        offset: u64,
+        data: Option<Box<dyn Any>>,
+    ) -> InterpResult<'tcx, CondvarId> {
+        let this = self.eval_context_mut();
+        this.create_id(
+            condvar,
+            offset,
+            |ecx| &mut ecx.machine.sync.condvars,
+            Condvar { data, ..Default::default() },
+        )
+    }
+
     fn condvar_get_or_create_id(
         &mut self,
         lock: &MPlaceTy<'tcx>,
         offset: u64,
+        initialize_data: impl for<'a> FnOnce(
+            &'a mut MiriInterpCx<'tcx>,
+        ) -> InterpResult<'tcx, Option<Box<dyn Any>>>,
     ) -> InterpResult<'tcx, CondvarId> {
         let this = self.eval_context_mut();
         this.get_or_create_id(
             lock,
             offset,
             |ecx| &mut ecx.machine.sync.condvars,
-            |_| Ok(Default::default()),
+            |ecx| initialize_data(ecx).map(|data| Condvar { data, ..Default::default() }),
         )?
         .ok_or_else(|| err_ub_format!("condvar has invalid ID").into())
     }
 
+    /// Retrieve the additional data stored for a condvar.
+    fn condvar_get_data<'a, T: 'static>(&'a mut self, id: CondvarId) -> Option<&'a T>
+    where
+        'tcx: 'a,
+    {
+        let this = self.eval_context_ref();
+        this.machine.sync.condvars[id].data.as_deref().and_then(|p| p.downcast_ref::<T>())
+    }
+
     #[inline]
     /// Get the id of the thread that currently owns this lock.
     fn mutex_get_owner(&mut self, id: MutexId) -> ThreadId {

src/shims/unix/sync.rs

@@ -206,7 +206,7 @@ fn translate_kind<'tcx>(ecx: &MiriInterpCx<'tcx>, kind: i32) -> InterpResult<'tc
 // - id: u32
 
 #[derive(Debug)]
-/// Additional data that may be used by shim implementations.
+/// Additional data that we attach with each rwlock instance.
 pub struct AdditionalRwLockData {
     /// The address of the rwlock.
     pub address: u64,
@@ -364,25 +364,32 @@ fn cond_clock_offset<'tcx>(ecx: &MiriInterpCx<'tcx>) -> u64 {
     offset
 }
 
+#[derive(Debug)]
+/// Additional data that we attach with each cond instance.
+struct AdditionalCondData {
+    /// The address of the cond.
+    address: u64,
+}
+
 fn cond_get_id<'tcx>(
     ecx: &mut MiriInterpCx<'tcx>,
     cond_ptr: &OpTy<'tcx>,
 ) -> InterpResult<'tcx, CondvarId> {
     let cond = ecx.deref_pointer(cond_ptr)?;
-    ecx.condvar_get_or_create_id(&cond, cond_id_offset(ecx)?)
-}
+    let address = cond.ptr().addr().bytes();
+    let id = ecx.condvar_get_or_create_id(&cond, cond_id_offset(ecx)?, |_| {
+        Ok(Some(Box::new(AdditionalCondData { address })))
+    })?;
 
-fn cond_reset_id<'tcx>(
-    ecx: &mut MiriInterpCx<'tcx>,
-    cond_ptr: &OpTy<'tcx>,
-) -> InterpResult<'tcx, ()> {
-    ecx.deref_pointer_and_write(
-        cond_ptr,
-        cond_id_offset(ecx)?,
-        Scalar::from_i32(0),
-        ecx.libc_ty_layout("pthread_cond_t"),
-        ecx.machine.layouts.u32,
-    )
+    // Check that the mutex has not been moved since last use.
+    let data = ecx
+        .condvar_get_data::<AdditionalCondData>(id)
+        .expect("data should always exist for pthreads");
+    if data.address != address {
+        throw_ub_format!("pthread_cond_t can't be moved after first use")
+    }
+
+    Ok(id)
 }
 
 fn cond_get_clock_id<'tcx>(
@@ -821,8 +828,13 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
             condattr_get_clock_id(this, attr_op)?
         };
 
-        // Write 0 to use the same code path as the static initializers.
-        cond_reset_id(this, cond_op)?;
+        let cond = this.deref_pointer(cond_op)?;
+        let address = cond.ptr().addr().bytes();
+        this.condvar_create(
+            &cond,
+            cond_id_offset(this)?,
+            Some(Box::new(AdditionalCondData { address })),
+        )?;
 
         cond_set_clock_id(this, cond_op, clock_id)?;
 

tests/fail-dep/concurrency/libc_pthread_cond_move.init.stderr

@@ -0,0 +1,20 @@
+error: Undefined Behavior: pthread_cond_t can't be moved after first use
+  --> $DIR/libc_pthread_cond_move.rs:LL:CC
+   |
+LL |         libc::pthread_cond_destroy(cond2.as_mut_ptr());
+   |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ pthread_cond_t can't be moved after first use
+   |
+   = help: this indicates a bug in the program: it performed an invalid operation, and caused Undefined Behavior
+   = help: see https://doc.rust-lang.org/nightly/reference/behavior-considered-undefined.html for further information
+   = note: BACKTRACE:
+   = note: inside `check` at $DIR/libc_pthread_cond_move.rs:LL:CC
+note: inside `main`
+  --> $DIR/libc_pthread_cond_move.rs:LL:CC
+   |
+LL |     check()
+   |     ^^^^^^^
+
+note: some details are omitted, run with `MIRIFLAGS=-Zmiri-backtrace=full` for a verbose backtrace
+
+error: aborting due to 1 previous error
+

tests/fail-dep/concurrency/libc_pthread_cond_move.rs

@@ -0,0 +1,37 @@
+//@revisions: static_initializer init
+//@ignore-target-windows: No pthreads on Windows
+
+/// Test that moving a pthread_cond between uses fails.
+
+fn main() {
+    check()
+}
+
+#[cfg(init)]
+fn check() {
+    unsafe {
+        use core::mem::MaybeUninit;
+        let mut cond = MaybeUninit::<libc::pthread_cond_t>::uninit();
+
+        libc::pthread_cond_init(cond.as_mut_ptr(), std::ptr::null());
+
+        // move pthread_cond_t
+        let mut cond2 = cond;
+
+        libc::pthread_cond_destroy(cond2.as_mut_ptr()); //~[init] ERROR: pthread_cond_t can't be moved after first use
+    }
+}
+
+#[cfg(static_initializer)]
+fn check() {
+    unsafe {
+        let mut cond = libc::PTHREAD_COND_INITIALIZER;
+
+        libc::pthread_cond_signal(&mut cond as *mut _);
+
+        // move pthread_cond_t
+        let mut cond2 = cond;
+
+        libc::pthread_cond_destroy(&mut cond2 as *mut _); //~[static_initializer] ERROR: pthread_cond_t can't be moved after first use
+    }
+}

tests/fail-dep/concurrency/libc_pthread_cond_move.static_initializer.stderr

@@ -0,0 +1,20 @@
+error: Undefined Behavior: pthread_cond_t can't be moved after first use
+  --> $DIR/libc_pthread_cond_move.rs:LL:CC
+   |
+LL |         libc::pthread_cond_destroy(&mut cond2 as *mut _);
+   |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ pthread_cond_t can't be moved after first use
+   |
+   = help: this indicates a bug in the program: it performed an invalid operation, and caused Undefined Behavior
+   = help: see https://doc.rust-lang.org/nightly/reference/behavior-considered-undefined.html for further information
+   = note: BACKTRACE:
+   = note: inside `check` at $DIR/libc_pthread_cond_move.rs:LL:CC
+note: inside `main`
+  --> $DIR/libc_pthread_cond_move.rs:LL:CC
+   |
+LL |     check()
+   |     ^^^^^^^
+
+note: some details are omitted, run with `MIRIFLAGS=-Zmiri-backtrace=full` for a verbose backtrace
+
+error: aborting due to 1 previous error
+