Auto merge of rust-lang#3871 - Mandragorian:detect_rwlock_move, r=RalfJung

detect when pthread_rwlock_t is moved

For some implementations of pthreads, the address of pthread_rwlock_t (or its fields) is used to identify the lock. That means that if the contents of a pthread_rwlock_t are moved in memory, effectively a new lock object is created, which is completely independted from the original. Thus we want to detect when when such objects are moved and show an error.

see also rust-lang#3749 for more context

Author: bors

Diff for: src/tools/miri/src/concurrency/sync.rs

@@ -105,6 +105,13 @@ struct Mutex {
 
 declare_id!(RwLockId);
 
+#[derive(Debug)]
+/// Additional data that may be used by shim implementations.
+pub struct AdditionalRwLockData {
+    /// The address of the rwlock.
+    pub address: u64,
+}
+
 /// The read-write lock state.
 #[derive(Default, Debug)]
 struct RwLock {
@@ -137,6 +144,9 @@ struct RwLock {
     /// locks.
     /// This is only relevant when there is an active reader.
     clock_current_readers: VClock,
+
+    /// Additional data that can be set by shim implementations.
+    data: Option<AdditionalRwLockData>,
 }
 
 declare_id!(CondvarId);
@@ -343,18 +353,31 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
         lock_op: &OpTy<'tcx>,
         lock_layout: TyAndLayout<'tcx>,
         offset: u64,
+        initialize_data: impl for<'a> FnOnce(
+            &'a mut MiriInterpCx<'tcx>,
+        )
+            -> InterpResult<'tcx, Option<AdditionalRwLockData>>,
     ) -> InterpResult<'tcx, RwLockId> {
         let this = self.eval_context_mut();
         this.get_or_create_id(
             lock_op,
             lock_layout,
             offset,
             |ecx| &mut ecx.machine.sync.rwlocks,
-            |_| Ok(Default::default()),
+            |ecx| initialize_data(ecx).map(|data| RwLock { data, ..Default::default() }),
         )?
         .ok_or_else(|| err_ub_format!("rwlock has invalid ID").into())
     }
 
+    /// Retrieve the additional data stored for a rwlock.
+    fn rwlock_get_data<'a>(&'a mut self, id: RwLockId) -> Option<&'a AdditionalRwLockData>
+    where
+        'tcx: 'a,
+    {
+        let this = self.eval_context_ref();
+        this.machine.sync.rwlocks[id].data.as_ref()
+    }
+
     fn condvar_get_or_create_id(
         &mut self,
         lock_op: &OpTy<'tcx>,

Diff for: src/tools/miri/src/lib.rs

@@ -134,8 +134,8 @@ pub use crate::concurrency::{
     data_race::{AtomicFenceOrd, AtomicReadOrd, AtomicRwOrd, AtomicWriteOrd, EvalContextExt as _},
     init_once::{EvalContextExt as _, InitOnceId},
     sync::{
-        AdditionalMutexData, CondvarId, EvalContextExt as _, MutexId, MutexKind, RwLockId,
-        SynchronizationObjects,
+        AdditionalMutexData, AdditionalRwLockData, CondvarId, EvalContextExt as _, MutexId,
+        MutexKind, RwLockId, SynchronizationObjects,
     },
     thread::{
         BlockReason, EvalContextExt as _, StackEmptyCallback, ThreadId, ThreadManager,

Diff for: src/tools/miri/src/shims/unix/sync.rs

@@ -213,11 +213,22 @@ fn rwlock_get_id<'tcx>(
     ecx: &mut MiriInterpCx<'tcx>,
     rwlock_op: &OpTy<'tcx>,
 ) -> InterpResult<'tcx, RwLockId> {
-    ecx.rwlock_get_or_create_id(
+    let address = ecx.read_pointer(rwlock_op)?.addr().bytes();
+
+    let id = ecx.rwlock_get_or_create_id(
         rwlock_op,
         ecx.libc_ty_layout("pthread_rwlock_t"),
         rwlock_id_offset(ecx)?,
-    )
+        |_| Ok(Some(AdditionalRwLockData { address })),
+    )?;
+
+    // Check that the rwlock has not been moved since last use.
+    let data = ecx.rwlock_get_data(id).expect("data should be always exist for pthreads");
+    if data.address != address {
+        throw_ub_format!("pthread_rwlock_t can't be moved after first use")
+    }
+
+    Ok(id)
 }
 
 // pthread_condattr_t.

Diff for: src/tools/miri/tests/fail-dep/concurrency/libx_pthread_rwlock_moved.rs

@@ -0,0 +1,14 @@
+//@ignore-target-windows: No pthreads on Windows
+
+fn main() {
+    unsafe {
+        let mut rw = libc::PTHREAD_RWLOCK_INITIALIZER;
+
+        libc::pthread_rwlock_rdlock(&mut rw as *mut _);
+
+        // Move rwlock
+        let mut rw2 = rw;
+
+        libc::pthread_rwlock_unlock(&mut rw2 as *mut _); //~ ERROR: pthread_rwlock_t can't be moved after first use
+    }
+}

Diff for: src/tools/miri/tests/fail-dep/concurrency/libx_pthread_rwlock_moved.stderr

@@ -0,0 +1,15 @@
+error: Undefined Behavior: pthread_rwlock_t can't be moved after first use
+  --> $DIR/libx_pthread_rwlock_moved.rs:LL:CC
+   |
+LL |         libc::pthread_rwlock_unlock(&mut rw2 as *mut _);
+   |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ pthread_rwlock_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 `main` at $DIR/libx_pthread_rwlock_moved.rs:LL:CC
+
+note: some details are omitted, run with `MIRIFLAGS=-Zmiri-backtrace=full` for a verbose backtrace
+
+error: aborting due to 1 previous error
+