From 8606efa5d24b1aeefa14b285920a0291953149fa Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Tue, 16 Apr 2024 14:13:18 +0200 Subject: [PATCH] Box::into_raw: make Miri understand that this is a box-to-raw cast --- library/alloc/src/boxed.rs | 8 ++++-- .../newtype_pair_retagging.stack.stderr | 2 +- .../newtype_retagging.stack.stderr | 2 +- .../miri/tests/pass/issues/issue-miri-3473.rs | 28 +++++++++++++++++++ .../pass/stacked-borrows/stacked-borrows.rs | 12 ++++++++ 5 files changed, 48 insertions(+), 4 deletions(-) create mode 100644 src/tools/miri/tests/pass/issues/issue-miri-3473.rs diff --git a/library/alloc/src/boxed.rs b/library/alloc/src/boxed.rs index 7c3fa2312e5..ed3ad8b39a5 100644 --- a/library/alloc/src/boxed.rs +++ b/library/alloc/src/boxed.rs @@ -1058,7 +1058,8 @@ impl Box { #[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. @@ -1112,7 +1113,10 @@ pub fn into_raw(b: Self) -> *mut T { 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) diff --git a/src/tools/miri/tests/fail/both_borrows/newtype_pair_retagging.stack.stderr b/src/tools/miri/tests/fail/both_borrows/newtype_pair_retagging.stack.stderr index 867907e98e6..c26c7f397b0 100644 --- a/src/tools/miri/tests/fail/both_borrows/newtype_pair_retagging.stack.stderr +++ b/src/tools/miri/tests/fail/both_borrows/newtype_pair_retagging.stack.stderr @@ -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: was created by a Unique retag at offsets [0x0..0x4] +help: 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)); diff --git a/src/tools/miri/tests/fail/both_borrows/newtype_retagging.stack.stderr b/src/tools/miri/tests/fail/both_borrows/newtype_retagging.stack.stderr index 56715938e97..ae54da70fe2 100644 --- a/src/tools/miri/tests/fail/both_borrows/newtype_retagging.stack.stderr +++ b/src/tools/miri/tests/fail/both_borrows/newtype_retagging.stack.stderr @@ -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: was created by a Unique retag at offsets [0x0..0x4] +help: 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)); diff --git a/src/tools/miri/tests/pass/issues/issue-miri-3473.rs b/src/tools/miri/tests/pass/issues/issue-miri-3473.rs new file mode 100644 index 00000000000..77b960c1cdc --- /dev/null +++ b/src/tools/miri/tests/pass/issues/issue-miri-3473.rs @@ -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, + 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::>()) = 2; + assert_eq!(*ptr.value(), 0); + drop(Box::from_raw(a)); + } +} diff --git a/src/tools/miri/tests/pass/stacked-borrows/stacked-borrows.rs b/src/tools/miri/tests/pass/stacked-borrows/stacked-borrows.rs index 734411ccc72..43ba490d5bb 100644 --- a/src/tools/miri/tests/pass/stacked-borrows/stacked-borrows.rs +++ b/src/tools/miri/tests/pass/stacked-borrows/stacked-borrows.rs @@ -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 `&`, @@ -263,3 +264,14 @@ pub fn lib2() { 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::(); // 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)); +} }