4

The Rust Reference seems to say that mutating an immutable local data (that's not inside an UnsafeCell) is undefined behavior:

Behavior considered undefined

  • Mutating immutable data. All data inside a const item is immutable. Moreover, all data reached through a shared reference or data owned by an immutable binding is immutable, unless that data is contained within an UnsafeCell<U>.

The following code mutates an immutable local variable by reinterpreting it as an AtomicU32. Currently the code runs just fine and prints the expected result, but is its behavior actually undefined?

use std::sync::atomic::{AtomicU32, Ordering};

#[repr(C, align(4))]
struct Bytes([u8; 4]);

fn main() {
    let bytes = Bytes([11; 4]);
    let x = unsafe { &*(&bytes as *const Bytes as *const AtomicU32) };
    x.store(12345, Ordering::SeqCst);
    println!("{:?}", bytes.0); // [57, 48, 0, 0]
}

Miri doesn't complain about the code example below where the bytes are mutable. Since those bytes are being mutated through a shared reference (&AtomicU32), it seems to me that according to The Rust Reference the code below should also have undefined behavior - given that "all data reached through a shared reference [..] is immutable" and "mutating immutable data [is considered undefined behavior]".

use std::sync::atomic::{AtomicU32, Ordering};

#[repr(C, align(4))]
struct Bytes([u8; 4]);

fn main() {
    let mut bytes = Bytes([11; 4]);
    let x = unsafe { &*(&mut bytes as *mut Bytes as *const AtomicU32) };
    x.store(12345, Ordering::SeqCst);
    println!("{:?}", bytes.0); // [57, 48, 0, 0]
}
StableGeneous
  • 119
  • 6
  • 2
    You're mutating `bytes` by reaching through an immutable reference `&bytes`, which the quote in your question pretty clearly says is undefined behavior. What else would you expect? – Frxstrem Feb 01 '21 at 18:22
  • I was just looking for a confirmation of the reference text. Do you have any thoughts about my follow-up code example? – StableGeneous Feb 01 '21 at 19:26
  • 2
    The follow-up example is fine, because the `&AtomicU32` is derived from a `&mut Bytes`. `&*(&bytes as *const AtomicU32)` would be unsound even though `bytes` itself is marked mutable, because the *provenance* of the pointer matters for what you can do with it. See also [Temporarily opt-in to shared mutation](https://ryhl.io/blog/temporary-shared-mutation/), it's basically the same thing using `Cell` but without `unsafe`. (Unfortunately there is no stable equivalent for `Atomic`s) – trent Feb 02 '21 at 14:20

1 Answers1

8

Yes, according to Miri:

error: Undefined Behavior: trying to reborrow for SharedReadWrite at alloc1377, but parent tag <untagged> does not have an appropriate item in the borrow stack
 --> src/main.rs:8:22
  |
8 |     let x = unsafe { &*(&bytes as *const Bytes as *const AtomicU32) };
  |                      ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ trying to reborrow for SharedReadWrite at alloc1377, but parent tag <untagged> does not have an appropriate item in the borrow stack
  |
  = help: this indicates a potential bug in the program: it performed an invalid operation, but the rules it violated are still experimental
  = help: see https://github.com/rust-lang/unsafe-code-guidelines/blob/master/wip/stacked-borrows.md for further information
          
  = note: inside `main` at src/main.rs:8:22
  = note: inside `<fn() as std::ops::FnOnce<()>>::call_once - shim(fn())` at /playground/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/core/src/ops/function.rs:227:5
  = note: inside `std::sys_common::backtrace::__rust_begin_short_backtrace::<fn(), ()>` at /playground/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/std/src/sys_common/backtrace.rs:125:18
  = note: inside closure at /playground/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/std/src/rt.rs:66:18
  = note: inside `std::ops::function::impls::<impl std::ops::FnOnce<()> for &dyn std::ops::Fn() -> i32 + std::marker::Sync + std::panic::RefUnwindSafe>::call_once` at /playground/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/core/src/ops/function.rs:259:13
  = note: inside `std::panicking::r#try::do_call::<&dyn std::ops::Fn() -> i32 + std::marker::Sync + std::panic::RefUnwindSafe, i32>` at /playground/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/std/src/panicking.rs:379:40
  = note: inside `std::panicking::r#try::<i32, &dyn std::ops::Fn() -> i32 + std::marker::Sync + std::panic::RefUnwindSafe>` at /playground/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/std/src/panicking.rs:343:19
  = note: inside `std::panic::catch_unwind::<&dyn std::ops::Fn() -> i32 + std::marker::Sync + std::panic::RefUnwindSafe, i32>` at /playground/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/std/src/panic.rs:396:14
  = note: inside `std::rt::lang_start_internal` at /playground/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/std/src/rt.rs:51:25
  = note: inside `std::rt::lang_start::<()>` at /playground/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/std/src/rt.rs:65:5

See also:

since those bytes are being mutated through a shared reference (&AtomicU32)

AtomicU32 contains an UnsafeCell, thus it meets the exemption criteria you've quoted:

        pub struct $atomic_type {
            v: UnsafeCell<$int_type>,
        }

This is part of the API as AtomicU32::from_mut, no need for unsafe.

Shepmaster
  • 388,571
  • 95
  • 1,107
  • 1,366
  • Thank you for pointing me to a useful tool (Miri). Do you have any thoughts about whether the follow-up code example I added to the question has undefined behavior? – StableGeneous Feb 01 '21 at 19:28