From 60c182d5dc78fbd9d509e3fa4944c0a99e38b175 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sat, 15 Nov 2025 09:20:49 +0100 Subject: [PATCH] apple os_unfair_lock: throw 'unsupported' when we would behave different from the real implementation --- src/shims/unix/macos/sync.rs | 64 ++++++++++--------- .../apple_os_unfair_lock_move_deadlock.rs | 2 +- .../apple_os_unfair_lock_move_deadlock.stderr | 6 +- 3 files changed, 39 insertions(+), 33 deletions(-) diff --git a/src/shims/unix/macos/sync.rs b/src/shims/unix/macos/sync.rs index d69d373b57..be32ca9abd 100644 --- a/src/shims/unix/macos/sync.rs +++ b/src/shims/unix/macos/sync.rs @@ -20,8 +20,14 @@ use crate::*; #[derive(Clone)] enum MacOsUnfairLock { - Active { mutex_ref: MutexRef }, - PermanentlyLocked, + Active { + mutex_ref: MutexRef, + }, + /// If a lock gets copied while being held, we put it in this state. + /// It seems like in the real implementation, the lock actually remembers who held it, + /// and still behaves as-if it was held by that thread in the new location. In Miri, we don't + /// know who actually owns this lock at the moment. + PermanentlyLockedByUnknown, } impl SyncObj for MacOsUnfairLock { @@ -93,10 +99,12 @@ trait EvalContextExtPriv<'tcx>: crate::MiriInterpCxExt<'tcx> { // locks when they get released, so it got copied while locked. Unfortunately // that is something `std` needs to support (the guard could have been leaked). // On the plus side, we know nobody was queued for the lock while it got copied; - // that would have been rejected by our `on_access`. So we behave like a - // futex-based lock would in this case: any attempt to acquire the lock will - // just wait forever, since there's nobody to wake us up. - interp_ok(MacOsUnfairLock::PermanentlyLocked) + // that would have been rejected by our `on_access`. + // The real implementation would apparently remember who held the old lock, and + // consider them to hold the copy as well -- but our copies don't preserve sync + // object metadata so we instead move the lock into a "permanently locked" + // state. + interp_ok(MacOsUnfairLock::PermanentlyLockedByUnknown) } else { throw_ub_format!("`os_unfair_lock` was not properly initialized at this location, or it got overwritten"); } @@ -303,18 +311,12 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { let this = self.eval_context_mut(); let MacOsUnfairLock::Active { mutex_ref } = 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> {} - |_this, _unblock: UnblockKind| { - panic!("we shouldn't wake up ever") - } - ), + // Trying to lock a perma-locked lock. On macOS this would block or abort depending + // on whether the current thread is considered to be the one holding this lock. We + // don't know who is considered to be holding the lock so we don't know what to do. + throw_unsup_format!( + "attempted to lock an os_unfair_lock that was copied while being locked" ); - return interp_ok(()); }; let mutex_ref = mutex_ref.clone(); @@ -342,15 +344,15 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { let this = self.eval_context_mut(); let MacOsUnfairLock::Active { mutex_ref } = this.os_unfair_lock_get_data(lock_op)? else { - // Trying to get a poisoned lock. That never works. + // Trying to lock a perma-locked lock. That behaves the same no matter who the owner is + // so we can implement the real behavior here. this.write_scalar(Scalar::from_bool(false), dest)?; return interp_ok(()); }; let mutex_ref = mutex_ref.clone(); if mutex_ref.owner().is_some() { - // Contrary to the blocking lock function, this does not check for - // reentrancy. + // Contrary to the blocking lock function, this does not check for reentrancy. this.write_scalar(Scalar::from_bool(false), dest)?; } else { this.mutex_lock(&mutex_ref)?; @@ -364,10 +366,10 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { let this = self.eval_context_mut(); let MacOsUnfairLock::Active { mutex_ref } = this.os_unfair_lock_get_data(lock_op)? else { - // A perma-locked lock is definitely not held by us. - throw_machine_stop!(TerminationInfo::Abort( - "attempted to unlock an os_unfair_lock not owned by the current thread".to_owned() - )); + // We don't know who the owner is so we cannot proceed. + throw_unsup_format!( + "attempted to unlock an os_unfair_lock that was copied while being locked" + ); }; let mutex_ref = mutex_ref.clone(); @@ -393,10 +395,10 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { let this = self.eval_context_mut(); let MacOsUnfairLock::Active { mutex_ref } = this.os_unfair_lock_get_data(lock_op)? else { - // A perma-locked lock is definitely not held by us. - throw_machine_stop!(TerminationInfo::Abort( - "called os_unfair_lock_assert_owner on an os_unfair_lock not owned by the current thread".to_owned() - )); + // We don't know who the owner is so we cannot proceed. + throw_unsup_format!( + "attempted to assert the owner of an os_unfair_lock that was copied while being locked" + ); }; let mutex_ref = mutex_ref.clone(); @@ -415,8 +417,10 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { let this = self.eval_context_mut(); let MacOsUnfairLock::Active { mutex_ref } = this.os_unfair_lock_get_data(lock_op)? else { - // A perma-locked lock is definitely not held by us. - return interp_ok(()); + // We don't know who the owner is so we cannot proceed. + throw_unsup_format!( + "attempted to assert the owner of an os_unfair_lock that was copied while being locked" + ); }; let mutex_ref = mutex_ref.clone(); diff --git a/tests/fail-dep/concurrency/apple_os_unfair_lock_move_deadlock.rs b/tests/fail-dep/concurrency/apple_os_unfair_lock_move_deadlock.rs index 8406143933..b776badcee 100644 --- a/tests/fail-dep/concurrency/apple_os_unfair_lock_move_deadlock.rs +++ b/tests/fail-dep/concurrency/apple_os_unfair_lock_move_deadlock.rs @@ -9,5 +9,5 @@ fn main() { let lock = lock; // This needs to either error or deadlock. unsafe { libc::os_unfair_lock_lock(lock.get()) }; - //~^ error: deadlock + //~^ error: lock an os_unfair_lock that was copied while being locked } diff --git a/tests/fail-dep/concurrency/apple_os_unfair_lock_move_deadlock.stderr b/tests/fail-dep/concurrency/apple_os_unfair_lock_move_deadlock.stderr index 8e7364226e..fdf19a445e 100644 --- a/tests/fail-dep/concurrency/apple_os_unfair_lock_move_deadlock.stderr +++ b/tests/fail-dep/concurrency/apple_os_unfair_lock_move_deadlock.stderr @@ -1,8 +1,10 @@ -error: the evaluated program deadlocked +error: unsupported operation: attempted to lock an os_unfair_lock that was copied while being locked --> tests/fail-dep/concurrency/apple_os_unfair_lock_move_deadlock.rs:LL:CC | LL | unsafe { libc::os_unfair_lock_lock(lock.get()) }; - | ^ this thread got stuck here + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ unsupported operation occurred here + | + = help: this is likely not a bug in the program; it indicates that the program performed an operation that Miri does not support note: some details are omitted, run with `MIRIFLAGS=-Zmiri-backtrace=full` for a verbose backtrace