Stacked Borrows violation when using bumpalo::boxed::Box instead of std::box::Box

[lukechu10]

I have already opened an issue on bumpalo's side here: fitzgen/bumpalo#187
However, I am not sure if this is a bumpalo issue or a miri issue so opening here as well.

The following code doesn't run with miri's Stacked Borrows:

use bumpalo::Bump;
use bumpalo::boxed::Box as BumpBox;

fn main() {
    let bump = Bump::new();
    let mut vec = Vec::new();

    let boxed = BumpBox::new_in(1, &bump);
    let raw = BumpBox::into_raw(boxed);
    dbg!(raw);
    vec.push(raw);

    let mut_ref = unsafe { &mut *raw };
    dbg!(mut_ref);

    for raw in vec {
        let _ = unsafe { BumpBox::from_raw(raw) };
    }
}

But if all instances of BumpBox are replaced with the Box from std, miri no longer complains.

Error:

error: Undefined Behavior: trying to retag from <5216> for Unique permission at alloc2489[0x1bc], but that tag does not exist in the borrow stack for this location
  --> src\main.rs:13:28
   |
13 |     let mut_ref = unsafe { &mut *raw };
   |                            ^^^^^^^^^
   |                            |
   |                            trying to retag from <5216> for Unique permission at alloc2489[0x1bc], but that tag does not exist in the borrow stack for this location
   |                            this error occurs as part of retag at alloc2489[0x1bc..0x1c0]
   |
   = 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: <5216> was created by a SharedReadWrite retag at offsets [0x1bc..0x1c0]
  --> src\main.rs:9:15
   |
9  |     let raw = BumpBox::into_raw(boxed);
   |               ^^^^^^^^^^^^^^^^^^^^^^^^
help: <5216> was later invalidated at offsets [0x1bc..0x1c0] by a Unique retag
  --> src\main.rs:9:15
   |
9  |     let raw = BumpBox::into_raw(boxed);
   |               ^^^^^^^^^^^^^^^^^^^^^^^^
   = note: BACKTRACE:
   = note: inside `main` at src\main.rs:13:28
   = note: inside `<fn() as std::ops::FnOnce<()>>::call_once - shim(fn())` at C:\Users\_\.rustup\toolchains\nightly-x86_64-pc-windows-msvc\lib\rustlib\src\rust\library\core\src\ops\function.rs:510:5
   = note: inside `std::sys_common::backtrace::__rust_begin_short_backtrace::<fn(), ()>` at C:\Users\_\.rustup\toolchains\nightly-x86_64-pc-windows-msvc\lib\rustlib\src\rust\library\std\src\sys_common\backtrace.rs:121:18
   = note: inside closure at C:\Users\_\.rustup\toolchains\nightly-x86_64-pc-windows-msvc\lib\rustlib\src\rust\library\std\src\rt.rs:166: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 C:\Users\_\.rustup\toolchains\nightly-x86_64-pc-windows-msvc\lib\rustlib\src\rust\library\core\src\ops\function.rs:609:13
   = note: inside `std::panicking::r#try::do_call::<&dyn std::ops::Fn() -> i32 + std::marker::Sync + std::panic::RefUnwindSafe, i32>` at C:\Users\_\.rustup\toolchains\nightly-x86_64-pc-windows-msvc\lib\rustlib\src\rust\library\std\src\panicking.rs:483:40
   = note: inside `std::panicking::r#try::<i32, &dyn std::ops::Fn() -> i32 + std::marker::Sync + std::panic::RefUnwindSafe>` at C:\Users\_\.rustup\toolchains\nightly-x86_64-pc-windows-msvc\lib\rustlib\src\rust\library\std\src\panicking.rs:447:19
   = note: inside `std::panic::catch_unwind::<&dyn std::ops::Fn() -> i32 + std::marker::Sync + std::panic::RefUnwindSafe, i32>` at C:\Users\_\.rustup\toolchains\nightly-x86_64-pc-windows-msvc\lib\rustlib\src\rust\library\std\src\panic.rs:137:14
   = note: inside closure at C:\Users\_\.rustup\toolchains\nightly-x86_64-pc-windows-msvc\lib\rustlib\src\rust\library\std\src\rt.rs:148:48
   = note: inside `std::panicking::r#try::do_call::<[closure@std::rt::lang_start_internal::{closure#2}], isize>` at C:\Users\_\.rustup\toolchains\nightly-x86_64-pc-windows-msvc\lib\rustlib\src\rust\library\std\src\panicking.rs:483:40
   = note: inside `std::panicking::r#try::<isize, [closure@std::rt::lang_start_internal::{closure#2}]>` at C:\Users\_\.rustup\toolchains\nightly-x86_64-pc-windows-msvc\lib\rustlib\src\rust\library\std\src\panicking.rs:447:19
   = note: inside `std::panic::catch_unwind::<[closure@std::rt::lang_start_internal::{closure#2}], isize>` at C:\Users\_\.rustup\toolchains\nightly-x86_64-pc-windows-msvc\lib\rustlib\src\rust\library\std\src\panic.rs:137:14
   = note: inside `std::rt::lang_start_internal` at C:\Users\_\.rustup\toolchains\nightly-x86_64-pc-windows-msvc\lib\rustlib\src\rust\library\std\src\rt.rs:148:20
   = note: inside `std::rt::lang_start::<()>` at C:\Users\_\.rustup\toolchains\nightly-x86_64-pc-windows-msvc\lib\rustlib\src\rust\library\std\src\rt.rs:165:17

