macOS os_unfair_lock: also store ID outside addressable memory

Author: RalfJung

src/tools/miri/src/concurrency/sync.rs

@@ -236,48 +236,26 @@ pub(super) trait EvalContextExtPriv<'tcx>: crate::MiriInterpCxExt<'tcx> {
     }
 }
 
-// Public interface to synchronization primitives. Please note that in most
-// cases, the function calls are infallible and it is the client's (shim
-// implementation's) responsibility to detect and deal with erroneous
-// situations.
-impl<'tcx> EvalContextExt<'tcx> for crate::MiriInterpCx<'tcx> {}
-pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
-    /// Eagerly create and initialize a new mutex.
-    fn mutex_create(&mut self) -> MutexId {
-        let this = self.eval_context_mut();
-        this.machine.sync.mutexes.push(Default::default())
-    }
-
-    /// Lazily create a new mutex.
-    /// `initialize_data` must return any additional data that a user wants to associate with the mutex.
-    fn mutex_get_or_create_id(
-        &mut self,
-        lock: &MPlaceTy<'tcx>,
-        offset: u64,
-    ) -> InterpResult<'tcx, MutexId> {
-        let this = self.eval_context_mut();
-        this.get_or_create_id(
-            lock,
-            offset,
-            |ecx| &mut ecx.machine.sync.mutexes,
-            |_ecx| interp_ok(Mutex::default()),
-        )?
-        .ok_or_else(|| err_ub_format!("mutex has invalid ID"))
-        .into()
+impl SynchronizationObjects {
+    pub fn mutex_create(&mut self) -> MutexId {
+        self.mutexes.push(Default::default())
     }
 
-    /// Eagerly create and initialize a new rwlock.
-    fn rwlock_create(&mut self) -> RwLockId {
-        let this = self.eval_context_mut();
-        this.machine.sync.rwlocks.push(Default::default())
+    pub fn rwlock_create(&mut self) -> RwLockId {
+        self.rwlocks.push(Default::default())
     }
 
-    /// Eagerly create and initialize a new condvar.
-    fn condvar_create(&mut self) -> CondvarId {
-        let this = self.eval_context_mut();
-        this.machine.sync.condvars.push(Default::default())
+    pub fn condvar_create(&mut self) -> CondvarId {
+        self.condvars.push(Default::default())
     }
+}
 
+// Public interface to synchronization primitives. Please note that in most
+// cases, the function calls are infallible and it is the client's (shim
+// implementation's) responsibility to detect and deal with erroneous
+// situations.
+impl<'tcx> EvalContextExt<'tcx> for crate::MiriInterpCx<'tcx> {}
+pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
     #[inline]
     /// Get the id of the thread that currently owns this lock.
     fn mutex_get_owner(&mut self, id: MutexId) -> ThreadId {

src/tools/miri/src/machine.rs

@@ -362,6 +362,12 @@ impl VisitProvenance for AllocExtra<'_> {
     }
 }
 
+impl<'tcx> AllocExtra<'tcx> {
+    pub fn get_sync<T: 'static>(&self, offset: Size) -> Option<&T> {
+        self.sync.get(&offset).and_then(|s| s.downcast_ref::<T>())
+    }
+}
+
 /// Precomputed layouts of primitive types
 pub struct PrimitiveLayouts<'tcx> {
     pub unit: TyAndLayout<'tcx>,

src/tools/miri/src/shims/unix/macos/sync.rs

@@ -12,15 +12,26 @@
 
 use crate::*;
 
+struct LockData {
+    id: MutexId,
+}
+
 impl<'tcx> EvalContextExtPriv<'tcx> for crate::MiriInterpCx<'tcx> {}
 trait EvalContextExtPriv<'tcx>: crate::MiriInterpCxExt<'tcx> {
     fn os_unfair_lock_getid(&mut self, lock_ptr: &OpTy<'tcx>) -> InterpResult<'tcx, MutexId> {
         let this = self.eval_context_mut();
         let lock = this.deref_pointer(lock_ptr)?;
-        // os_unfair_lock holds a 32-bit value, is initialized with zero and
-        // must be assumed to be opaque. Therefore, we can just store our
-        // internal mutex ID in the structure without anyone noticing.
-        this.mutex_get_or_create_id(&lock, /* offset */ 0)
+        // We store the mutex ID in the `sync` metadata. This means that when the lock is moved,
+        // that's just implicitly creating a new lock at the new location.
+        let (alloc, offset, _) = this.ptr_get_alloc_id(lock.ptr(), 0)?;
+        let (alloc_extra, machine) = this.get_alloc_extra_mut(alloc)?;
+        if let Some(data) = alloc_extra.get_sync::<LockData>(offset) {
+            interp_ok(data.id)
+        } else {
+            let id = machine.sync.mutex_create();
+            alloc_extra.sync.insert(offset, Box::new(LockData { id }));
+            interp_ok(id)
+        }
     }
 }
 

src/tools/miri/src/shims/unix/sync.rs

@@ -80,11 +80,9 @@ fn sync_get_data<'tcx, T: 'static + Copy>(
         // If it is initialized, it must be found in the "sync primitive" table,
         // or else it has been moved illegally.
         let (alloc, offset, _) = ecx.ptr_get_alloc_id(primitive.ptr(), 0)?;
-        let (alloc_extra, _machine) = ecx.get_alloc_extra_mut(alloc)?;
+        let alloc_extra = ecx.get_alloc_extra(alloc)?;
         let data = alloc_extra
-            .sync
-            .get(&offset)
-            .and_then(|s| s.downcast_ref::<T>())
+            .get_sync::<T>(offset)
             .ok_or_else(|| err_ub_format!("`{name}` can't be moved after first use"))?;
         interp_ok(Some(*data))
     } else {
@@ -174,7 +172,7 @@ enum MutexKind {
 
 #[derive(Debug, Clone, Copy)]
 /// Additional data that we attach with each mutex instance.
-pub struct MutexData {
+struct MutexData {
     id: MutexId,
     kind: MutexKind,
 }
@@ -228,7 +226,7 @@ fn mutex_create<'tcx>(
     kind: MutexKind,
 ) -> InterpResult<'tcx, MutexData> {
     let mutex = ecx.deref_pointer(mutex_ptr)?;
-    let id = ecx.mutex_create();
+    let id = ecx.machine.sync.mutex_create();
     let data = MutexData { id, kind };
     sync_init(ecx, &mutex, mutex_init_offset(ecx)?, data)?;
     interp_ok(data)
@@ -290,7 +288,7 @@ fn mutex_kind_from_static_initializer<'tcx>(
 
 #[derive(Debug, Copy, Clone)]
 /// Additional data that we attach with each rwlock instance.
-pub struct RwLockData {
+struct RwLockData {
     id: RwLockId,
 }
 
@@ -337,7 +335,7 @@ fn rwlock_get_data<'tcx>(
         )? {
             throw_unsup_format!("unsupported static initializer used for `pthread_rwlock_t`");
         }
-        let id = ecx.rwlock_create();
+        let id = ecx.machine.sync.rwlock_create();
         let data = RwLockData { id };
         sync_init(ecx, &rwlock, init_offset, data)?;
         interp_ok(data)
@@ -446,7 +444,7 @@ fn cond_create<'tcx>(
     clock: ClockId,
 ) -> InterpResult<'tcx, CondData> {
     let cond = ecx.deref_pointer(cond_ptr)?;
-    let id = ecx.condvar_create();
+    let id = ecx.machine.sync.condvar_create();
     let data = CondData { id, clock };
     sync_init(ecx, &cond, cond_init_offset(ecx)?, data)?;
     interp_ok(data)