Skip to content

Commit

Permalink
Rollup merge of rust-lang#124013 - RalfJung:box-to-raw, r=oli-obk
Browse files Browse the repository at this point in the history
Box::into_raw: make Miri understand that this is a box-to-raw cast

Turns out rust-lang#122647 went a bit too far in cleaning up `Box`... we still need a hack in `Box::into_raw`. The nicer fix would be to make Stacked Borrows not care about reference-to-raw-pointer casts, but it's unclear whether that will ever be possible without going to full Tree Borrows.

Fixes rust-lang/miri#3473.
  • Loading branch information
GuillaumeGomez authored Apr 16, 2024
2 parents 4764dce + 8606efa commit 51cfa95
Show file tree
Hide file tree
Showing 5 changed files with 48 additions and 4 deletions.
8 changes: 6 additions & 2 deletions library/alloc/src/boxed.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1058,7 +1058,8 @@ impl<T: ?Sized, A: Allocator> Box<T, A> {
#[stable(feature = "box_raw", since = "1.4.0")]
#[inline]
pub fn into_raw(b: Self) -> *mut T {
Self::into_raw_with_allocator(b).0
// Make sure Miri realizes that we transition from a noalias pointer to a raw pointer here.
unsafe { addr_of_mut!(*&mut *Self::into_raw_with_allocator(b).0) }
}

/// Consumes the `Box`, returning a wrapped raw pointer and the allocator.
Expand Down Expand Up @@ -1112,7 +1113,10 @@ impl<T: ?Sized, A: Allocator> Box<T, A> {
pub fn into_raw_with_allocator(b: Self) -> (*mut T, A) {
let mut b = mem::ManuallyDrop::new(b);
// We carefully get the raw pointer out in a way that Miri's aliasing model understands what
// is happening: using the primitive "deref" of `Box`.
// is happening: using the primitive "deref" of `Box`. In case `A` is *not* `Global`, we
// want *no* aliasing requirements here!
// In case `A` *is* `Global`, this does not quite have the right behavior; `into_raw`
// works around that.
let ptr = addr_of_mut!(**b);
let alloc = unsafe { ptr::read(&b.1) };
(ptr, alloc)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ LL | Box(unsafe { Unique::new_unchecked(raw) }, alloc)
|
= 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: <TAG> was created by a Unique retag at offsets [0x0..0x4]
help: <TAG> was created by a SharedReadWrite retag at offsets [0x0..0x4]
--> $DIR/newtype_pair_retagging.rs:LL:CC
|
LL | let ptr = Box::into_raw(Box::new(0i32));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ LL | Box(unsafe { Unique::new_unchecked(raw) }, alloc)
|
= 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: <TAG> was created by a Unique retag at offsets [0x0..0x4]
help: <TAG> was created by a SharedReadWrite retag at offsets [0x0..0x4]
--> $DIR/newtype_retagging.rs:LL:CC
|
LL | let ptr = Box::into_raw(Box::new(0i32));
Expand Down
28 changes: 28 additions & 0 deletions src/tools/miri/tests/pass/issues/issue-miri-3473.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
//@revisions: stack tree
//@[tree]compile-flags: -Zmiri-tree-borrows
use std::cell::UnsafeCell;

#[repr(C)]
#[derive(Default)]
struct Node {
_meta: UnsafeCell<usize>,
value: usize,
}

impl Node {
fn value(&self) -> &usize {
&self.value
}
}

/// This used to cause Stacked Borrows errors because of trouble around conversion
/// from Box to raw pointer.
fn main() {
unsafe {
let a = Box::into_raw(Box::new(Node::default()));
let ptr = &*a;
*UnsafeCell::raw_get(a.cast::<UnsafeCell<usize>>()) = 2;
assert_eq!(*ptr.value(), 0);
drop(Box::from_raw(a));
}
}
12 changes: 12 additions & 0 deletions src/tools/miri/tests/pass/stacked-borrows/stacked-borrows.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ fn main() {
wide_raw_ptr_in_tuple();
not_unpin_not_protected();
write_does_not_invalidate_all_aliases();
box_into_raw_allows_interior_mutable_alias();
}

// Make sure that reading from an `&mut` does, like reborrowing to `&`,
Expand Down Expand Up @@ -263,3 +264,14 @@ fn write_does_not_invalidate_all_aliases() {
other::lib2();
assert_eq!(*x, 1337); // oops, the value changed! I guess not all pointers were invalidated
}

fn box_into_raw_allows_interior_mutable_alias() { unsafe {
let b = Box::new(std::cell::Cell::new(42));
let raw = Box::into_raw(b);
let c = &*raw;
let d = raw.cast::<i32>(); // bypassing `Cell` -- only okay in Miri tests
// `c` and `d` should permit arbitrary aliasing with each other now.
*d = 1;
c.set(2);
drop(Box::from_raw(raw));
} }

0 comments on commit 51cfa95

Please sign in to comment.