std's `From<Box<T, A>> for Rc<T, A>` implementation is unsound when `A` is a custom allocator.

[steffahn]

/*
[dependencies]
bumpalo = { version = "3", features = ["allocator_api"] }
*/

#![feature(allocator_api)]

use bumpalo::Bump;
use std::rc::Rc;


fn main() {
    let bump = Bump::new();
    let boxx = Box::new_in(42, &bump);

    let rc = Rc::<i32, _>::from(boxx);
}

run on rustexplorer.com

Output:

free(): invalid pointer

Miri:

error: Undefined Behavior: deallocating 0x258bc[alloc1011]<2851> which does not point to the beginning of an object
   --> /home/frank/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/alloc/src/alloc.rs:117:14
    |
117 |     unsafe { __rust_dealloc(ptr, layout.size(), layout.align()) }
    |              ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ deallocating 0x258bc[alloc1011]<2851> which does not point to the beginning of an object
    |
    = help: this indicates a bug in the program: it performed an invalid operation, and caused Undefined Behavior
    = help: see https://doc.rust-lang.org/nightly/reference/behavior-considered-undefined.html for further information
    = note: BACKTRACE:
    = note: inside `std::alloc::dealloc` at /home/frank/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/alloc/src/alloc.rs:117:14: 117:64
    = note: inside `<std::alloc::Global as std::alloc::Allocator>::deallocate` at /home/frank/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/alloc/src/alloc.rs:254:22: 254:51
    = note: inside `<std::boxed::Box<std::mem::ManuallyDrop<i32>> as std::ops::Drop>::drop` at /home/frank/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/alloc/src/boxed.rs:1244:17: 1244:66
    = note: inside `std::ptr::drop_in_place::<std::boxed::Box<std::mem::ManuallyDrop<i32>>> - shim(Some(std::boxed::Box<std::mem::ManuallyDrop<i32>>))` at /home/frank/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/core/src/ptr/mod.rs:507:1: 507:56
    = note: inside `std::mem::drop::<std::boxed::Box<std::mem::ManuallyDrop<i32>>>` at /home/frank/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/core/src/mem/mod.rs:992:24: 992:25
    = note: inside `std::rc::Rc::<i32, &bumpalo::Bump>::from_box_in` at /home/frank/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/alloc/src/rc.rs:1928:13: 1928:22
    = note: inside `<std::rc::Rc<i32, &bumpalo::Bump> as std::convert::From<std::boxed::Box<i32, &bumpalo::Bump>>>::from` at /home/frank/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/alloc/src/rc.rs:2504:9: 2504:27
note: inside `main`
   --> src/main.rs:16:14
    |
16  |     let rc = Rc::<i32, _>::from(boxx);
    |              ^^^^^^^^^^^^^^^^^^^^^^^^

note: some details are omitted, run with `MIRIFLAGS=-Zmiri-backtrace=full` for a verbose backtrace

The underlying issue was found when reviewing the implementation of From<Box<T, A>> for Rc<T, A>. The code is a reproduction to trigger the unsoundness. The implementation in question features:

rust/library/alloc/src/rc.rs

Lines 1912 to 1932 in ca663b0

	#[cfg(not(no_global_oom_handling))]
	fn from_box_in(src: Box<T, A>) -> Rc<T, A> {
	unsafe {
	let value_size = size_of_val(&*src);
	let ptr = Self::allocate_for_ptr_in(&*src, Box::allocator(&src));
	// Copy value as bytes
	ptr::copy_nonoverlapping(
	&*src as *const T as *const u8,
	&mut (*ptr).value as *mut _ as *mut u8,
	value_size,
	);
	// Free the allocation without dropping its contents
	let (bptr, alloc) = Box::into_raw_with_allocator(src);
	let src = Box::from_raw(bptr as *mut mem::ManuallyDrop<T>);
	drop(src);
	Self::from_ptr_in(ptr, alloc)
	}
	}

Where Box<T, A> is turned into Box<MaybeUninit<T>> and dropped. Yes, that’s Box<MaybeUninit<T>, Global>, not Box<MaybeUninit<T>, A>.

The code for Arc seems to be doing the same thing.

rust/library/alloc/src/sync.rs

Lines 1858 to 1877 in ca663b0

	fn from_box_in(src: Box<T, A>) -> Arc<T, A> {
	unsafe {
	let value_size = size_of_val(&*src);
	let ptr = Self::allocate_for_ptr_in(&*src, Box::allocator(&src));
	// Copy value as bytes
	ptr::copy_nonoverlapping(
	&*src as *const T as *const u8,
	&mut (*ptr).data as *mut _ as *mut u8,
	value_size,
	);
	// Free the allocation without dropping its contents
	let (bptr, alloc) = Box::into_raw_with_allocator(src);
	let src = Box::from_raw(bptr as *mut mem::ManuallyDrop<T>);
	drop(src);
	Self::from_ptr_in(ptr, alloc)
	}
	}

For comparison: The code for From<Vec<T, A>> for Rc<[T]> (and the same thing for Arc) seems to properly use a Vec<MaybeUninit<T>, &A>… no actually, that’s a Vec<T, &A> with zero lengths… notably the allocator is correct though.

rust/library/alloc/src/rc.rs

Lines 2521 to 2535 in ca663b0

	#[inline]
	fn from(v: Vec<T, A>) -> Rc<[T], A> {
	unsafe {
	let (vec_ptr, len, cap, alloc) = v.into_raw_parts_with_alloc();
	let rc_ptr = Self::allocate_for_slice_in(len, &alloc);
	ptr::copy_nonoverlapping(vec_ptr, &mut (*rc_ptr).value as *mut [T] as *mut T, len);
	// Create a `Vec<T, &A>` with length 0, to deallocate the buffer
	// without dropping its contents or the allocator
	let _ = Vec::from_raw_parts_in(vec_ptr, 0, cap, &alloc);
	Self::from_ptr_in(rc_ptr, alloc)
	}
	}

So the buggy code for Box should just switch to create a Box<MaybeUninit<T>, &A> presumably.

@rustbot label I-unsound requires-nightly T-libs