4
votes

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]
}
1
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
I was just looking for a confirmation of the reference text. Do you have any thoughts about my follow-up code example?StableGeneous
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, it's basically the same thing using Cell but without unsafe. (Unfortunately there is no stable equivalent for Atomics)trentcl

1 Answers

7
votes

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.