Skip to content

Commit

Permalink
detect when pthread_cond_t is moved
Browse files Browse the repository at this point in the history
Closes #3749
  • Loading branch information
Mandragorian committed Sep 13, 2024
1 parent 11da987 commit 62cff38
Show file tree
Hide file tree
Showing 5 changed files with 138 additions and 5 deletions.
33 changes: 32 additions & 1 deletion src/concurrency/sync.rs
Original file line number Diff line number Diff line change
Expand Up @@ -134,6 +134,9 @@ struct Condvar {
/// Contains the clock of the last thread to
/// perform a condvar-signal.
clock: VClock,

/// Additional data that can be set by shim implementations.
data: Option<Box<dyn Any>>,
}

/// The futex state.
Expand Down Expand Up @@ -344,21 +347,49 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
this.machine.sync.rwlocks[id].data.as_deref().and_then(|p| p.downcast_ref::<T>())
}

/// Eagerly create and initialize a new condvar.
fn condvar_create(
&mut self,
condvar: &MPlaceTy<'tcx>,
offset: u64,
data: Option<Box<dyn Any>>,
) -> InterpResult<'tcx, CondvarId> {
let this = self.eval_context_mut();
this.create_id(
condvar,
offset,
|ecx| &mut ecx.machine.sync.condvars,
Condvar { data, ..Default::default() },
)
}

fn condvar_get_or_create_id(
&mut self,
lock: &MPlaceTy<'tcx>,
offset: u64,
initialize_data: impl for<'a> FnOnce(
&'a mut MiriInterpCx<'tcx>,
) -> InterpResult<'tcx, Option<Box<dyn Any>>>,
) -> InterpResult<'tcx, CondvarId> {
let this = self.eval_context_mut();
this.get_or_create_id(
lock,
offset,
|ecx| &mut ecx.machine.sync.condvars,
|_| Ok(Default::default()),
|ecx| initialize_data(ecx).map(|data| Condvar { data, ..Default::default() }),
)?
.ok_or_else(|| err_ub_format!("condvar has invalid ID").into())
}

/// Retrieve the additional data stored for a condvar.
fn condvar_get_data<'a, T: 'static>(&'a mut self, id: CondvarId) -> Option<&'a T>
where
'tcx: 'a,
{
let this = self.eval_context_ref();
this.machine.sync.condvars[id].data.as_deref().and_then(|p| p.downcast_ref::<T>())
}

#[inline]
/// Get the id of the thread that currently owns this lock.
fn mutex_get_owner(&mut self, id: MutexId) -> ThreadId {
Expand Down
33 changes: 29 additions & 4 deletions src/shims/unix/sync.rs
Original file line number Diff line number Diff line change
Expand Up @@ -206,7 +206,7 @@ fn translate_kind<'tcx>(ecx: &MiriInterpCx<'tcx>, kind: i32) -> InterpResult<'tc
// - id: u32

#[derive(Debug)]
/// Additional data that may be used by shim implementations.
/// Additional data that we attach with each rwlock instance.
pub struct AdditionalRwLockData {
/// The address of the rwlock.
pub address: u64,
Expand Down Expand Up @@ -364,12 +364,32 @@ fn cond_clock_offset<'tcx>(ecx: &MiriInterpCx<'tcx>) -> u64 {
offset
}

#[derive(Debug)]
/// Additional data that we attach with each cond instance.
struct AdditionalCondData {
/// The address of the cond.
address: u64,
}

fn cond_get_id<'tcx>(
ecx: &mut MiriInterpCx<'tcx>,
cond_ptr: &OpTy<'tcx>,
) -> InterpResult<'tcx, CondvarId> {
let cond = ecx.deref_pointer(cond_ptr)?;
ecx.condvar_get_or_create_id(&cond, cond_id_offset(ecx)?)
let address = cond.ptr().addr().bytes();
let id = ecx.condvar_get_or_create_id(&cond, cond_id_offset(ecx)?, |_| {
Ok(Some(Box::new(AdditionalCondData { address })))
})?;

// Check that the mutex has not been moved since last use.
let data = ecx
.condvar_get_data::<AdditionalCondData>(id)
.expect("data should always exist for pthreads");
if data.address != address {
throw_ub_format!("pthread_cond_t can't be moved after first use")
}

Ok(id)
}

fn cond_reset_id<'tcx>(
Expand Down Expand Up @@ -821,8 +841,13 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
condattr_get_clock_id(this, attr_op)?
};

// Write 0 to use the same code path as the static initializers.
cond_reset_id(this, cond_op)?;
let cond = this.deref_pointer(cond_op)?;
let address = cond.ptr().addr().bytes();
this.condvar_create(
&cond,
cond_id_offset(this)?,
Some(Box::new(AdditionalCondData { address })),
)?;

cond_set_clock_id(this, cond_op, clock_id)?;

Expand Down
20 changes: 20 additions & 0 deletions tests/fail-dep/concurrency/libc_pthread_cond_move.init.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
error: Undefined Behavior: pthread_cond_t can't be moved after first use
--> $DIR/libc_pthread_cond_move.rs:LL:CC
|
LL | libc::pthread_cond_destroy(cond2.as_mut_ptr());
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ pthread_cond_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_cond_move.rs:LL:CC
note: inside `main`
--> $DIR/libc_pthread_cond_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

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

/// Test that moving a pthread_cond between uses fails.

fn main() {
check()
}

#[cfg(init)]
fn check() {
unsafe {
use core::mem::MaybeUninit;
let mut cond = MaybeUninit::<libc::pthread_cond_t>::uninit();

libc::pthread_cond_init(cond.as_mut_ptr(), std::ptr::null());

// move pthread_cond_t
let mut cond2 = cond;

libc::pthread_cond_destroy(cond2.as_mut_ptr()); //~[init] ERROR: pthread_cond_t can't be moved after first use
}
}

#[cfg(static_initializer)]
fn check() {
unsafe {
let mut cond = libc::PTHREAD_COND_INITIALIZER;

libc::pthread_cond_signal(&mut cond as *mut _);

// move pthread_cond_t
let mut cond2 = cond;

libc::pthread_cond_destroy(&mut cond2 as *mut _); //~[static_initializer] ERROR: pthread_cond_t can't be moved after first use
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
error: Undefined Behavior: pthread_cond_t can't be moved after first use
--> $DIR/libc_pthread_cond_move.rs:LL:CC
|
LL | libc::pthread_cond_destroy(&mut cond2 as *mut _);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ pthread_cond_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_cond_move.rs:LL:CC
note: inside `main`
--> $DIR/libc_pthread_cond_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 62cff38

Please sign in to comment.