Improve information sharing across SB diagnostics

[saethlin]

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.

[saethlin]

I was reminded that this can probably be used to produce better tag-tracking diagnostics. Might implement that later...

[RalfJung]

I'm a big fan of the more detailed errors! However, I hope we can find a way to implement that that does not reply on passing information implicitly through global state...

[RalfJung]

Yes, much better. :)

[saethlin]

FYI this is now about a 10% perf regression on all benchmarks, on account of the requires_caller_location calls. Here's a profile of the backtraces benchmark if you're curious: flamegraph.svg.gz

I don't know why that function is so expensive, but the disassembly is massive.

[RalfJung]

Oh wow. Okay then let's do that yet, instead please open an issue to track further investigation.

[saethlin]

Fresh brain in the morning. I realized that I was pointlessly checking for #[track_caller] on a lot of frames. I just amended with this diff:

diff --git a/src/helpers.rs b/src/helpers.rs
index e3142caa..bf536c24 100644
--- a/src/helpers.rs
+++ b/src/helpers.rs
@@ -923,6 +923,8 @@ impl<'a, 'mir: 'a, 'tcx: 'a + 'mir> CurrentSpan<'a, 'mir, 'tcx> {
             .unwrap_or(rustc_span::DUMMY_SP)
     }
 
+    // Find the position of the inner-most frame which is part of the crate being
+    // compiled/executed, part of the Cargo workspace, and is also not #[track_caller].
     fn current_span_index(tcx: TyCtxt<'_>, machine: &Evaluator<'_, '_>) -> usize {
         machine
             .threads
@@ -930,11 +932,11 @@ impl<'a, 'mir: 'a, 'tcx: 'a + 'mir> CurrentSpan<'a, 'mir, 'tcx> {
             .iter()
             .enumerate()
             .rev()
-            // Skip #[track_caller] frames
-            .filter(|(_, frame)| !frame.instance.def.requires_caller_location(tcx))
             .find_map(|(idx, frame)| {
                 let def_id = frame.instance.def_id();
-                if def_id.is_local() || machine.local_crates.contains(&def_id.krate) {
+                if (def_id.is_local() || machine.local_crates.contains(&def_id.krate))
+                    && !frame.instance.def.requires_caller_location(tcx)
+                {
                     Some(idx)
                 } else {
                     None

This is now on average perf-neutral, with some small improvements and regressions. requires_caller_location is now ~1% of cycles which is still oddly high but not concerning for us at the moment IMO.

[RalfJung]

(Please don't rebase+amend unless there are conflicts. Rebasing destroys being able to get decent diffs from github for what you changed between revisions.)

[saethlin]

Please don't rebase+amend unless there are conflicts.

I was wondering how you were dealing with this before, sorry about that 🙃

[RalfJung]

For big PRs like this, ideally just keep adding new commits, then even after a rebase it's easy to see what is new. Squashing is an optimization that can happen when we are done. :)

[bors]

☔ The latest upstream changes (presumably #2488) made this pull request unmergeable. Please resolve the merge conflicts.

[RalfJung]

@rustbot author

[RalfJung]

I took the liberty to take care of the last nit, so that the nicer diagnostics can be part of the next Miri update. :)

@bors r+

[bors]

📌 Commit 0e5a53d has been approved by RalfJung

It is now in the queue for this repository.

[bors]

⌛ Testing commit 0e5a53d with merge bf38971...

[bors]

💔 Test failed - checks-actions

[RalfJung]

@bors r+

[bors]

📌 Commit 7397c8e has been approved by RalfJung

It is now in the queue for this repository.

[bors]

⌛ Testing commit 7397c8e with merge 09118da...

[bors]

☀️ Test successful - checks-actions
Approved by: RalfJung
Pushing 09118da to master...

[saethlin]

Thanks for taking care of this!