From 85e3acc947064454d907cc5a4195a7dcbc4193a9 Mon Sep 17 00:00:00 2001 From: Roy Ammerschuber Date: Wed, 3 Dec 2025 11:56:07 +0100 Subject: [PATCH] tree borrows: put accesses diagnostics data into single struct --- .../tree_borrows/diagnostics.rs | 66 ++++++--- .../src/borrow_tracker/tree_borrows/tree.rs | 130 +++++++----------- 2 files changed, 94 insertions(+), 102 deletions(-) diff --git a/src/tools/miri/src/borrow_tracker/tree_borrows/diagnostics.rs b/src/tools/miri/src/borrow_tracker/tree_borrows/diagnostics.rs index a91f35a9dcad..055baca8542f 100644 --- a/src/tools/miri/src/borrow_tracker/tree_borrows/diagnostics.rs +++ b/src/tools/miri/src/borrow_tracker/tree_borrows/diagnostics.rs @@ -77,6 +77,32 @@ pub struct Event { pub span: Span, } +/// Diagnostics data about the current access and the location we are accessing. +/// Used to create history events and errors. +#[derive(Clone, Debug)] +pub struct DiagnosticInfo { + pub alloc_id: AllocId, + pub span: Span, + /// The range the diagnostic actually applies to. + /// This is always a subset of `access_range`. + pub transition_range: Range, + /// The range the access is happening to. Is `None` if this is the protector release access + pub access_range: Option, + pub access_cause: AccessCause, +} +impl DiagnosticInfo { + /// Creates a history event. + pub fn create_event(&self, transition: PermTransition, is_foreign: bool) -> Event { + Event { + transition, + is_foreign, + access_cause: self.access_cause, + access_range: self.access_range, + transition_range: self.transition_range.clone(), + span: self.span, + } + } +} /// List of all events that affected a tag. /// NOTE: not all of these events are relevant for a particular location, /// the events should be filtered before the generation of diagnostics. @@ -280,32 +306,29 @@ impl History { pub(super) struct TbError<'node> { /// What failure occurred. pub error_kind: TransitionError, - /// The allocation in which the error is happening. - pub alloc_id: AllocId, - /// The offset (into the allocation) at which the conflict occurred. - pub error_offset: u64, + /// Diagnostic data about the access that caused the error. + pub access_info: &'node DiagnosticInfo, /// The tag on which the error was triggered. /// On protector violations, this is the tag that was protected. /// On accesses rejected due to insufficient permissions, this is the /// tag that lacked those permissions. - pub conflicting_info: &'node NodeDebugInfo, - // What kind of access caused this error (read, write, reborrow, deallocation) - pub access_cause: AccessCause, + pub conflicting_node_info: &'node NodeDebugInfo, /// Which tag, if any, the access that caused this error was made through, i.e. /// which tag was used to read/write/deallocate. /// Not set on wildcard accesses. - pub accessed_info: Option<&'node NodeDebugInfo>, + pub accessed_node_info: Option<&'node NodeDebugInfo>, } impl TbError<'_> { /// Produce a UB error. pub fn build<'tcx>(self) -> InterpErrorKind<'tcx> { use TransitionError::*; - let cause = self.access_cause; - let accessed = self.accessed_info; + let cause = self.access_info.access_cause; + let error_offset = self.access_info.transition_range.start; + let accessed = self.accessed_node_info; let accessed_str = - self.accessed_info.map(|v| format!("{v}")).unwrap_or_else(|| "".into()); - let conflicting = self.conflicting_info; + self.accessed_node_info.map(|v| format!("{v}")).unwrap_or_else(|| "".into()); + let conflicting = self.conflicting_node_info; // An access is considered conflicting if it happened through a // different tag than the one who caused UB. // When doing a wildcard access (where `accessed` is `None`) we @@ -316,9 +339,8 @@ impl TbError<'_> { // all tags through which an access would cause UB. let accessed_is_conflicting = accessed.map(|a| a.tag) == Some(conflicting.tag); let title = format!( - "{cause} through {accessed_str} at {alloc_id:?}[{offset:#x}] is forbidden", - alloc_id = self.alloc_id, - offset = self.error_offset + "{cause} through {accessed_str} at {alloc_id:?}[{error_offset:#x}] is forbidden", + alloc_id = self.access_info.alloc_id ); let (title, details, conflicting_tag_name) = match self.error_kind { ChildAccessForbidden(perm) => { @@ -362,13 +384,13 @@ impl TbError<'_> { } }; let mut history = HistoryData::default(); - if let Some(accessed_info) = self.accessed_info + if let Some(accessed_info) = self.accessed_node_info && !accessed_is_conflicting { history.extend(accessed_info.history.forget(), "accessed", false); } history.extend( - self.conflicting_info.history.extract_relevant(self.error_offset, self.error_kind), + self.conflicting_node_info.history.extract_relevant(error_offset, self.error_kind), conflicting_tag_name, true, ); @@ -379,12 +401,12 @@ impl TbError<'_> { /// Cannot access this allocation with wildcard provenance, as there are no /// valid exposed references for this access kind. pub fn no_valid_exposed_references_error<'tcx>( - alloc_id: AllocId, - offset: u64, - access_cause: AccessCause, + DiagnosticInfo { alloc_id, transition_range, access_cause, .. }: &DiagnosticInfo, ) -> InterpErrorKind<'tcx> { - let title = - format!("{access_cause} through at {alloc_id:?}[{offset:#x}] is forbidden"); + let title = format!( + "{access_cause} through at {alloc_id:?}[{offset:#x}] is forbidden", + offset = transition_range.start + ); let details = vec![format!("there are no exposed tags which may perform this access here")]; let history = HistoryData::default(); err_machine_stop!(TerminationInfo::TreeBorrowsUb { title, details, history }) 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 7630c73f4dac..900e9c3729c8 100644 --- a/src/tools/miri/src/borrow_tracker/tree_borrows/tree.rs +++ b/src/tools/miri/src/borrow_tracker/tree_borrows/tree.rs @@ -18,12 +18,12 @@ use rustc_data_structures::fx::FxHashSet; use rustc_span::Span; use smallvec::SmallVec; -use super::Permission; use super::diagnostics::{ - self, AccessCause, NodeDebugInfo, TbError, TransitionError, no_valid_exposed_references_error, + AccessCause, DiagnosticInfo, NodeDebugInfo, TbError, TransitionError, + no_valid_exposed_references_error, }; use super::foreign_access_skipping::IdempotentForeignAccess; -use super::perms::PermTransition; +use super::perms::{PermTransition, Permission}; use super::tree_visitor::{ChildrenVisitMode, ContinueTraversal, NodeAppArgs, TreeVisitor}; use super::unimap::{UniIndex, UniKeyMap, UniValMap}; use super::wildcard::WildcardState; @@ -91,12 +91,9 @@ impl LocationState { nodes: &mut UniValMap, wildcard_accesses: &mut UniValMap, access_kind: AccessKind, - access_cause: AccessCause, //diagnostics - access_range: Option, //diagnostics relatedness: AccessRelatedness, - span: Span, //diagnostics - location_range: Range, //diagnostics protected: bool, + diagnostics: &DiagnosticInfo, ) -> Result<(), TransitionError> { // Call this function now (i.e. only if we know `relatedness`), which // ensures it is only called when `skip_if_known_noop` returns @@ -107,14 +104,9 @@ impl LocationState { if !transition.is_noop() { let node = nodes.get_mut(idx).unwrap(); // Record the event as part of the history. - node.debug_info.history.push(diagnostics::Event { - transition, - is_foreign: relatedness.is_foreign(), - access_cause, - access_range, - transition_range: location_range, - span, - }); + node.debug_info + .history + .push(diagnostics.create_event(transition, relatedness.is_foreign())); // We need to update the wildcard state, if the permission // of an exposed pointer changes. @@ -546,7 +538,7 @@ impl<'tcx> Tree { prov, access_range, AccessKind::Write, - diagnostics::AccessCause::Dealloc, + AccessCause::Dealloc, global, alloc_id, span, @@ -560,6 +552,13 @@ impl<'tcx> Tree { // Check if this breaks any strong protector. // (Weak protectors are already handled by `perform_access`.) for (loc_range, loc) in self.locations.iter_mut(access_range.start, access_range.size) { + let diagnostics = DiagnosticInfo { + alloc_id, + span, + transition_range: loc_range, + access_range: Some(access_range), + access_cause: AccessCause::Dealloc, + }; // Checks the tree containing `idx` for strong protector violations. // It does this in traversal order. let mut check_tree = |idx| { @@ -586,12 +585,10 @@ impl<'tcx> Tree { && perm.accessed { Err(TbError { - conflicting_info: &node.debug_info, - access_cause: diagnostics::AccessCause::Dealloc, - alloc_id, - error_offset: loc_range.start, error_kind: TransitionError::ProtectedDealloc, - accessed_info: start_idx + access_info: &diagnostics, + conflicting_node_info: &node.debug_info, + accessed_node_info: start_idx .map(|idx| &args.nodes.get(idx).unwrap().debug_info), } .build()) @@ -649,18 +646,21 @@ impl<'tcx> Tree { }; // We iterate over affected locations and traverse the tree for each of them. for (loc_range, loc) in self.locations.iter_mut(access_range.start, access_range.size) { + let diagnostics = DiagnosticInfo { + access_cause, + access_range: Some(access_range), + alloc_id, + span, + transition_range: loc_range, + }; loc.perform_access( self.roots.iter().copied(), &mut self.nodes, source_idx, - loc_range, - Some(access_range), access_kind, - access_cause, global, - alloc_id, - span, ChildrenVisitMode::VisitChildrenOfAccessed, + &diagnostics, )?; } interp_ok(()) @@ -701,19 +701,21 @@ impl<'tcx> Tree { && let Some(access_kind) = p.permission.protector_end_access() && p.accessed { - let access_cause = diagnostics::AccessCause::FnExit(access_kind); + let diagnostics = DiagnosticInfo { + access_cause: AccessCause::FnExit(access_kind), + access_range: None, + alloc_id, + span, + transition_range: loc_range, + }; loc.perform_access( self.roots.iter().copied(), &mut self.nodes, Some(source_idx), - loc_range, - None, access_kind, - access_cause, global, - alloc_id, - span, ChildrenVisitMode::SkipChildrenOfAccessed, + &diagnostics, )?; } } @@ -882,27 +884,19 @@ impl<'tcx> LocationTree { roots: impl Iterator, nodes: &mut UniValMap, access_source: Option, - loc_range: Range, // diagnostics - access_range: Option, // diagnostics access_kind: AccessKind, - access_cause: diagnostics::AccessCause, // diagnostics global: &GlobalState, - alloc_id: AllocId, // diagnostics - span: Span, // diagnostics visit_children: ChildrenVisitMode, + diagnostics: &DiagnosticInfo, ) -> InterpResult<'tcx> { let accessed_root = if let Some(idx) = access_source { Some(self.perform_normal_access( idx, nodes, - loc_range.clone(), - access_range, access_kind, - access_cause, global, - alloc_id, - span, visit_children, + diagnostics, )?) } else { // `SkipChildrenOfAccessed` only gets set on protector release, which only @@ -936,13 +930,9 @@ impl<'tcx> LocationTree { access_source, /*max_local_tag*/ accessed_root_tag, nodes, - loc_range.clone(), - access_range, access_kind, - access_cause, global, - alloc_id, - span, + diagnostics, )?; } interp_ok(()) @@ -958,14 +948,10 @@ impl<'tcx> LocationTree { &mut self, access_source: UniIndex, nodes: &mut UniValMap, - loc_range: Range, // diagnostics - access_range: Option, // diagnostics access_kind: AccessKind, - access_cause: diagnostics::AccessCause, // diagnostics global: &GlobalState, - alloc_id: AllocId, // diagnostics - span: Span, // diagnostics visit_children: ChildrenVisitMode, + diagnostics: &DiagnosticInfo, ) -> InterpResult<'tcx, UniIndex> { // Performs the per-node work: // - insert the permission if it does not exist @@ -997,21 +983,18 @@ impl<'tcx> LocationTree { args.nodes, &mut args.data.wildcard_accesses, access_kind, - access_cause, - access_range, args.rel_pos, - span, - loc_range.clone(), protected, + diagnostics, ) .map_err(|error_kind| { TbError { - conflicting_info: &args.nodes.get(args.idx).unwrap().debug_info, - access_cause, - alloc_id, - error_offset: loc_range.start, error_kind, - accessed_info: Some(&args.nodes.get(access_source).unwrap().debug_info), + access_info: diagnostics, + conflicting_node_info: &args.nodes.get(args.idx).unwrap().debug_info, + accessed_node_info: Some( + &args.nodes.get(access_source).unwrap().debug_info, + ), } .build() }) @@ -1040,13 +1023,9 @@ impl<'tcx> LocationTree { access_source: Option, max_local_tag: Option, nodes: &mut UniValMap, - loc_range: Range, // diagnostics - access_range: Option, // diagnostics access_kind: AccessKind, - access_cause: diagnostics::AccessCause, // diagnostics global: &GlobalState, - alloc_id: AllocId, // diagnostics - span: Span, // diagnostics + diagnostics: &DiagnosticInfo, ) -> InterpResult<'tcx> { let get_relatedness = |idx: UniIndex, node: &Node, loc: &LocationTree| { let wildcard_state = loc.wildcard_accesses.get(idx).cloned().unwrap_or_default(); @@ -1100,11 +1079,7 @@ impl<'tcx> LocationTree { // This can only happen if `root` is the main root: We set // `max_foreign_access==Write` on all wildcard roots, so at least a foreign access // is always possible on all nodes in a wildcard subtree. - return Err(no_valid_exposed_references_error( - alloc_id, - loc_range.start, - access_cause, - )); + return Err(no_valid_exposed_references_error(diagnostics)); }; let Some(relatedness) = wildcard_relatedness.to_relatedness() else { @@ -1123,22 +1098,17 @@ impl<'tcx> LocationTree { args.nodes, &mut args.data.wildcard_accesses, access_kind, - access_cause, - access_range, relatedness, - span, - loc_range.clone(), protected, + diagnostics, ) .map_err(|trans| { let node = args.nodes.get(args.idx).unwrap(); TbError { - conflicting_info: &node.debug_info, - access_cause, - alloc_id, - error_offset: loc_range.start, error_kind: trans, - accessed_info: access_source + access_info: diagnostics, + conflicting_node_info: &node.debug_info, + accessed_node_info: access_source .map(|idx| &args.nodes.get(idx).unwrap().debug_info), } .build()