Skip to content

Commit 88c2c81

Browse files
factor out tree_visitor into own file
1 parent 12b9ce2 commit 88c2c81

File tree

3 files changed

+296
-292
lines changed

3 files changed

+296
-292
lines changed

src/tools/miri/src/borrow_tracker/tree_borrows/mod.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ pub mod diagnostics;
1313
mod foreign_access_skipping;
1414
mod perms;
1515
mod tree;
16+
mod tree_visitor;
1617
mod unimap;
1718
mod wildcard;
1819

src/tools/miri/src/borrow_tracker/tree_borrows/tree.rs

Lines changed: 9 additions & 292 deletions
Original file line numberDiff line numberDiff line change
@@ -18,15 +18,16 @@ use rustc_data_structures::fx::FxHashSet;
1818
use rustc_span::Span;
1919
use smallvec::SmallVec;
2020

21-
use super::diagnostics::AccessCause;
22-
use super::wildcard::WildcardState;
23-
use crate::borrow_tracker::tree_borrows::Permission;
24-
use crate::borrow_tracker::tree_borrows::diagnostics::{
25-
self, NodeDebugInfo, TbError, TransitionError, no_valid_exposed_references_error,
21+
use super::Permission;
22+
use super::diagnostics::{
23+
self, AccessCause, NodeDebugInfo, TbError, TransitionError,
24+
no_valid_exposed_references_error,
2625
};
27-
use crate::borrow_tracker::tree_borrows::foreign_access_skipping::IdempotentForeignAccess;
28-
use crate::borrow_tracker::tree_borrows::perms::PermTransition;
29-
use crate::borrow_tracker::tree_borrows::unimap::{UniIndex, UniKeyMap, UniValMap};
26+
use super::foreign_access_skipping::IdempotentForeignAccess;
27+
use super::perms::PermTransition;
28+
use super::tree_visitor::{ChildrenVisitMode, ContinueTraversal, NodeAppArgs, TreeVisitor};
29+
use super::unimap::{UniIndex, UniKeyMap, UniValMap};
30+
use super::wildcard::WildcardState;
3031
use crate::borrow_tracker::{AccessKind, GlobalState, ProtectorKind};
3132
use crate::*;
3233

@@ -339,290 +340,6 @@ pub(super) struct Node {
339340
pub debug_info: NodeDebugInfo,
340341
}
341342

342-
/// Data given to the transition function
343-
struct NodeAppArgs<'visit> {
344-
/// The index of the current node.
345-
idx: UniIndex,
346-
/// Relative position of the access.
347-
rel_pos: AccessRelatedness,
348-
/// The node map of this tree.
349-
nodes: &'visit mut UniValMap<Node>,
350-
/// The permissions map of this tree.
351-
loc: &'visit mut LocationTree,
352-
}
353-
/// Internal contents of `Tree` with the minimum of mutable access for
354-
/// For soundness do not modify the children or parent indexes of nodes
355-
/// during traversal.
356-
struct TreeVisitor<'tree> {
357-
nodes: &'tree mut UniValMap<Node>,
358-
loc: &'tree mut LocationTree,
359-
}
360-
361-
/// Whether to continue exploring the children recursively or not.
362-
#[derive(Debug)]
363-
enum ContinueTraversal {
364-
Recurse,
365-
SkipSelfAndChildren,
366-
}
367-
368-
#[derive(Clone, Copy, Debug)]
369-
pub enum ChildrenVisitMode {
370-
VisitChildrenOfAccessed,
371-
SkipChildrenOfAccessed,
372-
}
373-
374-
enum RecursionState {
375-
BeforeChildren,
376-
AfterChildren,
377-
}
378-
379-
/// Stack of nodes left to explore in a tree traversal.
380-
/// See the docs of `traverse_this_parents_children_other` for details on the
381-
/// traversal order.
382-
struct TreeVisitorStack<NodeContinue, NodeApp> {
383-
/// Function describing whether to continue at a tag.
384-
/// This is only invoked for foreign accesses.
385-
f_continue: NodeContinue,
386-
/// Function to apply to each tag.
387-
f_propagate: NodeApp,
388-
/// Mutable state of the visit: the tags left to handle.
389-
/// Every tag pushed should eventually be handled,
390-
/// and the precise order is relevant for diagnostics.
391-
/// Since the traversal is piecewise bottom-up, we need to
392-
/// remember whether we're here initially, or after visiting all children.
393-
/// The last element indicates this.
394-
/// This is just an artifact of how you hand-roll recursion,
395-
/// it does not have a deeper meaning otherwise.
396-
stack: Vec<(UniIndex, AccessRelatedness, RecursionState)>,
397-
}
398-
399-
impl<NodeContinue, NodeApp, Err> TreeVisitorStack<NodeContinue, NodeApp>
400-
where
401-
NodeContinue: Fn(&NodeAppArgs<'_>) -> ContinueTraversal,
402-
NodeApp: FnMut(NodeAppArgs<'_>) -> Result<(), Err>,
403-
{
404-
fn should_continue_at(
405-
&self,
406-
this: &mut TreeVisitor<'_>,
407-
idx: UniIndex,
408-
rel_pos: AccessRelatedness,
409-
) -> ContinueTraversal {
410-
let args = NodeAppArgs { idx, rel_pos, nodes: this.nodes, loc: this.loc };
411-
(self.f_continue)(&args)
412-
}
413-
414-
fn propagate_at(
415-
&mut self,
416-
this: &mut TreeVisitor<'_>,
417-
idx: UniIndex,
418-
rel_pos: AccessRelatedness,
419-
) -> Result<(), Err> {
420-
(self.f_propagate)(NodeAppArgs { idx, rel_pos, nodes: this.nodes, loc: this.loc })
421-
}
422-
423-
/// Returns the root of this tree.
424-
fn go_upwards_from_accessed(
425-
&mut self,
426-
this: &mut TreeVisitor<'_>,
427-
accessed_node: UniIndex,
428-
visit_children: ChildrenVisitMode,
429-
) -> Result<UniIndex, Err> {
430-
// We want to visit the accessed node's children first.
431-
// However, we will below walk up our parents and push their children (our cousins)
432-
// onto the stack. To ensure correct iteration order, this method thus finishes
433-
// by reversing the stack. This only works if the stack is empty initially.
434-
assert!(self.stack.is_empty());
435-
// First, handle accessed node. A bunch of things need to
436-
// be handled differently here compared to the further parents
437-
// of `accesssed_node`.
438-
{
439-
self.propagate_at(this, accessed_node, AccessRelatedness::LocalAccess)?;
440-
if matches!(visit_children, ChildrenVisitMode::VisitChildrenOfAccessed) {
441-
let accessed_node = this.nodes.get(accessed_node).unwrap();
442-
// We `rev()` here because we reverse the entire stack later.
443-
for &child in accessed_node.children.iter().rev() {
444-
self.stack.push((
445-
child,
446-
AccessRelatedness::ForeignAccess,
447-
RecursionState::BeforeChildren,
448-
));
449-
}
450-
}
451-
}
452-
// Then, handle the accessed node's parents. Here, we need to
453-
// make sure we only mark the "cousin" subtrees for later visitation,
454-
// not the subtree that contains the accessed node.
455-
let mut last_node = accessed_node;
456-
while let Some(current) = this.nodes.get(last_node).unwrap().parent {
457-
self.propagate_at(this, current, AccessRelatedness::LocalAccess)?;
458-
let node = this.nodes.get(current).unwrap();
459-
// We `rev()` here because we reverse the entire stack later.
460-
for &child in node.children.iter().rev() {
461-
if last_node == child {
462-
continue;
463-
}
464-
self.stack.push((
465-
child,
466-
AccessRelatedness::ForeignAccess,
467-
RecursionState::BeforeChildren,
468-
));
469-
}
470-
last_node = current;
471-
}
472-
// Reverse the stack, as discussed above.
473-
self.stack.reverse();
474-
Ok(last_node)
475-
}
476-
477-
fn finish_foreign_accesses(&mut self, this: &mut TreeVisitor<'_>) -> Result<(), Err> {
478-
while let Some((idx, rel_pos, step)) = self.stack.last_mut() {
479-
let idx = *idx;
480-
let rel_pos = *rel_pos;
481-
match *step {
482-
// How to do bottom-up traversal, 101: Before you handle a node, you handle all children.
483-
// For this, you must first find the children, which is what this code here does.
484-
RecursionState::BeforeChildren => {
485-
// Next time we come back will be when all the children are handled.
486-
*step = RecursionState::AfterChildren;
487-
// Now push the children, except if we are told to skip this subtree.
488-
let handle_children = self.should_continue_at(this, idx, rel_pos);
489-
match handle_children {
490-
ContinueTraversal::Recurse => {
491-
let node = this.nodes.get(idx).unwrap();
492-
for &child in node.children.iter() {
493-
self.stack.push((child, rel_pos, RecursionState::BeforeChildren));
494-
}
495-
}
496-
ContinueTraversal::SkipSelfAndChildren => {
497-
// skip self
498-
self.stack.pop();
499-
continue;
500-
}
501-
}
502-
}
503-
// All the children are handled, let's actually visit this node
504-
RecursionState::AfterChildren => {
505-
self.stack.pop();
506-
self.propagate_at(this, idx, rel_pos)?;
507-
}
508-
}
509-
}
510-
Ok(())
511-
}
512-
513-
fn new(f_continue: NodeContinue, f_propagate: NodeApp) -> Self {
514-
Self { f_continue, f_propagate, stack: Vec::new() }
515-
}
516-
}
517-
518-
impl<'tree> TreeVisitor<'tree> {
519-
/// Applies `f_propagate` to every vertex of the tree in a piecewise bottom-up way: First, visit
520-
/// all ancestors of `start_idx` (starting with `start_idx` itself), then children of `start_idx`, then the rest,
521-
/// going bottom-up in each of these two "pieces" / sections.
522-
/// This ensures that errors are triggered in the following order
523-
/// - first invalid accesses with insufficient permissions, closest to the accessed node first,
524-
/// - then protector violations, bottom-up, starting with the children of the accessed node, and then
525-
/// going upwards and outwards.
526-
///
527-
/// The following graphic visualizes it, with numbers indicating visitation order and `start_idx` being
528-
/// the node that is visited first ("1"):
529-
///
530-
/// ```text
531-
/// 3
532-
/// /|
533-
/// / |
534-
/// 9 2
535-
/// | |\
536-
/// | | \
537-
/// 8 1 7
538-
/// / \
539-
/// 4 6
540-
/// |
541-
/// 5
542-
/// ```
543-
///
544-
/// `f_propagate` should follow the following format: for a given `Node` it updates its
545-
/// `Permission` depending on the position relative to `start_idx` (given by an
546-
/// `AccessRelatedness`).
547-
/// `f_continue` is called earlier on foreign nodes, and describes whether to even start
548-
/// visiting the subtree at that node. If it e.g. returns `SkipSelfAndChildren` on node 6
549-
/// above, then nodes 5 _and_ 6 would not be visited by `f_propagate`. It is not used for
550-
/// notes having a child access (nodes 1, 2, 3).
551-
///
552-
/// Finally, remember that the iteration order is not relevant for UB, it only affects
553-
/// diagnostics. It also affects tree traversal optimizations built on top of this, so
554-
/// those need to be reviewed carefully as well whenever this changes.
555-
///
556-
/// Returns the index of the root of the accessed tree.
557-
fn traverse_this_parents_children_other<Err>(
558-
mut self,
559-
start_idx: UniIndex,
560-
f_continue: impl Fn(&NodeAppArgs<'_>) -> ContinueTraversal,
561-
f_propagate: impl FnMut(NodeAppArgs<'_>) -> Result<(), Err>,
562-
) -> Result<UniIndex, Err> {
563-
let mut stack = TreeVisitorStack::new(f_continue, f_propagate);
564-
// Visits the accessed node itself, and all its parents, i.e. all nodes
565-
// undergoing a child access. Also pushes the children and the other
566-
// cousin nodes (i.e. all nodes undergoing a foreign access) to the stack
567-
// to be processed later.
568-
let root = stack.go_upwards_from_accessed(
569-
&mut self,
570-
start_idx,
571-
ChildrenVisitMode::VisitChildrenOfAccessed,
572-
)?;
573-
// Now visit all the foreign nodes we remembered earlier.
574-
// For this we go bottom-up, but also allow f_continue to skip entire
575-
// subtrees from being visited if it would be a NOP.
576-
stack.finish_foreign_accesses(&mut self)?;
577-
Ok(root)
578-
}
579-
580-
/// Like `traverse_this_parents_children_other`, but skips the children of `start_idx`.
581-
///
582-
/// Returns the index of the root of the accessed tree.
583-
fn traverse_nonchildren<Err>(
584-
mut self,
585-
start_idx: UniIndex,
586-
f_continue: impl Fn(&NodeAppArgs<'_>) -> ContinueTraversal,
587-
f_propagate: impl FnMut(NodeAppArgs<'_>) -> Result<(), Err>,
588-
) -> Result<UniIndex, Err> {
589-
let mut stack = TreeVisitorStack::new(f_continue, f_propagate);
590-
// Visits the accessed node itself, and all its parents, i.e. all nodes
591-
// undergoing a child access. Also pushes the other cousin nodes to the
592-
// stack, but not the children of the accessed node.
593-
let root = stack.go_upwards_from_accessed(
594-
&mut self,
595-
start_idx,
596-
ChildrenVisitMode::SkipChildrenOfAccessed,
597-
)?;
598-
// Now visit all the foreign nodes we remembered earlier.
599-
// For this we go bottom-up, but also allow f_continue to skip entire
600-
// subtrees from being visited if it would be a NOP.
601-
stack.finish_foreign_accesses(&mut self)?;
602-
Ok(root)
603-
}
604-
605-
/// Traverses all children of `start_idx` including `start_idx` itself.
606-
/// Uses `f_continue` to filter out subtrees and then processes each node
607-
/// with `f_propagate` so that the children get processed before their
608-
/// parents.
609-
fn traverse_children_this<Err>(
610-
mut self,
611-
start_idx: UniIndex,
612-
f_continue: impl Fn(&NodeAppArgs<'_>) -> ContinueTraversal,
613-
f_propagate: impl FnMut(NodeAppArgs<'_>) -> Result<(), Err>,
614-
) -> Result<(), Err> {
615-
let mut stack = TreeVisitorStack::new(f_continue, f_propagate);
616-
617-
stack.stack.push((
618-
start_idx,
619-
AccessRelatedness::ForeignAccess,
620-
RecursionState::BeforeChildren,
621-
));
622-
stack.finish_foreign_accesses(&mut self)
623-
}
624-
}
625-
626343
impl Tree {
627344
/// Create a new tree, with only a root pointer.
628345
pub fn new(root_tag: BorTag, size: Size, span: Span) -> Self {

0 commit comments

Comments
 (0)