ptr::copy and ptr::swap are doing untyped copies

[RalfJung]

The consensus in #63159 seemed to be that these operations should be "untyped", i.e., they should treat the data as raw bytes, should work when these bytes violate the validity invariant of T, and should exactly preserve the initialization state of the bytes that are being copied. This is already somewhat implied by the description of "copying/swapping size*N bytes" (rather than "N instances of T").

The implementations mostly already work that way (well, for LLVM's intrinsics the documentation is not precise enough to say what exactly happens to poison, but if this ever gets clarified to something that would not perfectly preserve poison, then I strongly assume there will be some way to make a copy that does perfectly preserve poison). However, I had to adjust swap_nonoverlapping; after @scottmcm's recent changes, that one (sometimes) made a typed copy. (Note that mem::swap, which works on mutable references, is unchanged. It is documented as "swapping the values at two mutable locations", which to me strongly indicates that it is indeed typed. It is also safe and can rely on &mut T pointing to a valid T as part of its safety invariant.)

On top of adding a test (that will be run by Miri), this PR then also adjusts the documentation to indeed stably promise the untyped semantics. I assume this means the PR has to go through t-libs (and maybe t-lang?) FCP.

Fixes #63159

[rust-highfive]

r? @joshtriplett

(rust-highfive has picked a reviewer for you, use r? to override)

[RalfJung]

r? rust-lang/libs-api @rustbot label +T-libs-api -T-libs

[bors]

☔ The latest upstream changes (presumably #97742) made this pull request unmergeable. Please resolve the merge conflicts.

[dtolnay]

@rust-lang/libs-api:
@rfcbot fcp merge

This PR concerns whether code like the following is guaranteed to be okay, or is UB:

let mut x = 5u8;
let mut y = 6u8;

unsafe {
    ptr::swap(&mut x as *mut u8 as *mut bool, &mut y as *mut u8 as *mut bool)
}
// and similarly, swap_nonoverlapping, copy, copy_nonoverlapping

Due to x and y not being a valid value of type bool, if ptr::swap::<T> were doing something like this (a "typed copy") then that would be bad:

unsafe {
    let tmp: T = ptr::read(x); // UB if x does not point to valid T
    ptr::write(x, ptr::read(y));
    ptr::write(y, tmp);
}

This PR adds a guarantee that these core::ptr functions all involve "untyped copy" i.e. copying count * size_of::<T>() maybe-uninitialized bytes. The behavior of the operation depends only on the static size of T, not on the specific choice of T in any way other than size. (Technically T's alignment is relevant too. But its validity invariants are not.)

A case more resembling real-world code than the bool one above is:

#[repr(C)]
struct Struct {
    a: u16,
    /* padding */
    b: u32,
    c: u32,
}

let ptr1, ptr2: *mut Struct;

unsafe {
    // swap the first 8 bytes (a and b)
    ptr::swap_nonoverlapping(ptr1 as *mut u8, ptr2 as *mut u8, 8)
}

If swap_nonoverlapping is a "typed copy" then this is 🤷 (rust-lang/unsafe-code-guidelines#71) but most likely not okay. Padding bytes are not necessarily initialized and manipulating a byte with uninitialized contents as u8 is not currently well-defined.

If swap_nonoverlapping is an "untyped copy" then this is fine independent of the outcome of that integer validity discussion.

As a related effect, specifying this as an "untyped copy" requires that the implementation copies all the padding in the case of {copy,swap}{,_nonoverlapping}::<Struct>, even if it would be more efficient to not copy padding. Thus it's possible that you would get more efficient code from ptr::write(dst, ptr::read(src)) (typed copy) compared to ptr::copy_nonoverlapping(src, dst, 1) (untyped copy). Here is an example in which this is a very significant difference:

#[repr(align(128))]
#[derive(Copy, Clone)]
pub struct A(u8);

pub unsafe fn typed_copy(src: *const A, dst: *mut A) {
    (*dst).0 = (*src).0;
}

pub unsafe fn untyped_copy(src: *const A, dst: *mut A) {
    std::ptr::copy_nonoverlapping(src, dst, 1);
}

example::typed_copy:
        mov     al, byte ptr [rdi]
        mov     byte ptr [rsi], al
        ret

example::untyped_copy:
        movaps  xmm0, xmmword ptr [rdi + 112]
        movaps  xmmword ptr [rsi + 112], xmm0
        movaps  xmm0, xmmword ptr [rdi + 96]
        movaps  xmmword ptr [rsi + 96], xmm0
        movaps  xmm0, xmmword ptr [rdi + 80]
        movaps  xmmword ptr [rsi + 80], xmm0
        movaps  xmm0, xmmword ptr [rdi + 64]
        movaps  xmmword ptr [rsi + 64], xmm0
        movaps  xmm0, xmmword ptr [rdi]
        movaps  xmm1, xmmword ptr [rdi + 16]
        movaps  xmm2, xmmword ptr [rdi + 32]
        movaps  xmm3, xmmword ptr [rdi + 48]
        movaps  xmmword ptr [rsi + 48], xmm3
        movaps  xmmword ptr [rsi + 32], xmm2
        movaps  xmmword ptr [rsi + 16], xmm1
        movaps  xmmword ptr [rsi], xmm0
        ret

[rfcbot]

Team member @dtolnay has proposed to merge this. The next step is review by the rest of the tagged team members:

• @Amanieu
• @BurntSushi
• @dtolnay
• @joshtriplett
• @m-ou-se
• @yaahc

No concerns currently listed.

Once a majority of reviewers approve (and at most 2 approvals are outstanding), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up!

See this document for info about what commands tagged team members can give me.

[RalfJung]

That's an impressive summary, thanks. :-)

[RalfJung]

@Amanieu @BurntSushi @joshtriplett there are checkboxes waiting for you here. :)

[BurntSushi]

cc @rust-lang/lang

I'm not sure we need "official" sign off from the lang team or not (I don't really have an opinion either way), but I think it would be good to at least loop them in.

[rfcbot]

🔔 This is now entering its final comment period, as per the review above. 🔔

[scottmcm]

Since this can be implemented via swapping MaybeUninit<_>s, I do think this is officially fully in libs-api's purview, but since memcpy is so core I'll bring it up at the next lang meeting to make sure people know about it just in case.

[rfcbot]

The final comment period, with a disposition to merge, as per the review above, is now complete.

As the automated representative of the governance process, I would like to thank the author for their work and everyone else who contributed.

This will be merged soon.

[scottmcm]

With the FCP complete, the code changes all look good to me.

r? @scottmcm
@bors r+

[bors]

📌 Commit cb7cd97 has been approved by scottmcm