From 42479673c56606e04a474b981384e805f71644a2 Mon Sep 17 00:00:00 2001 From: Roy Ammerschuber Date: Thu, 5 Feb 2026 15:05:51 +0100 Subject: [PATCH] simplify wildcard datastructure --- .../src/borrow_tracker/tree_borrows/tree.rs | 79 +-- .../borrow_tracker/tree_borrows/wildcard.rs | 585 +++++------------- 2 files changed, 187 insertions(+), 477 deletions(-) diff --git a/src/tools/miri/src/borrow_tracker/tree_borrows/tree.rs b/src/tools/miri/src/borrow_tracker/tree_borrows/tree.rs index 2c6be522837c..fc8271454796 100644 --- a/src/tools/miri/src/borrow_tracker/tree_borrows/tree.rs +++ b/src/tools/miri/src/borrow_tracker/tree_borrows/tree.rs @@ -26,7 +26,7 @@ use super::foreign_access_skipping::IdempotentForeignAccess; use super::perms::{PermTransition, Permission}; use super::tree_visitor::{ChildrenVisitMode, ContinueTraversal, NodeAppArgs, TreeVisitor}; use super::unimap::{UniIndex, UniKeyMap, UniValMap}; -use super::wildcard::WildcardState; +use super::wildcard::ExposedCache; use crate::borrow_tracker::{AccessKind, GlobalState, ProtectorKind}; use crate::*; @@ -89,7 +89,7 @@ impl LocationState { &mut self, idx: UniIndex, nodes: &mut UniValMap, - wildcard_accesses: &mut UniValMap, + exposed_cache: &mut ExposedCache, access_kind: AccessKind, relatedness: AccessRelatedness, protected: bool, @@ -99,7 +99,7 @@ impl LocationState { // ensures it is only called when `skip_if_known_noop` returns // `Recurse`, due to the contract of `traverse_this_parents_children_other`. self.record_new_access(access_kind, relatedness); - + let old_access_level = self.permission.strongest_allowed_local_access(protected); let transition = self.perform_access(access_kind, relatedness, protected)?; if !transition.is_noop() { let node = nodes.get_mut(idx).unwrap(); @@ -111,8 +111,8 @@ 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_local_access(protected); - WildcardState::update_exposure(idx, access_type, nodes, wildcard_accesses); + let access_level = self.permission.strongest_allowed_local_access(protected); + exposed_cache.update_exposure(nodes, idx, old_access_level, access_level); } } Ok(()) @@ -261,14 +261,8 @@ pub struct LocationTree { /// /// We do uphold the fact that `keys(perms)` is a subset of `keys(nodes)` pub perms: UniValMap, - /// Maps a tag and a location to its wildcard access tracking information, - /// with possible lazy initialization. - /// - /// If this allocation doesn't have any exposed nodes, then this map doesn't get - /// initialized. This way we only need to allocate the map if we need it. - /// - /// NOTE: same guarantees on entry initialization as for `perms`. - pub wildcard_accesses: UniValMap, + /// Caches information about the relatedness of nodes for a wildcard access. + pub exposed_cache: ExposedCache, } /// Tree structure with both parents and children since we want to be /// able to traverse the tree efficiently in both directions. @@ -276,7 +270,7 @@ pub struct LocationTree { pub struct Tree { /// Mapping from tags to keys. The key obtained can then be used in /// any of the `UniValMap` relative to this allocation, i.e. - /// `nodes`, `LocationTree::perms` and `LocationTree::wildcard_accesses` + /// `nodes`, `LocationTree::perms` and `LocationTree::exposed_cache` /// of the same `Tree`. /// The parent-child relationship in `Node` is encoded in terms of these same /// keys, so traversing the entire tree needs exactly one access to @@ -372,8 +366,8 @@ impl Tree { IdempotentForeignAccess::None, ), ); - let wildcard_accesses = UniValMap::default(); - DedupRangeMap::new(size, LocationTree { perms, wildcard_accesses }) + let exposed_cache = ExposedCache::default(); + DedupRangeMap::new(size, LocationTree { perms, exposed_cache }) }; Self { roots: SmallVec::from_slice(&[root_idx]), nodes, locations, tag_mapping } } @@ -451,19 +445,9 @@ impl<'tcx> Tree { } } - // We need to ensure the consistency of the wildcard access tracking data structure. - // For this, we insert the correct entry for this tag based on its parent, if it exists. - // If we are inserting a new wildcard root (with Wildcard as parent_prov) then we insert - // the special wildcard root initial state instead. - for (_range, loc) in self.locations.iter_mut_all() { - if let Some(parent_idx) = parent_idx { - if let Some(parent_access) = loc.wildcard_accesses.get(parent_idx) { - loc.wildcard_accesses.insert(idx, parent_access.for_new_child()); - } - } else { - loc.wildcard_accesses.insert(idx, WildcardState::for_wildcard_root()); - } - } + // We don't have to update `exposed_cache` as the new node is not exposed and + // has no children so the default counts of 0 are correct. + // If the parent is a wildcard pointer, then it doesn't track SIFA and doesn't need to be updated. if let Some(parent_idx) = parent_idx { // Inserting the new perms might have broken the SIFA invariant (see @@ -807,7 +791,7 @@ impl Tree { let node = self.nodes.remove(this).unwrap(); for (_range, loc) in self.locations.iter_mut_all() { loc.perms.remove(this); - loc.wildcard_accesses.remove(this); + loc.exposed_cache.remove(this); } self.tag_mapping.remove(&node.tag); } @@ -943,7 +927,7 @@ impl<'tcx> LocationTree { }; let accessed_root_tag = accessed_root.map(|idx| nodes.get(idx).unwrap().tag); - for root in roots { + for (i, root) in roots.enumerate() { let tag = nodes.get(root).unwrap().tag; // On a protector release access we have to skip the children of the accessed tag. // However, if the tag has exposed children then some of the wildcard subtrees could @@ -981,6 +965,7 @@ impl<'tcx> LocationTree { access_kind, global, diagnostics, + /*is_wildcard_tree*/ i != 0, )?; } interp_ok(()) @@ -1029,7 +1014,7 @@ impl<'tcx> LocationTree { .perform_transition( args.idx, args.nodes, - &mut args.data.wildcard_accesses, + &mut args.data.exposed_cache, access_kind, args.rel_pos, protected, @@ -1074,12 +1059,18 @@ impl<'tcx> LocationTree { access_kind: AccessKind, global: &GlobalState, diagnostics: &DiagnosticInfo, + is_wildcard_tree: bool, ) -> InterpResult<'tcx> { let get_relatedness = |idx: UniIndex, node: &Node, loc: &LocationTree| { - let wildcard_state = loc.wildcard_accesses.get(idx).cloned().unwrap_or_default(); // If the tag is larger than `max_local_tag` then the access can only be foreign. let only_foreign = max_local_tag.is_some_and(|max_local_tag| max_local_tag < node.tag); - wildcard_state.access_relatedness(access_kind, only_foreign) + loc.exposed_cache.access_relatedness( + root, + idx, + access_kind, + is_wildcard_tree, + only_foreign, + ) }; // Whether there is an exposed node in this tree that allows this access. @@ -1156,7 +1147,7 @@ impl<'tcx> LocationTree { perm.perform_transition( args.idx, args.nodes, - &mut args.data.wildcard_accesses, + &mut args.data.exposed_cache, access_kind, relatedness, protected, @@ -1175,19 +1166,11 @@ impl<'tcx> LocationTree { }) }, )?; - // 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) - .unwrap() - .access_relatedness(access_kind, /* only_foreign */ true) - .is_none() - { + // If there is no exposed node in this tree that allows this access, then the access *must* + // be foreign to the entire subtree. Foreign accesses are only possible on wildcard subtrees + // as there are no ancestors to the main root. So if we do not find a valid exposed node in + // the main tree then this access is UB. + if !has_valid_exposed && !is_wildcard_tree { return Err(no_valid_exposed_references_error(diagnostics)).into(); } interp_ok(()) diff --git a/src/tools/miri/src/borrow_tracker/tree_borrows/wildcard.rs b/src/tools/miri/src/borrow_tracker/tree_borrows/wildcard.rs index b5ae0ee4c7d3..b03635de70ae 100644 --- a/src/tools/miri/src/borrow_tracker/tree_borrows/wildcard.rs +++ b/src/tools/miri/src/borrow_tracker/tree_borrows/wildcard.rs @@ -1,4 +1,3 @@ -use std::cmp::max; use std::fmt::Debug; use super::Tree; @@ -51,373 +50,141 @@ impl WildcardAccessRelatedness { } } +/// Caches information about where in the tree exposed nodes with permission to do reads/ rites are +/// located. [`ExposedCache`] stores this information a single location (or rather, a range of +/// homogeneous locations) for all nodes in an allocation. +/// +/// Nodes not in this map have a default [`ExposedCacheNode`], i.e. they have no exposed children. +/// In particular, this map remains empty (and thus consumes no memory) until the first +/// node in the tree gets exposed. +#[derive(Clone, Debug, Default, PartialEq, Eq)] +pub struct ExposedCache(UniValMap); + /// State per location per node keeping track of where relative to this /// node exposed nodes are and what access permissions they have. -/// -/// Designed to be completely determined by its parent, siblings and -/// direct children's max_local_access/max_foreign_access. -#[derive(Clone, Default, PartialEq, Eq)] -pub struct WildcardState { - /// How many of this node's direct children have `max_local_access()==Write`. - child_writes: u16, - /// How many of this node's direct children have `max_local_access()>=Read`. - child_reads: u16, - /// The maximum access level that could happen from an exposed node - /// that is foreign to this node. - /// - /// This is calculated as the `max()` of the parent's `max_foreign_access`, - /// `exposed_as` and the siblings' `max_local_access()`. - max_foreign_access: WildcardAccessLevel, - /// At what access level this node itself is exposed. - exposed_as: WildcardAccessLevel, +#[derive(Clone, Default, Debug, PartialEq, Eq)] +struct ExposedCacheNode { + /// How many local nodes (in this subtree) are exposed with write permissions. + local_writes: u16, + /// How many local nodes (in this subtree) are exposed with read permissions. + local_reads: u16, } -impl Debug for WildcardState { - fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - f.debug_struct("WildcardState") - .field("child_r/w", &(self.child_reads, self.child_writes)) - .field("foreign", &self.max_foreign_access) - .field("exposed_as", &self.exposed_as) - .finish() - } -} -impl WildcardState { - /// The maximum access level that could happen from an exposed - /// node that is local to this node. - fn max_local_access(&self) -> WildcardAccessLevel { - use WildcardAccessLevel::*; - max( - self.exposed_as, - if self.child_writes > 0 { - Write - } else if self.child_reads > 0 { - Read - } else { - None - }, - ) - } - /// From where relative to the node with this wildcard info a read or write access could happen. - /// If `only_foreign` is true then we treat `LocalAccess` as impossible. This means we return - /// `None` if only a `LocalAccess` is possible, and we treat `EitherAccess` as a - /// `ForeignAccess`. +impl ExposedCache { + /// Returns the relatedness of a wildcard access to a node. + /// + /// This function only considers a single subtree. If the current subtree does not contain + /// any valid exposed nodes then the function return `None`. + /// + /// * `root`: The root of the subtree the node belongs to. + /// * `id`: The id of the node. + /// * `kind`: The kind of the wildcard access. + /// * `is_wildcard_tree`: This nodes belongs to a wildcard subtree. + /// This means we always treat foreign accesses as possible. + /// * `only_foreign`: Assume the access cannot come from a local node. pub fn access_relatedness( &self, + root: UniIndex, + id: UniIndex, kind: AccessKind, + is_wildcard_tree: bool, only_foreign: bool, ) -> Option { - let rel = match kind { - AccessKind::Read => self.read_access_relatedness(), - AccessKind::Write => self.write_access_relatedness(), + // All nodes in the tree are local to the root, so we can use the root to get the total + // number of valid exposed nodes in the tree. + let root = self.0.get(root).cloned().unwrap_or_default(); + let node = self.0.get(id).cloned().unwrap_or_default(); + + let (total_num, local_num) = match kind { + AccessKind::Read => (root.local_reads, node.local_reads), + AccessKind::Write => (root.local_writes, node.local_writes), }; + + // If this is a wildcard tree then an access can always be foreign as + // it could come from another tree. + // We can represent this by adding 1 to the total which means there + // always exists a foreign exposed node. + // (We cannot bake this into the root's count as then if `node == root` it would + // affect both `total` and `local`.) + let total_num = total_num + u16::from(is_wildcard_tree); + + use WildcardAccessRelatedness::*; + let relatedness = if total_num == 0 { + // we return None if the tree does not contain any valid exposed nodes. + None + } else { + Some(if total_num == local_num { + // If all valid exposed nodes are local to this node then the access is local. + LocalAccess + } else if local_num == 0 { + // If the node does not have any exposed nodes as children then the access is foreign. + ForeignAccess + } else { + // If some but not all of the valid exposed nodes are local then we cannot determine the correct relatedness. + EitherAccess + }) + }; + if only_foreign { - use WildcardAccessRelatedness as E; - match rel { - Some(E::EitherAccess | E::ForeignAccess) => Some(E::ForeignAccess), - Some(E::LocalAccess) | None => None, + // This is definitely not a local access; clamp the result accordingly. + match relatedness { + Some(LocalAccess) => None, + Some(ForeignAccess) => Some(ForeignAccess), + Some(EitherAccess) => Some(ForeignAccess), + None => None, } } else { - rel + relatedness } } - - /// From where relative to the node with this wildcard info a read access could happen. - fn read_access_relatedness(&self) -> Option { - let has_foreign = self.max_foreign_access >= WildcardAccessLevel::Read; - let has_local = self.max_local_access() >= WildcardAccessLevel::Read; - use WildcardAccessRelatedness as E; - match (has_foreign, has_local) { - (true, true) => Some(E::EitherAccess), - (true, false) => Some(E::ForeignAccess), - (false, true) => Some(E::LocalAccess), - (false, false) => None, - } - } - - /// From where relative to the node with this wildcard info a write access could happen. - fn write_access_relatedness(&self) -> Option { - let has_foreign = self.max_foreign_access == WildcardAccessLevel::Write; - let has_local = self.max_local_access() == WildcardAccessLevel::Write; - use WildcardAccessRelatedness as E; - match (has_foreign, has_local) { - (true, true) => Some(E::EitherAccess), - (true, false) => Some(E::ForeignAccess), - (false, true) => Some(E::LocalAccess), - (false, false) => None, - } - } - - /// Gets the access tracking information for a new child node of a parent with this - /// wildcard info. - /// The new node doesn't have any child reads/writes, but calculates `max_foreign_access` - /// from its parent. - pub fn for_new_child(&self) -> Self { - Self { - max_foreign_access: max(self.max_foreign_access, self.max_local_access()), - ..Default::default() - } - } - /// Crates the initial `WildcardState` for a wildcard root. - /// This has `max_foreign_access==Write` as it actually is the child of *some* exposed node - /// through which we can receive foreign accesses. - /// - /// This is different from the main root which has `max_foreign_access==None`, since there - /// cannot be a foreign access to the root of the allocation. - pub fn for_wildcard_root() -> Self { - Self { max_foreign_access: WildcardAccessLevel::Write, ..Default::default() } - } - - /// Pushes the nodes of `children` onto the stack who's `max_foreign_access` - /// needs to be updated. - /// - /// * `children`: A list of nodes with the same parent. `children` doesn't - /// necessarily have to contain all children of parent, but can just be - /// a subset. - /// - /// * `child_reads`, `child_writes`: How many of `children` have `max_local_access()` - /// of at least `read`/`write` - /// - /// * `new_foreign_access`, `old_foreign_access`: - /// The max possible access level that is foreign to all `children` - /// (i.e., it is not local to *any* of them). - /// This can be calculated as the max of the parent's `exposed_as()`, `max_foreign_access` - /// and of all `max_local_access()` of any nodes with the same parent that are - /// not listed in `children`. - /// - /// This access level changed from `old` to `new`, which is why we need to - /// update `children`. - fn push_relevant_children( - stack: &mut Vec<(UniIndex, WildcardAccessLevel)>, - new_foreign_access: WildcardAccessLevel, - old_foreign_access: WildcardAccessLevel, - child_reads: u16, - child_writes: u16, - children: impl Iterator, - - wildcard_accesses: &UniValMap, - ) { - use WildcardAccessLevel::*; - - // Nothing changed so we don't need to update anything. - if new_foreign_access == old_foreign_access { - return; - } - - // We need to consider that the children's `max_local_access()` affect each - // other's `max_foreign_access`, but do not affect their own `max_foreign_access`. - - // The new `max_foreign_acces` for children with `max_local_access()==Write`. - let write_foreign_access = max( - new_foreign_access, - if child_writes > 1 { - // There exists at least one more child with exposed write access. - // This means that a foreign write through that node is possible. - Write - } else if child_reads > 1 { - // There exists at least one more child with exposed read access, - // but no other with write access. - // This means that a foreign read but no write through that node - // is possible. - Read - } else { - // There are no other nodes with read or write access. - // This means no foreign writes through other children are possible. - None - }, - ); - - // The new `max_foreign_acces` for children with `max_local_access()==Read`. - let read_foreign_access = max( - new_foreign_access, - if child_writes > 0 { - // There exists at least one child with write access (and it's not this one). - Write - } else if child_reads > 1 { - // There exists at least one more child with exposed read access, - // but no other with write access. - Read - } else { - // There are no other nodes with read or write access, - None - }, - ); - - // The new `max_foreign_acces` for children with `max_local_access()==None`. - let none_foreign_access = max( - new_foreign_access, - if child_writes > 0 { - // There exists at least one child with write access (and it's not this one). - Write - } else if child_reads > 0 { - // There exists at least one child with read access (and it's not this one), - // but none with write access. - Read - } else { - // No children are exposed as read or write. - None - }, - ); - - stack.extend(children.filter_map(|child| { - let state = wildcard_accesses.get(child).cloned().unwrap_or_default(); - - let new_foreign_access = match state.max_local_access() { - Write => write_foreign_access, - Read => read_foreign_access, - None => none_foreign_access, - }; - - if new_foreign_access != state.max_foreign_access { - Some((child, new_foreign_access)) - } else { - Option::None - } - })); - } - /// Update the tracking information of a tree, to reflect that the node specified by `id` is - /// now exposed with `new_exposed_as`. + /// now exposed with `new_exposed_as` permission. /// /// Propagates the Willard access information over the tree. This needs to be called every /// time the access level of an exposed node changes, to keep the state in sync with /// the rest of the tree. + /// + /// * `from`: The previous access level of the exposed node. + /// Set to `None` if the node was not exposed before. + /// * `to`: The new access level. pub fn update_exposure( - id: UniIndex, - new_exposed_as: WildcardAccessLevel, + &mut self, nodes: &UniValMap, - wildcard_accesses: &mut UniValMap, + id: UniIndex, + from: WildcardAccessLevel, + to: WildcardAccessLevel, ) { - let mut entry = wildcard_accesses.entry(id); - let src_state = entry.or_insert(Default::default()); - let old_exposed_as = src_state.exposed_as; - // If the exposure doesn't change, then we don't need to update anything. - if old_exposed_as == new_exposed_as { + if from == to { return; } - let src_old_local_access = src_state.max_local_access(); - - src_state.exposed_as = new_exposed_as; - - let src_new_local_access = src_state.max_local_access(); - - // Stack of nodes for which the max_foreign_access field needs to be updated. - // Will be filled with the children of this node and its parents children before - // we begin downwards traversal. - let mut stack: Vec<(UniIndex, WildcardAccessLevel)> = Vec::new(); - - // Add the direct children of this node to the stack. - { + // Update the counts of this node and all its ancestors. + let mut next_id = Some(id); + while let Some(id) = next_id { let node = nodes.get(id).unwrap(); - Self::push_relevant_children( - &mut stack, - // new_foreign_access - max(src_state.max_foreign_access, new_exposed_as), - // old_foreign_access - max(src_state.max_foreign_access, old_exposed_as), - // Consider all children. - src_state.child_reads, - src_state.child_writes, - node.children.iter().copied(), - wildcard_accesses, - ); - } - // We need to propagate the tracking info up the tree, for this we traverse - // up the parents. - // We can skip propagating info to the parent and siblings of a node if its - // access didn't change. - { - // The child from which we came. - let mut child = id; - // This is the `max_local_access()` of the child we came from, before - // this update... - let mut old_child_access = src_old_local_access; - // and after this update. - let mut new_child_access = src_new_local_access; - while let Some(parent_id) = nodes.get(child).unwrap().parent { - let parent_node = nodes.get(parent_id).unwrap(); - let mut entry = wildcard_accesses.entry(parent_id); - let parent_state = entry.or_insert(Default::default()); + let mut state = self.0.entry(id); + let state = state.or_insert(Default::default()); - let old_parent_local_access = parent_state.max_local_access(); - use WildcardAccessLevel::*; - // Updating this node's tracking state for its children. - match (old_child_access, new_child_access) { - (None | Read, Write) => parent_state.child_writes += 1, - (Write, None | Read) => parent_state.child_writes -= 1, - _ => {} - } - match (old_child_access, new_child_access) { - (None, Read | Write) => parent_state.child_reads += 1, - (Read | Write, None) => parent_state.child_reads -= 1, - _ => {} - } - - let new_parent_local_access = parent_state.max_local_access(); - - { - // We need to update the `max_foreign_access` of `child`'s - // siblings. For this we can reuse the `push_relevant_children` - // function. - // - // We pass it just the siblings without child itself. Since - // `child`'s `max_local_access()` is foreign to all of its - // siblings we can pass it as part of the foreign access. - - let parent_access = - max(parent_state.exposed_as, parent_state.max_foreign_access); - // This is how many of `child`'s siblings have read/write local access. - // If `child` itself has access, then we need to subtract its access from the count. - let sibling_reads = - parent_state.child_reads - if new_child_access >= Read { 1 } else { 0 }; - let sibling_writes = - parent_state.child_writes - if new_child_access >= Write { 1 } else { 0 }; - Self::push_relevant_children( - &mut stack, - // new_foreign_access - max(parent_access, new_child_access), - // old_foreign_access - max(parent_access, old_child_access), - // Consider only siblings of child. - sibling_reads, - sibling_writes, - parent_node.children.iter().copied().filter(|id| child != *id), - wildcard_accesses, - ); - } - if old_parent_local_access == new_parent_local_access { - // We didn't change `max_local_access()` for parent, so we don't need to propagate further upwards. - break; - } - - old_child_access = old_parent_local_access; - new_child_access = new_parent_local_access; - child = parent_id; + use WildcardAccessLevel::*; + match (from, to) { + (None | Read, Write) => state.local_writes += 1, + (Write, None | Read) => state.local_writes -= 1, + _ => {} } + match (from, to) { + (None, Read | Write) => state.local_reads += 1, + (Read | Write, None) => state.local_reads -= 1, + _ => {} + } + next_id = node.parent; } - // Traverses down the tree to update max_foreign_access fields of children and cousins who need to be updated. - while let Some((id, new_access)) = stack.pop() { - let node = nodes.get(id).unwrap(); - let mut entry = wildcard_accesses.entry(id); - let state = entry.or_insert(Default::default()); - - let old_access = state.max_foreign_access; - state.max_foreign_access = new_access; - - Self::push_relevant_children( - &mut stack, - // new_foreign_access - max(state.exposed_as, new_access), - // old_foreign_access - max(state.exposed_as, old_access), - // Consider all children. - state.child_reads, - state.child_writes, - node.children.iter().copied(), - wildcard_accesses, - ); - } + } + /// Removes a node from the datastructure. + /// + /// The caller needs to ensure that the node does not have any children. + pub fn remove(&mut self, idx: UniIndex) { + self.0.remove(idx); } } @@ -428,25 +195,28 @@ impl Tree { pub fn expose_tag(&mut self, tag: BorTag, protected: bool) { let id = self.tag_mapping.get(&tag).unwrap(); let node = self.nodes.get_mut(id).unwrap(); - node.is_exposed = true; - let node = self.nodes.get(id).unwrap(); + if !node.is_exposed { + node.is_exposed = true; + let node = self.nodes.get(id).unwrap(); - // When the first tag gets exposed then we initialize the - // wildcard state for every node and location in the tree. - for (_, loc) in self.locations.iter_mut_all() { - let perm = loc - .perms - .get(id) - .map(|p| p.permission()) - .unwrap_or_else(|| node.default_location_state().permission()); + for (_, loc) in self.locations.iter_mut_all() { + let perm = loc + .perms + .get(id) + .map(|p| p.permission()) + .unwrap_or_else(|| node.default_location_state().permission()); - let access_type = perm.strongest_allowed_local_access(protected); - WildcardState::update_exposure( - id, - access_type, - &self.nodes, - &mut loc.wildcard_accesses, - ); + let access_level = perm.strongest_allowed_local_access(protected); + // An unexposed node gets treated as access level `None`. Therefore, + // the initial exposure transitions from `None` to the node's actual + // `access_level`. + loc.exposed_cache.update_exposure( + &self.nodes, + id, + WildcardAccessLevel::None, + access_level, + ); + } } } @@ -457,10 +227,19 @@ impl Tree { // We check if the node is already exposed, as we don't want to expose any // nodes which aren't already exposed. - - if self.nodes.get(idx).unwrap().is_exposed { - // Updates the exposure to the new permission on every location. - self.expose_tag(tag, /* protected */ false); + let node = self.nodes.get(idx).unwrap(); + if node.is_exposed { + for (_, loc) in self.locations.iter_mut_all() { + let perm = loc + .perms + .get(idx) + .map(|p| p.permission()) + .unwrap_or_else(|| node.default_location_state().permission()); + // We are transitioning from protected to unprotected. + let old_access_type = perm.strongest_allowed_local_access(/*protected*/ true); + let access_type = perm.strongest_allowed_local_access(/*protected*/ false); + loc.exposed_cache.update_exposure(&self.nodes, idx, old_access_type, access_type); + } } } } @@ -472,20 +251,15 @@ impl Tree { pub fn verify_wildcard_consistency(&self, global: &GlobalState) { // We rely on the fact that `roots` is ordered according to tag from low to high. assert!(self.roots.is_sorted_by_key(|idx| self.nodes.get(*idx).unwrap().tag)); - let main_root_idx = self.roots[0]; let protected_tags = &global.borrow().protected_tags; for (_, loc) in self.locations.iter_all() { - let wildcard_accesses = &loc.wildcard_accesses; + let exposed_cache = &loc.exposed_cache; let perms = &loc.perms; - // Checks if accesses is empty. - if wildcard_accesses.is_empty() { - return; - } for (id, node) in self.nodes.iter() { - let state = wildcard_accesses.get(id).unwrap(); + let state = exposed_cache.0.get(id).cloned().unwrap_or_default(); - let expected_exposed_as = if node.is_exposed { + let exposed_as = if node.is_exposed { let perm = perms.get(id).copied().unwrap_or_else(|| node.default_location_state()); @@ -495,72 +269,25 @@ impl Tree { WildcardAccessLevel::None }; - // The foreign wildcard accesses possible at a node are determined by which - // accesses can originate from their siblings, their parent, and from above - // their parent. - let expected_max_foreign_access = if let Some(parent) = node.parent { - let parent_node = self.nodes.get(parent).unwrap(); - let parent_state = wildcard_accesses.get(parent).unwrap(); - - let max_sibling_access = parent_node - .children - .iter() - .copied() - .filter(|child| *child != id) - .map(|child| { - let state = wildcard_accesses.get(child).unwrap(); - state.max_local_access() - }) - .fold(WildcardAccessLevel::None, max); - - max_sibling_access - .max(parent_state.max_foreign_access) - .max(parent_state.exposed_as) - } else { - if main_root_idx == id { - // There can never be a foreign access to the root of the allocation. - // So its foreign access level is always `None`. - WildcardAccessLevel::None - } else { - // For wildcard roots any access on a different subtree can be foreign - // to it. So a wildcard root has the maximum possible foreign access - // level. - WildcardAccessLevel::Write - } - }; - - // Count how many children can be the source of wildcard reads or writes - // (either directly, or via their children). - let child_accesses = node.children.iter().copied().map(|child| { - let state = wildcard_accesses.get(child).unwrap(); - state.max_local_access() - }); - let expected_child_reads = - child_accesses.clone().filter(|a| *a >= WildcardAccessLevel::Read).count(); - let expected_child_writes = - child_accesses.filter(|a| *a >= WildcardAccessLevel::Write).count(); - + let (child_reads, child_writes) = node + .children + .iter() + .copied() + .map(|id| exposed_cache.0.get(id).cloned().unwrap_or_default()) + .fold((0, 0), |acc, wc| (acc.0 + wc.local_reads, acc.1 + wc.local_writes)); + let expected_reads = + child_reads + u16::from(exposed_as >= WildcardAccessLevel::Read); + let expected_writes = + child_writes + u16::from(exposed_as >= WildcardAccessLevel::Write); assert_eq!( - expected_exposed_as, state.exposed_as, - "tag {:?} (id:{id:?}) should be exposed as {expected_exposed_as:?} but is exposed as {:?}", - node.tag, state.exposed_as + state.local_reads, expected_reads, + "expected {:?}'s (id:{id:?}) local_reads to be {expected_reads:?} instead of {:?} (child_reads: {child_reads:?}, exposed_as: {exposed_as:?})", + node.tag, state.local_reads ); assert_eq!( - expected_max_foreign_access, state.max_foreign_access, - "expected {:?}'s (id:{id:?}) max_foreign_access to be {:?} instead of {:?}", - node.tag, expected_max_foreign_access, state.max_foreign_access - ); - let child_reads: usize = state.child_reads.into(); - assert_eq!( - expected_child_reads, child_reads, - "expected {:?}'s (id:{id:?}) child_reads to be {} instead of {}", - node.tag, expected_child_reads, child_reads - ); - let child_writes: usize = state.child_writes.into(); - assert_eq!( - expected_child_writes, child_writes, - "expected {:?}'s (id:{id:?}) child_writes to be {} instead of {}", - node.tag, expected_child_writes, child_writes + state.local_writes, expected_writes, + "expected {:?}'s (id:{id:?}) local_writes to be {expected_writes:?} instead of {:?} (child_writes: {child_writes:?}, exposed_as: {exposed_as:?})", + node.tag, state.local_writes ); } }