Unsoundness for types containing niches

[dtolnay]

I believe it's incorrect to implement Atomic<T> in terms of UnsafeCell<T>, and it needs to be UnsafeCell<MaybeUninit<T>> instead, to prevent code outside the cell from observing partially initialized state.

Here is an example of safe code that reproduces UB from a data race:

use atomic::Atomic;
use std::num::NonZeroU128;
use std::sync::atomic::Ordering;
use std::thread;

enum Enum {
    NeverConstructed,
    Cell(Atomic<NonZeroU128>),
}

static STATIC: Enum = Enum::Cell(Atomic::new(match NonZeroU128::new(1) {
    Some(nonzero) => nonzero,
    None => unreachable!(),
}));

fn main() {
    thread::spawn(|| {
        let cell = match &STATIC {
            Enum::NeverConstructed => unreachable!(),
            Enum::Cell(cell) => cell,
        };
        let x = NonZeroU128::new(0xFFFFFFFF_FFFFFFFF_00000000_00000000).unwrap();
        let y = NonZeroU128::new(0x00000000_00000000_FFFFFFFF_FFFFFFFF).unwrap();
        loop {
            cell.store(x, Ordering::Release);
            cell.store(y, Ordering::Release);
        }
    });

    loop {
        if let Enum::NeverConstructed = STATIC {
            unreachable!(":(");
        }
    }
}

$ cargo run

warning: variant is never constructed: `NeverConstructed`
 --> src/main.rs:7:5
  |
7 |     NeverConstructed,
  |     ^^^^^^^^^^^^^^^^
  |
  = note: `#[warn(dead_code)]` on by default

warning: `repro` (bin "repro") generated 1 warning
    Finished dev [unoptimized + debuginfo] target(s) in 0.23s
     Running `target/debug/repro`

thread 'main' panicked at 'internal error: entered unreachable code: :(', src/main.rs:32:13
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

[Amanieu]

As per the discussion in rust-lang/rust#87341 I think this should be fixed at the language level, not in this crate.

[dtolnay]

Even if fixed in rustc, this crate would remain prone to data races/UB on stable for the next 11 weeks and on older compilers permanently. I think it's worth considering a fix in this crate even if a fix in rustc is happening. This isn't a theoretical "hypothetically a future rustc could miscompule your code because there is theoretically UB here" — it's already observably doing the wrong thing on code that's reasonably typical of real-world usage of this crate.