From 7660ddefe98feeaaff64f573d36245def048baa9 Mon Sep 17 00:00:00 2001 From: joboet Date: Sat, 13 Jul 2024 18:25:41 +0200 Subject: [PATCH] implement the `os_unfair_lock` functions on macOS --- src/concurrency/sync.rs | 21 +++- src/provenance_gc.rs | 11 ++ src/shims/unix/macos/foreign_items.rs | 22 ++++ src/shims/unix/macos/mod.rs | 1 + src/shims/unix/macos/sync.rs | 107 ++++++++++++++++++ src/shims/unix/sync.rs | 2 +- .../apple_os_unfair_lock_assert_not_owner.rs | 13 +++ ...ple_os_unfair_lock_assert_not_owner.stderr | 13 +++ .../apple_os_unfair_lock_assert_owner.rs | 12 ++ .../apple_os_unfair_lock_assert_owner.stderr | 13 +++ .../apple_os_unfair_lock_reentrant.rs | 13 +++ .../apple_os_unfair_lock_reentrant.stderr | 13 +++ .../apple_os_unfair_lock_unowned.rs | 12 ++ .../apple_os_unfair_lock_unowned.stderr | 13 +++ .../concurrency/apple-os-unfair-lock.rs | 25 ++++ 15 files changed, 284 insertions(+), 7 deletions(-) create mode 100644 src/shims/unix/macos/sync.rs create mode 100644 tests/fail-dep/concurrency/apple_os_unfair_lock_assert_not_owner.rs create mode 100644 tests/fail-dep/concurrency/apple_os_unfair_lock_assert_not_owner.stderr create mode 100644 tests/fail-dep/concurrency/apple_os_unfair_lock_assert_owner.rs create mode 100644 tests/fail-dep/concurrency/apple_os_unfair_lock_assert_owner.stderr create mode 100644 tests/fail-dep/concurrency/apple_os_unfair_lock_reentrant.rs create mode 100644 tests/fail-dep/concurrency/apple_os_unfair_lock_reentrant.stderr create mode 100644 tests/fail-dep/concurrency/apple_os_unfair_lock_unowned.rs create mode 100644 tests/fail-dep/concurrency/apple_os_unfair_lock_unowned.stderr create mode 100644 tests/pass-dep/concurrency/apple-os-unfair-lock.rs diff --git a/src/concurrency/sync.rs b/src/concurrency/sync.rs index 91865a2192..d0c9a4600e 100644 --- a/src/concurrency/sync.rs +++ b/src/concurrency/sync.rs @@ -269,7 +269,7 @@ pub(super) trait EvalContextExtPriv<'tcx>: crate::MiriInterpCxExt<'tcx> { let this = self.eval_context_mut(); if this.mutex_is_locked(mutex) { assert_ne!(this.mutex_get_owner(mutex), this.active_thread()); - this.mutex_enqueue_and_block(mutex, retval, dest); + this.mutex_enqueue_and_block(mutex, Some((retval, dest))); } else { // We can have it right now! this.mutex_lock(mutex); @@ -390,9 +390,15 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { } /// Put the thread into the queue waiting for the mutex. - /// Once the Mutex becomes available, `retval` will be written to `dest`. + /// + /// Once the Mutex becomes available and if it exists, `retval_dest.0` will + /// be written to `retval_dest.1`. #[inline] - fn mutex_enqueue_and_block(&mut self, id: MutexId, retval: Scalar, dest: MPlaceTy<'tcx>) { + fn mutex_enqueue_and_block( + &mut self, + id: MutexId, + retval_dest: Option<(Scalar, MPlaceTy<'tcx>)>, + ) { let this = self.eval_context_mut(); assert!(this.mutex_is_locked(id), "queing on unlocked mutex"); let thread = this.active_thread(); @@ -403,13 +409,16 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { callback!( @capture<'tcx> { id: MutexId, - retval: Scalar, - dest: MPlaceTy<'tcx>, + retval_dest: Option<(Scalar, MPlaceTy<'tcx>)>, } @unblock = |this| { assert!(!this.mutex_is_locked(id)); this.mutex_lock(id); - this.write_scalar(retval, &dest)?; + + if let Some((retval, dest)) = retval_dest { + this.write_scalar(retval, &dest)?; + } + Ok(()) } ), diff --git a/src/provenance_gc.rs b/src/provenance_gc.rs index af980ca481..8edd80744d 100644 --- a/src/provenance_gc.rs +++ b/src/provenance_gc.rs @@ -30,6 +30,17 @@ impl VisitProvenance for Option { } } +impl VisitProvenance for (A, B) +where + A: VisitProvenance, + B: VisitProvenance, +{ + fn visit_provenance(&self, visit: &mut VisitWith<'_>) { + self.0.visit_provenance(visit); + self.1.visit_provenance(visit); + } +} + impl VisitProvenance for std::cell::RefCell { fn visit_provenance(&self, visit: &mut VisitWith<'_>) { self.borrow().visit_provenance(visit) diff --git a/src/shims/unix/macos/foreign_items.rs b/src/shims/unix/macos/foreign_items.rs index aefb5b2de5..47b887dec9 100644 --- a/src/shims/unix/macos/foreign_items.rs +++ b/src/shims/unix/macos/foreign_items.rs @@ -1,6 +1,7 @@ use rustc_span::Symbol; use rustc_target::spec::abi::Abi; +use super::sync::EvalContextExt as _; use crate::shims::unix::*; use crate::*; @@ -174,6 +175,27 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { this.write_scalar(res, dest)?; } + "os_unfair_lock_lock" => { + let [lock_op] = this.check_shim(abi, Abi::C { unwind: false }, link_name, args)?; + this.os_unfair_lock_lock(lock_op)?; + } + "os_unfair_lock_trylock" => { + let [lock_op] = this.check_shim(abi, Abi::C { unwind: false }, link_name, args)?; + this.os_unfair_lock_trylock(lock_op, dest)?; + } + "os_unfair_lock_unlock" => { + let [lock_op] = this.check_shim(abi, Abi::C { unwind: false }, link_name, args)?; + this.os_unfair_lock_unlock(lock_op)?; + } + "os_unfair_lock_assert_owner" => { + let [lock_op] = this.check_shim(abi, Abi::C { unwind: false }, link_name, args)?; + this.os_unfair_lock_assert_owner(lock_op)?; + } + "os_unfair_lock_assert_not_owner" => { + let [lock_op] = this.check_shim(abi, Abi::C { unwind: false }, link_name, args)?; + this.os_unfair_lock_assert_not_owner(lock_op)?; + } + _ => return Ok(EmulateItemResult::NotSupported), }; diff --git a/src/shims/unix/macos/mod.rs b/src/shims/unix/macos/mod.rs index 09c6507b24..50fb2b9d32 100644 --- a/src/shims/unix/macos/mod.rs +++ b/src/shims/unix/macos/mod.rs @@ -1 +1,2 @@ pub mod foreign_items; +pub mod sync; diff --git a/src/shims/unix/macos/sync.rs b/src/shims/unix/macos/sync.rs new file mode 100644 index 0000000000..5e5fccb587 --- /dev/null +++ b/src/shims/unix/macos/sync.rs @@ -0,0 +1,107 @@ +//! Contains macOS-specific synchronization functions. +//! +//! For `os_unfair_lock`, see the documentation +//! +//! and in case of underspecification its implementation +//! . +//! +//! Note that we don't emulate every edge-case behaviour of the locks. Notably, +//! we don't abort when locking a lock owned by a thread that has already exited +//! and we do not detect copying of the lock, but macOS doesn't guarantee anything +//! in that case either. + +use crate::*; + +impl<'tcx> EvalContextExtPriv<'tcx> for crate::MiriInterpCx<'tcx> {} +trait EvalContextExtPriv<'tcx>: crate::MiriInterpCxExt<'tcx> { + fn os_unfair_lock_getid(&mut self, lock_op: &OpTy<'tcx>) -> InterpResult<'tcx, MutexId> { + let this = self.eval_context_mut(); + // 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) + } +} + +impl<'tcx> EvalContextExt<'tcx> for crate::MiriInterpCx<'tcx> {} +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)?; + if this.mutex_is_locked(id) { + if this.mutex_get_owner(id) == this.active_thread() { + // Matching the current macOS implementation: abort on reentrant locking. + throw_machine_stop!(TerminationInfo::Abort( + "attempted to lock an os_unfair_lock that is already locked by the current thread".to_owned() + )); + } + + this.mutex_enqueue_and_block(id, None); + } else { + this.mutex_lock(id); + } + + Ok(()) + } + + fn os_unfair_lock_trylock( + &mut self, + lock_op: &OpTy<'tcx>, + dest: &MPlaceTy<'tcx>, + ) -> InterpResult<'tcx> { + let this = self.eval_context_mut(); + + let id = this.os_unfair_lock_getid(lock_op)?; + if this.mutex_is_locked(id) { + // Contrary to the blocking lock function, this does not check for + // reentrancy. + this.write_scalar(Scalar::from_bool(false), dest)?; + } else { + this.mutex_lock(id); + this.write_scalar(Scalar::from_bool(true), dest)?; + } + + Ok(()) + } + + 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)?; + if this.mutex_unlock(id)?.is_none() { + // Matching the current macOS implementation: abort. + throw_machine_stop!(TerminationInfo::Abort( + "attempted to unlock an os_unfair_lock not owned by the current thread".to_owned() + )); + } + + 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)?; + 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() + )); + } + + 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)?; + 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() + )); + } + + Ok(()) + } +} diff --git a/src/shims/unix/sync.rs b/src/shims/unix/sync.rs index be6732b1b6..e8653117ae 100644 --- a/src/shims/unix/sync.rs +++ b/src/shims/unix/sync.rs @@ -473,7 +473,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { let ret = if this.mutex_is_locked(id) { let owner_thread = this.mutex_get_owner(id); if owner_thread != this.active_thread() { - this.mutex_enqueue_and_block(id, Scalar::from_i32(0), dest.clone()); + this.mutex_enqueue_and_block(id, Some((Scalar::from_i32(0), dest.clone()))); return Ok(()); } else { // Trying to acquire the same mutex again. diff --git a/tests/fail-dep/concurrency/apple_os_unfair_lock_assert_not_owner.rs b/tests/fail-dep/concurrency/apple_os_unfair_lock_assert_not_owner.rs new file mode 100644 index 0000000000..d6604f3713 --- /dev/null +++ b/tests/fail-dep/concurrency/apple_os_unfair_lock_assert_not_owner.rs @@ -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()); + libc::os_unfair_lock_assert_not_owner(lock.get()); + //~^ error: abnormal termination: called os_unfair_lock_assert_not_owner on an os_unfair_lock owned by the current thread + } +} diff --git a/tests/fail-dep/concurrency/apple_os_unfair_lock_assert_not_owner.stderr b/tests/fail-dep/concurrency/apple_os_unfair_lock_assert_not_owner.stderr new file mode 100644 index 0000000000..7e890681c4 --- /dev/null +++ b/tests/fail-dep/concurrency/apple_os_unfair_lock_assert_not_owner.stderr @@ -0,0 +1,13 @@ +error: abnormal termination: called os_unfair_lock_assert_not_owner on an os_unfair_lock owned by the current thread + --> $DIR/apple_os_unfair_lock_assert_not_owner.rs:LL:CC + | +LL | libc::os_unfair_lock_assert_not_owner(lock.get()); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ called os_unfair_lock_assert_not_owner on an os_unfair_lock owned by the current thread + | + = note: BACKTRACE: + = note: inside `main` at $DIR/apple_os_unfair_lock_assert_not_owner.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 + diff --git a/tests/fail-dep/concurrency/apple_os_unfair_lock_assert_owner.rs b/tests/fail-dep/concurrency/apple_os_unfair_lock_assert_owner.rs new file mode 100644 index 0000000000..ddd8b572ea --- /dev/null +++ b/tests/fail-dep/concurrency/apple_os_unfair_lock_assert_owner.rs @@ -0,0 +1,12 @@ +//@ only-target-darwin + +use std::cell::UnsafeCell; + +fn main() { + let lock = UnsafeCell::new(libc::OS_UNFAIR_LOCK_INIT); + + unsafe { + libc::os_unfair_lock_assert_owner(lock.get()); + //~^ error: abnormal termination: called os_unfair_lock_assert_owner on an os_unfair_lock not owned by the current thread + } +} diff --git a/tests/fail-dep/concurrency/apple_os_unfair_lock_assert_owner.stderr b/tests/fail-dep/concurrency/apple_os_unfair_lock_assert_owner.stderr new file mode 100644 index 0000000000..3724f7996f --- /dev/null +++ b/tests/fail-dep/concurrency/apple_os_unfair_lock_assert_owner.stderr @@ -0,0 +1,13 @@ +error: abnormal termination: called os_unfair_lock_assert_owner on an os_unfair_lock not owned by the current thread + --> $DIR/apple_os_unfair_lock_assert_owner.rs:LL:CC + | +LL | libc::os_unfair_lock_assert_owner(lock.get()); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ called os_unfair_lock_assert_owner on an os_unfair_lock not owned by the current thread + | + = note: BACKTRACE: + = note: inside `main` at $DIR/apple_os_unfair_lock_assert_owner.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 + diff --git a/tests/fail-dep/concurrency/apple_os_unfair_lock_reentrant.rs b/tests/fail-dep/concurrency/apple_os_unfair_lock_reentrant.rs new file mode 100644 index 0000000000..eb98adeba0 --- /dev/null +++ b/tests/fail-dep/concurrency/apple_os_unfair_lock_reentrant.rs @@ -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()); + libc::os_unfair_lock_lock(lock.get()); + //~^ error: abnormal termination: attempted to lock an os_unfair_lock that is already locked by the current thread + } +} diff --git a/tests/fail-dep/concurrency/apple_os_unfair_lock_reentrant.stderr b/tests/fail-dep/concurrency/apple_os_unfair_lock_reentrant.stderr new file mode 100644 index 0000000000..644462a1b0 --- /dev/null +++ b/tests/fail-dep/concurrency/apple_os_unfair_lock_reentrant.stderr @@ -0,0 +1,13 @@ +error: abnormal termination: attempted to lock an os_unfair_lock that is already locked by the current thread + --> $DIR/apple_os_unfair_lock_reentrant.rs:LL:CC + | +LL | libc::os_unfair_lock_lock(lock.get()); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ attempted to lock an os_unfair_lock that is already locked by the current thread + | + = note: BACKTRACE: + = note: inside `main` at $DIR/apple_os_unfair_lock_reentrant.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 + diff --git a/tests/fail-dep/concurrency/apple_os_unfair_lock_unowned.rs b/tests/fail-dep/concurrency/apple_os_unfair_lock_unowned.rs new file mode 100644 index 0000000000..aed467552a --- /dev/null +++ b/tests/fail-dep/concurrency/apple_os_unfair_lock_unowned.rs @@ -0,0 +1,12 @@ +//@ only-target-darwin + +use std::cell::UnsafeCell; + +fn main() { + let lock = UnsafeCell::new(libc::OS_UNFAIR_LOCK_INIT); + + unsafe { + libc::os_unfair_lock_unlock(lock.get()); + //~^ error: abnormal termination: attempted to unlock an os_unfair_lock not owned by the current thread + } +} diff --git a/tests/fail-dep/concurrency/apple_os_unfair_lock_unowned.stderr b/tests/fail-dep/concurrency/apple_os_unfair_lock_unowned.stderr new file mode 100644 index 0000000000..6a8d12fa80 --- /dev/null +++ b/tests/fail-dep/concurrency/apple_os_unfair_lock_unowned.stderr @@ -0,0 +1,13 @@ +error: abnormal termination: attempted to unlock an os_unfair_lock not owned by the current thread + --> $DIR/apple_os_unfair_lock_unowned.rs:LL:CC + | +LL | libc::os_unfair_lock_unlock(lock.get()); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ attempted to unlock an os_unfair_lock not owned by the current thread + | + = note: BACKTRACE: + = note: inside `main` at $DIR/apple_os_unfair_lock_unowned.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 + diff --git a/tests/pass-dep/concurrency/apple-os-unfair-lock.rs b/tests/pass-dep/concurrency/apple-os-unfair-lock.rs new file mode 100644 index 0000000000..c2b9c37bbf --- /dev/null +++ b/tests/pass-dep/concurrency/apple-os-unfair-lock.rs @@ -0,0 +1,25 @@ +//@ 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()); + libc::os_unfair_lock_assert_owner(lock.get()); + assert!(!libc::os_unfair_lock_trylock(lock.get())); + libc::os_unfair_lock_unlock(lock.get()); + + libc::os_unfair_lock_assert_not_owner(lock.get()); + } + + // `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. + let lock = lock; + let locked = unsafe { libc::os_unfair_lock_trylock(lock.get()) }; + assert!(locked); + let _lock = lock; +}