Skip to content

Commit d84ce6b

Browse files
MandragorianMandragorian
committed
Detect pthread_mutex_t is moved
See: #3749
1 parent 4645e6a commit d84ce6b

10 files changed

+234
-6
lines changed

‎src/concurrency/sync.rs

Lines changed: 41 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -66,6 +66,30 @@ pub(super) use declare_id;
6666

6767
declare_id!(MutexId);
6868

69+
/// The mutex kind.
70+
#[derive(Debug, Clone, Copy)]
71+
pub struct MutexKind(i32);
72+
73+
impl MutexKind {
74+
pub fn new(k: i32) -> Self {
75+
Self(k)
76+
}
77+
78+
pub fn to_i32(&self) -> i32 {
79+
self.0
80+
}
81+
}
82+
83+
#[derive(Debug)]
84+
/// Additional data that may be used by shim implementations.
85+
pub struct AdditionalMutexData {
86+
/// The mutex kind, used by some mutex implementations like pthreads mutexes.
87+
pub kind: MutexKind,
88+
89+
/// The address of the mutex.
90+
pub address: Pointer,
91+
}
92+
6993
/// The mutex state.
7094
#[derive(Default, Debug)]
7195
struct Mutex {
@@ -77,6 +101,9 @@ struct Mutex {
77101
queue: VecDeque<ThreadId>,
78102
/// Mutex clock. This tracks the moment of the last unlock.
79103
clock: VClock,
104+
105+
/// Additional data that can be set by shim implementations.
106+
data: Option<AdditionalMutexData>,
80107
}
81108

82109
declare_id!(RwLockId);
@@ -175,6 +202,7 @@ pub(super) trait EvalContextExtPriv<'tcx>: crate::MiriInterpCxExt<'tcx> {
175202
lock_layout: TyAndLayout<'tcx>,
176203
offset: u64,
177204
get_objs: impl for<'a> Fn(&'a mut MiriInterpCx<'tcx>) -> &'a mut IndexVec<Id, T>,
205+
initialize_data: impl FnOnce() -> InterpResult<'tcx, Option<AdditionalMutexData>>,
178206
) -> InterpResult<'tcx, Option<Id>> {
179207
let this = self.eval_context_mut();
180208
let value_place =
@@ -198,6 +226,8 @@ pub(super) trait EvalContextExtPriv<'tcx>: crate::MiriInterpCxExt<'tcx> {
198226
// state.
199227
let new_index = get_objs(this).push(T::default());
200228
assert_eq!(next_index, new_index);
229+
let data = initialize_data()?;
230+
get_objs(this)[new_index].data = data;
201231
Some(new_index)
202232
} else {
203233
let id = Id::from_u32(old.to_u32().expect("layout is u32"));
@@ -241,10 +271,20 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
241271
lock_op: &OpTy<'tcx>,
242272
lock_layout: TyAndLayout<'tcx>,
243273
offset: u64,
274+
initialize_data: impl FnOnce() -> InterpResult<'tcx, Option<AdditionalMutexData>>,
244275
) -> InterpResult<'tcx, MutexId> {
245276
let this = self.eval_context_mut();
246277
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())
278+
.ok_or_else(|| err_ub_format!("mutex has invalid ID").into(), initialize_data)
279+
}
280+
281+
/// Retrieve the additional data stored for a mutex.
282+
fn mutex_get_data<'a>(&'a mut self, id: MutexId) -> Option<&'a AdditionalMutexData>
283+
where
284+
'tcx: 'a,
285+
{
286+
let this = self.eval_context_ref();
287+
this.machine.sync.mutexes[id].data.as_ref()
248288
}
249289

250290
fn rwlock_get_or_create_id(

‎src/lib.rs

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -130,7 +130,10 @@ pub use crate::concurrency::{
130130
cpu_affinity::MAX_CPUS,
131131
data_race::{AtomicFenceOrd, AtomicReadOrd, AtomicRwOrd, AtomicWriteOrd, EvalContextExt as _},
132132
init_once::{EvalContextExt as _, InitOnceId},
133-
sync::{CondvarId, EvalContextExt as _, MutexId, RwLockId, SynchronizationObjects},
133+
sync::{
134+
AdditionalMutexData, CondvarId, EvalContextExt as _, MutexId, MutexKind, RwLockId,
135+
SynchronizationObjects,
136+
},
134137
thread::{
135138
BlockReason, EvalContextExt as _, StackEmptyCallback, ThreadId, ThreadManager,
136139
TimeoutAnchor, TimeoutClock, UnblockCallback,

‎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)
22+
this.mutex_get_or_create_id(lock_op, this.libc_ty_layout("os_unfair_lock"), 0, || Ok(None))
2323
}
2424
}
2525

