Skip to content

Miri detect invalid deallocation where things seems fine #2751

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

Closed
trinity-1686a opened this issue Jan 8, 2023 · 5 comments
Closed

Miri detect invalid deallocation where things seems fine #2751

trinity-1686a opened this issue Jan 8, 2023 · 5 comments

Comments

@trinity-1686a
Copy link

I tried to do some things with Strings, and got Miri complaining about attempting to deallocate.

This code is from String::from_raw_parts documentation, except for s.reserve(1) which should have no safety impact. In general I have no issue if string capacity == string len, but issues when they differ.

use std::mem;

fn main() {
unsafe {
    let mut s = String::from("hello");
    s.reserve(1); // <= not in std example

    // Prevent automatically dropping the String's data
    let mut s = mem::ManuallyDrop::new(s);

    let ptr = s.as_mut_ptr();
    let len = s.len();
    let capacity = s.capacity();

    let s = String::from_raw_parts(ptr, len, capacity);

    assert_eq!(String::from("hello"), s);
}
}

miri error:

error: Undefined Behavior: attempting deallocation using <3737> at alloc1883, but that tag does not exist in the borrow stack for this location
   --> /playground/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/alloc/src/alloc.rs:113:14
    |
113 |     unsafe { __rust_dealloc(ptr, layout.size(), layout.align()) }
    |              ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ attempting deallocation using <3737> at alloc1883, but that tag does not exist in the borrow stack for this location
    |
    = help: this indicates a potential bug in the program: it performed an invalid operation, but the Stacked Borrows rules it violated are still experimental
    = help: see https://github.com/rust-lang/unsafe-code-guidelines/blob/master/wip/stacked-borrows.md for further information
help: <3737> was created by a SharedReadWrite retag at offsets [0x0..0x5]
   --> src/main.rs:11:15
    |
11  |     let ptr = s.as_mut_ptr();
    |               ^^^^^^^^^^^^^^
    = note: BACKTRACE (of the first span):
    = note: inside `std::alloc::dealloc` at /playground/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/alloc/src/alloc.rs:113:14: 113:64
    = note: inside `<std::alloc::Global as std::alloc::Allocator>::deallocate` at /playground/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/alloc/src/alloc.rs:250:22: 250:51
    = note: inside `<alloc::raw_vec::RawVec<u8> as std::ops::Drop>::drop` at /playground/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/alloc/src/raw_vec.rs:479:22: 479:56
    = note: inside `std::ptr::drop_in_place::<alloc::raw_vec::RawVec<u8>> - shim(Some(alloc::raw_vec::RawVec<u8>))` at /playground/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/core/src/ptr/mod.rs:490:1: 490:56
    = note: inside `std::ptr::drop_in_place::<std::vec::Vec<u8>> - shim(Some(std::vec::Vec<u8>))` at /playground/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/core/src/ptr/mod.rs:490:1: 490:56
    = note: inside `std::ptr::drop_in_place::<std::string::String> - shim(Some(std::string::String))` at /playground/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/core/src/ptr/mod.rs:490:1: 490:56
note: inside `main`
   --> src/main.rs:18:1
    |
18  | }
    | ^

note: some details are omitted, run with `MIRIFLAGS=-Zmiri-backtrace=full` for a verbose backtrace
@bjorn3
Copy link
Member

bjorn3 commented Jan 8, 2023

This seems to be an issue in the documentation, albeit a tricky one. The as_mut_ptr method comes from str, not String. The deref from String to str shrinks the region to which the str and thus as_mut_ptr result is valid for to fit exactly the size and not the capacity of the string. I think the only way to do this currently are s.into_bytes().as_mut_ptr() (which uses Vec::as_mut_ptr, which does cover the whole capacity) or s.into_raw_parts(), which is unstable.

@bjorn3
Copy link
Member

bjorn3 commented Jan 8, 2023

One way to fix this would be to add an as_mut_ptr method directly to String.

@trinity-1686a
Copy link
Author

converting to a Vec<u8> before calling ManuallyDrop::new does indeed make Miri happy. Should I open an issue on rust repository then?

@bjorn3
Copy link
Member

bjorn3 commented Jan 8, 2023

I think so.

@trinity-1686a
Copy link
Author

closing in favor of rust-lang/rust#106593

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

2 participants