From 8b3637635b46d74dfa7bcd1a7d30910c02b48ec4 Mon Sep 17 00:00:00 2001 From: Xinglu Chen Date: Wed, 30 Apr 2025 10:22:15 +0200 Subject: [PATCH 1/2] Correctly handle interior mutable data in `Box` Previously, the pointee type would be behind a `*const` pointer, so `ty_is_freeze` would always be `true`, even if there was an `UnsafeCell` in `Box`. --- .../src/borrow_tracker/tree_borrows/mod.rs | 2 +- .../pass/tree_borrows/cell-inside-box.rs | 19 +++++++++++++++++++ .../pass/tree_borrows/cell-inside-box.stderr | 6 ++++++ 3 files changed, 26 insertions(+), 1 deletion(-) create mode 100644 src/tools/miri/tests/pass/tree_borrows/cell-inside-box.rs create mode 100644 src/tools/miri/tests/pass/tree_borrows/cell-inside-box.stderr diff --git a/src/tools/miri/src/borrow_tracker/tree_borrows/mod.rs b/src/tools/miri/src/borrow_tracker/tree_borrows/mod.rs index ce7d7ab25d76..17e452ec5b45 100644 --- a/src/tools/miri/src/borrow_tracker/tree_borrows/mod.rs +++ b/src/tools/miri/src/borrow_tracker/tree_borrows/mod.rs @@ -173,7 +173,7 @@ impl<'tcx> NewPermission { pointee.is_unpin(*cx.tcx, cx.typing_env()).then_some(()).map(|()| { // Regular `Unpin` box, give it `noalias` but only a weak protector // because it is valid to deallocate it within the function. - let ty_is_freeze = ty.is_freeze(*cx.tcx, cx.typing_env()); + let ty_is_freeze = pointee.is_freeze(*cx.tcx, cx.typing_env()); let protected = kind == RetagKind::FnEntry; let initial_state = Permission::new_reserved(ty_is_freeze, protected); Self { diff --git a/src/tools/miri/tests/pass/tree_borrows/cell-inside-box.rs b/src/tools/miri/tests/pass/tree_borrows/cell-inside-box.rs new file mode 100644 index 000000000000..a2eb4ede44b2 --- /dev/null +++ b/src/tools/miri/tests/pass/tree_borrows/cell-inside-box.rs @@ -0,0 +1,19 @@ +//@compile-flags: -Zmiri-tree-borrows +#![feature(box_as_ptr)] +#[path = "../../utils/mod.rs"] +#[macro_use] +mod utils; + +use std::cell::UnsafeCell; + +pub fn main() { + let cell = UnsafeCell::new(42); + let mut root = Box::new(cell); + + let a = Box::as_mut_ptr(&mut root); + unsafe { + name!(a); + let alloc_id = alloc_id!(a); + print_state!(alloc_id); + } +} diff --git a/src/tools/miri/tests/pass/tree_borrows/cell-inside-box.stderr b/src/tools/miri/tests/pass/tree_borrows/cell-inside-box.stderr new file mode 100644 index 000000000000..89421380a3ff --- /dev/null +++ b/src/tools/miri/tests/pass/tree_borrows/cell-inside-box.stderr @@ -0,0 +1,6 @@ +────────────────────────────────────────────────── +Warning: this tree is indicative only. Some tags may have been hidden. +0.. 4 +| Act | └─┬── +| ReIM| └──── +────────────────────────────────────────────────── From 45d4bb2541c240087c86fa1eb742c04feabe9392 Mon Sep 17 00:00:00 2001 From: Xinglu Chen Date: Fri, 2 May 2025 19:46:09 +0200 Subject: [PATCH 2/2] Construct test so that it would fail for old code --- .../pass/tree_borrows/cell-inside-box.rs | 24 +++++++++++++++---- .../pass/tree_borrows/cell-inside-box.stderr | 3 ++- 2 files changed, 22 insertions(+), 5 deletions(-) diff --git a/src/tools/miri/tests/pass/tree_borrows/cell-inside-box.rs b/src/tools/miri/tests/pass/tree_borrows/cell-inside-box.rs index a2eb4ede44b2..adf2f4e845b5 100644 --- a/src/tools/miri/tests/pass/tree_borrows/cell-inside-box.rs +++ b/src/tools/miri/tests/pass/tree_borrows/cell-inside-box.rs @@ -8,12 +8,28 @@ use std::cell::UnsafeCell; pub fn main() { let cell = UnsafeCell::new(42); - let mut root = Box::new(cell); + let box1 = Box::new(cell); - let a = Box::as_mut_ptr(&mut root); unsafe { - name!(a); - let alloc_id = alloc_id!(a); + let ptr1: *mut UnsafeCell = Box::into_raw(box1); + name!(ptr1); + + let mut box2 = Box::from_raw(ptr1); + // `ptr2` will be a descendant of `ptr1`. + let ptr2: *mut UnsafeCell = Box::as_mut_ptr(&mut box2); + name!(ptr2); + + // We perform a write through `x`. + // Because `ptr1` is ReservedIM, a child write will make it transition to Active. + // Because `ptr2` is ReservedIM, a foreign write doesn't have any effect on it. + let x = (*ptr1).get(); + *x = 1; + + // We can still read from `ptr2`. + let val = *(*ptr2).get(); + assert_eq!(val, 1); + + let alloc_id = alloc_id!(ptr1); print_state!(alloc_id); } } diff --git a/src/tools/miri/tests/pass/tree_borrows/cell-inside-box.stderr b/src/tools/miri/tests/pass/tree_borrows/cell-inside-box.stderr index 89421380a3ff..5dbfff718b1e 100644 --- a/src/tools/miri/tests/pass/tree_borrows/cell-inside-box.stderr +++ b/src/tools/miri/tests/pass/tree_borrows/cell-inside-box.stderr @@ -2,5 +2,6 @@ Warning: this tree is indicative only. Some tags may have been hidden. 0.. 4 | Act | └─┬── -| ReIM| └──── +| Act | └─┬── +| ReIM| └──── ──────────────────────────────────────────────────