Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Miri does not catch UB when swap_nonoverlapping is used on the same ptr #4188

Closed
Phoqinu opened this issue Feb 11, 2025 · 3 comments · Fixed by rust-lang/rust#136890
Closed
Assignees

Comments

@Phoqinu
Copy link

Phoqinu commented Feb 11, 2025

Hello, I was writing a lot of unsafe in my lib and it passed tests with cargo miri test but running cargo test crashed with:

unsafe precondition(s) violated: ptr::swap_nonoverlapping requires that both pointer arguments are aligned and non-null and the specified memory ranges do not overlap

So I made this example:

use std::{
    alloc::{Layout, alloc, dealloc},
    ptr,
};

fn main() {
    let layout = Layout::new::<usize>();
    let ptr_layout = Layout::array::<usize>(2).unwrap();
    let ptr = unsafe { alloc(ptr_layout) };

    unsafe { ptr.cast::<usize>().add(0).write(0_usize) };
    unsafe { ptr.cast::<usize>().add(1).write(0_usize) };

    unsafe {
        ptr::swap_nonoverlapping(ptr, ptr, layout.size());
    }

    unsafe {
        dealloc(ptr, ptr_layout);
    };
}

One of the Safety requirements of ptr::swap_nonoverlapping is The region of memory beginning at x with a size of count * size_of::<T>() bytes must not overlap with the region of memory beginning at y with the same size. but miri does not catch this.

While it panics in debug mode without running miri I think miri should catch this... or not ?

@bjorn3
Copy link
Member

bjorn3 commented Feb 11, 2025

Looks like swap_nonoverlapping uses assert_unsafe_precondition with check_language_ub, which disables the assertion inside miri under the assumption that miri would eventually catch the UB, but the current implementation of swap_nonoverlapping doesn't seem to trigger language UB, so miri never catches the documented library UB. Maybe the assert_unsafe_precondition call should be changed to ude check_library_ub instead, which runs in miri too.

@saethlin
Copy link
Member

I agree with that assessment. @Phoqinu are you interested in fixing this?

@Phoqinu
Copy link
Author

Phoqinu commented Feb 11, 2025

I agree with that assessment. @Phoqinu are you interested in fixing this?

I'll leave that to others 👍

@saethlin saethlin self-assigned this Feb 11, 2025
rust-timer added a commit to rust-lang-ci/rust that referenced this issue Feb 12, 2025
Rollup merge of rust-lang#136890 - saethlin:swap_nonoverlapping, r=RalfJung

Change swap_nonoverlapping from lang to library UB

The implementation of ptr::swap_nonoverlapping does not always escalate its safety contract to language UB, so it should be `check_library_ub`.

Fixes rust-lang/miri#4188
github-actions bot pushed a commit to rust-lang/rustc-dev-guide that referenced this issue Feb 13, 2025
Change swap_nonoverlapping from lang to library UB

The implementation of ptr::swap_nonoverlapping does not always escalate its safety contract to language UB, so it should be `check_library_ub`.

Fixes rust-lang/miri#4188
github-actions bot pushed a commit that referenced this issue Feb 13, 2025
Change swap_nonoverlapping from lang to library UB

The implementation of ptr::swap_nonoverlapping does not always escalate its safety contract to language UB, so it should be `check_library_ub`.

Fixes #4188
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants