Footgun: compare_exchange may fail if T contains unused bytes

[binomial0]

Atomic::compare_exchange compares the bitvalues of the contents and its first argument. In Rust, even values of simple types can be considered equal without being bitwise-equal. I ran into this with Atomic<Option<NonNull<[f32]>>>.

Option<NonNull<[f32]>> is 128 bits large. The old value had a bit pattern of 0x0000009ace77f1030000000000000000, which corresponds to the value None. The value of None that I supplied had a bit pattern of all zeros, so the compare_exchange failed.

Maybe it's obvious that compare_exchange is not reliable for types without primitive bitwise equality, but I think it should still be documented. I can open a PR, but I'm not completely sure how to explain this in the docs.

We could also try to provide a solution, adding a function somewhat like this:

fn compare_exchange_careful(&self, mut current: T, new: T, success: Ordering, failure: Ordering) -> Result<T, T> {
    loop {
        match self.compare_exchange(current, new, success, failure) {
            Ok(v) => return Ok(v),
            Err(old_value) => if old_value == current {
                current = old_value;
                continue;
            } else {
                return Err(old_value);
            }
        }
    }
}

[Amanieu]

Yes, the docs should definitely clarify that bitwise comparisons are used and give your example as a potential pitfall. In fact, the docs should recommend sticking to primitive types: in your case you should probably used Atomic<(*mut f32, usize)> instead which doesn't have this issue.

[notgull]

Isn't this unsound undefined behavior? Since you're casting padding bytes (uninitialized bytes) to an integer type (where all bytes are expected to be initialized), you're practically calling MaybeUninit::uninit().assume_injt(), which is bad news.

I know that this is the primary motivation behind atomic-maybe-uninit. It might be worth it to have a NoPaddingBytes marker trait, or to only take types that are bytemuck::NoUninit.

[Amanieu]

I just had a look at atomic-maybe-uninit and it looks great. I would happily accept a PR which changes this crate to use atomic-maybe-uninit instead of the core atomic types.

[notgull]

Unfortunately I don't have the bandwidth to make a PR here for the foreseeable future