Skip to content

Commit

Permalink
Rollup merge of rust-lang#128778 - RalfJung:atomic-read-read-races, r…
Browse files Browse the repository at this point in the history
…=Mark-Simulacrum

atomics: allow atomic and non-atomic reads to race

We currently define our atomics in terms of C++ `atomic_ref`. That has the unfortunate side-effect of making it UB for an atomic and a non-atomic read to race (concretely, [this code](https://play.rust-lang.org/?version=stable&mode=debug&edition=2021&gist=d1a743774e60923db33def7fe314d754) has UB). There's really no good reason for this, all the academic models of the C++ memory model I am aware of allow this -- C++ just disallows this because of their insistence on an "object model" with typed memory, where `atomic_ref` temporarily creates an "atomic object" that may not be accesses via regular non-atomic operations.

So instead of tying our operations to `atomic_ref`, let us tie them directly to the underlying C++ memory model. I am not sure what is the best way to phrase this, so here's a first attempt.

We also carve out an exception from the "no mixed-size atomic accesses" rule to permit mixed-size atomic reads -- given that we permit mixed-size non-atomic reads, it seems odd that this would be disallowed for atomic reads. However, when an atomic write races with any other atomic operation, they must use the same size.

With this change, it is finally the case that every non-atomic access can be replaced by an atomic access without introducing UB.

Cc `@rust-lang/opsem` `@chorman0773` `@m-ou-se` `@WaffleLapkin` `@Amanieu`

Fixes rust-lang/unsafe-code-guidelines#483
  • Loading branch information
matthiaskrgr authored Sep 28, 2024
2 parents 098ada1 + 81fcbcd commit 7a5052a
Show file tree
Hide file tree
Showing 2 changed files with 56 additions and 35 deletions.
10 changes: 6 additions & 4 deletions core/src/cell.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1895,11 +1895,17 @@ impl<T: ?Sized + fmt::Display> fmt::Display for RefMut<'_, T> {
/// uniqueness guarantee for mutable references is unaffected. There is *no* legal way to obtain
/// aliasing `&mut`, not even with `UnsafeCell<T>`.
///
/// `UnsafeCell` does nothing to avoid data races; they are still undefined behavior. If multiple
/// threads have access to the same `UnsafeCell`, they must follow the usual rules of the
/// [concurrent memory model]: conflicting non-synchronized accesses must be done via the APIs in
/// [`core::sync::atomic`].
///
/// The `UnsafeCell` API itself is technically very simple: [`.get()`] gives you a raw pointer
/// `*mut T` to its contents. It is up to _you_ as the abstraction designer to use that raw pointer
/// correctly.
///
/// [`.get()`]: `UnsafeCell::get`
/// [concurrent memory model]: ../sync/atomic/index.html#memory-model-for-atomic-accesses
///
/// The precise Rust aliasing rules are somewhat in flux, but the main points are not contentious:
///
Expand All @@ -1922,10 +1928,6 @@ impl<T: ?Sized + fmt::Display> fmt::Display for RefMut<'_, T> {
/// live memory and the compiler is allowed to insert spurious reads if it can prove that this
/// memory has not yet been deallocated.
///
/// - At all times, you must avoid data races. If multiple threads have access to
/// the same `UnsafeCell`, then any writes must have a proper happens-before relation to all other
/// accesses (or use atomics).
///
/// To assist with proper design, the following scenarios are explicitly declared legal
/// for single-threaded code:
///
Expand Down
81 changes: 50 additions & 31 deletions core/src/sync/atomic.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,25 +24,42 @@
//!
//! ## Memory model for atomic accesses
//!
//! Rust atomics currently follow the same rules as [C++20 atomics][cpp], specifically `atomic_ref`.
//! Basically, creating a *shared reference* to one of the Rust atomic types corresponds to creating
//! an `atomic_ref` in C++; the `atomic_ref` is destroyed when the lifetime of the shared reference
//! ends. A Rust atomic type that is exclusively owned or behind a mutable reference does *not*
//! correspond to an “atomic object” in C++, since the underlying primitive can be mutably accessed,
//! for example with `get_mut`, to perform non-atomic operations.
//! Rust atomics currently follow the same rules as [C++20 atomics][cpp], specifically the rules
//! from the [`intro.races`][cpp-intro.races] section, without the "consume" memory ordering. Since
//! C++ uses an object-based memory model whereas Rust is access-based, a bit of translation work
//! has to be done to apply the C++ rules to Rust: whenever C++ talks about "the value of an
//! object", we understand that to mean the resulting bytes obtained when doing a read. When the C++
//! standard talks about "the value of an atomic object", this refers to the result of doing an
//! atomic load (via the operations provided in this module). A "modification of an atomic object"
//! refers to an atomic store.
//!
//! [cpp]: https://en.cppreference.com/w/cpp/atomic
//! The end result is *almost* equivalent to saying that creating a *shared reference* to one of the
//! Rust atomic types corresponds to creating an `atomic_ref` in C++, with the `atomic_ref` being
//! destroyed when the lifetime of the shared reference ends. The main difference is that Rust
//! permits concurrent atomic and non-atomic reads to the same memory as those cause no issue in the
//! C++ memory model, they are just forbidden in C++ because memory is partitioned into "atomic
//! objects" and "non-atomic objects" (with `atomic_ref` temporarily converting a non-atomic object
//! into an atomic object).
//!
//! The most important aspect of this model is that *data races* are undefined behavior. A data race
//! is defined as conflicting non-synchronized accesses where at least one of the accesses is
//! non-atomic. Here, accesses are *conflicting* if they affect overlapping regions of memory and at
//! least one of them is a write. They are *non-synchronized* if neither of them *happens-before*
//! the other, according to the happens-before order of the memory model.
//!
//! Each method takes an [`Ordering`] which represents the strength of
//! the memory barrier for that operation. These orderings are the
//! same as the [C++20 atomic orderings][1]. For more information see the [nomicon][2].
//! The other possible cause of undefined behavior in the memory model are mixed-size accesses: Rust
//! inherits the C++ limitation that non-synchronized conflicting atomic accesses may not partially
//! overlap. In other words, every pair of non-synchronized atomic accesses must be either disjoint,
//! access the exact same memory (including using the same access size), or both be reads.
//!
//! [1]: https://en.cppreference.com/w/cpp/atomic/memory_order
//! [2]: ../../../nomicon/atomics.html
//! Each atomic access takes an [`Ordering`] which defines how the operation interacts with the
//! happens-before order. These orderings behave the same as the corresponding [C++20 atomic
//! orderings][cpp_memory_order]. For more information, see the [nomicon].
//!
//! Since C++ does not support mixing atomic and non-atomic accesses, or non-synchronized
//! different-sized accesses to the same data, Rust does not support those operations either.
//! Note that both of those restrictions only apply if the accesses are non-synchronized.
//! [cpp]: https://en.cppreference.com/w/cpp/atomic
//! [cpp-intro.races]: https://timsong-cpp.github.io/cppwp/n4868/intro.multithread#intro.races
//! [cpp_memory_order]: https://en.cppreference.com/w/cpp/atomic/memory_order
//! [nomicon]: ../../../nomicon/atomics.html
//!
//! ```rust,no_run undefined_behavior
//! use std::sync::atomic::{AtomicU16, AtomicU8, Ordering};
Expand All @@ -52,27 +69,29 @@
//! let atomic = AtomicU16::new(0);
//!
//! thread::scope(|s| {
//! // This is UB: mixing atomic and non-atomic accesses
//! s.spawn(|| atomic.store(1, Ordering::Relaxed));
//! s.spawn(|| unsafe { atomic.as_ptr().write(2) });
//! // This is UB: conflicting non-synchronized accesses, at least one of which is non-atomic.
//! s.spawn(|| atomic.store(1, Ordering::Relaxed)); // atomic store
//! s.spawn(|| unsafe { atomic.as_ptr().write(2) }); // non-atomic write
//! });
//!
//! thread::scope(|s| {
//! // This is UB: even reads are not allowed to be mixed
//! s.spawn(|| atomic.load(Ordering::Relaxed));
//! s.spawn(|| unsafe { atomic.as_ptr().read() });
//! // This is fine: the accesses do not conflict (as none of them performs any modification).
//! // In C++ this would be disallowed since creating an `atomic_ref` precludes
//! // further non-atomic accesses, but Rust does not have that limitation.
//! s.spawn(|| atomic.load(Ordering::Relaxed)); // atomic load
//! s.spawn(|| unsafe { atomic.as_ptr().read() }); // non-atomic read
//! });
//!
//! thread::scope(|s| {
//! // This is fine, `join` synchronizes the code in a way such that atomic
//! // and non-atomic accesses can't happen "at the same time"
//! let handle = s.spawn(|| atomic.store(1, Ordering::Relaxed));
//! handle.join().unwrap();
//! s.spawn(|| unsafe { atomic.as_ptr().write(2) });
//! // This is fine: `join` synchronizes the code in a way such that the atomic
//! // store happens-before the non-atomic write.
//! let handle = s.spawn(|| atomic.store(1, Ordering::Relaxed)); // atomic store
//! handle.join().unwrap(); // synchronize
//! s.spawn(|| unsafe { atomic.as_ptr().write(2) }); // non-atomic write
//! });
//!
//! thread::scope(|s| {
//! // This is UB: using different-sized atomic accesses to the same data
//! // This is UB: non-synchronized conflicting differently-sized atomic accesses.
//! s.spawn(|| atomic.store(1, Ordering::Relaxed));
//! s.spawn(|| unsafe {
//! let differently_sized = transmute::<&AtomicU16, &AtomicU8>(&atomic);
Expand All @@ -81,8 +100,8 @@
//! });
//!
//! thread::scope(|s| {
//! // This is fine, `join` synchronizes the code in a way such that
//! // differently-sized accesses can't happen "at the same time"
//! // This is fine: `join` synchronizes the code in a way such that
//! // the 1-byte store happens-before the 2-byte store.
//! let handle = s.spawn(|| atomic.store(1, Ordering::Relaxed));
//! handle.join().unwrap();
//! s.spawn(|| unsafe {
Expand Down Expand Up @@ -137,7 +156,7 @@
//!
//! # Atomic accesses to read-only memory
//!
//! In general, *all* atomic accesses on read-only memory are Undefined Behavior. For instance, attempting
//! In general, *all* atomic accesses on read-only memory are undefined behavior. For instance, attempting
//! to do a `compare_exchange` that will definitely fail (making it conceptually a read-only
//! operation) can still cause a segmentation fault if the underlying memory page is mapped read-only. Since
//! atomic `load`s might be implemented using compare-exchange operations, even a `load` can fault
Expand All @@ -153,7 +172,7 @@
//!
//! As an exception from the general rule stated above, "sufficiently small" atomic loads with
//! `Ordering::Relaxed` are implemented in a way that works on read-only memory, and are hence not
//! Undefined Behavior. The exact size limit for what makes a load "sufficiently small" varies
//! undefined behavior. The exact size limit for what makes a load "sufficiently small" varies
//! depending on the target:
//!
//! | `target_arch` | Size limit |
Expand Down

0 comments on commit 7a5052a

Please sign in to comment.