AtomicCell is unsound on types containing niches

[dtolnay]

I believe it's incorrect to implement AtomicCell<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:

use crossbeam_utils::atomic::AtomicCell;
use std::num::NonZeroU128;
use std::thread;

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

static STATIC: Enum = Enum::Cell(AtomicCell::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);
            cell.store(y);
        }
    });

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

$ cargo run

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

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

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

[taiki-e]

Thanks for the report! I've filed #834 to fix this.

[RalfJung]

Since rust-lang/rust#99011 (which landed a few months after this issue), UnsafeCell<T> suppresses niches, exactly due to problems like this. So I don't think there's any need for crossbeam to do anything itself here any more.

[taiki-e]

Since that fix is not yet available in our MSRV, we cannot remove the workaround on our part yet, but it would make sense to update the comment to mention that it has been fixed in 1.64.