Skip to content

std::ptr::swap_nonoverlapping is not always untyped #134713

Closed
@zachs18

Description

@zachs18
Contributor

Code

I tried this code:

#![allow(unused)]
use std::mem::{size_of, align_of};

#[repr(C)]
struct Foo(usize, u8);

fn main() {
    let buf1: [usize; 2] = [1000, 2000];
    let buf2: [usize; 2] = [3000, 4000];
    
    // Foo and [usize; 2] have the same size and alignment,
    // so swap_nonoverlapping should treat them the same
    assert_eq!(size_of::<Foo>(), size_of::<[usize; 2]>());
    assert_eq!(align_of::<Foo>(), align_of::<[usize; 2]>());
    
    let mut b1 = buf1;
    let mut b2 = buf2;
    // Safety: b1 and b2 are distinct local variables,
    // with the same size and alignment as Foo.
    unsafe {
        std::ptr::swap_nonoverlapping(
            b1.as_mut_ptr().cast::<Foo>(),
            b2.as_mut_ptr().cast::<Foo>(),
            1,
        );
    }
    assert_eq!(b1, buf2); // Fails: [3000, 160] != [3000, 4000] or [3000, 1952] != [3000, 4000]
    assert_eq!(b2, buf1); // Fails: [1000, 208] != [1000, 2000] or [1000, 4048] != [1000, 2000]
}

godbolt link

I expected to see this happen: The two assert_eq!s should succeed; b1 and b2 should be completely swapped, since std::ptr::swap_nonoverlapping::<T> claims to swap bytes, not Ts.

Instead, this happened: They are not entirely swapped. swap_nonoverlapping appears to be doing a typed swap at Foo, skipping/zeroing/not-correctly-swapping the 7 padding bytes at the end of Foo.

I think this only happens with types where size_of::<T>() > size_of::<usize>() from looking at the implementation, but I'm not sure.

In debug mode:
Rust 1.61-1.70 appear to consistently give [3000, 160].
Rust 1.71-nightly appear to consistently give [3000, 1952].
4000 is 0x0fa0
160 is 0x00a0
1952 is 0x07a0
2000 is 0x07d0
so it looks like Rust 1.61-1.70 are zeroing the padding bytes, and Rust 1.71-nightly are ignoring them.

Version it worked on

It most recently worked on: Rust 1.60

Version with regression

rustc --version --verbose:

rustc 1.61.0 (fe5b13d68 2022-05-18)
binary: rustc
commit-hash: fe5b13d681f25ee6474be29d748c65adcd91f69e
commit-date: 2022-05-18
host: x86_64-unknown-linux-gnu
release: 1.61.0
LLVM version: 14.0.0

@rustbot modify labels: +regression-from-stable-to-stable -regression-untriaged

Activity

added
C-bugCategory: This is a bug.
regression-untriagedUntriaged performance or correctness regression.
on Dec 23, 2024
added
I-prioritizeIssue: Indicates that prioritization has been requested for this issue.
needs-triageThis issue may need triage. Remove it if it has been sufficiently triaged.
regression-from-stable-to-stablePerformance or correctness regression from one stable version to another.
and removed
regression-untriagedUntriaged performance or correctness regression.
on Dec 23, 2024
scottmcm

scottmcm commented on Dec 24, 2024

@scottmcm
Member

This is the classic problem that MaybeUninit promised two things and it can't actually promise both:

  • That it holds any byte pattern
  • That it has the same ABI

So copying as MaybeUninit<T> doesn't do what it says it would :(

GKFX

GKFX commented on Dec 27, 2024

@GKFX
Contributor

I don't see any part of the MaybeUninit<T> documentation that would make the padding bytes of T not still be padding bytes of MaybeUninit<T>. So it doesn't look like swap_nonoverlapping_simple_untyped can be expected to copy padding bytes. Possibly the answer is that all of the copying should be done as MaybeUninit integers, never MaybeUninit<T>, or copy_nonoverlapping<T> should be used.

added a commit that references this issue on Dec 31, 2024

Auto merge of rust-lang#134954 - scottmcm:more-swap-tuning, r=<try>

407c8de
self-assigned this
on Dec 31, 2024
added
T-compilerRelevant to the compiler team, which will review and decide on the PR/issue.
T-libsRelevant to the library team, which will review and decide on the PR/issue.
and removed
needs-triageThis issue may need triage. Remove it if it has been sufficiently triaged.
on Jan 8, 2025

13 remaining items

Loading
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

C-bugCategory: This is a bug.P-highHigh priorityT-compilerRelevant to the compiler team, which will review and decide on the PR/issue.T-libsRelevant to the library team, which will review and decide on the PR/issue.regression-from-stable-to-stablePerformance or correctness regression from one stable version to another.

Type

No type

Projects

No projects

Milestone

No milestone

Relationships

None yet

    Participants

    @Amanieu@RalfJung@Manishearth@GKFX@zachs18

    Issue actions

      `std::ptr::swap_nonoverlapping` is not always untyped · Issue #134713 · rust-lang/rust