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 detects a violation of stacked borrows when joining &str with -Zmiri-tag-raw-pointers #91574

Closed
saethlin opened this issue Dec 5, 2021 · 3 comments
Labels
C-bug Category: This is a bug.

Comments

@saethlin
Copy link
Member

saethlin commented Dec 5, 2021

Originally found by @5225225, I'm just the one with energy at the moment to write this up.

I tried this code:

fn main() {
    ["", ""].join(" ");
}

MIRIFLAGS="-Zmiri-tag-raw-pointers" cargo miri run

I expected to see this happen: Miri does not detect UB

Instead, this happened: Miri detects UB (experimentally)

error: Undefined Behavior: trying to reborrow for Unique at alloc1190, but parent tag <2797> does not have an appropriate item in the borrow stack
   --> /home/ben/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/core/src/slice/mod.rs:421:18
    |
421 |         unsafe { &mut *index.get_unchecked_mut(self) }
    |                  ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ trying to reborrow for Unique at alloc1190, but parent tag <2797> does not have an appropriate item in the borrow stack
    |
    = help: this indicates a potential bug in the program: it performed an invalid operation, but the 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

Meta

rustc --version --verbose:

rustc 1.59.0-nightly (acbe4443c 2021-12-02)
binary: rustc
commit-hash: acbe4443cc4c9695c0b74a7b64b60333c990a400
commit-date: 2021-12-02
host: x86_64-unknown-linux-gnu
release: 1.59.0-nightly
LLVM version: 13.0.0
Backtrace

error: Undefined Behavior: trying to reborrow for Unique at alloc1190, but parent tag <2797> does not have an appropriate item in the borrow stack
   --> /home/ben/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/core/src/slice/mod.rs:421:18
    |
421 |         unsafe { &mut *index.get_unchecked_mut(self) }
    |                  ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ trying to reborrow for Unique at alloc1190, but parent tag <2797> does not have an appropriate item in the borrow stack
    |
    = help: this indicates a potential bug in the program: it performed an invalid operation, but the 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

    = note: inside `core::slice::<impl [u8]>::get_unchecked_mut::<std::ops::Range<usize>>` at /home/ben/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/core/src/slice/mod.rs:421:18
    = note: inside `std::str::join_generic_copy::<str, u8, &str>` at /home/ben/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/alloc/src/str.rs:181:22
    = note: inside `std::str::<impl std::slice::Join<&str> for [&str]>::join` at /home/ben/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/alloc/src/str.rs:93:46
    = note: inside `std::slice::<impl [&str]>::join::<&str>` at /home/ben/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/alloc/src/slice.rs:633:9
note: inside `main` at src/main.rs:2:5
   --> src/main.rs:2:5
    |
2   |     ["", ""].join(" ");
    |     ^^^^^^^^^^^^^^^^^^
    = note: inside `<fn() as std::ops::FnOnce<()>>::call_once - shim(fn())` at /home/ben/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/core/src/ops/function.rs:227:5
    = note: inside `std::sys_common::backtrace::__rust_begin_short_backtrace::<fn(), ()>` at /home/ben/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/std/src/sys_common/backtrace.rs:123:18
    = note: inside closure at /home/ben/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/std/src/rt.rs:145:18
    = note: inside `std::ops::function::impls::<impl std::ops::FnOnce<()> for &dyn std::ops::Fn() -> i32 + std::marker::Sync + std::panic::RefUnwindSafe>::call_once` at /home/ben/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/core/src/ops/function.rs:259:13
    = note: inside `std::panicking::r#try::do_call::<&dyn std::ops::Fn() -> i32 + std::marker::Sync + std::panic::RefUnwindSafe, i32>` at /home/ben/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/std/src/panicking.rs:406:40
    = note: inside `std::panicking::r#try::<i32, &dyn std::ops::Fn() -> i32 + std::marker::Sync + std::panic::RefUnwindSafe>` at /home/ben/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/std/src/panicking.rs:370:19
    = note: inside `std::panic::catch_unwind::<&dyn std::ops::Fn() -> i32 + std::marker::Sync + std::panic::RefUnwindSafe, i32>` at /home/ben/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/std/src/panic.rs:133:14
    = note: inside closure at /home/ben/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/std/src/rt.rs:128:48
    = note: inside `std::panicking::r#try::do_call::<[closure@std::rt::lang_start_internal::{closure#2}], isize>` at /home/ben/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/std/src/panicking.rs:406:40
    = note: inside `std::panicking::r#try::<isize, [closure@std::rt::lang_start_internal::{closure#2}]>` at /home/ben/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/std/src/panicking.rs:370:19
    = note: inside `std::panic::catch_unwind::<[closure@std::rt::lang_start_internal::{closure#2}], isize>` at /home/ben/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/std/src/panic.rs:133:14
    = note: inside `std::rt::lang_start_internal` at /home/ben/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/std/src/rt.rs:128:20
    = note: inside `std::rt::lang_start::<()>` at /home/ben/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/std/src/rt.rs:144:17

error: aborting due to previous error

@saethlin saethlin added the C-bug Category: This is a bug. label Dec 5, 2021
@saethlin saethlin changed the title Miri detects a violation of stacked borrows in when joining &str with -Zmiri-track-raw-pointers Miri detects a violation of stacked borrows when joining &str with -Zmiri-track-raw-pointers Dec 5, 2021
@saethlin saethlin changed the title Miri detects a violation of stacked borrows when joining &str with -Zmiri-track-raw-pointers Miri detects a violation of stacked borrows when joining &str with -Zmiri-tag-raw-pointers Dec 6, 2021
@saethlin
Copy link
Member Author

saethlin commented Dec 8, 2021

I think this is UB even in the absence of stacked borrows because this code tries to construct a slice of uninit T:

let pos = result.len();
let target = result.get_unchecked_mut(pos..reserved_len);

Therefore, I think the way to go is replacing this with &[MaybeUninit<T>], because redoing all the slice copying logic with raw pointers seems messy.

@RalfJung
Copy link
Member

RalfJung commented Dec 9, 2021

I think this is UB even in the absence of stacked borrows because this code tries to construct a slice of uninit T:

Yeah, it technically is, but inside the standard library we sometimes exploit the fact that we know that currently, rustc does not actually make use of that particular UB. (And I personally think we should allow references to uninit data, unless the type is entirely uninhabited. But that is a discussion for rust-lang/unsafe-code-guidelines#77.)

The aliasing issue seems more severe.

matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Dec 11, 2021
…, r=dtolnay

Use spare_capacity_mut instead of invalid unchecked indexing when joining str

This is a fix for rust-lang#91574

I think in general I'd prefer to see this code implemented with raw pointers or `MaybeUninit::write_slice`, but there's existing code in here based on copying from slice to slice, so converting everything from `&[T]` to `&[MaybeUninit<T>]` is less disruptive.
bors added a commit to rust-lang-ci/rust that referenced this issue Dec 14, 2021
…r=dtolnay

Use spare_capacity_mut instead of invalid unchecked indexing when joining str

This is a fix for rust-lang#91574

I think in general I'd prefer to see this code implemented with raw pointers or `MaybeUninit::write_slice`, but there's existing code in here based on copying from slice to slice, so converting everything from `&[T]` to `&[MaybeUninit<T>]` is less disruptive.
@saethlin
Copy link
Member Author

Closed by #91680

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-bug Category: This is a bug.
Projects
None yet
Development

No branches or pull requests

2 participants