Skip to content

Commit 0385748

Browse files
Mandragorianshrirambalaji
authored andcommitted
Detect pthread_mutex_t is moved
See: rust-lang#3749
1 parent bfe5595 commit 0385748

File tree

8 files changed

+310
-116
lines changed

8 files changed

+310
-116
lines changed

src/tools/miri/src/concurrency/init_once.rs

+8-2
Original file line numberDiff line numberDiff line change
@@ -35,8 +35,14 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
3535
offset: u64,
3636
) -> InterpResult<'tcx, InitOnceId> {
3737
let this = self.eval_context_mut();
38-
this.get_or_create_id(lock_op, lock_layout, offset, |ecx| &mut ecx.machine.sync.init_onces)?
39-
.ok_or_else(|| err_ub_format!("init_once has invalid ID").into())
38+
this.get_or_create_id(
39+
lock_op,
40+
lock_layout,
41+
offset,
42+
|ecx| &mut ecx.machine.sync.init_onces,
43+
|_| Ok(Default::default()),
44+
)?
45+
.ok_or_else(|| err_ub_format!("init_once has invalid ID").into())
4046
}
4147

4248
#[inline]

src/tools/miri/src/concurrency/sync.rs

+111-8
Original file line numberDiff line numberDiff line change
@@ -66,6 +66,27 @@ pub(super) use declare_id;
6666

6767
declare_id!(MutexId);
6868

