TB: Make FnEntry access on protected locations be a write under certain circumstances

Author: JoJoDeveloping

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

@@ -19,7 +19,7 @@ pub enum AccessCause {
     Explicit(AccessKind),
     Reborrow,
     Dealloc,
-    FnExit,
+    FnExit(AccessKind),
 }
 
 impl fmt::Display for AccessCause {
@@ -28,7 +28,8 @@ impl fmt::Display for AccessCause {
             Self::Explicit(kind) => write!(f, "{kind}"),
             Self::Reborrow => write!(f, "reborrow"),
             Self::Dealloc => write!(f, "deallocation"),
-            Self::FnExit => write!(f, "protector release"),
+            Self::FnExit(AccessKind::Read) => write!(f, "protector release read"),
+            Self::FnExit(AccessKind::Write) => write!(f, "protector release write"),
         }
     }
 }
@@ -40,7 +41,7 @@ impl AccessCause {
             Self::Explicit(kind) => format!("{rel} {kind}"),
             Self::Reborrow => format!("reborrow (acting as a {rel} read access)"),
             Self::Dealloc => format!("deallocation (acting as a {rel} write access)"),
-            Self::FnExit => format!("protector release (acting as a {rel} read access)"),
+            Self::FnExit(kind) => format!("protector release (acting as a {rel} {kind})"),
         }
     }
 }

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

@@ -68,13 +68,11 @@ impl<'tcx> Tree {
         let global = machine.borrow_tracker.as_ref().unwrap();
         let span = machine.current_span();
         self.perform_access(
-            access_kind,
             tag,
-            Some(range),
+            Some((range, access_kind, diagnostics::AccessCause::Explicit(access_kind))),
             global,
             alloc_id,
             span,
-            diagnostics::AccessCause::Explicit(access_kind),
         )
     }
 
@@ -115,15 +113,8 @@ impl<'tcx> Tree {
         alloc_id: AllocId, // diagnostics
     ) -> InterpResult<'tcx> {
         let span = machine.current_span();
-        self.perform_access(
-            AccessKind::Read,
-            tag,
-            None, // no specified range because it occurs on the entire allocation
-            global,
-            alloc_id,
-            span,
-            diagnostics::AccessCause::FnExit,
-        )
+        // `None` makes it the magic on-protector-end operation
+        self.perform_access(tag, None, global, alloc_id, span)
     }
 }
 
@@ -297,13 +288,11 @@ trait EvalContextPrivExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
 
         // All reborrows incur a (possibly zero-sized) read access to the parent
         tree_borrows.perform_access(
-            AccessKind::Read,
             orig_tag,
-            Some(range),
+            Some((range, AccessKind::Read, diagnostics::AccessCause::Reborrow)),
             this.machine.borrow_tracker.as_ref().unwrap(),
             alloc_id,
             this.machine.current_span(),
-            diagnostics::AccessCause::Reborrow,
         )?;
         // Record the parent-child pair in the tree.
         tree_borrows.new_child(orig_tag, new_tag, new_perm.initial_state, range, span)?;

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

@@ -186,6 +186,10 @@ impl Permission {
     pub fn is_disabled(&self) -> bool {
         self.inner == Disabled
     }
+    /// Check if `self` is the post-child-write state of a pointer (is `Active`).
+    pub fn is_active(&self) -> bool {
+        self.inner == Active
+    }
 
     /// Default initial permission of the root of a new tree at inbounds positions.
     /// Must *only* be used for the root, this is not in general an "initial" permission!

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

@@ -530,13 +530,11 @@ impl<'tcx> Tree {
         span: Span,        // diagnostics
     ) -> InterpResult<'tcx> {
         self.perform_access(
-            AccessKind::Write,
             tag,
-            Some(access_range),
+            Some((access_range, AccessKind::Write, diagnostics::AccessCause::Dealloc)),
             global,
             alloc_id,
             span,
-            diagnostics::AccessCause::Dealloc,
         )?;
         for (perms_range, perms) in self.rperms.iter_mut(access_range.start, access_range.size) {
             TreeVisitor { nodes: &mut self.nodes, tag_mapping: &self.tag_mapping, perms }
@@ -570,12 +568,16 @@ impl<'tcx> Tree {
     }
 
     /// Map the per-node and per-location `LocationState::perform_access`
-    /// to each location of `access_range`, on every tag of the allocation.
+    /// to each location of the first component of `access_range_and_kind`,
+    /// on every tag of the allocation.
     ///
