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 Aug 29, 2024
1 parent 4645e6a commit d84ce6b
Show file tree
Hide file tree
Showing 10 changed files with 234 additions and 6 deletions.
42 changes: 41 additions & 1 deletion src/concurrency/sync.rs
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,30 @@ pub(super) use declare_id;

declare_id!(MutexId);

/// The mutex kind.
#[derive(Debug, Clone, Copy)]
pub struct MutexKind(i32);

impl MutexKind {
pub fn new(k: i32) -> Self {
Self(k)
}

pub fn to_i32(&self) -> i32 {
self.0
}
}

#[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: Pointer,
}

/// The mutex state.
#[derive(Default, Debug)]
struct Mutex {
Expand All @@ -77,6 +101,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 @@ -175,6 +202,7 @@ pub(super) trait EvalContextExtPriv<'tcx>: crate::MiriInterpCxExt<'tcx> {
lock_layout: TyAndLayout<'tcx>,
offset: u64,
get_objs: impl for<'a> Fn(&'a mut MiriInterpCx<'tcx>) -> &'a mut IndexVec<Id, T>,
initialize_data: impl FnOnce() -> InterpResult<'tcx, Option<AdditionalMutexData>>,
) -> InterpResult<'tcx, Option<Id>> {
let this = self.eval_context_mut();
let value_place =
Expand All @@ -198,6 +226,8 @@ pub(super) trait EvalContextExtPriv<'tcx>: crate::MiriInterpCxExt<'tcx> {
// state.
let new_index = get_objs(this).push(T::default());
assert_eq!(next_index, new_index);
let data = initialize_data()?;
get_objs(this)[new_index].data = data;
Some(new_index)
} else {
let id = Id::from_u32(old.to_u32().expect("layout is u32"));
Expand Down Expand Up @@ -241,10 +271,20 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
lock_op: &OpTy<'tcx>,
lock_layout: TyAndLayout<'tcx>,
offset: u64,
initialize_data: impl FnOnce() -> 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())
.ok_or_else(|| err_ub_format!("mutex has invalid ID").into(), initialize_data)
}

/// 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 Down
5 changes: 4 additions & 1 deletion 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/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
44 changes: 41 additions & 3 deletions src/shims/unix/sync.rs
Original file line number Diff line number Diff line change
Expand Up @@ -118,13 +118,29 @@ fn mutex_get_id<'tcx>(
ecx: &mut MiriInterpCx<'tcx>,
mutex_op: &OpTy<'tcx>,
) -> InterpResult<'tcx, MutexId> {
let address = ecx.deref_pointer_as(mutex_op, ecx.libc_ty_layout("pthread_mutex_t"))?.ptr();
let kind = MutexKind::new(mutex_get_kind(ecx, mutex_op)?);
ecx.mutex_get_or_create_id(
mutex_op,
ecx.libc_ty_layout("pthread_mutex_t"),
mutex_id_offset(ecx)?,
|| Ok(Some(AdditionalMutexData { kind, address })),
)
}

fn mutex_check_moved<'tcx>(
ecx: &mut MiriInterpCx<'tcx>,
mutex_op: &OpTy<'tcx>,
id: MutexId,
) -> InterpResult<'tcx, ()> {
let addr = ecx.deref_pointer_as(mutex_op, ecx.libc_ty_layout("pthread_mutex_t"))?.ptr().addr();
let data = ecx.mutex_get_data(id).expect("data should be always exist for pthreads");
if data.address.addr() != addr {
throw_ub_format!("pthread_mutex_t can't be moved after first use")
}
Ok(())
}

fn mutex_reset_id<'tcx>(
ecx: &mut MiriInterpCx<'tcx>,
mutex_op: &OpTy<'tcx>,
Expand Down Expand Up @@ -457,6 +473,10 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {

mutex_set_kind(this, mutex_op, kind)?;

// Fetch (and ignore) the mutex's id to go through the initialization
// code, and thus register the mutex's address.
mutex_get_id(this, mutex_op)?;

Ok(())
}

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

let kind = mutex_get_kind(this, mutex_op)?;
let id = mutex_get_id(this, mutex_op)?;
let kind = this
.mutex_get_data(id)
.expect("data should always exist for pthread mutexes")
.kind
.to_i32();

mutex_check_moved(this, mutex_op, id)?;

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

let kind = mutex_get_kind(this, mutex_op)?;
let id = mutex_get_id(this, mutex_op)?;
let kind = this
.mutex_get_data(id)
.expect("data should always exist for pthread mutexes")
.kind
.to_i32();

mutex_check_moved(this, mutex_op, id)?;

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

let kind = mutex_get_kind(this, mutex_op)?;
let id = mutex_get_id(this, mutex_op)?;
let kind = this
.mutex_get_data(id)
.expect("data should always exist for pthread mutexes")
.kind
.to_i32();

mutex_check_moved(this, mutex_op, id)?;

if let Some(_old_locked_count) = this.mutex_unlock(id)? {
// The mutex was locked by the current thread.
Expand Down
20 changes: 20 additions & 0 deletions tests/fail-dep/concurrency/libc_pthread_mutex_move.init.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
error: Undefined Behavior: pthread_mutex_t can't be moved after first use
--> $DIR/libc_pthread_mutex_move.rs:LL:CC
|
LL | libc::pthread_mutex_lock(&mut m2 as *mut _);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ pthread_mutex_t can't be moved after first use
|
= help: this indicates a bug in the program: it performed an invalid operation, and caused Undefined Behavior
= help: see https://doc.rust-lang.org/nightly/reference/behavior-considered-undefined.html for further information
= note: BACKTRACE:
= note: inside `check` at $DIR/libc_pthread_mutex_move.rs:LL:CC
note: inside `main`
--> $DIR/libc_pthread_mutex_move.rs:LL:CC
|
LL | check();
| ^^^^^^^

note: some details are omitted, run with `MIRIFLAGS=-Zmiri-backtrace=full` for a verbose backtrace

error: aborting due to 1 previous error

20 changes: 20 additions & 0 deletions tests/fail-dep/concurrency/libc_pthread_mutex_move.lock.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
error: Undefined Behavior: pthread_mutex_t can't be moved after first use
--> $DIR/libc_pthread_mutex_move.rs:LL:CC
|
LL | libc::pthread_mutex_lock(&mut m2 as *mut _);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ pthread_mutex_t can't be moved after first use
|
= help: this indicates a bug in the program: it performed an invalid operation, and caused Undefined Behavior
= help: see https://doc.rust-lang.org/nightly/reference/behavior-considered-undefined.html for further information
= note: BACKTRACE:
= note: inside `check` at $DIR/libc_pthread_mutex_move.rs:LL:CC
note: inside `main`
--> $DIR/libc_pthread_mutex_move.rs:LL:CC
|
LL | check();
| ^^^^^^^

note: some details are omitted, run with `MIRIFLAGS=-Zmiri-backtrace=full` for a verbose backtrace

error: aborting due to 1 previous error

52 changes: 52 additions & 0 deletions tests/fail-dep/concurrency/libc_pthread_mutex_move.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
//@ignore-target-windows: No pthreads on Windows
//@revisions: lock trylock unlock init

fn main() {
check();
}

#[cfg(init)]
fn check() {
unsafe {
let mut m: libc::pthread_mutex_t = std::mem::zeroed();
assert_eq!(libc::pthread_mutex_init(&mut m as *mut _, std::ptr::null()), 0);

let mut m2 = m;
libc::pthread_mutex_lock(&mut m2 as *mut _); //~[init] ERROR: pthread_mutex_t can't be moved after first use
}
}

#[cfg(lock)]
fn check() {
unsafe {
let mut m: libc::pthread_mutex_t = libc::PTHREAD_MUTEX_INITIALIZER;
libc::pthread_mutex_lock(&mut m as *mut _);
// libc::pthread_mutex_unlock(&mut m as *mut _);

let mut m2 = m;
libc::pthread_mutex_lock(&mut m2 as *mut _); //~[lock] ERROR: pthread_mutex_t can't be moved after first use
}
}

#[cfg(trylock)]
fn check() {
unsafe {
let mut m: libc::pthread_mutex_t = libc::PTHREAD_MUTEX_INITIALIZER;
libc::pthread_mutex_trylock(&mut m as *mut _);
// libc::pthread_mutex_unlock(&mut m as *mut _);

let mut m2 = m;
libc::pthread_mutex_trylock(&mut m2 as *mut _); //~[trylock] ERROR: pthread_mutex_t can't be moved after first use
}
}

#[cfg(unlock)]
fn check() {
unsafe {
let mut m: libc::pthread_mutex_t = libc::PTHREAD_RECURSIVE_MUTEX_INITIALIZER_NP;
libc::pthread_mutex_unlock(&mut m as *mut _);

let mut m2 = m;
libc::pthread_mutex_unlock(&mut m2 as *mut _); //~[unlock] ERROR: pthread_mutex_t can't be moved after first use
}
}
15 changes: 15 additions & 0 deletions tests/fail-dep/concurrency/libc_pthread_mutex_move.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
error: Undefined Behavior: pthread_mutex_t can't be moved
--> $DIR/libc_pthread_mutex_move.rs:LL:CC
|
LL | libc::pthread_mutex_lock(&mut mutex2 as *mut _);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ pthread_mutex_t can't be moved
|
= help: this indicates a bug in the program: it performed an invalid operation, and caused Undefined Behavior
= help: see https://doc.rust-lang.org/nightly/reference/behavior-considered-undefined.html for further information
= note: BACKTRACE:
= note: inside `main` at $DIR/libc_pthread_mutex_move.rs:LL:CC

note: some details are omitted, run with `MIRIFLAGS=-Zmiri-backtrace=full` for a verbose backtrace

error: aborting due to 1 previous error

20 changes: 20 additions & 0 deletions tests/fail-dep/concurrency/libc_pthread_mutex_move.trylock.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
error: Undefined Behavior: pthread_mutex_t can't be moved after first use
--> $DIR/libc_pthread_mutex_move.rs:LL:CC
|
LL | libc::pthread_mutex_trylock(&mut m2 as *mut _);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ pthread_mutex_t can't be moved after first use
|
= help: this indicates a bug in the program: it performed an invalid operation, and caused Undefined Behavior
= help: see https://doc.rust-lang.org/nightly/reference/behavior-considered-undefined.html for further information
= note: BACKTRACE:
= note: inside `check` at $DIR/libc_pthread_mutex_move.rs:LL:CC
note: inside `main`
--> $DIR/libc_pthread_mutex_move.rs:LL:CC
|
LL | check();
| ^^^^^^^

note: some details are omitted, run with `MIRIFLAGS=-Zmiri-backtrace=full` for a verbose backtrace

error: aborting due to 1 previous error

20 changes: 20 additions & 0 deletions tests/fail-dep/concurrency/libc_pthread_mutex_move.unlock.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
error: Undefined Behavior: pthread_mutex_t can't be moved after first use
--> $DIR/libc_pthread_mutex_move.rs:LL:CC
|
LL | libc::pthread_mutex_unlock(&mut m2 as *mut _);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ pthread_mutex_t can't be moved after first use
|
= help: this indicates a bug in the program: it performed an invalid operation, and caused Undefined Behavior
= help: see https://doc.rust-lang.org/nightly/reference/behavior-considered-undefined.html for further information
= note: BACKTRACE:
= note: inside `check` at $DIR/libc_pthread_mutex_move.rs:LL:CC
note: inside `main`
--> $DIR/libc_pthread_mutex_move.rs:LL:CC
|
LL | check();
| ^^^^^^^

note: some details are omitted, run with `MIRIFLAGS=-Zmiri-backtrace=full` for a verbose backtrace

error: aborting due to 1 previous error

0 comments on commit d84ce6b

Please sign in to comment.