Skip to content

Commit

Permalink
Detect pthread_mutex_t is moved
Browse files Browse the repository at this point in the history
  • Loading branch information
Mandragorian committed Sep 5, 2024
1 parent ad7a1aa commit 5f22103
Show file tree
Hide file tree
Showing 8 changed files with 310 additions and 116 deletions.
10 changes: 8 additions & 2 deletions src/tools/miri/src/concurrency/init_once.rs
Original file line number Diff line number Diff line change
Expand Up @@ -35,8 +35,14 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
offset: u64,
) -> InterpResult<'tcx, InitOnceId> {
let this = self.eval_context_mut();
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())
this.get_or_create_id(
lock_op,
lock_layout,
offset,
|ecx| &mut ecx.machine.sync.init_onces,
|_| Ok(Default::default()),
)?
.ok_or_else(|| err_ub_format!("init_once has invalid ID").into())
}

#[inline]
Expand Down
119 changes: 111 additions & 8 deletions src/tools/miri/src/concurrency/sync.rs
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,27 @@ pub(super) use declare_id;

declare_id!(MutexId);

/// The mutex kind.
#[derive(Debug, Clone, Copy)]
#[non_exhaustive]
pub enum MutexKind {
Invalid,
Normal,
Default,
Recursive,
ErrorCheck,
}

#[derive(Debug)]
/// Additional data that may be used by shim implementations.
pub struct AdditionalMutexData {
/// The mutex kind, used by some mutex implementations like pthreads mutexes.
pub kind: MutexKind,

/// The address of the mutex.
pub address: u64,
}

/// The mutex state.
#[derive(Default, Debug)]
struct Mutex {
Expand All @@ -77,6 +98,9 @@ struct Mutex {
queue: VecDeque<ThreadId>,
/// Mutex clock. This tracks the moment of the last unlock.
clock: VClock,

/// Additional data that can be set by shim implementations.
data: Option<AdditionalMutexData>,
}

declare_id!(RwLockId);
Expand Down Expand Up @@ -168,13 +192,15 @@ pub(super) trait EvalContextExtPriv<'tcx>: crate::MiriInterpCxExt<'tcx> {
/// Returns `None` if memory stores a non-zero invalid ID.
///
/// `get_objs` must return the `IndexVec` that stores all the objects of this type.
/// `create_obj` must create the new object if initialization is needed.
#[inline]
fn get_or_create_id<Id: SyncId + Idx, T: Default>(
fn get_or_create_id<Id: SyncId + Idx, T>(
&mut self,
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>,
create_obj: impl for<'a> FnOnce(&'a mut MiriInterpCx<'tcx>) -> InterpResult<'tcx, T>,
) -> InterpResult<'tcx, Option<Id>> {
let this = self.eval_context_mut();
let value_place =
Expand All @@ -196,7 +222,8 @@ pub(super) trait EvalContextExtPriv<'tcx>: crate::MiriInterpCxExt<'tcx> {
Ok(if success.to_bool().expect("compare_exchange's second return value is a bool") {
// 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());
let obj = create_obj(this)?;
let new_index = get_objs(this).push(obj);
assert_eq!(next_index, new_index);
Some(new_index)
} else {
Expand All @@ -210,6 +237,32 @@ pub(super) trait EvalContextExtPriv<'tcx>: crate::MiriInterpCxExt<'tcx> {
})
}

/// Eagerly creates a Miri sync structure.
///
/// `create_id` will store the index of the sync_structure in the memory pointed to by
/// `lock_op`, so that future calls to `get_or_create_id` will see it as initialized.
/// - `lock_op` must hold a pointer to the sync structure.
/// - `lock_layout` must be the memory layout of the sync structure.
/// - `offset` must be the offset inside the sync structure where its miri id will be stored.
/// - `get_objs` is described in `get_or_create_id`.
/// - `obj` must be the new sync object.
fn create_id<Id: SyncId + Idx, T>(
&mut self,
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>,
obj: T,
) -> InterpResult<'tcx, 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 new_index = get_objs(this).push(obj);
this.write_scalar(Scalar::from_u32(new_index.to_u32()), &value_place)?;
Ok(new_index)
}

fn condvar_reacquire_mutex(
&mut self,
mutex: MutexId,
Expand All @@ -236,15 +289,53 @@ pub(super) trait EvalContextExtPriv<'tcx>: crate::MiriInterpCxExt<'tcx> {
// 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,
lock_op: &OpTy<'tcx>,
lock_layout: TyAndLayout<'tcx>,
offset: u64,
data: Option<AdditionalMutexData>,
) -> InterpResult<'tcx, MutexId> {
let this = self.eval_context_mut();
this.create_id(
lock_op,
lock_layout,
offset,
|ecx| &mut ecx.machine.sync.mutexes,
Mutex { data, ..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_op: &OpTy<'tcx>,
lock_layout: TyAndLayout<'tcx>,
offset: u64,
initialize_data: impl for<'a> FnOnce(
&'a mut MiriInterpCx<'tcx>,
) -> InterpResult<'tcx, Option<AdditionalMutexData>>,
) -> InterpResult<'tcx, MutexId> {
let this = self.eval_context_mut();
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())
this.get_or_create_id(
lock_op,
lock_layout,
offset,
|ecx| &mut ecx.machine.sync.mutexes,
|ecx| initialize_data(ecx).map(|data| Mutex { data, ..Default::default() }),
)?
.ok_or_else(|| err_ub_format!("mutex has invalid ID").into())
}

/// Retrieve the additional data stored for a mutex.
fn mutex_get_data<'a>(&'a mut self, id: MutexId) -> Option<&'a AdditionalMutexData>
where
'tcx: 'a,
{
let this = self.eval_context_ref();
this.machine.sync.mutexes[id].data.as_ref()
}

fn rwlock_get_or_create_id(
Expand All @@ -254,8 +345,14 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
offset: u64,
) -> InterpResult<'tcx, RwLockId> {
let this = self.eval_context_mut();
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())
this.get_or_create_id(
lock_op,
lock_layout,
offset,
|ecx| &mut ecx.machine.sync.rwlocks,
|_| Ok(Default::default()),
)?
.ok_or_else(|| err_ub_format!("rwlock has invalid ID").into())
}

