code reuse for sync ids

beepster4096 · RalfJung · commit 7cf32a7d4777 · 2022-10-20T22:19:06.000+02:00
diff --git a/src/tools/miri/src/concurrency/sync.rs b/src/tools/miri/src/concurrency/sync.rs
@@ -11,6 +11,11 @@ use super::thread::MachineCallback;
 use super::vector_clock::VClock;
 use crate::*;
 
+pub trait SyncId {
+    fn from_u32(id: u32) -> Self;
+    fn to_u32_scalar(&self) -> Scalar<Provenance>;
+}
+
 /// We cannot use the `newtype_index!` macro because we have to use 0 as a
 /// sentinel value meaning that the identifier is not assigned. This is because
 /// the pthreads static initializers initialize memory with zeros (see the
@@ -22,11 +27,14 @@ macro_rules! declare_id {
         #[derive(Clone, Copy, Debug, PartialOrd, Ord, PartialEq, Eq, Hash)]
         pub struct $name(NonZeroU32);
 
-        impl $name {
+        impl SyncId for $name {
             // Panics if `id == 0`.
-            pub fn from_u32(id: u32) -> Self {
+            fn from_u32(id: u32) -> Self {
                 Self(NonZeroU32::new(id).unwrap())
             }
+            fn to_u32_scalar(&self) -> Scalar<Provenance> {
+                Scalar::from_u32(self.0.get())
+            }
         }
 
         impl Idx for $name {
@@ -166,7 +174,7 @@ impl<'mir, 'tcx> std::fmt::Debug for InitOnceWaiter<'mir, 'tcx> {
     }
 }
 
-#[derive(Default, Debug, Copy, Clone, PartialEq, Eq,)]
+#[derive(Default, Debug, Copy, Clone, PartialEq, Eq)]
 /// The current status of a one time initialization.
 pub enum InitOnceStatus {
     #[default]
@@ -212,6 +220,37 @@ impl<'mir, 'tcx> VisitTags for SynchronizationState<'mir, 'tcx> {
 // Private extension trait for local helper methods
 impl<'mir, 'tcx: 'mir> EvalContextExtPriv<'mir, 'tcx> for crate::MiriInterpCx<'mir, 'tcx> {}
 trait EvalContextExtPriv<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
+    #[inline]
+    // Miri sync structures contain zero-initialized ids stored at some offset behind a pointer
+    fn get_or_create_id<Id: SyncId>(
+        &mut self,
+        next_id: Id,
+        lock_op: &OpTy<'tcx, Provenance>,
+        offset: u64,
+    ) -> InterpResult<'tcx, Option<Id>> {
+        let this = self.eval_context_mut();
+        let value_place =
+            this.deref_operand_and_offset(lock_op, offset, this.machine.layouts.u32)?;
+
+        let (old, success) = this
+            .atomic_compare_exchange_scalar(
+                &value_place,
+                &ImmTy::from_uint(0u32, this.machine.layouts.u32),
+                next_id.to_u32_scalar(),
+                AtomicRwOrd::Relaxed,
+                AtomicReadOrd::Relaxed,
+                false,
+            )?
+            .to_scalar_pair();
+
+        Ok(if success.to_bool().expect("compare_exchange's second return value is a bool") {
+            // Caller of the closure needs to allocate next_id
+            None
+        } else {
+            Some(Id::from_u32(old.to_u32().expect("layout is u32")))
+        })
+    }
+
     /// Take a reader out of the queue waiting for the lock.
     /// Returns `true` if some thread got the rwlock.
     #[inline]
@@ -261,6 +300,48 @@ trait EvalContextExtPriv<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
 // situations.
 impl<'mir, 'tcx: 'mir> EvalContextExt<'mir, 'tcx> for crate::MiriInterpCx<'mir, 'tcx> {}
 pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
+    fn mutex_get_or_create_id(
+        &mut self,
+        lock_op: &OpTy<'tcx, Provenance>,
+        offset: u64,
+    ) -> InterpResult<'tcx, MutexId> {
+        let this = self.eval_context_mut();
+        this.mutex_get_or_create(|ecx, next_id| Ok(ecx.get_or_create_id(next_id, lock_op, offset)?))
+    }
+
+    fn rwlock_get_or_create_id(
+        &mut self,
+        lock_op: &OpTy<'tcx, Provenance>,
+        offset: u64,
+    ) -> InterpResult<'tcx, RwLockId> {
+        let this = self.eval_context_mut();
+        this.rwlock_get_or_create(
+            |ecx, next_id| Ok(ecx.get_or_create_id(next_id, lock_op, offset)?),
+        )
+    }
+
+    fn condvar_get_or_create_id(
+        &mut self,
+        lock_op: &OpTy<'tcx, Provenance>,
+        offset: u64,
+    ) -> InterpResult<'tcx, CondvarId> {
+        let this = self.eval_context_mut();
+        this.condvar_get_or_create(|ecx, next_id| {
+            Ok(ecx.get_or_create_id(next_id, lock_op, offset)?)
+        })
+    }
+
+    fn init_once_get_or_create_id(
+        &mut self,
+        lock_op: &OpTy<'tcx, Provenance>,
+        offset: u64,
+    ) -> InterpResult<'tcx, InitOnceId> {
+        let this = self.eval_context_mut();
+        this.init_once_get_or_create(|ecx, next_id| {
+            Ok(ecx.get_or_create_id(next_id, lock_op, offset)?)
+        })
+    }
+
     #[inline]
     /// Create state for a new mutex.
     fn mutex_create(&mut self) -> MutexId {
diff --git a/src/tools/miri/src/lib.rs b/src/tools/miri/src/lib.rs
@@ -87,7 +87,7 @@ pub use crate::concurrency::{
         AtomicFenceOrd, AtomicReadOrd, AtomicRwOrd, AtomicWriteOrd,
         EvalContextExt as DataRaceEvalContextExt,
     },
-    sync::{CondvarId, EvalContextExt as SyncEvalContextExt, InitOnceId, MutexId, RwLockId},
+    sync::{CondvarId, EvalContextExt as SyncEvalContextExt, InitOnceId, MutexId, RwLockId, SyncId},
     thread::{
         EvalContextExt as ThreadsEvalContextExt, SchedulingAction, ThreadId, ThreadManager,
         ThreadState, Time,
diff --git a/src/tools/miri/src/shims/unix/sync.rs b/src/tools/miri/src/shims/unix/sync.rs
@@ -108,33 +108,6 @@ fn mutex_set_id<'mir, 'tcx: 'mir>(
     )
 }
 
