Skip to content

Commit

Permalink
Rollup merge of rust-lang#66705 - pitdicker:atomic_mut_ptr, r=KodrAus
Browse files Browse the repository at this point in the history
Atomic as_mut_ptr

I encountered the following pattern a few times: In Rust we use some atomic type like `AtomicI32`, and an FFI interface exposes this as `*mut i32` (or some similar `libc` type).

It was not obvious to me if a just transmuting a pointer to the atomic was acceptable, or if this should use a cast that goes through an `UnsafeCell`. See rust-lang#66136 (comment)

Transmuting the pointer directly:
```rust
let atomic = AtomicI32::new(1);
let ptr = &atomic as *const AtomicI32 as *mut i32;
unsafe {
    ffi(ptr);
}
```

A dance with `UnsafeCell`:
```rust
let atomic = AtomicI32::new(1);
unsafe {
    let ptr = (&*(&atomic as *const AtomicI32 as *const UnsafeCell<i32>)).get();
    ffi(ptr);
}
```

Maybe in the end both ways could be valid. But why not expose a direct method to get a pointer from the standard library?

An `as_mut_ptr` method on atomics can be safe, because only the use of the resulting pointer is where things can get unsafe. I documented its use for FFI, and "Doing non-atomic reads and writes on the resulting integer can be a data race."

The standard library could make use this method in a few places in the WASM module.

cc @RalfJung as you answered my original question.
  • Loading branch information
Centril authored Nov 30, 2019
2 parents 3af14f9 + d34090a commit 123406c
Show file tree
Hide file tree
Showing 5 changed files with 80 additions and 5 deletions.
74 changes: 74 additions & 0 deletions src/libcore/sync/atomic.rs
Original file line number Diff line number Diff line change
Expand Up @@ -802,6 +802,43 @@ impl AtomicBool {
pub fn fetch_xor(&self, val: bool, order: Ordering) -> bool {
unsafe { atomic_xor(self.v.get(), val as u8, order) != 0 }
}

/// Returns a mutable pointer to the underlying [`bool`].
///
/// Doing non-atomic reads and writes on the resulting integer can be a data race.
/// This method is mostly useful for FFI, where the function signature may use
/// `*mut bool` instead of `&AtomicBool`.
///
/// Returning an `*mut` pointer from a shared reference to this atomic is safe because the
/// atomic types work with interior mutability. All modifications of an atomic change the value
/// through a shared reference, and can do so safely as long as they use atomic operations. Any
/// use of the returned raw pointer requires an `unsafe` block and still has to uphold the same
/// restriction: operations on it must be atomic.
///
/// [`bool`]: ../../../std/primitive.bool.html
///
/// # Examples
///
/// ```ignore (extern-declaration)
/// # fn main() {
/// use std::sync::atomic::AtomicBool;
/// extern {
/// fn my_atomic_op(arg: *mut bool);
/// }
///
/// let mut atomic = AtomicBool::new(true);
/// unsafe {
/// my_atomic_op(atomic.as_mut_ptr());
/// }
/// # }
/// ```
#[inline]
#[unstable(feature = "atomic_mut_ptr",
reason = "recently added",
issue = "66893")]
pub fn as_mut_ptr(&self) -> *mut bool {
self.v.get() as *mut bool
}
}

#[cfg(target_has_atomic_load_store = "ptr")]
Expand Down Expand Up @@ -1891,6 +1928,43 @@ assert_eq!(min_foo, 12);
}
}

doc_comment! {
concat!("Returns a mutable pointer to the underlying integer.
Doing non-atomic reads and writes on the resulting integer can be a data race.
This method is mostly useful for FFI, where the function signature may use
`*mut ", stringify!($int_type), "` instead of `&", stringify!($atomic_type), "`.
Returning an `*mut` pointer from a shared reference to this atomic is safe because the
atomic types work with interior mutability. All modifications of an atomic change the value
through a shared reference, and can do so safely as long as they use atomic operations. Any
use of the returned raw pointer requires an `unsafe` block and still has to uphold the same
restriction: operations on it must be atomic.
# Examples
```ignore (extern-declaration)
# fn main() {
", $extra_feature, "use std::sync::atomic::", stringify!($atomic_type), ";
extern {
fn my_atomic_op(arg: *mut ", stringify!($int_type), ");
}
let mut atomic = ", stringify!($atomic_type), "::new(1);
unsafe {
my_atomic_op(atomic.as_mut_ptr());
}
# }
```"),
#[inline]
#[unstable(feature = "atomic_mut_ptr",
reason = "recently added",
issue = "66893")]
pub fn as_mut_ptr(&self) -> *mut $int_type {
self.v.get()
}
}
}
}
}
Expand Down
1 change: 1 addition & 0 deletions src/libstd/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -234,6 +234,7 @@
#![feature(allocator_internals)]
#![feature(allow_internal_unsafe)]
#![feature(allow_internal_unstable)]
#![feature(atomic_mut_ptr)]
#![feature(arbitrary_self_types)]
#![feature(array_error_internals)]
#![feature(asm)]
Expand Down
4 changes: 2 additions & 2 deletions src/libstd/sys/wasm/alloc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ mod lock {
//
// unsafe {
// let r = core::arch::wasm32::i32_atomic_wait(
// &LOCKED as *const AtomicI32 as *mut i32,
// LOCKED.as_mut_ptr(),
// 1, // expected value
// -1, // timeout
// );
Expand Down Expand Up @@ -143,7 +143,7 @@ mod lock {
//
// unsafe {
// core::arch::wasm32::atomic_notify(
// &LOCKED as *const AtomicI32 as *mut i32,
// LOCKED.as_mut_ptr(),
// 1, // only one thread
// );
// }
Expand Down
2 changes: 1 addition & 1 deletion src/libstd/sys/wasm/condvar_atomics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,6 @@ impl Condvar {
#[inline]
fn ptr(&self) -> *mut i32 {
assert_eq!(mem::size_of::<usize>(), mem::size_of::<i32>());
&self.cnt as *const AtomicUsize as *mut i32
self.cnt.as_mut_ptr() as *mut i32
}
}
4 changes: 2 additions & 2 deletions src/libstd/sys/wasm/mutex_atomics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ impl Mutex {
#[inline]
fn ptr(&self) -> *mut i32 {
assert_eq!(mem::size_of::<usize>(), mem::size_of::<i32>());
&self.locked as *const AtomicUsize as *mut isize as *mut i32
self.locked.as_mut_ptr() as *mut i32
}
}

Expand Down Expand Up @@ -145,6 +145,6 @@ impl ReentrantMutex {

#[inline]
fn ptr(&self) -> *mut i32 {
&self.owner as *const AtomicU32 as *mut i32
self.owner.as_mut_ptr() as *mut i32
}
}

0 comments on commit 123406c

Please sign in to comment.