From 4352e93eae638f0e1b74ff6b2f6055c8bb044a21 Mon Sep 17 00:00:00 2001 From: Roy Ammerschuber Date: Tue, 2 Dec 2025 17:46:44 +0100 Subject: [PATCH 1/6] detect properly if there are valid exposed nodes --- src/borrow_tracker/tree_borrows/tree.rs | 26 ++++++++++- src/borrow_tracker/tree_borrows/wildcard.rs | 8 ++++ ...cross_tree_update_main_invalid_exposed2.rs | 46 +++++++++++++++++++ ...s_tree_update_main_invalid_exposed2.stderr | 14 ++++++ 4 files changed, 92 insertions(+), 2 deletions(-) create mode 100644 tests/fail/tree_borrows/wildcard/cross_tree_update_main_invalid_exposed2.rs create mode 100644 tests/fail/tree_borrows/wildcard/cross_tree_update_main_invalid_exposed2.stderr diff --git a/src/borrow_tracker/tree_borrows/tree.rs b/src/borrow_tracker/tree_borrows/tree.rs index 900e9c3729..540abba7b5 100644 --- a/src/borrow_tracker/tree_borrows/tree.rs +++ b/src/borrow_tracker/tree_borrows/tree.rs @@ -1034,6 +1034,8 @@ impl<'tcx> LocationTree { wildcard_state.access_relatedness(access_kind, only_foreign) }; + let mut has_exposed = false; + // This does a traversal across the tree updating children before their parents. The // difference to `perform_normal_access` is that we take the access relatedness from // the wildcard tracking state of the node instead of from the visitor itself. @@ -1082,6 +1084,17 @@ impl<'tcx> LocationTree { return Err(no_valid_exposed_references_error(diagnostics)); }; + let mut entry = args.data.perms.entry(args.idx); + let perm = entry.or_insert(node.default_location_state()); + + if node.is_exposed + && perm.permission.strongest_allowed_child_access(protected) + >= access_kind.into() + && max_local_tag.is_none_or(|max_local_tag| max_local_tag > node.tag) + { + has_exposed = true; + } + let Some(relatedness) = wildcard_relatedness.to_relatedness() else { // If the access type is Either, then we do not apply any transition // to this node, but we still update each of its children. @@ -1090,8 +1103,6 @@ impl<'tcx> LocationTree { return Ok(()); }; - let mut entry = args.data.perms.entry(args.idx); - let perm = entry.or_insert(node.default_location_state()); // We know the exact relatedness, so we can actually do precise checks. perm.perform_transition( args.idx, @@ -1115,6 +1126,17 @@ impl<'tcx> LocationTree { }) }, )?; + if !has_exposed + && self + .wildcard_accesses + .get(root) + .unwrap() + .access_relatedness(access_kind, true) + .is_none() + { + return Err(no_valid_exposed_references_error(alloc_id, loc_range.start, access_cause)) + .into(); + } interp_ok(()) } } diff --git a/src/borrow_tracker/tree_borrows/wildcard.rs b/src/borrow_tracker/tree_borrows/wildcard.rs index 3b55a9e36e..84ceb2a3f1 100644 --- a/src/borrow_tracker/tree_borrows/wildcard.rs +++ b/src/borrow_tracker/tree_borrows/wildcard.rs @@ -20,6 +20,14 @@ pub enum WildcardAccessLevel { Read, Write, } +impl From for WildcardAccessLevel { + fn from(value: AccessKind) -> Self { + match value { + AccessKind::Read => Self::Read, + AccessKind::Write => Self::Write, + } + } +} /// Where the access happened relative to the current node. #[derive(Clone, Copy, Debug, PartialEq, Eq)] diff --git a/tests/fail/tree_borrows/wildcard/cross_tree_update_main_invalid_exposed2.rs b/tests/fail/tree_borrows/wildcard/cross_tree_update_main_invalid_exposed2.rs new file mode 100644 index 0000000000..0d9ef466f7 --- /dev/null +++ b/tests/fail/tree_borrows/wildcard/cross_tree_update_main_invalid_exposed2.rs @@ -0,0 +1,46 @@ +//@compile-flags: -Zmiri-tree-borrows -Zmiri-permissive-provenance +use std::cell::Cell; + +/// Checks how accesses from one subtree affect other subtrees. +/// This test checks that an access from a subtree performs a +/// wildcard access on all earlier trees, and that local +/// accesses are treated as access errors for tags that are +/// larger than the root of the accessed subtree. +pub fn main() { + let mut x: u32 = 42; + + let ptr_base = &mut x as *mut u32; + let ref1 = unsafe { &mut *ptr_base }; + let int1 = ref1 as *mut u32 as usize; + let wild = int1 as *mut u32; + + // Activates ref1. + *ref1 = 4; + + let ref2 = unsafe { &mut *wild }; + + // Freezes ref1. + let ref3 = unsafe { &mut *(ptr_base as *mut Cell) }; + let _int3 = ref3 as *mut Cell as usize; + let ref4 = unsafe { &mut *(ptr_base as *mut Cell) }; + let _int4 = ref4 as *mut Cell as usize; + + // ┌──────────────┐ + // │ │ + // │ptr_base(Act) ├───────────┬──────────────────┐ * + // │ │ │ │ │ + // └──────┬───────┘ │ │ │ + // │ │ │ │ + // │ │ │ │ + // ▼ ▼ ▼ ▼ + // ┌─────────────┐ ┌────────────┐ ┌────────────┐ ┌───────────┐ + // │ │ │ │ │ │ │ │ + // │ ref1(Frz)* │ │ ref3(ReIM)*│ │ ref4(ReIM)*│ │ ref2(Res) │ + // │ │ │ │ │ │ │ │ + // └─────────────┘ └────────────┘ └────────────┘ └───────────┘ + + // Performs a wildcard access on the main root. However, as there are + // no exposed tags with write permissions and a tag smaller than ref2 + // this access fails. + *ref2 = 13; //~ ERROR: /write access through .* is forbidden/ +} diff --git a/tests/fail/tree_borrows/wildcard/cross_tree_update_main_invalid_exposed2.stderr b/tests/fail/tree_borrows/wildcard/cross_tree_update_main_invalid_exposed2.stderr new file mode 100644 index 0000000000..1f7a3d6182 --- /dev/null +++ b/tests/fail/tree_borrows/wildcard/cross_tree_update_main_invalid_exposed2.stderr @@ -0,0 +1,14 @@ +error: Undefined Behavior: write access through at ALLOC[0x0] is forbidden + --> tests/fail/tree_borrows/wildcard/cross_tree_update_main_invalid_exposed2.rs:LL:CC + | +LL | *ref2 = 13; + | ^^^^^^^^^^ Undefined Behavior occurred here + | + = help: this indicates a potential bug in the program: it performed an invalid operation, but the Tree Borrows rules it violated are still experimental + = help: see https://github.com/rust-lang/unsafe-code-guidelines/blob/master/wip/tree-borrows.md for further information + = help: there are no exposed tags which may perform this access here + +note: some details are omitted, run with `MIRIFLAGS=-Zmiri-backtrace=full` for a verbose backtrace + +error: aborting due to 1 previous error + From 55e4fb50eac22b207741c16df563acdb32f03505 Mon Sep 17 00:00:00 2001 From: Roy Ammerschuber Date: Wed, 10 Dec 2025 09:35:37 +0100 Subject: [PATCH 2/6] add comments --- src/borrow_tracker/tree_borrows/tree.rs | 4 ++++ .../wildcard/cross_tree_update_main_invalid_exposed2.rs | 2 ++ 2 files changed, 6 insertions(+) diff --git a/src/borrow_tracker/tree_borrows/tree.rs b/src/borrow_tracker/tree_borrows/tree.rs index 540abba7b5..4ee00aa4cd 100644 --- a/src/borrow_tracker/tree_borrows/tree.rs +++ b/src/borrow_tracker/tree_borrows/tree.rs @@ -1087,6 +1087,7 @@ impl<'tcx> LocationTree { let mut entry = args.data.perms.entry(args.idx); let perm = entry.or_insert(node.default_location_state()); + // We only count exposed nodes through which an access could happen. if node.is_exposed && perm.permission.strongest_allowed_child_access(protected) >= access_kind.into() @@ -1126,7 +1127,10 @@ impl<'tcx> LocationTree { }) }, )?; + // If this is the main subtree and it doesn't contain any exposed children through which the access could happen then the access + // is invalid. if !has_exposed + // Check that this is the main subtree. && self .wildcard_accesses .get(root) diff --git a/tests/fail/tree_borrows/wildcard/cross_tree_update_main_invalid_exposed2.rs b/tests/fail/tree_borrows/wildcard/cross_tree_update_main_invalid_exposed2.rs index 0d9ef466f7..ca430cad16 100644 --- a/tests/fail/tree_borrows/wildcard/cross_tree_update_main_invalid_exposed2.rs +++ b/tests/fail/tree_borrows/wildcard/cross_tree_update_main_invalid_exposed2.rs @@ -6,6 +6,8 @@ use std::cell::Cell; /// wildcard access on all earlier trees, and that local /// accesses are treated as access errors for tags that are /// larger than the root of the accessed subtree. +/// This tests the case were we have multiple exposed nodes on +/// the main tree that are invalid because their tag is too large. pub fn main() { let mut x: u32 = 42; From dc8d21fc6cccf2dfd232d4bb1835df18d192457a Mon Sep 17 00:00:00 2001 From: Roy Ammerschuber Date: Wed, 10 Dec 2025 17:27:22 +0100 Subject: [PATCH 3/6] fix rebase issues --- src/borrow_tracker/tree_borrows/tree.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/borrow_tracker/tree_borrows/tree.rs b/src/borrow_tracker/tree_borrows/tree.rs index 4ee00aa4cd..ed315dd99d 100644 --- a/src/borrow_tracker/tree_borrows/tree.rs +++ b/src/borrow_tracker/tree_borrows/tree.rs @@ -1138,8 +1138,7 @@ impl<'tcx> LocationTree { .access_relatedness(access_kind, true) .is_none() { - return Err(no_valid_exposed_references_error(alloc_id, loc_range.start, access_cause)) - .into(); + return Err(no_valid_exposed_references_error(diagnostics)).into(); } interp_ok(()) } From 07f7e6cc84037a4855b23a9051f243329ab9cc75 Mon Sep 17 00:00:00 2001 From: Roy Ammerschuber Date: Tue, 16 Dec 2025 20:03:39 +0100 Subject: [PATCH 4/6] various changes --- src/borrow_tracker/tree_borrows/perms.rs | 12 ++++++------ src/borrow_tracker/tree_borrows/tree.rs | 19 +++++++++++-------- src/borrow_tracker/tree_borrows/wildcard.rs | 12 +++++++----- 3 files changed, 24 insertions(+), 19 deletions(-) diff --git a/src/borrow_tracker/tree_borrows/perms.rs b/src/borrow_tracker/tree_borrows/perms.rs index bd4573f940..064c1cc5b9 100644 --- a/src/borrow_tracker/tree_borrows/perms.rs +++ b/src/borrow_tracker/tree_borrows/perms.rs @@ -374,9 +374,9 @@ impl Permission { self.inner.strongest_idempotent_foreign_access(prot) } - /// Returns the strongest access allowed from a child to this node without + /// Returns the strongest access allowed that is local to this node without /// causing UB (only considers possible transitions to this permission). - pub fn strongest_allowed_child_access(&self, protected: bool) -> WildcardAccessLevel { + pub fn strongest_allowed_local_access(&self, protected: bool) -> WildcardAccessLevel { match self.inner { // Everything except disabled can be accessed by read access. Disabled => WildcardAccessLevel::None, @@ -794,9 +794,9 @@ mod propagation_optimization_checks { /// Checks that `strongest_allowed_child_access` correctly /// represents which transitions are possible. #[test] - fn strongest_allowed_child_access() { + fn strongest_allowed_local_access() { for (permission, protected) in <(Permission, bool)>::exhaustive() { - let strongest_child_access = permission.strongest_allowed_child_access(protected); + let strongest_local_access = permission.strongest_allowed_local_access(protected); let is_read_valid = Permission::perform_access( AccessKind::Read, @@ -814,8 +814,8 @@ mod propagation_optimization_checks { ) .is_some(); - assert_eq!(is_read_valid, strongest_child_access >= WildcardAccessLevel::Read); - assert_eq!(is_write_valid, strongest_child_access >= WildcardAccessLevel::Write); + assert_eq!(is_read_valid, strongest_local_access >= WildcardAccessLevel::Read); + assert_eq!(is_write_valid, strongest_local_access >= WildcardAccessLevel::Write); } } } diff --git a/src/borrow_tracker/tree_borrows/tree.rs b/src/borrow_tracker/tree_borrows/tree.rs index ed315dd99d..76e0483004 100644 --- a/src/borrow_tracker/tree_borrows/tree.rs +++ b/src/borrow_tracker/tree_borrows/tree.rs @@ -111,7 +111,7 @@ impl LocationState { // We need to update the wildcard state, if the permission // of an exposed pointer changes. if node.is_exposed { - let access_type = self.permission.strongest_allowed_child_access(protected); + let access_type = self.permission.strongest_allowed_local_access(protected); WildcardState::update_exposure(idx, access_type, nodes, wildcard_accesses); } } @@ -1089,9 +1089,8 @@ impl<'tcx> LocationTree { // We only count exposed nodes through which an access could happen. if node.is_exposed - && perm.permission.strongest_allowed_child_access(protected) - >= access_kind.into() - && max_local_tag.is_none_or(|max_local_tag| max_local_tag > node.tag) + && perm.permission.strongest_allowed_local_access(protected).allows(access_kind) + && max_local_tag.is_none_or(|max_local_tag| max_local_tag >= node.tag) { has_exposed = true; } @@ -1127,15 +1126,19 @@ impl<'tcx> LocationTree { }) }, )?; - // If this is the main subtree and it doesn't contain any exposed children through which the access could happen then the access - // is invalid. + // For a wildcard access to be valid each subtree needs to either contain an exposed tag + // through which the access could have happened or a foreign access to the subtree must + // be possible. If neither of these is the case than the access is UB. + // In reality this is only ever UB on the main subtree as all other trees always allow + // foreign accesses. if !has_exposed - // Check that this is the main subtree. + // Check that no access that is foreign to this subtree is possible. + // (Its only impossible for the main subtree). && self .wildcard_accesses .get(root) .unwrap() - .access_relatedness(access_kind, true) + .access_relatedness(access_kind, /* only_foreign */ true) .is_none() { return Err(no_valid_exposed_references_error(diagnostics)).into(); diff --git a/src/borrow_tracker/tree_borrows/wildcard.rs b/src/borrow_tracker/tree_borrows/wildcard.rs index 84ceb2a3f1..a4baf85514 100644 --- a/src/borrow_tracker/tree_borrows/wildcard.rs +++ b/src/borrow_tracker/tree_borrows/wildcard.rs @@ -20,12 +20,14 @@ pub enum WildcardAccessLevel { Read, Write, } -impl From for WildcardAccessLevel { - fn from(value: AccessKind) -> Self { - match value { +impl WildcardAccessLevel { + /// Weather this access kind is allowed at this level. + pub fn allows(self, kind: AccessKind) -> bool { + let required_level = match kind { AccessKind::Read => Self::Read, AccessKind::Write => Self::Write, - } + }; + required_level <= self } } @@ -438,7 +440,7 @@ impl Tree { .map(|p| p.permission()) .unwrap_or_else(|| node.default_location_state().permission()); - let access_type = perm.strongest_allowed_child_access(protected); + let access_type = perm.strongest_allowed_local_access(protected); WildcardState::update_exposure( id, access_type, From 2081d1a7201e51751b8b9c6927dae8802e3c4181 Mon Sep 17 00:00:00 2001 From: Roy Ammerschuber Date: Tue, 16 Dec 2025 20:14:15 +0100 Subject: [PATCH 5/6] fix code behind flag --- src/borrow_tracker/tree_borrows/wildcard.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/borrow_tracker/tree_borrows/wildcard.rs b/src/borrow_tracker/tree_borrows/wildcard.rs index a4baf85514..b5ae0ee4c7 100644 --- a/src/borrow_tracker/tree_borrows/wildcard.rs +++ b/src/borrow_tracker/tree_borrows/wildcard.rs @@ -490,7 +490,7 @@ impl Tree { perms.get(id).copied().unwrap_or_else(|| node.default_location_state()); perm.permission() - .strongest_allowed_child_access(protected_tags.contains_key(&node.tag)) + .strongest_allowed_local_access(protected_tags.contains_key(&node.tag)) } else { WildcardAccessLevel::None }; From 661c0f25e11084e735693322e3fcab08715ecc37 Mon Sep 17 00:00:00 2001 From: Roy Ammerschuber Date: Sun, 21 Dec 2025 14:48:51 +0100 Subject: [PATCH 6/6] improve comments --- src/borrow_tracker/tree_borrows/tree.rs | 19 +++++++++---------- 1 file changed, 9 insertions(+), 10 deletions(-) diff --git a/src/borrow_tracker/tree_borrows/tree.rs b/src/borrow_tracker/tree_borrows/tree.rs index 76e0483004..c8e27ebf0f 100644 --- a/src/borrow_tracker/tree_borrows/tree.rs +++ b/src/borrow_tracker/tree_borrows/tree.rs @@ -1034,7 +1034,8 @@ impl<'tcx> LocationTree { wildcard_state.access_relatedness(access_kind, only_foreign) }; - let mut has_exposed = false; + // Whether there is an exposed node in this tree that allows this access. + let mut has_valid_exposed = false; // This does a traversal across the tree updating children before their parents. The // difference to `perform_normal_access` is that we take the access relatedness from @@ -1092,7 +1093,7 @@ impl<'tcx> LocationTree { && perm.permission.strongest_allowed_local_access(protected).allows(access_kind) && max_local_tag.is_none_or(|max_local_tag| max_local_tag >= node.tag) { - has_exposed = true; + has_valid_exposed = true; } let Some(relatedness) = wildcard_relatedness.to_relatedness() else { @@ -1126,14 +1127,12 @@ impl<'tcx> LocationTree { }) }, )?; - // For a wildcard access to be valid each subtree needs to either contain an exposed tag - // through which the access could have happened or a foreign access to the subtree must - // be possible. If neither of these is the case than the access is UB. - // In reality this is only ever UB on the main subtree as all other trees always allow - // foreign accesses. - if !has_exposed - // Check that no access that is foreign to this subtree is possible. - // (Its only impossible for the main subtree). + // If there is no exposed node in this tree that allows this access, then the + // access *must* be foreign. So we check if the root of this tree would allow this + // as a foreign access, and if not, then we can error. + // In practice, all wildcard trees accept foreign accesses, but the main tree does + // not, so this catches UB when none of the nodes in the main tree allows this access. + if !has_valid_exposed && self .wildcard_accesses .get(root)