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

Reading dropped object's memory succeeds #62135

Closed
lineaar opened this issue Jun 26, 2019 · 6 comments
Closed

Reading dropped object's memory succeeds #62135

lineaar opened this issue Jun 26, 2019 · 6 comments
Labels
A-miri Area: The miri tool

Comments

@lineaar
Copy link

lineaar commented Jun 26, 2019

Consider the following (https://play.rust-lang.org/?version=stable&mode=debug&edition=2018&gist=3937a5d7f026fb9905f3a2ba3f7e2faf)

#[repr(C, packed)]
struct S {
    f1: u16,
    f2: u8,
}

impl Drop for S {
    fn drop(&mut self) {
        let p = self as *const S as *const u8;
        println!("Invalid p = {:?}", p);
    }
}

impl S {
    fn new() -> Self {
        S { f1: 6, f2: 6 }
    }
}

fn main() {
    let size = std::mem::size_of::<S>();
    let mut buf = [0; 8];
    let s = S::new();
    let p = &s as *const S as *const u8;

    println!("  Valid p = {:?}", p);

    assert_eq!(3, size);

    drop(s);

    for i in 0..size {
        buf[i] = unsafe { *p.add(i) };
    }

    for e in &buf {
        print!("|{}|", e);
    }
}

When reading from p, the original contents of the structure are read. Running this snippet in miri results in an error. What is interesting is the cause of the error:

println!("  Valid p = {:?}", p);

Another curious bit is that changing the attributes of the structure S to the following produces a warning.

#[repr(packed)]
@jonas-schievink
Copy link
Contributor

This just looks like UB, what did you expect to happen here?

@lineaar
Copy link
Author

lineaar commented Jun 26, 2019

  • In regards to reading from p, I expected it to not compile since s moved out and p points to s.
  • In the case where you would remove the two for loops, I expected the code to run successfully under miri.
  • Using #[repr(packed)] or #[repr(C,packed)], I expected to have warnings regarding unused fields in both cases.

@jonas-schievink
Copy link
Contributor

In regards to reading from p, I expected it to not compile since s moved out and p points to s.

p is a raw pointer, which aren't checked like references are. Check the book for more info: https://doc.rust-lang.org/book/ch19-01-unsafe-rust.html#dereferencing-a-raw-pointer

Using #[repr(packed)] or #[repr(C,packed)], I expected to have warnings regarding unused fields in both cases.

This might be intentional since the struct will likely be used for FFI, but could also be a bug.

@lineaar
Copy link
Author

lineaar commented Jun 27, 2019

raw pointers aren’t guaranteed to point to valid memory

Thank you for pointing that out.

This might be intentional since the struct will likely be used for FFI, but could also be a bug.

Would you recommend to keep the issue open if that is the case?

@RalfJung
Copy link
Member

RalfJung commented Jun 28, 2019

(Please Cc me for Miri bugs.)

The reason it does not work in Miri is that arithmetic on integers obtained from pointers is not fully supported yet. Hence you cannot print pointer values with Miri. However, we are very close to getting there! This is tracked at rust-lang/miri#224.

@jonas-schievink jonas-schievink added the A-miri Area: The miri tool label Jul 28, 2019
@RalfJung
Copy link
Member

RalfJung commented Aug 4, 2019

I don't think there is a Mri bug to track here. These days Miri can run the example, and it works and no UB is found. Whether use-after-drop should be UB is being discussed at rust-lang/unsafe-code-guidelines#188.

What remains is the fact that unused fields do not produce a warning with repr(C). I do not know if that is deliberate. @lineaar if you think that is a bug, could you open a new bug for that? Reusing the one here would be just confusing, too much discussion is about other issues. (In general, please always open separate issues for separate bugs. Mixing multiple bugs in one GH issue just ends up in a mess.)

@RalfJung RalfJung closed this as completed Aug 4, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-miri Area: The miri tool
Projects
None yet
Development

No branches or pull requests

3 participants