Tree Borrows: multiple invalid exposed nodes on main subtree

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);
         }
     }
 }

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);
             }
         }
@@ -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());
+
+                // We only count exposed nodes through which an access could happen.
+                if node.is_exposed
+                    && 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;
+                }
+
                 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,23 @@ 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).
+            && self
+                .wildcard_accesses
+                .get(root)
+                .unwrap()
+                .access_relatedness(access_kind, /* only_foreign */ true)
+                .is_none()
+        {
+            return Err(no_valid_exposed_references_error(diagnostics)).into();
+        }
         interp_ok(())
     }
 }

src/borrow_tracker/tree_borrows/wildcard.rs

@@ -20,6 +20,16 @@ pub enum WildcardAccessLevel {
     Read,
     Write,
 }
+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
+    }
+}
 
 /// Where the access happened relative to the current node.
 #[derive(Clone, Copy, Debug, PartialEq, Eq)]
@@ -430,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,
@@ -480,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
                 };

tests/fail/tree_borrows/wildcard/cross_tree_update_main_invalid_exposed2.rs

@@ -0,0 +1,48 @@
+//@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.
+/// 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;
+
+    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<u32>) };
+    let _int3 = ref3 as *mut Cell<u32> as usize;
+    let ref4 = unsafe { &mut *(ptr_base as *mut Cell<u32>) };
+    let _int4 = ref4 as *mut Cell<u32> 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/
+}

tests/fail/tree_borrows/wildcard/cross_tree_update_main_invalid_exposed2.stderr

@@ -0,0 +1,14 @@
+error: Undefined Behavior: write access through <wildcard> 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
+