Skip to content

Commit

Permalink
Auto merge of rust-lang#3745 - joboet:os_unfair_lock, r=RalfJung
Browse files Browse the repository at this point in the history
  • Loading branch information
bors committed Jul 14, 2024
2 parents 5f99349 + 32221c3 commit e90f047
Show file tree
Hide file tree
Showing 15 changed files with 284 additions and 7 deletions.
21 changes: 15 additions & 6 deletions src/tools/miri/src/concurrency/sync.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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();
Expand All @@ -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(())
}
),
Expand Down
11 changes: 11 additions & 0 deletions src/tools/miri/src/provenance_gc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,17 @@ impl<T: VisitProvenance> VisitProvenance for Option<T> {
}
}

impl<A, B> 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<T: VisitProvenance> VisitProvenance for std::cell::RefCell<T> {
fn visit_provenance(&self, visit: &mut VisitWith<'_>) {
self.borrow().visit_provenance(visit)
Expand Down
22 changes: 22 additions & 0 deletions src/tools/miri/src/shims/unix/macos/foreign_items.rs
Original file line number Diff line number Diff line change
@@ -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::*;

Expand Down Expand Up @@ -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),
};

Expand Down
1 change: 1 addition & 0 deletions src/tools/miri/src/shims/unix/macos/mod.rs
Original file line number Diff line number Diff line change
@@ -1 +1,2 @@
pub mod foreign_items;
pub mod sync;
107 changes: 107 additions & 0 deletions src/tools/miri/src/shims/unix/macos/sync.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,107 @@
//! Contains macOS-specific synchronization functions.
//!
//! For `os_unfair_lock`, see the documentation
//! <https://developer.apple.com/documentation/os/synchronization?language=objc>
//! and in case of underspecification its implementation
//! <https://github.com/apple-oss-distributions/libplatform/blob/a00a4cc36da2110578bcf3b8eeeeb93dcc7f4e11/src/os/lock.c#L645>.
//!
//! 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(())
}
}
2 changes: 1 addition & 1 deletion src/tools/miri/src/shims/unix/sync.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
Original file line number Diff line number Diff line change
@@ -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
}
}
Original file line number Diff line number Diff line change
@@ -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

Original file line number Diff line number Diff line change
@@ -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
}
}
Original file line number Diff line number Diff line change
@@ -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

Original file line number Diff line number Diff line change
@@ -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
}
}
Original file line number Diff line number Diff line change
@@ -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

Original file line number Diff line number Diff line change
@@ -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
}
}
Original file line number Diff line number Diff line change
@@ -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

25 changes: 25 additions & 0 deletions src/tools/miri/tests/pass-dep/concurrency/apple-os-unfair-lock.rs
Original file line number Diff line number Diff line change
@@ -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;
}

0 comments on commit e90f047

Please sign in to comment.