rust-lang / miri

An interpreter for Rust's mid-level intermediate representation
Apache License 2.0
4.6k stars 346 forks source link

`ptr::copy` structs of different sizes with a pointer member results in uninitialized memory? #4024

Closed Supreeeme closed 2 days ago

Supreeeme commented 2 days ago
#[repr(C)]
struct A {
    ty: i32,
    ptr: *const std::ffi::c_void,
    data: [i32; 128]
}

#[repr(C)]
struct B {  
    ty: i32,
    ptr: *const std::ffi::c_void,
    data: i32,
}

struct AWrapped(A);

fn main() {
    let b = B {
        ty: 1,
        ptr: std::ptr::null(),
        data: 24
    };

    let mut a = A {
        ty: 0,
        ptr: std::ptr::null(),
        data: [0; 128]
    };
    unsafe {
        (&mut a as *mut A).cast::<B>().copy_from(&b, 1);
    }

    let a = AWrapped(a);
}

Miri says:

error: Undefined Behavior: constructing invalid value at .data[5]: encountered uninitialized memory, but expected an integer
  --> src/main.rs:33:22
   |
33 |     let a = AWrapped(a);
   |                      ^ constructing invalid value at .data[5]: encountered uninitialized memory, but expected an integer
   |

Why is this memory uninitialized? If I remove the ptr member it's fine.

bjorn3 commented 2 days ago

I think the issue is that B ends with padding bytes to ensure that the size is a multiple of the alignment, which when copied over a clobbers part of a.data with uninit memory.

RalfJung commented 2 days ago

Indeed. copy_from copies size_of::<B>() many bytes, and B has 8 padding bytes (4 between ty and ptr, and 4 after data), which are uninitialized in your case.

Supreeeme commented 2 days ago

So padding bytes are considered uninitialized? And is there no safe way to achieve this transformation then?

RalfJung commented 2 days ago

Copying a value of type B will always leave the padding bytes at the destination uninitialized. Note that the copy that makes padding uninit in your code is the let b = ...;. The operation copy_from itself is a raw, "untyped" copy, meaning that it copies all the bytes exactly as they were. You could carefully initialize the padding in B and then use "untyped" copies that preserve this property, but I would advice against this -- it is a very advanced strategy and requires great care.

I don't know what exactly you are trying to achieve. It seems like you are thinking of B as being a prefix of A, but it is not a prefix -- B has padding where A has data. On a 64bit system, you can make it a prefix by setting

#[repr(C)]
struct B {  
    ty: i32,
    ptr: *const std::ffi::c_void,
    data: [i32; 2],
}

This B has the same size as yours, but is actually a prefix of A.

If that's not what you are looking for, I suggest opening a thread on https://users.rust-lang.org/ where you explain what you want to achieve, and what you already tried.

RalfJung commented 2 days ago

I will close this issue since there is no Miri bug here and I think we have explained why Miri behaves the way it does. Feel free to ask further questions about Miri here if you want. For general guidance about how to write unsafe Rust, I'd refer you to other venues -- we don't have the capacity to provide that kind of support, unfortunately.