user0/rust
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

Merge pull request #4740 from royAmmerschuber/feature/refactor-diagnostics-struct

Browse source 
TreeBorrows: Put accesses diagnostic parameters into a single struct
This commit is contained in:
Ralf Jung 2025-12-09 23:16:47 +00:00 • committed by GitHub
commit 0830b84bba
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
2 changed files with 94 additions and 102 deletions
Download patch file Download diff file Expand all files Collapse all files

66
src/tools/miri/src/borrow_tracker/tree_borrows/diagnostics.rs
View file

@ -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<u64>,
/// The range the access is happening to. Is `None` if this is the protector release access
pub access_range: Option<AllocRange>,
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(|| "<wildcard>".into());
let conflicting = self.conflicting_info;
self.accessed_node_info.map(|v| format!("{v}")).unwrap_or_else(|| "<wildcard>".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 <wildcard> at {alloc_id:?}[{offset:#x}] is forbidden");
let title = format!(
"{access_cause} through <wildcard> 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 })

130
src/tools/miri/src/borrow_tracker/tree_borrows/tree.rs
View file

@ -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<Node>,
wildcard_accesses: &mut UniValMap<WildcardState>,
access_kind: AccessKind,
access_cause: AccessCause, //diagnostics
access_range: Option<AllocRange>, //diagnostics
relatedness: AccessRelatedness,
span: Span, //diagnostics
location_range: Range<u64>, //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<Item = UniIndex>,
nodes: &mut UniValMap<Node>,
access_source: Option<UniIndex>,
loc_range: Range<u64>, // diagnostics
access_range: Option<AllocRange>, // 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<Node>,
loc_range: Range<u64>, // diagnostics
access_range: Option<AllocRange>, // 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<UniIndex>,
max_local_tag: Option<BorTag>,
nodes: &mut UniValMap<Node>,
loc_range: Range<u64>, // diagnostics
access_range: Option<AllocRange>, // 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()