‎src/shims/unix/sync.rs

Lines changed: 41 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -118,13 +118,29 @@ fn mutex_get_id<'tcx>(
118118
ecx: &mut MiriInterpCx<'tcx>,
119119
mutex_op: &OpTy<'tcx>,
120120
) -> InterpResult<'tcx, MutexId> {
121+
let address = ecx.deref_pointer_as(mutex_op, ecx.libc_ty_layout("pthread_mutex_t"))?.ptr();
122+
let kind = MutexKind::new(mutex_get_kind(ecx, mutex_op)?);
121123
ecx.mutex_get_or_create_id(
122124
mutex_op,
123125
ecx.libc_ty_layout("pthread_mutex_t"),
124126
mutex_id_offset(ecx)?,
127+
|| Ok(Some(AdditionalMutexData { kind, address })),
125128
)
126129
}
127130

131+
fn mutex_check_moved<'tcx>(
132+
ecx: &mut MiriInterpCx<'tcx>,
133+
mutex_op: &OpTy<'tcx>,
134+
id: MutexId,
135+
) -> InterpResult<'tcx, ()> {
136+
let addr = ecx.deref_pointer_as(mutex_op, ecx.libc_ty_layout("pthread_mutex_t"))?.ptr().addr();
137+
let data = ecx.mutex_get_data(id).expect("data should be always exist for pthreads");
138+
if data.address.addr() != addr {
139+
throw_ub_format!("pthread_mutex_t can't be moved after first use")
140+
}
141+
Ok(())
142+
}
143+
128144
fn mutex_reset_id<'tcx>(
129145
ecx: &mut MiriInterpCx<'tcx>,
130146
mutex_op: &OpTy<'tcx>,
@@ -457,6 +473,10 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
457473

458474
mutex_set_kind(this, mutex_op, kind)?;
459475

476+
// Fetch (and ignore) the mutex's id to go through the initialization
477+
// code, and thus register the mutex's address.
478+
mutex_get_id(this, mutex_op)?;
479+
460480
Ok(())
461481
}
462482

@@ -467,8 +487,14 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
467487
) -> InterpResult<'tcx> {
468488
let this = self.eval_context_mut();
469489

470-
let kind = mutex_get_kind(this, mutex_op)?;
471490
let id = mutex_get_id(this, mutex_op)?;
491+
let kind = this
492+
.mutex_get_data(id)
493+
.expect("data should always exist for pthread mutexes")
494+
.kind
495+
.to_i32();
496+
497+
mutex_check_moved(this, mutex_op, id)?;
472498

