reference
reference copied to clipboard
a more reasonable explanation for atomic operation on static is unsafe
a more reasonable explanation for atomic operation on static is unsafe
While you’re at it improving this example:
I think this example should probably also be re-written to use add_of_mut. There’s nothing “safe” about this function as it stands right now, under up-to-date Rust semantics.
Even something like this (that doesn’t even use the reference) is already considered UB by miri:
use std::thread;
static mut LEVELS: u32 = 0;
fn bump_levels_unsafe2() {
let _ = unsafe { &mut LEVELS };
}
fn main() {
thread::scope(|s| {
s.spawn(bump_levels_unsafe2);
s.spawn(bump_levels_unsafe2);
});
}
error: Undefined Behavior: Data race detected between (1) non-atomic write on thread `<unnamed>` and (2) non-atomic write on thread `<unnamed>` at alloc1. (2) just happened here
--> src/main.rs:6:22
|
6 | let _ = unsafe { &mut LEVELS };
| ^^^^^^^^^^^ Data race detected between (1) non-atomic write on thread `<unnamed>` and (2) non-atomic write on thread `<unnamed>` at alloc1. (2) just happened here
|
help: and (1) occurred earlier here
--> src/main.rs:6:22
|
6 | let _ = unsafe { &mut LEVELS };
| ^^^^^^^^^^^
= help: this indicates a bug in the program: it performed an invalid operation, and caused Undefined Behavior
= help: see https://doc.rust-lang.org/nightly/reference/behavior-considered-undefined.html for further information
= note: BACKTRACE (of the first span):
= note: inside `bump_levels_unsafe2` at src/main.rs:6:22: 6:33
Also, Rustc rightfully warns and suggests usage of addr_of_mut
warning: mutable reference of mutable static is discouraged
--> src/main.rs:6:22
|
6 | let _ = unsafe { &mut LEVELS };
| ^^^^^^^^^^^ mutable reference of mutable static
|
= note: for more information, see issue #114447 <https://github.com/rust-lang/rust/issues/114447>
= note: reference of mutable static is a hard error from 2024 edition
= note: mutable statics can be written to by multiple threads: aliasing violations or data races will cause undefined behavior
= note: `#[warn(static_mut_ref)]` on by default
help: mutable references are dangerous since if there's any other pointer or reference used for that static while the reference lives, that's UB; use `addr_of_mut!` instead to create a raw pointer
|
6 | let _ = unsafe { addr_of_mut!(LEVELS) };
| ~~~~~~~~~~~~~~~~~~~~
And given that it’s a very low-level thing, this atomic_add function, not using any safe wrapper type (like AtomicU32) but operating on the u32 directly, it’s only natural that *mut u32 is the right argument type here (not &mut u32!)… so the call would then look like atomic_add(addr_of_mut!(LEVELS), 1);
If you want, you can also improve the style and refactor the usage of early-return statements (return EXPR;) at the end of the functions into ordinary final expression in a block for return value.
Closing in favor of #1544, thanks!