fn condvar_get_or_create_id(
Expand All @@ -265,8 +362,14 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
offset: u64,
) -> InterpResult<'tcx, CondvarId> {
let this = self.eval_context_mut();
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())
this.get_or_create_id(
lock_op,
lock_layout,
offset,
|ecx| &mut ecx.machine.sync.condvars,
|_| Ok(Default::default()),
)?
.ok_or_else(|| err_ub_format!("condvar has invalid ID").into())
}

#[inline]
Expand Down
5 changes: 4 additions & 1 deletion src/tools/miri/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,10 @@ pub use crate::concurrency::{
cpu_affinity::MAX_CPUS,
data_race::{AtomicFenceOrd, AtomicReadOrd, AtomicRwOrd, AtomicWriteOrd, EvalContextExt as _},
init_once::{EvalContextExt as _, InitOnceId},
sync::{CondvarId, EvalContextExt as _, MutexId, RwLockId, SynchronizationObjects},
sync::{
AdditionalMutexData, CondvarId, EvalContextExt as _, MutexId, MutexKind, RwLockId,
SynchronizationObjects,
},
thread::{
BlockReason, EvalContextExt as _, StackEmptyCallback, ThreadId, ThreadManager,
TimeoutAnchor, TimeoutClock, UnblockCallback,
Expand Down
2 changes: 1 addition & 1 deletion src/tools/miri/src/shims/unix/macos/sync.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ trait EvalContextExtPriv<'tcx>: crate::MiriInterpCxExt<'tcx> {
// 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_op, this.libc_ty_layout("os_unfair_lock"), 0)
this.mutex_get_or_create_id(lock_op, this.libc_ty_layout("os_unfair_lock"), 0, |_| Ok(None))
}
}

Expand Down
Loading

0 comments on commit 5f22103

Please sign in to comment.