mirror of
https://github.com/rust-lang/rust.git
synced 2025-02-09 21:42:44 +00:00
ensure that a macOS os_unfair_lock that is moved while being held is not implicitly unlocked
This commit is contained in:
parent
4e14ad6d62
commit
a802fd9c11
@ -223,13 +223,15 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
|
||||
}
|
||||
|
||||
/// Helper for lazily initialized `alloc_extra.sync` data:
|
||||
/// Checks if the primitive is initialized, and return its associated data if so.
|
||||
/// Otherwise, calls `new_data` to initialize the primitive.
|
||||
/// Checks if the primitive is initialized:
|
||||
/// - If yes, fetches the data from `alloc_extra.sync`, or calls `missing_data` if that fails
|
||||
/// and stores that in `alloc_extra.sync`.
|
||||
/// - Otherwise, calls `new_data` to initialize the primitive.
|
||||
fn lazy_sync_get_data<T: 'static + Copy>(
|
||||
&mut self,
|
||||
primitive: &MPlaceTy<'tcx>,
|
||||
init_offset: Size,
|
||||
name: &str,
|
||||
missing_data: impl FnOnce() -> InterpResult<'tcx, T>,
|
||||
new_data: impl FnOnce(&mut MiriInterpCx<'tcx>) -> InterpResult<'tcx, T>,
|
||||
) -> InterpResult<'tcx, T> {
|
||||
let this = self.eval_context_mut();
|
||||
@ -254,11 +256,14 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
|
||||
// If it is initialized, it must be found in the "sync primitive" table,
|
||||
// or else it has been moved illegally.
|
||||
let (alloc, offset, _) = this.ptr_get_alloc_id(primitive.ptr(), 0)?;
|
||||
let alloc_extra = this.get_alloc_extra(alloc)?;
|
||||
let data = alloc_extra
|
||||
.get_sync::<T>(offset)
|
||||
.ok_or_else(|| err_ub_format!("`{name}` can't be moved after first use"))?;
|
||||
interp_ok(*data)
|
||||
let (alloc_extra, _machine) = this.get_alloc_extra_mut(alloc)?;
|
||||
if let Some(data) = alloc_extra.get_sync::<T>(offset) {
|
||||
interp_ok(*data)
|
||||
} else {
|
||||
let data = missing_data()?;
|
||||
alloc_extra.sync.insert(offset, Box::new(data));
|
||||
interp_ok(data)
|
||||
}
|
||||
} else {
|
||||
let data = new_data(this)?;
|
||||
this.lazy_sync_init(primitive, init_offset, data)?;
|
||||
|
@ -59,7 +59,7 @@ macro_rules! callback {
|
||||
@unblock = |$this:ident| $unblock:block
|
||||
) => {
|
||||
callback!(
|
||||
@capture<$tcx, $($lft),*> { $($name: $type),+ }
|
||||
@capture<$tcx, $($lft),*> { $($name: $type),* }
|
||||
@unblock = |$this| $unblock
|
||||
@timeout = |_this| {
|
||||
unreachable!(
|
||||
|
@ -10,24 +10,42 @@
|
||||
//! and we do not detect copying of the lock, but macOS doesn't guarantee anything
|
||||
//! in that case either.
|
||||
|
||||
use rustc_target::abi::Size;
|
||||
|
||||
use crate::*;
|
||||
|
||||
struct MacOsUnfairLock {
|
||||
id: MutexId,
|
||||
#[derive(Copy, Clone)]
|
||||
enum MacOsUnfairLock {
|
||||
Poisoned,
|
||||
Active { id: MutexId },
|
||||
}
|
||||
|
||||
impl<'tcx> EvalContextExtPriv<'tcx> for crate::MiriInterpCx<'tcx> {}
|
||||
trait EvalContextExtPriv<'tcx>: crate::MiriInterpCxExt<'tcx> {
|
||||
fn os_unfair_lock_getid(&mut self, lock_ptr: &OpTy<'tcx>) -> InterpResult<'tcx, MutexId> {
|
||||
fn os_unfair_lock_get_data(
|
||||
&mut self,
|
||||
lock_ptr: &OpTy<'tcx>,
|
||||
) -> InterpResult<'tcx, MacOsUnfairLock> {
|
||||
let this = self.eval_context_mut();
|
||||
let lock = this.deref_pointer(lock_ptr)?;
|
||||
// We store the mutex ID in the `sync` metadata. This means that when the lock is moved,
|
||||
// that's just implicitly creating a new lock at the new location.
|
||||
let data = this.get_sync_or_init(lock.ptr(), |machine| {
|
||||
let id = machine.sync.mutex_create();
|
||||
interp_ok(MacOsUnfairLock { id })
|
||||
})?;
|
||||
interp_ok(data.id)
|
||||
this.lazy_sync_get_data(
|
||||
&lock,
|
||||
Size::ZERO, // offset for init tracking
|
||||
|| {
|
||||
// If we get here, due to how we reset things to zero in `os_unfair_lock_unlock`,
|
||||
// this means the lock was moved while locked. This can happen with a `std` lock,
|
||||
// but then any future attempt to unlock will just deadlock. In practice, terrible
|
||||
// things can probably happen if you swap two locked locks, since they'd wake up
|
||||
// from the wrong queue... we just won't catch all UB of this library API then (we
|
||||
// would need to store some unique identifer in-memory for this, instead of a static
|
||||
// LAZY_INIT_COOKIE). This can't be hit via `std::sync::Mutex`.
|
||||
interp_ok(MacOsUnfairLock::Poisoned)
|
||||
},
|
||||
|ecx| {
|
||||
let id = ecx.machine.sync.mutex_create();
|
||||
interp_ok(MacOsUnfairLock::Active { id })
|
||||
},
|
||||
)
|
||||
}
|
||||
}
|
||||
|
||||
@ -36,7 +54,21 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
|
||||
fn os_unfair_lock_lock(&mut self, lock_op: &OpTy<'tcx>) -> InterpResult<'tcx> {
|
||||
let this = self.eval_context_mut();
|
||||
|
||||
let id = this.os_unfair_lock_getid(lock_op)?;
|
||||
let MacOsUnfairLock::Active { id } = this.os_unfair_lock_get_data(lock_op)? else {
|
||||
// Trying to get a poisoned lock. Just block forever...
|
||||
this.block_thread(
|
||||
BlockReason::Sleep,
|
||||
None,
|
||||
callback!(
|
||||
@capture<'tcx> {}
|
||||
@unblock = |_this| {
|
||||
panic!("we shouldn't wake up ever")
|
||||
}
|
||||
),
|
||||
);
|
||||
return interp_ok(());
|
||||
};
|
||||
|
||||
if this.mutex_is_locked(id) {
|
||||
if this.mutex_get_owner(id) == this.active_thread() {
|
||||
// Matching the current macOS implementation: abort on reentrant locking.
|
||||
@ -60,7 +92,12 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
|
||||
) -> InterpResult<'tcx> {
|
||||
let this = self.eval_context_mut();
|
||||
|
||||
let id = this.os_unfair_lock_getid(lock_op)?;
|
||||
let MacOsUnfairLock::Active { id } = this.os_unfair_lock_get_data(lock_op)? else {
|
||||
// Trying to get a poisoned lock. That never works.
|
||||
this.write_scalar(Scalar::from_bool(false), dest)?;
|
||||
return interp_ok(());
|
||||
};
|
||||
|
||||
if this.mutex_is_locked(id) {
|
||||
// Contrary to the blocking lock function, this does not check for
|
||||
// reentrancy.
|
||||
@ -76,7 +113,14 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
|
||||
fn os_unfair_lock_unlock(&mut self, lock_op: &OpTy<'tcx>) -> InterpResult<'tcx> {
|
||||
let this = self.eval_context_mut();
|
||||
|
||||
let id = this.os_unfair_lock_getid(lock_op)?;
|
||||
let MacOsUnfairLock::Active { id } = this.os_unfair_lock_get_data(lock_op)? else {
|
||||
// The lock is poisoned, who knows who owns it... we'll pretend: someone else.
|
||||
throw_machine_stop!(TerminationInfo::Abort(
|
||||
"attempted to unlock an os_unfair_lock not owned by the current thread".to_owned()
|
||||
));
|
||||
};
|
||||
|
||||
// Now, unlock.
|
||||
if this.mutex_unlock(id)?.is_none() {
|
||||
// Matching the current macOS implementation: abort.
|
||||
throw_machine_stop!(TerminationInfo::Abort(
|
||||
@ -84,32 +128,56 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
|
||||
));
|
||||
}
|
||||
|
||||
// If the lock is not locked by anyone now, it went quer.
|
||||
// Reset to zero so that it can be moved and initialized again for the next phase.
|
||||
if !this.mutex_is_locked(id) {
|
||||
let lock_place = this.deref_pointer_as(lock_op, this.machine.layouts.u32)?;
|
||||
this.write_scalar_atomic(Scalar::from_u32(0), &lock_place, AtomicWriteOrd::Relaxed)?;
|
||||
}
|
||||
|
||||
interp_ok(())
|
||||
}
|
||||
|
||||
fn os_unfair_lock_assert_owner(&mut self, lock_op: &OpTy<'tcx>) -> InterpResult<'tcx> {
|
||||
let this = self.eval_context_mut();
|
||||
|
||||
let id = this.os_unfair_lock_getid(lock_op)?;
|
||||
let MacOsUnfairLock::Active { id } = this.os_unfair_lock_get_data(lock_op)? else {
|
||||
// The lock is poisoned, who knows who owns it... we'll pretend: someone else.
|
||||
throw_machine_stop!(TerminationInfo::Abort(
|
||||
"called os_unfair_lock_assert_owner on an os_unfair_lock not owned by the current thread".to_owned()
|
||||
));
|
||||
};
|
||||
if !this.mutex_is_locked(id) || this.mutex_get_owner(id) != this.active_thread() {
|
||||
throw_machine_stop!(TerminationInfo::Abort(
|
||||
"called os_unfair_lock_assert_owner on an os_unfair_lock not owned by the current thread".to_owned()
|
||||
));
|
||||
}
|
||||
|
||||
// The lock is definitely not quiet since we are the owner.
|
||||
|
||||
interp_ok(())
|
||||
}
|
||||
|
||||
fn os_unfair_lock_assert_not_owner(&mut self, lock_op: &OpTy<'tcx>) -> InterpResult<'tcx> {
|
||||
let this = self.eval_context_mut();
|
||||
|
||||
let id = this.os_unfair_lock_getid(lock_op)?;
|
||||
let MacOsUnfairLock::Active { id } = this.os_unfair_lock_get_data(lock_op)? else {
|
||||
// The lock is poisoned, who knows who owns it... we'll pretend: someone else.
|
||||
return interp_ok(());
|
||||
};
|
||||
if this.mutex_is_locked(id) && this.mutex_get_owner(id) == this.active_thread() {
|
||||
throw_machine_stop!(TerminationInfo::Abort(
|
||||
"called os_unfair_lock_assert_not_owner on an os_unfair_lock owned by the current thread".to_owned()
|
||||
));
|
||||
}
|
||||
|
||||
// If the lock is not locked by anyone now, it went quer.
|
||||
// Reset to zero so that it can be moved and initialized again for the next phase.
|
||||
if !this.mutex_is_locked(id) {
|
||||
let lock_place = this.deref_pointer_as(lock_op, this.machine.layouts.u32)?;
|
||||
this.write_scalar_atomic(Scalar::from_u32(0), &lock_place, AtomicWriteOrd::Relaxed)?;
|
||||
}
|
||||
|
||||
interp_ok(())
|
||||
}
|
||||
}
|
||||
|
@ -189,11 +189,16 @@ fn mutex_get_data<'tcx, 'a>(
|
||||
mutex_ptr: &OpTy<'tcx>,
|
||||
) -> InterpResult<'tcx, PthreadMutex> {
|
||||
let mutex = ecx.deref_pointer(mutex_ptr)?;
|
||||
ecx.lazy_sync_get_data(&mutex, mutex_init_offset(ecx)?, "pthread_mutex_t", |ecx| {
|
||||
let kind = mutex_kind_from_static_initializer(ecx, &mutex)?;
|
||||
let id = ecx.machine.sync.mutex_create();
|
||||
interp_ok(PthreadMutex { id, kind })
|
||||
})
|
||||
ecx.lazy_sync_get_data(
|
||||
&mutex,
|
||||
mutex_init_offset(ecx)?,
|
||||
|| throw_ub_format!("`pthread_mutex_t` can't be moved after first use"),
|
||||
|ecx| {
|
||||
let kind = mutex_kind_from_static_initializer(ecx, &mutex)?;
|
||||
let id = ecx.machine.sync.mutex_create();
|
||||
interp_ok(PthreadMutex { id, kind })
|
||||
},
|
||||
)
|
||||
}
|
||||
|
||||
/// Returns the kind of a static initializer.
|
||||
@ -261,17 +266,22 @@ fn rwlock_get_data<'tcx>(
|
||||
rwlock_ptr: &OpTy<'tcx>,
|
||||
) -> InterpResult<'tcx, PthreadRwLock> {
|
||||
let rwlock = ecx.deref_pointer(rwlock_ptr)?;
|
||||
ecx.lazy_sync_get_data(&rwlock, rwlock_init_offset(ecx)?, "pthread_rwlock_t", |ecx| {
|
||||
if !bytewise_equal_atomic_relaxed(
|
||||
ecx,
|
||||
&rwlock,
|
||||
&ecx.eval_path(&["libc", "PTHREAD_RWLOCK_INITIALIZER"]),
|
||||
)? {
|
||||
throw_unsup_format!("unsupported static initializer used for `pthread_rwlock_t`");
|
||||
}
|
||||
let id = ecx.machine.sync.rwlock_create();
|
||||
interp_ok(PthreadRwLock { id })
|
||||
})
|
||||
ecx.lazy_sync_get_data(
|
||||
&rwlock,
|
||||
rwlock_init_offset(ecx)?,
|
||||
|| throw_ub_format!("`pthread_rwlock_t` can't be moved after first use"),
|
||||
|ecx| {
|
||||
if !bytewise_equal_atomic_relaxed(
|
||||
ecx,
|
||||
&rwlock,
|
||||
&ecx.eval_path(&["libc", "PTHREAD_RWLOCK_INITIALIZER"]),
|
||||
)? {
|
||||
throw_unsup_format!("unsupported static initializer used for `pthread_rwlock_t`");
|
||||
}
|
||||
let id = ecx.machine.sync.rwlock_create();
|
||||
interp_ok(PthreadRwLock { id })
|
||||
},
|
||||
)
|
||||
}
|
||||
|
||||
// # pthread_condattr_t
|
||||
@ -386,18 +396,23 @@ fn cond_get_data<'tcx>(
|
||||
cond_ptr: &OpTy<'tcx>,
|
||||
) -> InterpResult<'tcx, PthreadCondvar> {
|
||||
let cond = ecx.deref_pointer(cond_ptr)?;
|
||||
ecx.lazy_sync_get_data(&cond, cond_init_offset(ecx)?, "pthread_cond_t", |ecx| {
|
||||
if !bytewise_equal_atomic_relaxed(
|
||||
ecx,
|
||||
&cond,
|
||||
&ecx.eval_path(&["libc", "PTHREAD_COND_INITIALIZER"]),
|
||||
)? {
|
||||
throw_unsup_format!("unsupported static initializer used for `pthread_cond_t`");
|
||||
}
|
||||
// This used the static initializer. The clock there is always CLOCK_REALTIME.
|
||||
let id = ecx.machine.sync.condvar_create();
|
||||
interp_ok(PthreadCondvar { id, clock: ClockId::Realtime })
|
||||
})
|
||||
ecx.lazy_sync_get_data(
|
||||
&cond,
|
||||
cond_init_offset(ecx)?,
|
||||
|| throw_ub_format!("`pthread_cond_t` can't be moved after first use"),
|
||||
|ecx| {
|
||||
if !bytewise_equal_atomic_relaxed(
|
||||
ecx,
|
||||
&cond,
|
||||
&ecx.eval_path(&["libc", "PTHREAD_COND_INITIALIZER"]),
|
||||
)? {
|
||||
throw_unsup_format!("unsupported static initializer used for `pthread_cond_t`");
|
||||
}
|
||||
// This used the static initializer. The clock there is always CLOCK_REALTIME.
|
||||
let id = ecx.machine.sync.condvar_create();
|
||||
interp_ok(PthreadCondvar { id, clock: ClockId::Realtime })
|
||||
},
|
||||
)
|
||||
}
|
||||
|
||||
impl<'tcx> EvalContextExt<'tcx> for crate::MiriInterpCx<'tcx> {}
|
||||
|
@ -24,11 +24,16 @@ trait EvalContextExtPriv<'tcx>: crate::MiriInterpCxExt<'tcx> {
|
||||
let init_once = this.deref_pointer(init_once_ptr)?;
|
||||
let init_offset = Size::ZERO;
|
||||
|
||||
this.lazy_sync_get_data(&init_once, init_offset, "INIT_ONCE", |this| {
|
||||
// TODO: check that this is still all-zero.
|
||||
let id = this.machine.sync.init_once_create();
|
||||
interp_ok(WindowsInitOnce { id })
|
||||
})
|
||||
this.lazy_sync_get_data(
|
||||
&init_once,
|
||||
init_offset,
|
||||
|| throw_ub_format!("`INIT_ONCE` can't be moved after first use"),
|
||||
|this| {
|
||||
// TODO: check that this is still all-zero.
|
||||
let id = this.machine.sync.init_once_create();
|
||||
interp_ok(WindowsInitOnce { id })
|
||||
},
|
||||
)
|
||||
}
|
||||
|
||||
/// Returns `true` if we were succssful, `false` if we would block.
|
||||
|
@ -0,0 +1,13 @@
|
||||
//@only-target: darwin
|
||||
|
||||
use std::cell::UnsafeCell;
|
||||
|
||||
fn main() {
|
||||
let lock = UnsafeCell::new(libc::OS_UNFAIR_LOCK_INIT);
|
||||
|
||||
unsafe { libc::os_unfair_lock_lock(lock.get()) };
|
||||
let lock = lock;
|
||||
// This needs to either error or deadlock.
|
||||
unsafe { libc::os_unfair_lock_lock(lock.get()) };
|
||||
//~^ error: deadlock
|
||||
}
|
@ -0,0 +1,13 @@
|
||||
error: deadlock: the evaluated program deadlocked
|
||||
--> tests/fail-dep/concurrency/apple_os_unfair_lock_move_deadlock.rs:LL:CC
|
||||
|
|
||||
LL | unsafe { libc::os_unfair_lock_lock(lock.get()) };
|
||||
| ^ the evaluated program deadlocked
|
||||
|
|
||||
= note: BACKTRACE:
|
||||
= note: inside `main` at tests/fail-dep/concurrency/apple_os_unfair_lock_move_deadlock.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
|
||||
|
@ -16,8 +16,8 @@ fn main() {
|
||||
|
||||
// `os_unfair_lock`s can be moved and leaked.
|
||||
// In the real implementation, even moving it while locked is possible
|
||||
// (and "forks" the lock, i.e. old and new location have independent wait queues);
|
||||
// Miri behavior differs here and anyway none of this is documented.
|
||||
// (and "forks" the lock, i.e. old and new location have independent wait queues).
|
||||
// We only test the somewhat sane case of moving while unlocked that `std` plans to rely on.
|
||||
let lock = lock;
|
||||
let locked = unsafe { libc::os_unfair_lock_trylock(lock.get()) };
|
||||
assert!(locked);
|
||||
|
Loading…
Reference in New Issue
Block a user