69+
/// The mutex kind.
70+
#[derive(Debug, Clone, Copy)]
71+
#[non_exhaustive]
72+
pub enum MutexKind {
73+
Invalid,
74+
Normal,
75+
Default,
76+
Recursive,
77+
ErrorCheck,
78+
}
79+
80+
#[derive(Debug)]
81+
/// Additional data that may be used by shim implementations.
82+
pub struct AdditionalMutexData {
83+
/// The mutex kind, used by some mutex implementations like pthreads mutexes.
84+
pub kind: MutexKind,
85+
86+
/// The address of the mutex.
87+
pub address: u64,
88+
}
89+
6990
/// The mutex state.
7091
#[derive(Default, Debug)]
7192
struct Mutex {
@@ -77,6 +98,9 @@ struct Mutex {
7798
queue: VecDeque<ThreadId>,
7899
/// Mutex clock. This tracks the moment of the last unlock.
79100
clock: VClock,
101+
102+
/// Additional data that can be set by shim implementations.
103+
data: Option<AdditionalMutexData>,
80104
}
81105

82106
declare_id!(RwLockId);
@@ -168,13 +192,15 @@ pub(super) trait EvalContextExtPriv<'tcx>: crate::MiriInterpCxExt<'tcx> {
168192
/// Returns `None` if memory stores a non-zero invalid ID.
169193
///
170194
/// `get_objs` must return the `IndexVec` that stores all the objects of this type.
195+
/// `create_obj` must create the new object if initialization is needed.
171196
#[inline]
172-
fn get_or_create_id<Id: SyncId + Idx, T: Default>(
197+
fn get_or_create_id<Id: SyncId + Idx, T>(
173198
&mut self,
174199
lock_op: &OpTy<'tcx>,
175200
lock_layout: TyAndLayout<'tcx>,
176201
offset: u64,
177202
get_objs: impl for<'a> Fn(&'a mut MiriInterpCx<'tcx>) -> &'a mut IndexVec<Id, T>,
203+
create_obj: impl for<'a> FnOnce(&'a mut MiriInterpCx<'tcx>) -> InterpResult<'tcx, T>,
178204
) -> InterpResult<'tcx, Option<Id>> {
179205
let this = self.eval_context_mut();
180206
let value_place =
@@ -196,7 +222,8 @@ pub(super) trait EvalContextExtPriv<'tcx>: crate::MiriInterpCxExt<'tcx> {
196222
Ok(if success.to_bool().expect("compare_exchange's second return value is a bool") {
197223
// We set the in-memory ID to `next_index`, now also create this object in the machine
198224
// state.
199-
let new_index = get_objs(this).push(T::default());
225+
let obj = create_obj(this)?;
226+
let new_index = get_objs(this).push(obj);
200227
assert_eq!(next_index, new_index);
201228
Some(new_index)
202229
} else {
@@ -210,6 +237,32 @@ pub(super) trait EvalContextExtPriv<'tcx>: crate::MiriInterpCxExt<'tcx> {
210237
})
211238
}
212239

240+
/// Eagerly creates a Miri sync structure.
241+
///
242+
/// `create_id` will store the index of the sync_structure in the memory pointed to by
243+
/// `lock_op`, so that future calls to `get_or_create_id` will see it as initialized.
244+
/// - `lock_op` must hold a pointer to the sync structure.
245+
/// - `lock_layout` must be the memory layout of the sync structure.
246+
/// - `offset` must be the offset inside the sync structure where its miri id will be stored.
247+
/// - `get_objs` is described in `get_or_create_id`.
248+
/// - `obj` must be the new sync object.
249+
fn create_id<Id: SyncId + Idx, T>(
250+
&mut self,
251+
lock_op: &OpTy<'tcx>,
252+
lock_layout: TyAndLayout<'tcx>,
253+
offset: u64,
254+
get_objs: impl for<'a> Fn(&'a mut MiriInterpCx<'tcx>) -> &'a mut IndexVec<Id, T>,
255+
obj: T,
256+
) -> InterpResult<'tcx, Id> {
257+
let this = self.eval_context_mut();
258+
let value_place =
259+
this.deref_pointer_and_offset(lock_op, offset, lock_layout, this.machine.layouts.u32)?;
260+
261+
let new_index = get_objs(this).push(obj);
262+
this.write_scalar(Scalar::from_u32(new_index.to_u32()), &value_place)?;
263+
Ok(new_index)
264+
}
265+
213266
fn condvar_reacquire_mutex(
214267
&mut self,
215268
mutex: MutexId,
@@ -236,15 +289,53 @@ pub(super) trait EvalContextExtPriv<'tcx>: crate::MiriInterpCxExt<'tcx> {
236289
// situations.
237290
impl<'tcx> EvalContextExt<'tcx> for crate::MiriInterpCx<'tcx> {}
238291
pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
292+
/// Eagerly create and initialize a new mutex.
293+
fn mutex_create(
294+
&mut self,
295+
lock_op: &OpTy<'tcx>,
296+
lock_layout: TyAndLayout<'tcx>,
297+
offset: u64,
298+
data: Option<AdditionalMutexData>,
299+
) -> InterpResult<'tcx, MutexId> {
300+
let this = self.eval_context_mut();
301+
this.create_id(
302+
lock_op,
303+
lock_layout,
304+
offset,
305+
|ecx| &mut ecx.machine.sync.mutexes,
306+
Mutex { data, ..Default::default() },
307+
)
308+
}
309+
310+
/// Lazily create a new mutex.
311+
/// `initialize_data` must return any additional data that a user wants to associate with the mutex.
239312
fn mutex_get_or_create_id(
240313
&mut self,
241314
lock_op: &OpTy<'tcx>,
242315
lock_layout: TyAndLayout<'tcx>,
243316
offset: u64,
317+
initialize_data: impl for<'a> FnOnce(
318+
&'a mut MiriInterpCx<'tcx>,
319+
) -> InterpResult<'tcx, Option<AdditionalMutexData>>,
244320
) -> InterpResult<'tcx, MutexId> {
245321
let this = self.eval_context_mut();
246-
this.get_or_create_id(lock_op, lock_layout, offset, |ecx| &mut ecx.machine.sync.mutexes)?
247-
.ok_or_else(|| err_ub_format!("mutex has invalid ID").into())
322+
this.get_or_create_id(
323+
lock_op,
324+
lock_layout,
325+
offset,
326+
|ecx| &mut ecx.machine.sync.mutexes,
327+
|ecx| initialize_data(ecx).map(|data| Mutex { data, ..Default::default() }),
328+
)?
329+
.ok_or_else(|| err_ub_format!("mutex has invalid ID").into())
330+
}
331+
332+
/// Retrieve the additional data stored for a mutex.
333+
fn mutex_get_data<'a>(&'a mut self, id: MutexId) -> Option<&'a AdditionalMutexData>
334+
where
335+
'tcx: 'a,
336+
{
337+
let this = self.eval_context_ref();
338+
this.machine.sync.mutexes[id].data.as_ref()
248339
}
249340

250341
fn rwlock_get_or_create_id(
@@ -254,8 +345,14 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
254345
offset: u64,
255346
) -> InterpResult<'tcx, RwLockId> {
256347
let this = self.eval_context_mut();
257-
this.get_or_create_id(lock_op, lock_layout, offset, |ecx| &mut ecx.machine.sync.rwlocks)?
258-
.ok_or_else(|| err_ub_format!("rwlock has invalid ID").into())
348+
this.get_or_create_id(
349+
lock_op,
350+
lock_layout,
351+
offset,
352+
|ecx| &mut ecx.machine.sync.rwlocks,
353+
|_| Ok(Default::default()),
354+
)?
355+
.ok_or_else(|| err_ub_format!("rwlock has invalid ID").into())
259356
}
260357