473499
let ret = if this.mutex_is_locked(id) {
474500
let owner_thread = this.mutex_get_owner(id);
@@ -504,8 +530,14 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
504530
fn pthread_mutex_trylock(&mut self, mutex_op: &OpTy<'tcx>) -> InterpResult<'tcx, Scalar> {
505531
let this = self.eval_context_mut();
506532

507-
let kind = mutex_get_kind(this, mutex_op)?;
508533
let id = mutex_get_id(this, mutex_op)?;
534+
let kind = this
535+
.mutex_get_data(id)
536+
.expect("data should always exist for pthread mutexes")
537+
.kind
538+
.to_i32();
539+
540+
mutex_check_moved(this, mutex_op, id)?;
509541

510542
Ok(Scalar::from_i32(if this.mutex_is_locked(id) {
511543
let owner_thread = this.mutex_get_owner(id);
@@ -536,8 +568,14 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
536568
fn pthread_mutex_unlock(&mut self, mutex_op: &OpTy<'tcx>) -> InterpResult<'tcx, Scalar> {
537569
let this = self.eval_context_mut();
538570

539-
let kind = mutex_get_kind(this, mutex_op)?;
540571
let id = mutex_get_id(this, mutex_op)?;
572+
let kind = this
573+
.mutex_get_data(id)
574+
.expect("data should always exist for pthread mutexes")
575+
.kind
576+
.to_i32();
577+
578+
mutex_check_moved(this, mutex_op, id)?;
541579

542580
if let Some(_old_locked_count) = this.mutex_unlock(id)? {
543581
// The mutex was locked by the current thread.

‎tests/fail-dep/concurrency/libc_pthread_mutex_move.init.stderr

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
error: Undefined Behavior: pthread_mutex_t can't be moved after first use
2+
--> $DIR/libc_pthread_mutex_move.rs:LL:CC
3+
|
4+
LL | libc::pthread_mutex_lock(&mut m2 as *mut _);
5+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ pthread_mutex_t can't be moved after first use
6+
|
7+
= help: this indicates a bug in the program: it performed an invalid operation, and caused Undefined Behavior
8+
= help: see https://doc.rust-lang.org/nightly/reference/behavior-considered-undefined.html for further information
9+
= note: BACKTRACE:
10+
= note: inside `check` at $DIR/libc_pthread_mutex_move.rs:LL:CC
11+
note: inside `main`
12+
--> $DIR/libc_pthread_mutex_move.rs:LL:CC
13+
|
14+
LL | check();
15+
| ^^^^^^^
16+
17+
note: some details are omitted, run with `MIRIFLAGS=-Zmiri-backtrace=full` for a verbose backtrace
18+
19+
error: aborting due to 1 previous error
20+

‎tests/fail-dep/concurrency/libc_pthread_mutex_move.lock.stderr

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
error: Undefined Behavior: pthread_mutex_t can't be moved after first use
2+
--> $DIR/libc_pthread_mutex_move.rs:LL:CC
3+
|
4+
LL | libc::pthread_mutex_lock(&mut m2 as *mut _);
5+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ pthread_mutex_t can't be moved after first use
6+
|
7+
= help: this indicates a bug in the program: it performed an invalid operation, and caused Undefined Behavior
8+
= help: see https://doc.rust-lang.org/nightly/reference/behavior-considered-undefined.html for further information
9+
= note: BACKTRACE:
10+
= note: inside `check` at $DIR/libc_pthread_mutex_move.rs:LL:CC
11+
note: inside `main`
12+
--> $DIR/libc_pthread_mutex_move.rs:LL:CC
13+
|
14+
LL | check();
15+
| ^^^^^^^
16+
17+
note: some details are omitted, run with `MIRIFLAGS=-Zmiri-backtrace=full` for a verbose backtrace
18+
19+
error: aborting due to 1 previous error
20+

‎tests/fail-dep/concurrency/libc_pthread_mutex_move.rs

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,52 @@
1+
//@ignore-target-windows: No pthreads on Windows
2+
//@revisions: lock trylock unlock init
3+
4+
fn main() {
5+
check();
6+
}
7+
8+
#[cfg(init)]
9+
fn check() {
10+
unsafe {
11+
let mut m: libc::pthread_mutex_t = std::mem::zeroed();
12+
assert_eq!(libc::pthread_mutex_init(&mut m as *mut _, std::ptr::null()), 0);
13+
14+
let mut m2 = m;
15+
libc::pthread_mutex_lock(&mut m2 as *mut _); //~[init] ERROR: pthread_mutex_t can't be moved after first use
16+
}
17+
}
18+
19+
#[cfg(lock)]
20+
fn check() {
21+
unsafe {
22+
let mut m: libc::pthread_mutex_t = libc::PTHREAD_MUTEX_INITIALIZER;
23+
libc::pthread_mutex_lock(&mut m as *mut _);
24+
// libc::pthread_mutex_unlock(&mut m as *mut _);
25+
26+
let mut m2 = m;
27+
libc::pthread_mutex_lock(&mut m2 as *mut _); //~[lock] ERROR: pthread_mutex_t can't be moved after first use
28+
}
29+
}
30+
31+
#[cfg(trylock)]
32+
fn check() {
33+
unsafe {
34+
let mut m: libc::pthread_mutex_t = libc::PTHREAD_MUTEX_INITIALIZER;
35+
libc::pthread_mutex_trylock(&mut m as *mut _);
36+
// libc::pthread_mutex_unlock(&mut m as *mut _);
37+
38+
let mut m2 = m;
39+
libc::pthread_mutex_trylock(&mut m2 as *mut _); //~[trylock] ERROR: pthread_mutex_t can't be moved after first use
40+
}
41+
}
42+
43+
#[cfg(unlock)]
44+
fn check() {
45+
unsafe {
46+
let mut m: libc::pthread_mutex_t = libc::PTHREAD_RECURSIVE_MUTEX_INITIALIZER_NP;
47+
libc::pthread_mutex_unlock(&mut m as *mut _);
48+
49+
let mut m2 = m;
50+
libc::pthread_mutex_unlock(&mut m2 as *mut _); //~[unlock] ERROR: pthread_mutex_t can't be moved after first use
51+
}
52+
}

‎tests/fail-dep/concurrency/libc_pthread_mutex_move.stderr

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
error: Undefined Behavior: pthread_mutex_t can't be moved
2+
--> $DIR/libc_pthread_mutex_move.rs:LL:CC
3+
|
4+
LL | libc::pthread_mutex_lock(&mut mutex2 as *mut _);
5+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ pthread_mutex_t can't be moved
6+
|
7+
= help: this indicates a bug in the program: it performed an invalid operation, and caused Undefined Behavior
8+
= help: see https://doc.rust-lang.org/nightly/reference/behavior-considered-undefined.html for further information
9+
= note: BACKTRACE:
10+
= note: inside `main` at $DIR/libc_pthread_mutex_move.rs:LL:CC
11+
12+
note: some details are omitted, run with `MIRIFLAGS=-Zmiri-backtrace=full` for a verbose backtrace
13+
14+
error: aborting due to 1 previous error
15+

‎tests/fail-dep/concurrency/libc_pthread_mutex_move.trylock.stderr

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
error: Undefined Behavior: pthread_mutex_t can't be moved after first use
2+
--> $DIR/libc_pthread_mutex_move.rs:LL:CC
3+
|
4+
LL | libc::pthread_mutex_trylock(&mut m2 as *mut _);
5+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ pthread_mutex_t can't be moved after first use
6+
|
7+
= help: this indicates a bug in the program: it performed an invalid operation, and caused Undefined Behavior
8+
= help: see https://doc.rust-lang.org/nightly/reference/behavior-considered-undefined.html for further information
9+
= note: BACKTRACE:
10+
= note: inside `check` at $DIR/libc_pthread_mutex_move.rs:LL:CC
11+
note: inside `main`
12+
--> $DIR/libc_pthread_mutex_move.rs:LL:CC
13+
|
14+
LL | check();
15+
| ^^^^^^^
16+
17+
note: some details are omitted, run with `MIRIFLAGS=-Zmiri-backtrace=full` for a verbose backtrace
18+
19+
error: aborting due to 1 previous error
20+

‎tests/fail-dep/concurrency/libc_pthread_mutex_move.unlock.stderr

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
error: Undefined Behavior: pthread_mutex_t can't be moved after first use
2+
--> $DIR/libc_pthread_mutex_move.rs:LL:CC
3+
|
4+
LL | libc::pthread_mutex_unlock(&mut m2 as *mut _);
5+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ pthread_mutex_t can't be moved after first use
6+
|
7+
= help: this indicates a bug in the program: it performed an invalid operation, and caused Undefined Behavior
8+
= help: see https://doc.rust-lang.org/nightly/reference/behavior-considered-undefined.html for further information
9+
= note: BACKTRACE:
10+
= note: inside `check` at $DIR/libc_pthread_mutex_move.rs:LL:CC
11+
note: inside `main`
12+
--> $DIR/libc_pthread_mutex_move.rs:LL:CC
13+
|
14+
LL | check();
15+
| ^^^^^^^
16+
17+
note: some details are omitted, run with `MIRIFLAGS=-Zmiri-backtrace=full` for a verbose backtrace
18+
19+
error: aborting due to 1 previous error
20+

0 commit comments

Comments
 (0)