Skip to content
This repository was archived by the owner on May 28, 2025. It is now read-only.

Commit 14e72e7

Browse files
saethlinRalfJung
authored andcommitted
Improve information sharing across SB diagnostics
Previous Stacked Borrows diagnostics were missing a lot of information about the state of the interpreter, and it was difficult to add additional state because it was threaded through all the intervening function signatures. This change factors a lot of the arguments which used to be passed individually to many stacked borrows functions into a single `DiagnosticCx`, which is built in `Stacks::for_each`, and since it wraps a handle to `AllocHistory`, we can now handle more nuanced things like heterogeneous borrow of `!Freeze` types.
1 parent 46da748 commit 14e72e7

File tree

77 files changed

+876
-521
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

77 files changed

+876
-521
lines changed

src/diagnostics.rs

Lines changed: 8 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -178,20 +178,15 @@ pub fn report_error<'tcx, 'mir>(
178178
(None, format!("this indicates a potential bug in the program: it performed an invalid operation, but the Stacked Borrows rules it violated are still experimental")),
179179
(None, format!("see {url} for further information")),
180180
];
181-
match history {
182-
Some(TagHistory::Tagged {tag, created: (created_range, created_span), invalidated, protected }) => {
183-
let msg = format!("{tag:?} was created by a retag at offsets {created_range:?}");
184-
helps.push((Some(*created_span), msg));
185-
if let Some((invalidated_range, invalidated_span)) = invalidated {
186-
let msg = format!("{tag:?} was later invalidated at offsets {invalidated_range:?}");
187-
helps.push((Some(*invalidated_span), msg));
188-
}
189-
if let Some((protecting_tag, protecting_tag_span, protection_span)) = protected {
190-
helps.push((Some(*protecting_tag_span), format!("{tag:?} was protected due to {protecting_tag:?} which was created here")));
191-
helps.push((Some(*protection_span), format!("this protector is live for this call")));
192-
}
181+
if let Some(TagHistory {created, invalidated, protected}) = history.clone() {
182+
helps.push((Some(created.1), created.0));
183+
if let Some((msg, span)) = invalidated {
184+
helps.push((Some(span), msg));
185+
}
186+
if let Some([(protector_msg, protector_span), (protection_msg, protection_span)]) = protected {
187+
helps.push((Some(protector_span), protector_msg));
188+
helps.push((Some(protection_span), protection_msg));
193189
}
194-
None => {}
195190
}
196191
helps
197192
}

src/helpers.rs

Lines changed: 43 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -876,8 +876,8 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
876876
}
877877