261358
fn condvar_get_or_create_id(
@@ -265,8 +362,14 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
265362
offset: u64,
266363
) -> InterpResult<'tcx, CondvarId> {
267364
let this = self.eval_context_mut();
268-
this.get_or_create_id(lock_op, lock_layout, offset, |ecx| &mut ecx.machine.sync.condvars)?
269-
.ok_or_else(|| err_ub_format!("condvar has invalid ID").into())
365+
this.get_or_create_id(
366+
lock_op,
367+
lock_layout,
368+
offset,
369+
|ecx| &mut ecx.machine.sync.condvars,
370+
|_| Ok(Default::default()),
371+
)?
372+
.ok_or_else(|| err_ub_format!("condvar has invalid ID").into())
270373
}
271374

272375
#[inline]

src/tools/miri/src/lib.rs

+4-1
Original file line numberDiff line numberDiff line change
@@ -133,7 +133,10 @@ pub use crate::concurrency::{
133133
cpu_affinity::MAX_CPUS,
134134
data_race::{AtomicFenceOrd, AtomicReadOrd, AtomicRwOrd, AtomicWriteOrd, EvalContextExt as _},
135135
init_once::{EvalContextExt as _, InitOnceId},
136-
sync::{CondvarId, EvalContextExt as _, MutexId, RwLockId, SynchronizationObjects},
136+
sync::{
137+
AdditionalMutexData, CondvarId, EvalContextExt as _, MutexId, MutexKind, RwLockId,
138+
SynchronizationObjects,
139+
},
137140
thread::{
138141
BlockReason, EvalContextExt as _, StackEmptyCallback, ThreadId, ThreadManager,
139142
TimeoutAnchor, TimeoutClock, UnblockCallback,

src/tools/miri/src/shims/unix/macos/sync.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ trait EvalContextExtPriv<'tcx>: crate::MiriInterpCxExt<'tcx> {
1919
// os_unfair_lock holds a 32-bit value, is initialized with zero and
2020
// must be assumed to be opaque. Therefore, we can just store our
2121
// internal mutex ID in the structure without anyone noticing.
22-
this.mutex_get_or_create_id(lock_op, this.libc_ty_layout("os_unfair_lock"), 0)
22+
this.mutex_get_or_create_id(lock_op, this.libc_ty_layout("os_unfair_lock"), 0, |_| Ok(None))
2323
}
2424
}
2525

0 commit comments

Comments
 (0)