Skip to content

Commit

Permalink
simplify synchronization object creation logic
Browse files Browse the repository at this point in the history
  • Loading branch information
@RalfJung
RalfJung committed Aug 17, 2024
1 parent 0058752 commit 7c81120
Show file tree
Hide file tree
Showing 2 changed files with 27 additions and 102 deletions.
26 changes: 2 additions & 24 deletions src/tools/miri/src/concurrency/init_once.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand All @@ -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]
Expand Down
103 changes: 25 additions & 78 deletions src/tools/miri/src/concurrency/sync.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -164,100 +164,50 @@ 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,
)?
.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(
Expand Down Expand Up @@ -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(
Expand All @@ -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(
Expand All @@ -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]
Expand Down

0 comments on commit 7c81120

Please sign in to comment.