878878
impl<'mir, 'tcx> Evaluator<'mir, 'tcx> {
879-
pub fn current_span(&self) -> CurrentSpan<'_, 'mir, 'tcx> {
880-
CurrentSpan { span: None, machine: self }
879+
pub fn current_span(&self, tcx: TyCtxt<'tcx>) -> CurrentSpan<'_, 'mir, 'tcx> {
880+
CurrentSpan { span: None, machine: self, tcx }
881881
}
882882
}
883883

@@ -888,27 +888,61 @@ impl<'mir, 'tcx> Evaluator<'mir, 'tcx> {
888888
#[derive(Clone)]
889889
pub struct CurrentSpan<'a, 'mir, 'tcx> {
890890
span: Option<Span>,
891+
tcx: TyCtxt<'tcx>,
891892
machine: &'a Evaluator<'mir, 'tcx>,
892893
}
893894

894-
impl<'a, 'mir, 'tcx> CurrentSpan<'a, 'mir, 'tcx> {
895+
impl<'a, 'mir: 'a, 'tcx: 'a + 'mir> CurrentSpan<'a, 'mir, 'tcx> {
896+
/// Get the current span, skipping non-local frames.
897+
/// This function is backed by a cache, and can be assumed to be very fast.
895898
pub fn get(&mut self) -> Span {
896-
*self.span.get_or_insert_with(|| Self::current_span(self.machine))
899+
*self.span.get_or_insert_with(|| Self::current_span(self.tcx, self.machine))
897900
}
898901

902+
/// Similar to `CurrentSpan::get`, but retrieves the parent frame of the first non-local frame.
903+
/// This is useful when we are processing something which occurs on function-entry and we want
904+
/// to point at the call to the function, not the function definition generally.
899905
#[inline(never)]
900-
fn current_span(machine: &Evaluator<'_, '_>) -> Span {
906+
pub fn get_parent(&mut self) -> Span {
907+
let idx = Self::current_span_index(self.tcx, self.machine);
908+
Self::nth_span(self.machine, idx.wrapping_sub(1))
909+
}
910+
911+
#[inline(never)]
912+
fn current_span(tcx: TyCtxt<'_>, machine: &Evaluator<'_, '_>) -> Span {
913+
let idx = Self::current_span_index(tcx, machine);
914+
Self::nth_span(machine, idx)
915+
}
916+
917+
fn nth_span(machine: &Evaluator<'_, '_>, idx: usize) -> Span {
918+
machine
919+
.threads
920+
.active_thread_stack()
921+
.get(idx)
922+
.map(Frame::current_span)
923+
.unwrap_or(rustc_span::DUMMY_SP)
924+
}
925+
926+
// Find the position of the inner-most frame which is part of the crate being
927+
// compiled/executed, part of the Cargo workspace, and is also not #[track_caller].
928+
fn current_span_index(tcx: TyCtxt<'_>, machine: &Evaluator<'_, '_>) -> usize {
901929
machine
902930
.threads
903931
.active_thread_stack()
904932
.iter()
933+
.enumerate()
905934
.rev()
906-
.find(|frame| {
935+
.find_map(|(idx, frame)| {
907936
let def_id = frame.instance.def_id();
908-
def_id.is_local() || machine.local_crates.contains(&def_id.krate)
937+
if (def_id.is_local() || machine.local_crates.contains(&def_id.krate))
938+
&& !frame.instance.def.requires_caller_location(tcx)
939+
{
940+
Some(idx)
941+
} else {
942+
None
943+
}
909944
})
910-
.map(|frame| frame.current_span())
911-
.unwrap_or(rustc_span::DUMMY_SP)
945+
.unwrap_or(0)
912946
}
913947
}
914948

src/machine.rs

Lines changed: 11 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -137,7 +137,7 @@ pub enum Provenance {
137137
}
138138

139139
/// The "extra" information a pointer has over a regular AllocId.
140-
#[derive(Copy, Clone)]
140+
#[derive(Copy, Clone, PartialEq)]
141141
pub enum ProvenanceExtra {
142142
Concrete(SbTag),
143143
Wildcard,
@@ -706,15 +706,10 @@ impl<'mir, 'tcx> Machine<'mir, 'tcx> for Evaluator<'mir, 'tcx> {
706706
}
707707

708708
let alloc = alloc.into_owned();
709-
let stacks = ecx.machine.stacked_borrows.as_ref().map(|stacked_borrows| {
710-
Stacks::new_allocation(
711-
id,
712-
alloc.size(),
713-
stacked_borrows,
714-
kind,
715-
ecx.machine.current_span(),
716-
)
717-
});
709+
let stacks =
710+
ecx.machine.stacked_borrows.as_ref().map(|stacked_borrows| {
711+
Stacks::new_allocation(id, alloc.size(), stacked_borrows, kind)
712+
});
718713
let race_alloc = ecx.machine.data_race.as_ref().map(|data_race| {
719714
data_race::AllocExtra::new_allocation(
720715
data_race,
@@ -808,7 +803,7 @@ impl<'mir, 'tcx> Machine<'mir, 'tcx> for Evaluator<'mir, 'tcx> {
808803

809804
#[inline(always)]
810805
fn before_memory_read(
811-
_tcx: TyCtxt<'tcx>,
806+
tcx: TyCtxt<'tcx>,
812807
machine: &Self,
813808
alloc_extra: &AllocExtra,
814809
(alloc_id, prov_extra): (AllocId, Self::ProvenanceExtra),
@@ -828,7 +823,7 @@ impl<'mir, 'tcx> Machine<'mir, 'tcx> for Evaluator<'mir, 'tcx> {
828823
prov_extra,
829824
range,
830825
machine.stacked_borrows.as_ref().unwrap(),
831-
machine.current_span(),
826+
machine.current_span(tcx),
832827
&machine.threads,
833828
)?;
834829
}
@@ -840,7 +835,7 @@ impl<'mir, 'tcx> Machine<'mir, 'tcx> for Evaluator<'mir, 'tcx> {
840835

841836
#[inline(always)]
842837
fn before_memory_write(
843-
_tcx: TyCtxt<'tcx>,
838+
tcx: TyCtxt<'tcx>,
844839
machine: &mut Self,
845840
alloc_extra: &mut AllocExtra,
846841
(alloc_id, prov_extra): (AllocId, Self::ProvenanceExtra),
@@ -860,7 +855,7 @@ impl<'mir, 'tcx> Machine<'mir, 'tcx> for Evaluator<'mir, 'tcx> {
860855
prov_extra,
861856
range,
862857
machine.stacked_borrows.as_ref().unwrap(),
863-
machine.current_span(),
858+
machine.current_span(tcx),
864859
&machine.threads,
865860
)?;
866861
}
@@ -872,7 +867,7 @@ impl<'mir, 'tcx> Machine<'mir, 'tcx> for Evaluator<'mir, 'tcx> {
872867

873868
#[inline(always)]
874869
fn before_memory_deallocation(
875-
_tcx: TyCtxt<'tcx>,
870+
tcx: TyCtxt<'tcx>,
876871
machine: &mut Self,
877872
alloc_extra: &mut AllocExtra,
878873
(alloc_id, prove_extra): (AllocId, Self::ProvenanceExtra),
@@ -895,6 +890,7 @@ impl<'mir, 'tcx> Machine<'mir, 'tcx> for Evaluator<'mir, 'tcx> {
895890
prove_extra,
896891
range,
897892
machine.stacked_borrows.as_ref().unwrap(),
893+
machine.current_span(tcx),
898894
&machine.threads,
899895
)
900896
} else {

0 commit comments

Comments
 (0)