Skip to content

Commit 51dbed2

Browse files
RalfJungRalfJung
committed
sync methods: pass around places, not pointer-typed operands
1 parent 0aa3fd4 commit 51dbed2

File tree

5 files changed

+69
-95
lines changed

5 files changed

+69
-95
lines changed

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

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
use std::collections::VecDeque;
22

33
use rustc_index::Idx;
4-
use rustc_middle::ty::layout::TyAndLayout;
54

65
use super::sync::EvalContextExtPriv as _;
76
use super::vector_clock::VClock;
@@ -30,14 +29,12 @@ impl<'tcx> EvalContextExt<'tcx> for crate::MiriInterpCx<'tcx> {}
3029
pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
3130
fn init_once_get_or_create_id(
3231
&mut self,
33-
lock_op: &OpTy<'tcx>,
34-
lock_layout: TyAndLayout<'tcx>,
32+
lock: &MPlaceTy<'tcx>,
3533
offset: u64,
3634
) -> InterpResult<'tcx, InitOnceId> {
3735
let this = self.eval_context_mut();
3836
this.get_or_create_id(
39-
lock_op,
40-
lock_layout,
37+
lock,
4138
offset,
4239
|ecx| &mut ecx.machine.sync.init_onces,
4340
|_| Ok(Default::default()),

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

Lines changed: 19 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ use std::time::Duration;
44

55
use rustc_data_structures::fx::FxHashMap;
66
use rustc_index::{Idx, IndexVec};
7-
use rustc_middle::ty::layout::TyAndLayout;
7+
use rustc_target::abi::Size;
88

99
use super::init_once::InitOnce;
1010
use super::vector_clock::VClock;
@@ -206,21 +206,21 @@ pub(super) trait EvalContextExtPriv<'tcx>: crate::MiriInterpCxExt<'tcx> {
206206
#[inline]
207207
fn get_or_create_id<Id: SyncId + Idx, T>(
208208
&mut self,
209-
lock_op: &OpTy<'tcx>,
210-
lock_layout: TyAndLayout<'tcx>,
209+
lock: &MPlaceTy<'tcx>,
211210
offset: u64,
212211
get_objs: impl for<'a> Fn(&'a mut MiriInterpCx<'tcx>) -> &'a mut IndexVec<Id, T>,
213212
create_obj: impl for<'a> FnOnce(&'a mut MiriInterpCx<'tcx>) -> InterpResult<'tcx, T>,
214213
) -> InterpResult<'tcx, Option<Id>> {
215214
let this = self.eval_context_mut();
216-
let value_place =
217-
this.deref_pointer_and_offset(lock_op, offset, lock_layout, this.machine.layouts.u32)?;
215+
let offset = Size::from_bytes(offset);
216+
assert!(lock.layout.size >= offset + this.machine.layouts.u32.size);
217+
let id_place = lock.offset(offset, this.machine.layouts.u32, this)?;
218218
let next_index = get_objs(this).next_index();
219219

220220
// Since we are lazy, this update has to be atomic.
221221
let (old, success) = this
222222
.atomic_compare_exchange_scalar(
223-
&value_place,
223+
&id_place,
224224
&ImmTy::from_uint(0u32, this.machine.layouts.u32),
225225
Scalar::from_u32(next_index.to_u32()),
226226
AtomicRwOrd::Relaxed, // deliberately *no* synchronization
@@ -258,18 +258,18 @@ pub(super) trait EvalContextExtPriv<'tcx>: crate::MiriInterpCxExt<'tcx> {
258258
/// - `obj` must be the new sync object.
259259
fn create_id<Id: SyncId + Idx, T>(
260260
&mut self,
261-
lock_op: &OpTy<'tcx>,
262-
lock_layout: TyAndLayout<'tcx>,
261+
lock: &MPlaceTy<'tcx>,
263262
offset: u64,
264263
get_objs: impl for<'a> Fn(&'a mut MiriInterpCx<'tcx>) -> &'a mut IndexVec<Id, T>,
265264
obj: T,
266265
) -> InterpResult<'tcx, Id> {
267266
let this = self.eval_context_mut();
268-
let value_place =
269-
this.deref_pointer_and_offset(lock_op, offset, lock_layout, this.machine.layouts.u32)?;
267+
let offset = Size::from_bytes(offset);
268+
assert!(lock.layout.size >= offset + this.machine.layouts.u32.size);
269+
let id_place = lock.offset(offset, this.machine.layouts.u32, this)?;
270270

271271
let new_index = get_objs(this).push(obj);
272-
this.write_scalar(Scalar::from_u32(new_index.to_u32()), &value_place)?;
272+
this.write_scalar(Scalar::from_u32(new_index.to_u32()), &id_place)?;
273273
Ok(new_index)
274274
}
275275

@@ -302,15 +302,13 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
302302
/// Eagerly create and initialize a new mutex.
303303
fn mutex_create(
304304
&mut self,
305-
lock_op: &OpTy<'tcx>,
306-
lock_layout: TyAndLayout<'tcx>,
305+
lock: &MPlaceTy<'tcx>,
307306
offset: u64,
308307
data: Option<AdditionalMutexData>,
309308
) -> InterpResult<'tcx, MutexId> {
310309
let this = self.eval_context_mut();
311310
this.create_id(
312-
lock_op,
313-
lock_layout,
311+
lock,
314312
offset,
315313
|ecx| &mut ecx.machine.sync.mutexes,
316314
Mutex { data, ..Default::default() },
@@ -321,17 +319,15 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
321319
/// `initialize_data` must return any additional data that a user wants to associate with the mutex.
322320
fn mutex_get_or_create_id(
323321
&mut self,
324-
lock_op: &OpTy<'tcx>,
325-
lock_layout: TyAndLayout<'tcx>,
322+
lock: &MPlaceTy<'tcx>,
326323
offset: u64,
327324
initialize_data: impl for<'a> FnOnce(
328325
&'a mut MiriInterpCx<'tcx>,
329326
) -> InterpResult<'tcx, Option<AdditionalMutexData>>,
330327
) -> InterpResult<'tcx, MutexId> {
331328
let this = self.eval_context_mut();
332329
this.get_or_create_id(
333-
lock_op,
334-
lock_layout,
330+
lock,
335331
offset,
336332
|ecx| &mut ecx.machine.sync.mutexes,
337333
|ecx| initialize_data(ecx).map(|data| Mutex { data, ..Default::default() }),
@@ -350,8 +346,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
350346

351347
fn rwlock_get_or_create_id(
352348
&mut self,
353-
lock_op: &OpTy<'tcx>,
354-
lock_layout: TyAndLayout<'tcx>,
349+
lock: &MPlaceTy<'tcx>,
355350
offset: u64,
356351
initialize_data: impl for<'a> FnOnce(
357352
&'a mut MiriInterpCx<'tcx>,
@@ -360,8 +355,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
360355
) -> InterpResult<'tcx, RwLockId> {
361356
let this = self.eval_context_mut();
362357
this.get_or_create_id(
363-
lock_op,
364-
lock_layout,
358+
lock,
365359
offset,
366360
|ecx| &mut ecx.machine.sync.rwlocks,
367361
|ecx| initialize_data(ecx).map(|data| RwLock { data, ..Default::default() }),
@@ -380,14 +374,12 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
380374

381375
fn condvar_get_or_create_id(
382376
&mut self,
383-
lock_op: &OpTy<'tcx>,
384-
lock_layout: TyAndLayout<'tcx>,
377+
lock: &MPlaceTy<'tcx>,
385378
offset: u64,
386379
) -> InterpResult<'tcx, CondvarId> {
387380
let this = self.eval_context_mut();
388381
this.get_or_create_id(
389-
lock_op,
390-
lock_layout,
382+
lock,
391383
offset,
392384
|ecx| &mut ecx.machine.sync.condvars,
393385
|_| Ok(Default::default()),

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

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,12 +14,13 @@ use crate::*;
1414

1515
impl<'tcx> EvalContextExtPriv<'tcx> for crate::MiriInterpCx<'tcx> {}
1616
trait EvalContextExtPriv<'tcx>: crate::MiriInterpCxExt<'tcx> {
17-
fn os_unfair_lock_getid(&mut self, lock_op: &OpTy<'tcx>) -> InterpResult<'tcx, MutexId> {
17+
fn os_unfair_lock_getid(&mut self, lock_ptr: &OpTy<'tcx>) -> InterpResult<'tcx, MutexId> {
1818
let this = self.eval_context_mut();
19+
let lock = this.deref_pointer(lock_ptr)?;
1920
// os_unfair_lock holds a 32-bit value, is initialized with zero and
2021
// must be assumed to be opaque. Therefore, we can just store our
2122
// 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))
23+
this.mutex_get_or_create_id(&lock, 0, |_| Ok(None))
2324
}
2425
}
2526

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

Lines changed: 42 additions & 59 deletions
Original file line numberDiff line numberDiff line change
@@ -18,10 +18,10 @@ fn mutexattr_kind_offset<'tcx>(ecx: &MiriInterpCx<'tcx>) -> InterpResult<'tcx, u
1818

1919
fn mutexattr_get_kind<'tcx>(
2020
ecx: &MiriInterpCx<'tcx>,
21-
attr_op: &OpTy<'tcx>,
21+
attr_ptr: &OpTy<'tcx>,
2222
) -> InterpResult<'tcx, i32> {
2323
ecx.deref_pointer_and_read(
24-
attr_op,
24+
attr_ptr,
2525
mutexattr_kind_offset(ecx)?,
2626
ecx.libc_ty_layout("pthread_mutexattr_t"),
2727
ecx.machine.layouts.i32,
@@ -31,11 +31,11 @@ fn mutexattr_get_kind<'tcx>(
3131

3232
fn mutexattr_set_kind<'tcx>(
3333
ecx: &mut MiriInterpCx<'tcx>,
34-
attr_op: &OpTy<'tcx>,
34+
attr_ptr: &OpTy<'tcx>,
3535
kind: i32,
3636
) -> InterpResult<'tcx, ()> {
3737
ecx.deref_pointer_and_write(
38-
attr_op,
38+
attr_ptr,
3939
mutexattr_kind_offset(ecx)?,
4040
Scalar::from_i32(kind),
4141
ecx.libc_ty_layout("pthread_mutexattr_t"),
@@ -94,15 +94,14 @@ fn mutex_id_offset<'tcx>(ecx: &MiriInterpCx<'tcx>) -> InterpResult<'tcx, u64> {
9494
/// Eagerly create and initialize a new mutex.
9595
fn mutex_create<'tcx>(
9696
ecx: &mut MiriInterpCx<'tcx>,
97-
mutex_op: &OpTy<'tcx>,
97+
mutex_ptr: &OpTy<'tcx>,
9898
kind: i32,
9999
) -> InterpResult<'tcx> {
100-
// FIXME: might be worth changing mutex_create to take the mplace
101-
// rather than the `OpTy`.
102-
let address = ecx.read_pointer(mutex_op)?.addr().bytes();
100+
let mutex = ecx.deref_pointer(mutex_ptr)?;
101+
let address = mutex.ptr().addr().bytes();
103102
let kind = translate_kind(ecx, kind)?;
104103
let data = Some(AdditionalMutexData { address, kind });
105-
ecx.mutex_create(mutex_op, ecx.libc_ty_layout("pthread_mutex_t"), mutex_id_offset(ecx)?, data)?;
104+
ecx.mutex_create(&mutex, mutex_id_offset(ecx)?, data)?;
106105
Ok(())
107106
}
108107

@@ -112,24 +111,18 @@ fn mutex_create<'tcx>(
112111
/// return an error if it has.
113112
fn mutex_get_id<'tcx>(
114113
ecx: &mut MiriInterpCx<'tcx>,
115-
mutex_op: &OpTy<'tcx>,
114+
mutex_ptr: &OpTy<'tcx>,
116115
) -> InterpResult<'tcx, MutexId> {
117-
let address = ecx.read_pointer(mutex_op)?.addr().bytes();
118-
119-
// FIXME: might be worth changing mutex_get_or_create_id to take the mplace
120-
// rather than the `OpTy`.
121-
let id = ecx.mutex_get_or_create_id(
122-
mutex_op,
123-
ecx.libc_ty_layout("pthread_mutex_t"),
124-
mutex_id_offset(ecx)?,
125-
|ecx| {
126-
// This is called if a static initializer was used and the lock has not been assigned
127-
// an ID yet. We have to determine the mutex kind from the static initializer.
128-
let kind = kind_from_static_initializer(ecx, mutex_op)?;
129-
130-
Ok(Some(AdditionalMutexData { kind, address }))
131-
},
132-
)?;
116+
let mutex = ecx.deref_pointer(mutex_ptr)?;
117+
let address = mutex.ptr().addr().bytes();
118+
119+
let id = ecx.mutex_get_or_create_id(&mutex, mutex_id_offset(ecx)?, |ecx| {
120+
// This is called if a static initializer was used and the lock has not been assigned
121+
// an ID yet. We have to determine the mutex kind from the static initializer.
122+
let kind = kind_from_static_initializer(ecx, &mutex)?;
123+
124+
Ok(Some(AdditionalMutexData { kind, address }))
125+
})?;
133126

134127
// Check that the mutex has not been moved since last use.
135128
let data = ecx.mutex_get_data(id).expect("data should be always exist for pthreads");
@@ -143,20 +136,15 @@ fn mutex_get_id<'tcx>(
143136
/// Returns the kind of a static initializer.
144137
fn kind_from_static_initializer<'tcx>(
145138
ecx: &MiriInterpCx<'tcx>,
146-
mutex_op: &OpTy<'tcx>,
139+
mutex: &MPlaceTy<'tcx>,
147140
) -> InterpResult<'tcx, MutexKind> {
148141
// Only linux has static initializers other than PTHREAD_MUTEX_DEFAULT.
149142
let kind = match &*ecx.tcx.sess.target.os {
150143
"linux" => {
151144
let offset = if ecx.pointer_size().bytes() == 8 { 16 } else { 12 };
152-
153-
ecx.deref_pointer_and_read(
154-
mutex_op,
155-
offset,
156-
ecx.libc_ty_layout("pthread_mutex_t"),
157-
ecx.machine.layouts.i32,
158-
)?
159-
.to_i32()?
145+
let kind_place =
146+
mutex.offset(Size::from_bytes(offset), ecx.machine.layouts.i32, ecx)?;
147+
ecx.read_scalar(&kind_place)?.to_i32()?
160148
}
161149
| "illumos" | "solaris" | "macos" => ecx.eval_libc_i32("PTHREAD_MUTEX_DEFAULT"),
162150
os => throw_unsup_format!("`pthread_mutex` is not supported on {os}"),
@@ -211,16 +199,14 @@ fn rwlock_id_offset<'tcx>(ecx: &MiriInterpCx<'tcx>) -> InterpResult<'tcx, u64> {
211199

212200
fn rwlock_get_id<'tcx>(
213201
ecx: &mut MiriInterpCx<'tcx>,
214-
rwlock_op: &OpTy<'tcx>,
202+
rwlock_ptr: &OpTy<'tcx>,
215203
) -> InterpResult<'tcx, RwLockId> {
216-
let address = ecx.read_pointer(rwlock_op)?.addr().bytes();
204+
let rwlock = ecx.deref_pointer(rwlock_ptr)?;
205+
let address = rwlock.ptr().addr().bytes();
217206

218-
let id = ecx.rwlock_get_or_create_id(
219-
rwlock_op,
220-
ecx.libc_ty_layout("pthread_rwlock_t"),
221-
rwlock_id_offset(ecx)?,
222-
|_| Ok(Some(AdditionalRwLockData { address })),
223-
)?;
207+
let id = ecx.rwlock_get_or_create_id(&rwlock, rwlock_id_offset(ecx)?, |_| {
208+
Ok(Some(AdditionalRwLockData { address }))
209+
})?;
224210

225211
// Check that the rwlock has not been moved since last use.
226212
let data = ecx.rwlock_get_data(id).expect("data should be always exist for pthreads");
@@ -246,10 +232,10 @@ fn condattr_clock_offset<'tcx>(ecx: &MiriInterpCx<'tcx>) -> InterpResult<'tcx, u
246232

247233
fn condattr_get_clock_id<'tcx>(
248234
ecx: &MiriInterpCx<'tcx>,
249-
attr_op: &OpTy<'tcx>,
235+
attr_ptr: &OpTy<'tcx>,
250236
) -> InterpResult<'tcx, i32> {
251237
ecx.deref_pointer_and_read(
252-
attr_op,
238+
attr_ptr,
253239
condattr_clock_offset(ecx)?,
254240
ecx.libc_ty_layout("pthread_condattr_t"),
255241
ecx.machine.layouts.i32,
@@ -259,11 +245,11 @@ fn condattr_get_clock_id<'tcx>(
259245

260246
fn condattr_set_clock_id<'tcx>(
261247
ecx: &mut MiriInterpCx<'tcx>,
262-
attr_op: &OpTy<'tcx>,
248+
attr_ptr: &OpTy<'tcx>,
263249
clock_id: i32,
264250
) -> InterpResult<'tcx, ()> {
265251
ecx.deref_pointer_and_write(
266-
attr_op,
252+
attr_ptr,
267253
condattr_clock_offset(ecx)?,
268254
Scalar::from_i32(clock_id),
269255
ecx.libc_ty_layout("pthread_condattr_t"),
@@ -337,21 +323,18 @@ fn cond_clock_offset<'tcx>(ecx: &MiriInterpCx<'tcx>) -> u64 {
337323

338324
fn cond_get_id<'tcx>(
339325
ecx: &mut MiriInterpCx<'tcx>,
340-
cond_op: &OpTy<'tcx>,
326+
cond_ptr: &OpTy<'tcx>,
341327
) -> InterpResult<'tcx, CondvarId> {
342-
ecx.condvar_get_or_create_id(
343-
cond_op,
344-
ecx.libc_ty_layout("pthread_cond_t"),
345-
cond_id_offset(ecx)?,
346-
)
328+
let cond = ecx.deref_pointer(cond_ptr)?;
329+
ecx.condvar_get_or_create_id(&cond, cond_id_offset(ecx)?)
347330
}
348331

