Skip to content

Commit

Permalink
macOS os_unfair_lock: also store ID outside addressable memory
Browse files Browse the repository at this point in the history
  • Loading branch information
RalfJung committed Oct 14, 2024
1 parent 17f0aed commit 8f342ed
Show file tree
Hide file tree
Showing 4 changed files with 42 additions and 49 deletions.
50 changes: 14 additions & 36 deletions src/tools/miri/src/concurrency/sync.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
6 changes: 6 additions & 0 deletions src/tools/miri/src/machine.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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>,
Expand Down
19 changes: 15 additions & 4 deletions src/tools/miri/src/shims/unix/macos/sync.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
}
}

Expand Down
16 changes: 7 additions & 9 deletions src/tools/miri/src/shims/unix/sync.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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,
}
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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,
}

Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down

0 comments on commit 8f342ed

Please sign in to comment.