-fn mutex_get_or_create_id<'mir, 'tcx: 'mir>(
-    ecx: &mut MiriInterpCx<'mir, 'tcx>,
-    mutex_op: &OpTy<'tcx, Provenance>,
-) -> InterpResult<'tcx, MutexId> {
-    let value_place = ecx.deref_operand_and_offset(mutex_op, 4, ecx.machine.layouts.u32)?;
-
-    ecx.mutex_get_or_create(|ecx, next_id| {
-        let (old, success) = ecx
-            .atomic_compare_exchange_scalar(
-                &value_place,
-                &ImmTy::from_uint(0u32, ecx.machine.layouts.u32),
-                next_id.to_u32_scalar(),
-                AtomicRwOrd::Relaxed,
-                AtomicReadOrd::Relaxed,
-                false,
-            )?
-            .to_scalar_pair();
-
-        Ok(if success.to_bool().expect("compare_exchange's second return value is a bool") {
-            // Caller of the closure needs to allocate next_id
-            None
-        } else {
-            Some(MutexId::from_u32(old.to_u32().expect("layout is u32")))
-        })
-    })
-}
-
 // pthread_rwlock_t is between 32 and 56 bytes, depending on the platform.
 
 // Our chosen memory layout for the emulated rwlock (does not have to match the platform layout!):
@@ -149,33 +122,6 @@ fn rwlock_get_id<'mir, 'tcx: 'mir>(
     ecx.read_scalar_at_offset_atomic(rwlock_op, 4, ecx.machine.layouts.u32, AtomicReadOrd::Relaxed)
 }
 
