diff options
| -rw-r--r-- | src/tools/miri/src/concurrency/init_once.rs | 26 | ||||
| -rw-r--r-- | src/tools/miri/src/concurrency/sync.rs | 103 |
2 files changed, 27 insertions, 102 deletions
diff --git a/src/tools/miri/src/concurrency/init_once.rs b/src/tools/miri/src/concurrency/init_once.rs index 9145ef32c52..c23e5737280 100644 --- a/src/tools/miri/src/concurrency/init_once.rs +++ b/src/tools/miri/src/concurrency/init_once.rs @@ -26,27 +26,6 @@ pub(super) struct InitOnce { clock: VClock, } -impl<'tcx> EvalContextExtPriv<'tcx> for crate::MiriInterpCx<'tcx> {} -trait EvalContextExtPriv<'tcx>: crate::MiriInterpCxExt<'tcx> { - /// Provides the closure with the next InitOnceId. Creates that InitOnce if the closure returns None, - /// otherwise returns the value from the closure. - #[inline] - fn init_once_get_or_create<F>(&mut self, existing: F) -> InterpResult<'tcx, InitOnceId> - where - F: FnOnce(&mut MiriInterpCx<'tcx>, InitOnceId) -> InterpResult<'tcx, Option<InitOnceId>>, - { - let this = self.eval_context_mut(); - let next_index = this.machine.sync.init_onces.next_index(); - if let Some(old) = existing(this, next_index)? { - Ok(old) - } else { - let new_index = this.machine.sync.init_onces.push(Default::default()); - assert_eq!(next_index, new_index); - Ok(new_index) - } - } -} - impl<'tcx> EvalContextExt<'tcx> for crate::MiriInterpCx<'tcx> {} pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { fn init_once_get_or_create_id( @@ -56,9 +35,8 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { offset: u64, ) -> InterpResult<'tcx, InitOnceId> { let this = self.eval_context_mut(); - this.init_once_get_or_create(|ecx, next_id| { - ecx.get_or_create_id(next_id, lock_op, lock_layout, offset) - }) + this.get_or_create_id(lock_op, lock_layout, offset, |ecx| &mut ecx.machine.sync.init_onces)? + .ok_or_else(|| err_ub_format!("init_once has invalid ID").into()) } #[inline] diff --git a/src/tools/miri/src/concurrency/sync.rs b/src/tools/miri/src/concurrency/sync.rs index d0c9a4600e8..d972831c768 100644 --- a/src/tools/miri/src/concurrency/sync.rs +++ b/src/tools/miri/src/concurrency/sync.rs @@ -164,25 +164,29 @@ pub struct SynchronizationObjects { impl<'tcx> EvalContextExtPriv<'tcx> for crate::MiriInterpCx<'tcx> {} pub(super) trait EvalContextExtPriv<'tcx>: crate::MiriInterpCxExt<'tcx> { /// Lazily initialize the ID of this Miri sync structure. - /// ('0' indicates uninit.) + /// If memory stores '0', that indicates uninit and we generate a new instance. + /// Returns `None` if memory stores a non-zero invalid ID. + /// + /// `get_objs` must return the `IndexVec` that stores all the objects of this type. #[inline] - fn get_or_create_id<Id: SyncId>( + fn get_or_create_id<Id: SyncId + Idx, T: Default>( &mut self, - next_id: Id, lock_op: &OpTy<'tcx>, lock_layout: TyAndLayout<'tcx>, offset: u64, + get_objs: impl for<'a> Fn(&'a mut MiriInterpCx<'tcx>) -> &'a mut IndexVec<Id, T>, ) -> InterpResult<'tcx, Option<Id>> { let this = self.eval_context_mut(); let value_place = this.deref_pointer_and_offset(lock_op, offset, lock_layout, this.machine.layouts.u32)?; + let next_index = get_objs(this).next_index(); // Since we are lazy, this update has to be atomic. let (old, success) = this .atomic_compare_exchange_scalar( &value_place, &ImmTy::from_uint(0u32, this.machine.layouts.u32), - Scalar::from_u32(next_id.to_u32()), + Scalar::from_u32(next_index.to_u32()), AtomicRwOrd::Relaxed, // deliberately *no* synchronization AtomicReadOrd::Relaxed, false, @@ -190,74 +194,20 @@ pub(super) trait EvalContextExtPriv<'tcx>: crate::MiriInterpCxExt<'tcx> { .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"))) - }) - } - - /// Provides the closure with the next MutexId. Creates that mutex if the closure returns None, - /// otherwise returns the value from the closure. - #[inline] - fn mutex_get_or_create<F>(&mut self, existing: F) -> InterpResult<'tcx, MutexId> - where - F: FnOnce(&mut MiriInterpCx<'tcx>, MutexId) -> InterpResult<'tcx, Option<MutexId>>, - { - let this = self.eval_context_mut(); - let next_index = this.machine.sync.mutexes.next_index(); - if let Some(old) = existing(this, next_index)? { - if this.machine.sync.mutexes.get(old).is_none() { - throw_ub_format!("mutex has invalid ID"); - } - Ok(old) - } else { - let new_index = this.machine.sync.mutexes.push(Default::default()); + // We set the in-memory ID to `next_index`, now also create this object in the machine + // state. + let new_index = get_objs(this).push(T::default()); assert_eq!(next_index, new_index); - Ok(new_index) - } - } - - /// Provides the closure with the next RwLockId. Creates that RwLock if the closure returns None, - /// otherwise returns the value from the closure. - #[inline] - fn rwlock_get_or_create<F>(&mut self, existing: F) -> InterpResult<'tcx, RwLockId> - where - F: FnOnce(&mut MiriInterpCx<'tcx>, RwLockId) -> InterpResult<'tcx, Option<RwLockId>>, - { - let this = self.eval_context_mut(); - let next_index = this.machine.sync.rwlocks.next_index(); - if let Some(old) = existing(this, next_index)? { - if this.machine.sync.rwlocks.get(old).is_none() { - throw_ub_format!("rwlock has invalid ID"); - } - Ok(old) + Some(new_index) } else { - let new_index = this.machine.sync.rwlocks.push(Default::default()); - assert_eq!(next_index, new_index); - Ok(new_index) - } - } - - /// Provides the closure with the next CondvarId. Creates that Condvar if the closure returns None, - /// otherwise returns the value from the closure. - #[inline] - fn condvar_get_or_create<F>(&mut self, existing: F) -> InterpResult<'tcx, CondvarId> - where - F: FnOnce(&mut MiriInterpCx<'tcx>, CondvarId) -> InterpResult<'tcx, Option<CondvarId>>, - { - let this = self.eval_context_mut(); - let next_index = this.machine.sync.condvars.next_index(); - if let Some(old) = existing(this, next_index)? { - if this.machine.sync.condvars.get(old).is_none() { - throw_ub_format!("condvar has invalid ID"); + let id = Id::from_u32(old.to_u32().expect("layout is u32")); + if get_objs(this).get(id).is_none() { + // The in-memory ID is invalid. + None + } else { + Some(id) } - Ok(old) - } else { - let new_index = this.machine.sync.condvars.push(Default::default()); - assert_eq!(next_index, new_index); - Ok(new_index) - } + }) } fn condvar_reacquire_mutex( @@ -293,9 +243,8 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { offset: u64, ) -> InterpResult<'tcx, MutexId> { let this = self.eval_context_mut(); - this.mutex_get_or_create(|ecx, next_id| { - ecx.get_or_create_id(next_id, lock_op, lock_layout, offset) - }) + this.get_or_create_id(lock_op, lock_layout, offset, |ecx| &mut ecx.machine.sync.mutexes)? + .ok_or_else(|| err_ub_format!("mutex has invalid ID").into()) } fn rwlock_get_or_create_id( @@ -305,9 +254,8 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { offset: u64, ) -> InterpResult<'tcx, RwLockId> { let this = self.eval_context_mut(); - this.rwlock_get_or_create(|ecx, next_id| { - ecx.get_or_create_id(next_id, lock_op, lock_layout, offset) - }) + this.get_or_create_id(lock_op, lock_layout, offset, |ecx| &mut ecx.machine.sync.rwlocks)? + .ok_or_else(|| err_ub_format!("rwlock has invalid ID").into()) } fn condvar_get_or_create_id( @@ -317,9 +265,8 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { offset: u64, ) -> InterpResult<'tcx, CondvarId> { let this = self.eval_context_mut(); - this.condvar_get_or_create(|ecx, next_id| { - ecx.get_or_create_id(next_id, lock_op, lock_layout, offset) - }) + this.get_or_create_id(lock_op, lock_layout, offset, |ecx| &mut ecx.machine.sync.condvars)? + .ok_or_else(|| err_ub_format!("condvar has invalid ID").into()) } #[inline] |