Skip to content

Commit cd25fd2

Browse files
MandragorianMandragorian
committed
address comments
1 parent 18bad5a commit cd25fd2

File tree

4 files changed

+185
-101
lines changed

4 files changed

+185
-101
lines changed

‎src/concurrency/init_once.rs‎

Lines changed: 8 additions & 2 deletions
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/concurrency/sync.rs‎

Lines changed: 79 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -87,7 +87,7 @@ pub struct AdditionalMutexData {
8787
pub kind: MutexKind,
8888

8989
/// The address of the mutex.
90-
pub address: Pointer,
90+
pub address: u64,
9191
}
9292

9393
/// The mutex state.
@@ -195,14 +195,15 @@ pub(super) trait EvalContextExtPriv<'tcx>: crate::MiriInterpCxExt<'tcx> {
195195
/// Returns `None` if memory stores a non-zero invalid ID.
196196
///
197197
/// `get_objs` must return the `IndexVec` that stores all the objects of this type.
198+
/// `create_obj` must create the new object if initialization is needed.
198199
#[inline]
199-
fn get_or_create_id<Id: SyncId + Idx, T: Default>(
200+
fn get_or_create_id<Id: SyncId + Idx, T>(
200201
&mut self,
201202
lock_op: &OpTy<'tcx>,
202203
lock_layout: TyAndLayout<'tcx>,
203204
offset: u64,
204205
get_objs: impl for<'a> Fn(&'a mut MiriInterpCx<'tcx>) -> &'a mut IndexVec<Id, T>,
205-
initialize_data: impl FnOnce() -> InterpResult<'tcx, Option<AdditionalMutexData>>,
206+
create_obj: impl for<'a> FnOnce(&'a mut MiriInterpCx<'tcx>) -> InterpResult<'tcx, T>,
206207
) -> InterpResult<'tcx, Option<Id>> {
207208
let this = self.eval_context_mut();
208209
let value_place =
@@ -224,10 +225,9 @@ pub(super) trait EvalContextExtPriv<'tcx>: crate::MiriInterpCxExt<'tcx> {
224225
Ok(if success.to_bool().expect("compare_exchange's second return value is a bool") {
225226
// We set the in-memory ID to `next_index`, now also create this object in the machine
226227
// state.
227-
let new_index = get_objs(this).push(T::default());
228+
let obj = create_obj(this)?;
229+
let new_index = get_objs(this).push(obj);
228230
assert_eq!(next_index, new_index);
229-
let data = initialize_data()?;
230-
get_objs(this)[new_index].data = data;
231231
Some(new_index)
232232
} else {
233233
let id = Id::from_u32(old.to_u32().expect("layout is u32"));
@@ -240,6 +240,32 @@ pub(super) trait EvalContextExtPriv<'tcx>: crate::MiriInterpCxExt<'tcx> {
240240
})
241241
}
242242

243+
/// Eagerly creates a Miri sync structure.
244+
///
245+
/// `create_id` will store the index of the sync_structure in the memory pointed to by
246+
/// `lock_op`, so that future calls to `get_or_create_id` will see it as initialized.
247+
/// - `lock_op` must hold a pointer to the sync structure.
248+
/// - `lock_layout` must be the memory layout of the sync structure.
249+
/// - `offset` must be the offset inside the sync structure where its miri id will be stored.
250+
/// - `get_objs` is described in `get_or_create_id`.
251+
/// - `obj` must be the new sync object.
252+
fn create_id<Id: SyncId + Idx, T>(
253+
&mut self,
254+
lock_op: &OpTy<'tcx>,
255+
lock_layout: TyAndLayout<'tcx>,
256+
offset: u64,
257+
get_objs: impl for<'a> Fn(&'a mut MiriInterpCx<'tcx>) -> &'a mut IndexVec<Id, T>,
258+
obj: T,
259+
) -> InterpResult<'tcx, Id> {
260+
let this = self.eval_context_mut();
261+
let value_place =
262+
this.deref_pointer_and_offset(lock_op, offset, lock_layout, this.machine.layouts.u32)?;
263+
264+
let new_index = get_objs(this).push(obj);
265+
this.write_scalar(Scalar::from_u32(new_index.to_u32()), &value_place)?;
266+
Ok(new_index)
267+
}
268+
243269
fn condvar_reacquire_mutex(
244270
&mut self,
245271
mutex: MutexId,
@@ -266,16 +292,44 @@ pub(super) trait EvalContextExtPriv<'tcx>: crate::MiriInterpCxExt<'tcx> {
266292
// situations.
267293
impl<'tcx> EvalContextExt<'tcx> for crate::MiriInterpCx<'tcx> {}
268294
pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
295+
/// Eagerly create and initialize a new mutex.
296+
fn mutex_create(
297+
&mut self,
298+
lock_op: &OpTy<'tcx>,
299+
lock_layout: TyAndLayout<'tcx>,
300+
offset: u64,
301+
data: Option<AdditionalMutexData>,
302+
) -> InterpResult<'tcx, MutexId> {
303+
let this = self.eval_context_mut();
304+
this.create_id(
305+
lock_op,
306+
lock_layout,
307+
offset,
308+
|ecx| &mut ecx.machine.sync.mutexes,
309+
Mutex { data, ..Default::default() },
310+
)
311+
}
312+
313+
/// Lazily create a new mutex.
314+
/// `initialize_data` must return any additional data that a user wants to associate with the mutex.
269315
fn mutex_get_or_create_id(
270316
&mut self,
271317
lock_op: &OpTy<'tcx>,
272318
lock_layout: TyAndLayout<'tcx>,
273319
offset: u64,
274-
initialize_data: impl FnOnce() -> InterpResult<'tcx, Option<AdditionalMutexData>>,
320+
initialize_data: impl for<'a> FnOnce(
321+
&'a mut MiriInterpCx<'tcx>,
322+
) -> InterpResult<'tcx, Option<AdditionalMutexData>>,
275323
) -> InterpResult<'tcx, MutexId> {
276324
let this = self.eval_context_mut();
277-
this.get_or_create_id(lock_op, lock_layout, offset, |ecx| &mut ecx.machine.sync.mutexes)?
278-
.ok_or_else(|| err_ub_format!("mutex has invalid ID").into(), initialize_data)
325+
this.get_or_create_id(
326+
lock_op,
327+
lock_layout,
328+
offset,
329+
|ecx| &mut ecx.machine.sync.mutexes,
330+
|ecx| initialize_data(ecx).map(|data| Mutex { data, ..Default::default() }),
331+
)?
332+
.ok_or_else(|| err_ub_format!("mutex has invalid ID").into())
279333
}
280334

281335
/// Retrieve the additional data stored for a mutex.
@@ -294,8 +348,14 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
294348
offset: u64,
295349
) -> InterpResult<'tcx, RwLockId> {
296350
let this = self.eval_context_mut();
297-
this.get_or_create_id(lock_op, lock_layout, offset, |ecx| &mut ecx.machine.sync.rwlocks)?
298-
.ok_or_else(|| err_ub_format!("rwlock has invalid ID").into())
351+
this.get_or_create_id(
352+
lock_op,
353+
lock_layout,
354+
offset,
355+
|ecx| &mut ecx.machine.sync.rwlocks,
356+
|_| Ok(Default::default()),
357+
)?
358+
.ok_or_else(|| err_ub_format!("rwlock has invalid ID").into())
299359
}
300360

301361
fn condvar_get_or_create_id(
@@ -305,8 +365,14 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
305365
offset: u64,
306366
) -> InterpResult<'tcx, CondvarId> {
307367
let this = self.eval_context_mut();
308-
this.get_or_create_id(lock_op, lock_layout, offset, |ecx| &mut ecx.machine.sync.condvars)?
309-
.ok_or_else(|| err_ub_format!("condvar has invalid ID").into())
368+
this.get_or_create_id(
369+
lock_op,
370+
lock_layout,
371+
offset,
372+
|ecx| &mut ecx.machine.sync.condvars,
373+
|_| Ok(Default::default()),
374+
)?
375+
.ok_or_else(|| err_ub_format!("condvar has invalid ID").into())
310376
}
311377

312378
#[inline]

‎src/shims/unix/macos/sync.rs‎

Lines changed: 1 addition & 1 deletion
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, || Ok(None))
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)