error: aborting due to previous error

I am not sure if this is a false-positive from miri or a regression from bumpalo but the error is there with the following versions of bumpalo and miri:

• bumpalo: 3.5.0 and 3.11.1 (have tested both)
• miri 0.1.0 (42325c52 2022-11-11)

There was no error with previous versions of miri and same code.

This also errors when using the allocator api on nightly.

Possibly related to #2104

[saethlin]

I am not sure if this is a false-positive from miri or a regression from bumpalo

Neither. This is #2528, the previous behavior of Miri was a false negative. You can reproduce the old behavior with MIRIFLAGS=-Zmiri-retag-fields=none.

The issue here is that bumpalo::boxed::Box wraps a &mut T which means that rustc puts noalias on it when this type is passed to a function by value, so Miri is obligated to treat newtype wrappers around references the same way it does references.

The bug in bumpalo is the use of mem::forget. The current implementation of into_raw establishes sharing, then asserts uniqueness by passing a &mut wrapper by move, then attempts to continue sharing. This is a very common mistake, and using ManuallyDrop fixes this.

[lukechu10]

The bug in bumpalo is the use of mem::forget. The current implementation of into_raw establishes sharing, then asserts uniqueness by passing a &mut wrapper by move, then attempts to continue sharing. This is a very common mistake, and using ManuallyDrop fixes this.

If this were a bug in the implementation of bumpalo::boxed::Box::into_raw, how would this explain the error when using std instead with the allocator_api feature?

Edit: Oh wait, are we supposed to use into_raw_with_allocator/from_raw_in instead of just into_raw/frame_raw methods?

[RalfJung]

how would this explain the error when using std instead with the allocator_api feature?

Which error? This seems to be actually unrelated to #2104 if that's what you mean.

[saethlin]

But if all instances of BumpBox are replaced with the Box from std, miri no longer complains.

Miri doesn't complain because the standard library Box::into_raw is not buggy. bumpalo's is. If nobody beats me to it I can submit a PR that fixes this.

[lukechu10]

But if all instances of BumpBox are replaced with the Box from std, miri no longer complains.

Miri doesn't complain because the standard library Box::into_raw is not buggy. bumpalo's is. If nobody beats me to it I can submit a PR that fixes this.

Sorry got a little bit confused with the different methods. You're right, there is no error with Box::into_raw. By all means, please go ahead and submit a PR to bumpalo.

[lukechu10]

Also closing this now because this is more of a bumpalo issue than a miri issue.