349332
fn cond_reset_id<'tcx>(
350333
ecx: &mut MiriInterpCx<'tcx>,
351-
cond_op: &OpTy<'tcx>,
334+
cond_ptr: &OpTy<'tcx>,
352335
) -> InterpResult<'tcx, ()> {
353336
ecx.deref_pointer_and_write(
354-
cond_op,
337+
cond_ptr,
355338
cond_id_offset(ecx)?,
356339
Scalar::from_i32(0),
357340
ecx.libc_ty_layout("pthread_cond_t"),
@@ -361,10 +344,10 @@ fn cond_reset_id<'tcx>(
361344

362345
fn cond_get_clock_id<'tcx>(
363346
ecx: &MiriInterpCx<'tcx>,
364-
cond_op: &OpTy<'tcx>,
347+
cond_ptr: &OpTy<'tcx>,
365348
) -> InterpResult<'tcx, i32> {
366349
ecx.deref_pointer_and_read(
367-
cond_op,
350+
cond_ptr,
368351
cond_clock_offset(ecx),
369352
ecx.libc_ty_layout("pthread_cond_t"),
370353
ecx.machine.layouts.i32,
@@ -374,11 +357,11 @@ fn cond_get_clock_id<'tcx>(
374357

375358
fn cond_set_clock_id<'tcx>(
376359
ecx: &mut MiriInterpCx<'tcx>,
377-
cond_op: &OpTy<'tcx>,
360+
cond_ptr: &OpTy<'tcx>,
378361
clock_id: i32,
379362
) -> InterpResult<'tcx, ()> {
380363
ecx.deref_pointer_and_write(
381-
cond_op,
364+
cond_ptr,
382365
cond_clock_offset(ecx),
383366
Scalar::from_i32(clock_id),
384367
ecx.libc_ty_layout("pthread_cond_t"),

0 commit comments

Comments
 (0)