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 cannot represent partial pointers (for byte-wise copy and partial overwrite) #2181

Closed
Tracked by #2159
RalfJung opened this issue Jun 3, 2022 · 2 comments · Fixed by rust-lang/rust#104054
Closed
Tracked by #2159
Labels
A-interpreter Area: affects the core interpreter C-bug Category: This is a bug. I-false-UB Impact: makes Miri falsely report UB, i.e., a false positive (with default settings)

Comments

@RalfJung
Copy link
Member

RalfJung commented Jun 3, 2022

Miri's model of memory can only attach provenance to pointer-sized chunks of memory, but not to individual bytes. This leads to incorrect behavior in several situations:

  • We cannot copy a pointer one MaybeUninit<u8> at a time. In particular this affects ptr::swap_nonoverlapping.
  • We cannot overwrite some of the bytes of a pointer, but preserve provenance on the bytes that remain.
@RalfJung RalfJung changed the title Miri cannot represent partial pointers for byte-wise copy Miri cannot represent partial pointers (for byte-wise copy and partial overwrite) Jun 3, 2022
@RalfJung
Copy link
Member Author

RalfJung commented Jun 3, 2022

Testcases for the first problem

These should pass but currently raise an "unsupported operation".

use std::{
    mem::{self, MaybeUninit},
    ptr,
};

fn main() {
    let mut ptr1 = &1;
    let mut ptr2 = &2;

    // Swap them, bytewise.
    unsafe {
        ptr::swap_nonoverlapping(
            &mut ptr1 as *mut _ as *mut MaybeUninit<u8>,
            &mut ptr2 as *mut _ as *mut MaybeUninit<u8>,
            mem::size_of::<&i32>(),
        );
    }
    
    // Make sure they still work.
    assert_eq!(*ptr1, 2);
    assert_eq!(*ptr2, 1);

    // TODO: also test ptr::swap, ptr::copy, ptr::copy_nonoverlapping.
}
use std::mem::{self, MaybeUninit};

unsafe fn memcpy<T>(to: *mut T, from: *const T) {
    let to = to.cast::<MaybeUninit<u8>>();
    let from = from.cast::<MaybeUninit<u8>>();
    for i in 0..mem::size_of::<T>() {
        let b = from.add(i).read();
        to.add(i).write(b);
    }
}

fn main() {
    let ptr1 = &1;
    let mut ptr2 = &2;

    // Copy, bytewise.
    unsafe {
        memcpy(&mut ptr2, &ptr1);
    }

    // Make sure they still work.
    assert_eq!(*ptr1, 1);
    assert_eq!(*ptr2, 1);
}

Testcase for the second problem

This should pass but currently reports UB.

use std::mem::{self, MaybeUninit};

unsafe fn ptr_bytes<'x>(ptr: &'x mut &i32)
-> &'x mut [MaybeUninit<u8>; mem::size_of::<&i32>()]
{
    mem::transmute(ptr)
}

fn main() { unsafe {
    let mut ptr = &42;
    // Get a bytewise view of the pointer.
    let ptr_bytes = ptr_bytes(&mut ptr);

    // The 2 highest bytes are both 0.
    assert_eq!(*ptr_bytes[6].as_ptr().cast::<u8>(), 0);
    assert_eq!(*ptr_bytes[7].as_ptr().cast::<u8>(), 0);
    // Overwrite provenance on the last byte.
    ptr_bytes[7] = MaybeUninit::new(0);
    // Restore it from the previous byte.
    ptr_bytes[7] = ptr_bytes[6];

    // Now ptr should be good again.
    assert_eq!(*ptr, 42);
} }

@RalfJung
Copy link
Member Author

RalfJung commented Oct 8, 2022

Also this special case in the standard library should not be needed any more when this is properly implemented. And this one.

@RalfJung RalfJung added the I-false-UB Impact: makes Miri falsely report UB, i.e., a false positive (with default settings) label Nov 5, 2022
RalfJung pushed a commit to RalfJung/miri that referenced this issue Nov 16, 2022
interpret: support for per-byte provenance

Also factors the provenance map into its own module.

The third commit does the same for the init mask. I can move it in a separate PR if you prefer.

Fixes rust-lang#2181

r? `@oli-obk`
bjorn3 pushed a commit to rust-lang/rustc_codegen_cranelift that referenced this issue Nov 19, 2022
interpret: support for per-byte provenance

Also factors the provenance map into its own module.

The third commit does the same for the init mask. I can move it in a separate PR if you prefer.

Fixes rust-lang/miri#2181

r? `@oli-obk`
thomcc pushed a commit to tcdi/postgrestd that referenced this issue Feb 10, 2023
interpret: support for per-byte provenance

Also factors the provenance map into its own module.

The third commit does the same for the init mask. I can move it in a separate PR if you prefer.

Fixes rust-lang/miri#2181

r? `@oli-obk`
antoyo pushed a commit to rust-lang/rustc_codegen_gcc that referenced this issue Jun 3, 2023
interpret: support for per-byte provenance

Also factors the provenance map into its own module.

The third commit does the same for the init mask. I can move it in a separate PR if you prefer.

Fixes rust-lang/miri#2181

r? `@oli-obk`
RalfJung pushed a commit to RalfJung/rust-analyzer that referenced this issue Apr 20, 2024
interpret: support for per-byte provenance

Also factors the provenance map into its own module.

The third commit does the same for the init mask. I can move it in a separate PR if you prefer.

Fixes rust-lang/miri#2181

r? `@oli-obk`
RalfJung pushed a commit to RalfJung/rust-analyzer that referenced this issue Apr 27, 2024
interpret: support for per-byte provenance

Also factors the provenance map into its own module.

The third commit does the same for the init mask. I can move it in a separate PR if you prefer.

Fixes rust-lang/miri#2181

r? `@oli-obk`
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-interpreter Area: affects the core interpreter C-bug Category: This is a bug. I-false-UB Impact: makes Miri falsely report UB, i.e., a false positive (with default settings)
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant