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

uninitialized bytes in a local variable are not caught #2732

Closed
oconnor663 opened this issue Dec 20, 2022 · 6 comments
Closed

uninitialized bytes in a local variable are not caught #2732

oconnor663 opened this issue Dec 20, 2022 · 6 comments

Comments

@oconnor663
Copy link

oconnor663 commented Dec 20, 2022

Playground link

use std::ptr::copy_nonoverlapping;

#[derive(Debug, PartialEq)]
#[repr(C)]
struct Foo {
    x: u8,
    // uninitialized padding goes here
    y: u16,
}

fn main() {
    let foo = Foo { x: 1, y: 2 };

    // Copy the bytes of foo to a buffer, including the uninitialized padding byte.
    let mut buf = [0u8; 4];
    unsafe {
        copy_nonoverlapping(&foo as *const Foo as *const u8, buf.as_mut_ptr(), 4);
    }

    // Copy the buffer back to foo2. This passes Miri.
    let mut foo2 = Foo { x: 0, y: 0 };
    unsafe {
        copy_nonoverlapping(buf.as_ptr(), &mut foo2 as *mut Foo as *mut u8, 4);
    }
    assert_eq!(foo, foo2);

    // Uncommenting any of the following lines fails Miri.
    // drop(buf);
    // buf = buf;
    // buf[1];
}

This is similar to #2518 (uninitialized value behind a reference), except here the uninitialized value is in a local variable.

Is it expected that Miri doesn't currently trigger on this code? Any thoughts about whether this is likely to be UB in the future?

@oconnor663
Copy link
Author

oconnor663 commented Dec 20, 2022

I get the same result if I make buf a u64 instead of a [u8; 4]: https://play.rust-lang.org/?version=stable&mode=debug&edition=2021&gist=cf1dbcaf5f3906b60920e8cc20bf8e9f. So maybe it's not actually specific to arrays?

@oconnor663 oconnor663 changed the title uninitialized bytes in an array are not caught uninitialized bytes in a local variable are not caught Dec 20, 2022
@saethlin
Copy link
Member

The type of buf does not matter.

Writing this code is a very bad idea, but it isn't UB. Miri is correctly quiet here. The docs for copy_nonoverlapping say this:

The copy is “untyped” in the sense that data may be uninitialized or otherwise violate the requirements of T. The initialization state is preserved exactly.

And in this code, I think Ralf would say you didn't "use" buf. Though precisely what "use" means, I'm not exactly sure, though I think the terminology generally provides the right intuition. The Reference uses similar language around "producing a value" and explains a bit more precisely:

Producing an invalid value, even in private fields and locals. "Producing" a value happens any time a value is assigned to or read from a place, passed to a function/primitive operation or returned from a function/primitive operation.

And you do none of those things to buf, so Miri is silent.

@oconnor663
Copy link
Author

oconnor663 commented Dec 20, 2022

Yeah I didn't mean to imply that the second copy should be UB, and maybe it's a red herring that I should've left out. (It's an artifact of the original example that prompted this question.) Maybe this is a better, more minimal example:

struct Foo {
    _a: u8,
    _b: u16,
}

fn main() {
    let mut x = 0u64;
    unsafe {
        std::ptr::copy_nonoverlapping(
            &Foo { _a: 1, _b: 2 } as *const Foo as *const u8,
            &mut x as *mut u64 as *mut u8,
            4,
        );
    }
    // x is partially uninitialized
}

@saethlin
Copy link
Member

Yes, exactly the same logic applies here. You do not do anything with x so it doesn't matter that is partly initialized.

@RalfJung
Copy link
Member

RalfJung commented Dec 20, 2022 via email

@oconnor663
Copy link
Author

Good to know!

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

No branches or pull requests

3 participants