-fn rwlock_get_or_create_id<'mir, 'tcx: 'mir>(
-    ecx: &mut MiriInterpCx<'mir, 'tcx>,
-    rwlock_op: &OpTy<'tcx, Provenance>,
-) -> InterpResult<'tcx, RwLockId> {
-    let value_place = ecx.deref_operand_and_offset(rwlock_op, 4, ecx.machine.layouts.u32)?;
-
-    ecx.rwlock_get_or_create(|ecx, next_id| {
-        let (old, success) = ecx
-            .atomic_compare_exchange_scalar(
-                &value_place,
-                &ImmTy::from_uint(0u32, ecx.machine.layouts.u32),
-                next_id.to_u32_scalar(),
-                AtomicRwOrd::Relaxed,
-                AtomicReadOrd::Relaxed,
-                false,
-            )?
-            .to_scalar_pair();
-
-        Ok(if success.to_bool().expect("compare_exchange's second return value is a bool") {
-            // Caller of the closure needs to allocate next_id
-            None
-        } else {
-            Some(RwLockId::from_u32(old.to_u32().expect("layout is u32")))
-        })
-    })
-}
-
 // pthread_condattr_t
 
 // Our chosen memory layout for emulation (does not have to match the platform layout!):
@@ -232,33 +178,6 @@ fn cond_set_id<'mir, 'tcx: 'mir>(
     )
 }
 
-fn cond_get_or_create_id<'mir, 'tcx: 'mir>(
-    ecx: &mut MiriInterpCx<'mir, 'tcx>,
-    cond_op: &OpTy<'tcx, Provenance>,
-) -> InterpResult<'tcx, CondvarId> {
-    let value_place = ecx.deref_operand_and_offset(cond_op, 4, ecx.machine.layouts.u32)?;
-
-    ecx.condvar_get_or_create(|ecx, next_id| {
-        let (old, success) = ecx
-            .atomic_compare_exchange_scalar(
-                &value_place,
-                &ImmTy::from_uint(0u32, ecx.machine.layouts.u32),
-                next_id.to_u32_scalar(),
-                AtomicRwOrd::Relaxed,
-                AtomicReadOrd::Relaxed,
-                false,
-            )?
-            .to_scalar_pair();
-
-        Ok(if success.to_bool().expect("compare_exchange's second return value is a bool") {
-            // Caller of the closure needs to allocate next_id
-            None
-        } else {
-            Some(CondvarId::from_u32(old.to_u32().expect("layout is u32")))
-        })
-    })
-}
-
 fn cond_get_clock_id<'mir, 'tcx: 'mir>(
     ecx: &MiriInterpCx<'mir, 'tcx>,
     cond_op: &OpTy<'tcx, Provenance>,
@@ -435,7 +354,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
         let this = self.eval_context_mut();
 
         let kind = mutex_get_kind(this, mutex_op)?;