-    /// If `access_range` is `None`, this is interpreted as the special
+    /// If `access_range_and_kind` is `None`, this is interpreted as the special
     /// access that is applied on protector release:
     /// - the access will be applied only to initialized locations of the allocation,
-    /// - and it will not be visible to children.
+    /// - it will not be visible to children,
+    /// - it will be recorded as a `FnExit` diagnostic access
+    /// - and it will be a read except if the location is `Active`, i.e. has been written to,
+    ///   in which case it will be a write.
     ///
     /// `LocationState::perform_access` will take care of raising transition
     /// errors and updating the `initialized` status of each location,
@@ -585,13 +587,11 @@ impl<'tcx> Tree {
     /// - recording the history.
     pub fn perform_access(
         &mut self,
-        access_kind: AccessKind,
         tag: BorTag,
-        access_range: Option<AllocRange>,
+        access_range_and_kind: Option<(AllocRange, AccessKind, diagnostics::AccessCause)>,
         global: &GlobalState,
-        alloc_id: AllocId,                      // diagnostics
-        span: Span,                             // diagnostics
-        access_cause: diagnostics::AccessCause, // diagnostics
+        alloc_id: AllocId, // diagnostics
+        span: Span,        // diagnostics
     ) -> InterpResult<'tcx> {
         use std::ops::Range;
         // Performs the per-node work:
@@ -605,6 +605,8 @@ impl<'tcx> Tree {
         // `perms_range` is only for diagnostics (it is the range of
         // the `RangeMap` on which we are currently working).
         let node_app = |perms_range: Range<u64>,
+                        access_kind: AccessKind,
+                        access_cause: diagnostics::AccessCause,
                         args: NodeAppArgs<'_>|
          -> Result<ContinueTraversal, TransitionError> {
             let NodeAppArgs { node, mut perm, rel_pos } = args;
@@ -618,14 +620,13 @@ impl<'tcx> Tree {
 
             let protected = global.borrow().protected_tags.contains_key(&node.tag);
             let transition = old_state.perform_access(access_kind, rel_pos, protected)?;
-
             // Record the event as part of the history
             if !transition.is_noop() {
                 node.debug_info.history.push(diagnostics::Event {
                     transition,
                     is_foreign: rel_pos.is_foreign(),
                     access_cause,
-                    access_range,
+                    access_range: access_range_and_kind.map(|x| x.0),
                     transition_range: perms_range,
                     span,
                 });
@@ -636,6 +637,7 @@ impl<'tcx> Tree {
         // Error handler in case `node_app` goes wrong.
         // Wraps the faulty transition in more context for diagnostics.
         let err_handler = |perms_range: Range<u64>,
+                           access_cause: diagnostics::AccessCause,
                            args: ErrHandlerArgs<'_, TransitionError>|
          -> InterpError<'tcx> {
             let ErrHandlerArgs { error_kind, conflicting_info, accessed_info } = args;
@@ -650,16 +652,16 @@ impl<'tcx> Tree {
             .build()
         };
 
-        if let Some(access_range) = access_range {
+        if let Some((access_range, access_kind, access_cause)) = access_range_and_kind {
             // Default branch: this is a "normal" access through a known range.
             // We iterate over affected locations and traverse the tree for each of them.
             for (perms_range, perms) in self.rperms.iter_mut(access_range.start, access_range.size)
             {
                 TreeVisitor { nodes: &mut self.nodes, tag_mapping: &self.tag_mapping, perms }
                     .traverse_parents_this_children_others(
                         tag,
-                        |args| node_app(perms_range.clone(), args),
-                        |args| err_handler(perms_range.clone(), args),
+                        |args| node_app(perms_range.clone(), access_kind, access_cause, args),
+                        |args| err_handler(perms_range.clone(), access_cause, args),
                     )?;
             }
         } else {
@@ -678,11 +680,14 @@ impl<'tcx> Tree {
                 if let Some(p) = perms.get(idx)
                     && p.initialized
                 {
+                    let access_kind =
+                        if p.permission.is_active() { AccessKind::Write } else { AccessKind::Read };
+                    let access_cause = diagnostics::AccessCause::FnExit(access_kind);
                     TreeVisitor { nodes: &mut self.nodes, tag_mapping: &self.tag_mapping, perms }
                         .traverse_nonchildren(
                         tag,
-                        |args| node_app(perms_range.clone(), args),
-                        |args| err_handler(perms_range.clone(), args),
+                        |args| node_app(perms_range.clone(), access_kind, access_cause, args),
+                        |args| err_handler(perms_range.clone(), access_cause, args),
                     )?;
                 }
             }