Add safety section for atomic_load/store

[yilin0518]

This PR tries to add # Safety section for atomic_load/store in intrinsic module. I notice that some intrinsic unsafe functions already have # Safety section. And for these two functions, they have corresponding stable version functions in core/sync. But in the stable implementation, I notice that they first call an unsafe atomic_load/store defined in the same file(a private function without safety doc), and that unsafe function directly call atomic_load/store defined in intrinsic module(for example, atomic_load). Here is the implementaion of atomic_load used in AtomicBool::load:

#[inline]
#[cfg_attr(miri, track_caller)] // even without panics, this helps for Miri backtraces
unsafe fn atomic_load<T: Copy>(dst: *const T, order: Ordering) -> T {
    // SAFETY: the caller must uphold the safety contract for `atomic_load`.
    unsafe {
        match order {
            Relaxed => intrinsics::atomic_load::<T, { AO::Relaxed }>(dst),
            Acquire => intrinsics::atomic_load::<T, { AO::Acquire }>(dst),
            SeqCst => intrinsics::atomic_load::<T, { AO::SeqCst }>(dst),
            Release => panic!("there is no such thing as a release load"),
            AcqRel => panic!("there is no such thing as an acquire-release load"),
        }
    }
}

So I'm trying to add # Safety section for the intrinsic atomic_load/store. Although intrinsic API mainly used for Rust standary library, I think that adding # Safety section is needed because it pass a raw pointer. When writing the # Safety section for these two functions, I refer to read_volatile and write_volatile.

If needed, I will review all the atomic operations defined in intrinsic module. Thank you for your review and I'm looking forward to your feedback. Hoping this PR can improve the safety doc of Rust standard library.

[rustbot]

Some changes occurred to the intrinsics. Make sure the CTFE / Miri interpreter
gets adapted for the changes, if necessary.

cc @rust-lang/miri, @RalfJung, @oli-obk, @lcnr

[rustbot]

r? @JohnTitor

rustbot has assigned @JohnTitor.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

Why was this reviewer chosen?

The reviewer was selected based on:

• Owners of files modified in this PR: libs
• libs expanded to 12 candidates
• Random selection from 6 candidates

[RalfJung]

These intrinsics exist only for internal use. What is the point of repeating the docs of the public methods here?

[yilin0518]

The reason I refer to read_volatile and write_volatile is that load/store are similar to read/write, but also I add the atomic access and alignment requirement as complement.

Although these two functions are for internel use, adding safety section may still be useful? The implementation of AtomicBool::load internally call unsafe atomic_load, but atomic_load doe not have safety section, and the same as atomic_load in intrinsic.

My basic idea is this function use raw pointer and need the caller to ensure the safety, so it's necessary to add safety section. But as what you say, this is for internal use.

[hxuhack]

I think we need some policy on whether an intrinsic API needs a safety doc. Currently, some intrinsics have safety docs while others do not. See this table.

I agree with @yilin0518 that even if an intrinsic is intended for internal use only, documenting its safety requirements is still valuable for checking or verifying the soundness of the standard library.

[RalfJung]

I agree that ideally, intrinsics should have safety docs. OTOH, I would like to avoid duplicating complicated safety docs in cases where we have equivalent public stable functions with carefully crafted safety docs.

For atomic_load/store

• if VOLATILE = true, they are equivalent to load_volatile and friends. So we should just link there.
• if VOLATILE = false, they are equivalent to from_ptr followed by load and friends, so we should just link there.

[yilin0518]

@RalfJung thank you for your feedback and support! I have update the commit. Hoping this PR can improve the safety doc of intrinsics.

[RalfJung]

This works, thanks. :)

@bors r+ rollup

[rust-bors]

📌 Commit 8a1e154 has been approved by RalfJung

It is now in the queue for this repository.