-        let id = mutex_get_or_create_id(this, mutex_op)?;
+        let id = this.mutex_get_or_create_id(mutex_op, 4)?;
         let active_thread = this.get_active_thread();
 
         if this.mutex_is_locked(id) {
@@ -475,7 +394,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
         let this = self.eval_context_mut();
 
         let kind = mutex_get_kind(this, mutex_op)?;
-        let id = mutex_get_or_create_id(this, mutex_op)?;
+        let id = this.mutex_get_or_create_id(mutex_op, 4)?;
         let active_thread = this.get_active_thread();
 
         if this.mutex_is_locked(id) {
@@ -511,7 +430,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
         let this = self.eval_context_mut();
 
         let kind = mutex_get_kind(this, mutex_op)?;
-        let id = mutex_get_or_create_id(this, mutex_op)?;
+        let id = this.mutex_get_or_create_id(mutex_op, 4)?;
         let active_thread = this.get_active_thread();
 
         if let Some(_old_locked_count) = this.mutex_unlock(id, active_thread) {
@@ -545,7 +464,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
     ) -> InterpResult<'tcx, i32> {
         let this = self.eval_context_mut();
 
-        let id = mutex_get_or_create_id(this, mutex_op)?;
+        let id = this.mutex_get_or_create_id(mutex_op, 4)?;
 
         if this.mutex_is_locked(id) {
             throw_ub_format!("destroyed a locked mutex");
@@ -568,7 +487,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
     ) -> InterpResult<'tcx, i32> {
         let this = self.eval_context_mut();
 
-        let id = rwlock_get_or_create_id(this, rwlock_op)?;
+        let id = this.rwlock_get_or_create_id(rwlock_op, 4)?;
         let active_thread = this.get_active_thread();
 
         if this.rwlock_is_write_locked(id) {
@@ -586,7 +505,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
     ) -> InterpResult<'tcx, i32> {
         let this = self.eval_context_mut();
 
-        let id = rwlock_get_or_create_id(this, rwlock_op)?;
+        let id = this.rwlock_get_or_create_id(rwlock_op, 4)?;
         let active_thread = this.get_active_thread();
 
         if this.rwlock_is_write_locked(id) {
@@ -603,7 +522,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
     ) -> InterpResult<'tcx, i32> {
         let this = self.eval_context_mut();
 
-        let id = rwlock_get_or_create_id(this, rwlock_op)?;
+        let id = this.rwlock_get_or_create_id(rwlock_op, 4)?;
         let active_thread = this.get_active_thread();
 
         if this.rwlock_is_locked(id) {
@@ -633,7 +552,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
     ) -> InterpResult<'tcx, i32> {
         let this = self.eval_context_mut();
 
-        let id = rwlock_get_or_create_id(this, rwlock_op)?;
+        let id = this.rwlock_get_or_create_id(rwlock_op, 4)?;
         let active_thread = this.get_active_thread();
 
         if this.rwlock_is_locked(id) {
@@ -650,7 +569,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
     ) -> InterpResult<'tcx, i32> {
         let this = self.eval_context_mut();
 
-        let id = rwlock_get_or_create_id(this, rwlock_op)?;
+        let id = this.rwlock_get_or_create_id(rwlock_op, 4)?;
         let active_thread = this.get_active_thread();
 
         #[allow(clippy::if_same_then_else)]
@@ -669,7 +588,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
     ) -> InterpResult<'tcx, i32> {
         let this = self.eval_context_mut();
 
-        let id = rwlock_get_or_create_id(this, rwlock_op)?;
+        let id = this.rwlock_get_or_create_id(rwlock_op, 4)?;
 
         if this.rwlock_is_locked(id) {
             throw_ub_format!("destroyed a locked rwlock");
@@ -772,7 +691,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
 
     fn pthread_cond_signal(&mut self, cond_op: &OpTy<'tcx, Provenance>) -> InterpResult<'tcx, i32> {
         let this = self.eval_context_mut();
-        let id = cond_get_or_create_id(this, cond_op)?;
+        let id = this.condvar_get_or_create_id(cond_op, 4)?;
         if let Some((thread, mutex)) = this.condvar_signal(id) {
             post_cond_signal(this, thread, mutex)?;
         }
@@ -785,7 +704,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
         cond_op: &OpTy<'tcx, Provenance>,
     ) -> InterpResult<'tcx, i32> {
         let this = self.eval_context_mut();
-        let id = cond_get_or_create_id(this, cond_op)?;
+        let id = this.condvar_get_or_create_id(cond_op, 4)?;
 
         while let Some((thread, mutex)) = this.condvar_signal(id) {
             post_cond_signal(this, thread, mutex)?;
@@ -801,8 +720,8 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
     ) -> InterpResult<'tcx, i32> {
         let this = self.eval_context_mut();
 
-        let id = cond_get_or_create_id(this, cond_op)?;
-        let mutex_id = mutex_get_or_create_id(this, mutex_op)?;
+        let id = this.condvar_get_or_create_id(cond_op, 4)?;
+        let mutex_id = this.mutex_get_or_create_id(mutex_op, 4)?;
         let active_thread = this.get_active_thread();
 
         release_cond_mutex_and_block(this, active_thread, mutex_id)?;
@@ -822,8 +741,8 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
 
         this.check_no_isolation("`pthread_cond_timedwait`")?;
 
-        let id = cond_get_or_create_id(this, cond_op)?;
-        let mutex_id = mutex_get_or_create_id(this, mutex_op)?;
+        let id = this.condvar_get_or_create_id(cond_op, 4)?;
+        let mutex_id = this.mutex_get_or_create_id(mutex_op, 4)?;
         let active_thread = this.get_active_thread();
 
         // Extract the timeout.
@@ -899,7 +818,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
     ) -> InterpResult<'tcx, i32> {
         let this = self.eval_context_mut();
 
-        let id = cond_get_or_create_id(this, cond_op)?;
+        let id = this.condvar_get_or_create_id(cond_op, 4)?;
         if this.condvar_is_awaited(id) {
             throw_ub_format!("destroying an awaited conditional variable");
         }
diff --git a/src/tools/miri/src/shims/windows/sync.rs b/src/tools/miri/src/shims/windows/sync.rs
