Auto merge of #1987 - saethlin:short-backtraces, r=oli-obk

Prune stacktraces for tag-tracking diagnostics too

In https://github.com/rust-lang/miri/pull/1977 we forgot to use the pruning logic when printing tag-tracking messages. I just saw this output before this PR which is very silly:
```
test sanity ... note: tracking was triggered
   --> src/lib.rs:110:21
    |
110 |         unsafe { &*(&self.value[range] as *const [T] as *const Aligned<A, [T]>) }
    |                     ^^^^^^^^^^^^^^^^^^ created tag 160954
    |
    = note: inside `<Aligned<A4, [u8]> as std::ops::Index<std::ops::RangeTo<usize>>>::index` at src/lib.rs:110:21
note: inside `sanity` at src/lib.rs:261:30
   --> src/lib.rs:261:30
    |
261 |     let y: &Aligned<_, _> = &y[..2];
    |                              ^^^^^^
note: inside closure at src/lib.rs:229:1
   --> src/lib.rs:229:1
    |
228 |   #[test]
    |   ------- in this procedural macro expansion
229 | / fn sanity() {
230 | |     use core::mem;
231 | |
232 | |     let x: Aligned<A2, _> = Aligned([0u8; 3]);
...   |
284 | |     let _: &[u8] = y;
285 | | }
    | |_^
    = note: inside `<[closure@src/lib.rs:229:1: 285:2] as std::ops::FnOnce<()>>::call_once - shim` at /home/ben/.rustup/toolchains/miri/lib/rustlib/src/rust/library/core/src/ops/function.rs:227:5
    = note: inside `<fn() as std::ops::FnOnce<()>>::call_once - shim(fn())` at /home/ben/.rustup/toolchains/miri/lib/rustlib/src/rust/library/core/src/ops/function.rs:227:5
    = note: inside `test::__rust_begin_short_backtrace::<fn()>` at /home/ben/.rustup/toolchains/miri/lib/rustlib/src/rust/library/test/src/lib.rs:575:5
    = note: inside closure at /home/ben/.rustup/toolchains/miri/lib/rustlib/src/rust/library/test/src/lib.rs:566:30
    = note: inside `<[closure@test::run_test::{closure#1}] as std::ops::FnOnce<()>>::call_once - shim(vtable)` at /home/ben/.rustup/toolchains/miri/lib/rustlib/src/rust/library/core/src/ops/function.rs:227:5
    = note: inside `<std::boxed::Box<dyn std::ops::FnOnce() + std::marker::Send> as std::ops::FnOnce<()>>::call_once` at /home/ben/.rustup/toolchains/miri/lib/rustlib/src/rust/library/alloc/src/boxed.rs:1854:9
    = note: inside `<std::panic::AssertUnwindSafe<std::boxed::Box<dyn std::ops::FnOnce() + std::marker::Send>> as std::ops::FnOnce<()>>::call_once` at /home/ben/.rustup/toolchains/miri/lib/rustlib/src/rust/library/core/src/panic/unwind_safe.rs:271:9
    = note: inside `std::panicking::r#try::do_call::<std::panic::AssertUnwindSafe<std::boxed::Box<dyn std::ops::FnOnce() + std::marker::Send>>, ()>` at /home/ben/.rustup/toolchains/miri/lib/rustlib/src/rust/library/std/src/panicking.rs:492:40
    = note: inside `std::panicking::r#try::<(), std::panic::AssertUnwindSafe<std::boxed::Box<dyn std::ops::FnOnce() + std::marker::Send>>>` at /home/ben/.rustup/toolchains/miri/lib/rustlib/src/rust/library/std/src/panicking.rs:456:19
    = note: inside `std::panic::catch_unwind::<std::panic::AssertUnwindSafe<std::boxed::Box<dyn std::ops::FnOnce() + std::marker::Send>>, ()>` at /home/ben/.rustup/toolchains/miri/lib/rustlib/src/rust/library/std/src/panic.rs:137:14
    = note: inside `test::run_test_in_process` at /home/ben/.rustup/toolchains/miri/lib/rustlib/src/rust/library/test/src/lib.rs:598:18
    = note: inside closure at /home/ben/.rustup/toolchains/miri/lib/rustlib/src/rust/library/test/src/lib.rs:492:39
    = note: inside `test::run_test::run_test_inner` at /home/ben/.rustup/toolchains/miri/lib/rustlib/src/rust/library/test/src/lib.rs:530:13
    = note: inside `test::run_test` at /home/ben/.rustup/toolchains/miri/lib/rustlib/src/rust/library/test/src/lib.rs:562:28
    = note: inside `test::run_tests::<[closure@test::run_tests_console::{closure#2}]>` at /home/ben/.rustup/toolchains/miri/lib/rustlib/src/rust/library/test/src/lib.rs:305:17
    = note: inside `test::run_tests_console` at /home/ben/.rustup/toolchains/miri/lib/rustlib/src/rust/library/test/src/console.rs:286:5
    = note: inside `test::test_main` at /home/ben/.rustup/toolchains/miri/lib/rustlib/src/rust/library/test/src/lib.rs:116:15
    = note: inside `test::test_main_static` at /home/ben/.rustup/toolchains/miri/lib/rustlib/src/rust/library/test/src/lib.rs:135:5
    = note: inside `main`
    = note: inside `<fn() as std::ops::FnOnce<()>>::call_once - shim(fn())` at /home/ben/.rustup/toolchains/miri/lib/rustlib/src/rust/library/core/src/ops/function.rs:227:5
    = note: inside `std::sys_common::backtrace::__rust_begin_short_backtrace::<fn(), ()>` at /home/ben/.rustup/toolchains/miri/lib/rustlib/src/rust/library/std/src/sys_common/backtrace.rs:122:18
    = note: inside closure at /home/ben/.rustup/toolchains/miri/lib/rustlib/src/rust/library/std/src/rt.rs:145:18
    = note: inside `std::ops::function::impls::<impl std::ops::FnOnce<()> for &dyn std::ops::Fn() -> i32 + std::marker::Sync + std::panic::RefUnwindSafe>::call_once` at /home/ben/.rustup/toolchains/miri/lib/rustlib/src/rust/library/core/src/ops/function.rs:259:13
    = note: inside `std::panicking::r#try::do_call::<&dyn std::ops::Fn() -> i32 + std::marker::Sync + std::panic::RefUnwindSafe, i32>` at /home/ben/.rustup/toolchains/miri/lib/rustlib/src/rust/library/std/src/panicking.rs:492:40
    = note: inside `std::panicking::r#try::<i32, &dyn std::ops::Fn() -> i32 + std::marker::Sync + std::panic::RefUnwindSafe>` at /home/ben/.rustup/toolchains/miri/lib/rustlib/src/rust/library/std/src/panicking.rs:456:19
    = note: inside `std::panic::catch_unwind::<&dyn std::ops::Fn() -> i32 + std::marker::Sync + std::panic::RefUnwindSafe, i32>` at /home/ben/.rustup/toolchains/miri/lib/rustlib/src/rust/library/std/src/panic.rs:137:14
    = note: inside closure at /home/ben/.rustup/toolchains/miri/lib/rustlib/src/rust/library/std/src/rt.rs:128:48
    = note: inside `std::panicking::r#try::do_call::<[closure@std::rt::lang_start_internal::{closure#2}], isize>` at /home/ben/.rustup/toolchains/miri/lib/rustlib/src/rust/library/std/src/panicking.rs:492:40
    = note: inside `std::panicking::r#try::<isize, [closure@std::rt::lang_start_internal::{closure#2}]>` at /home/ben/.rustup/toolchains/miri/lib/rustlib/src/rust/library/std/src/panicking.rs:456:19
    = note: inside `std::panic::catch_unwind::<[closure@std::rt::lang_start_internal::{closure#2}], isize>` at /home/ben/.rustup/toolchains/miri/lib/rustlib/src/rust/library/std/src/panic.rs:137:14
    = note: inside `std::rt::lang_start_internal` at /home/ben/.rustup/toolchains/miri/lib/rustlib/src/rust/library/std/src/rt.rs:128:20
    = note: inside `std::rt::lang_start::<()>` at /home/ben/.rustup/toolchains/miri/lib/rustlib/src/rust/library/std/src/rt.rs:144:17
    = note: this note originates in the attribute macro `test` (in Nightly builds, run with -Z macro-backtrace for more info)

error: Undefined Behavior: trying to reborrow for SharedReadOnly at alloc61593+0x2, but parent tag <160954> does not have an appropriate item in the borrow stack
   --> src/lib.rs:261:30
    |
261 |     let y: &Aligned<_, _> = &y[..2];
    |                              ^^^^^^ trying to reborrow for SharedReadOnly at alloc61593+0x2, but parent tag <160954> does not have an appropriate item in the borrow stack
    |
    = help: this indicates a potential bug in the program: it performed an invalid operation, but the rules it violated are still experimental
    = help: see https://github.com/rust-lang/unsafe-code-guidelines/blob/master/wip/stacked-borrows.md for further information

    = note: inside `sanity` at src/lib.rs:261:30
note: inside closure at src/lib.rs:229:1
   --> src/lib.rs:229:1
    |
228 |   #[test]
    |   ------- in this procedural macro expansion
229 | / fn sanity() {
230 | |     use core::mem;
231 | |
232 | |     let x: Aligned<A2, _> = Aligned([0u8; 3]);
...   |
284 | |     let _: &[u8] = y;
285 | | }
    | |_^
    = note: this error originates in the attribute macro `test` (in Nightly builds, run with -Z macro-backtrace for more info)

note: some details are omitted, run with `MIRIFLAGS=-Zmiri-backtrace=full` for a verbose backtrace

error: aborting due to previous error

error: test failed, to rerun pass '--lib'
```
This commit is contained in:
bors 2022-02-26 08:56:02 +00:00
commit 144d56532c

View File

@ -75,6 +75,54 @@ enum DiagLevel {
Note,
}
fn has_local_frame(stacktrace: &[FrameInfo<'_>]) -> bool {
stacktrace.iter().any(|frame| frame.instance.def_id().is_local())
}
fn prune_stacktrace<'mir, 'tcx>(
ecx: &InterpCx<'mir, 'tcx, Evaluator<'mir, 'tcx>>,
mut stacktrace: Vec<FrameInfo<'tcx>>,
) -> Vec<FrameInfo<'tcx>> {
match ecx.machine.backtrace_style {
BacktraceStyle::Off => {
// Retain one frame so that we can print a span for the error itself
stacktrace.truncate(1);
}
BacktraceStyle::Short => {
// Only prune frames if there is at least one local frame. This check ensures that if
// we get a backtrace that never makes it to the user code because it has detected a
// bug in the Rust runtime, we don't prune away every frame.
if has_local_frame(&stacktrace) {
// This is part of the logic that `std` uses to select the relevant part of a
// backtrace. But here, we only look for __rust_begin_short_backtrace, not
// __rust_end_short_backtrace because the end symbol comes from a call to the default
// panic handler.
stacktrace = stacktrace
.into_iter()
.take_while(|frame| {
let def_id = frame.instance.def_id();
let path = ecx.tcx.tcx.def_path_str(def_id);
!path.contains("__rust_begin_short_backtrace")
})
.collect::<Vec<_>>();
// After we prune frames from the bottom, there are a few left that are part of the
// Rust runtime. So we remove frames until we get to a local symbol, which should be
// main or a test.
// This len check ensures that we don't somehow remove every frame, as doing so breaks
// the primary error message.
while stacktrace.len() > 1
&& stacktrace.last().map_or(false, |e| !e.instance.def_id().is_local())
{
stacktrace.pop();
}
}
}
BacktraceStyle::Full => {}
}
stacktrace
}
/// Emit a custom diagnostic without going through the miri-engine machinery
pub fn report_error<'tcx, 'mir>(
ecx: &InterpCx<'mir, 'tcx, Evaluator<'mir, 'tcx>>,
@ -157,46 +205,8 @@ pub fn report_error<'tcx, 'mir>(
}
};
let mut stacktrace = ecx.generate_stacktrace();
let has_local_frame = stacktrace.iter().any(|frame| frame.instance.def_id().is_local());
match ecx.machine.backtrace_style {
BacktraceStyle::Off => {
// Retain one frame so that we can print a span for the error itself
stacktrace.truncate(1);
}
BacktraceStyle::Short => {
// Only prune frames if there is at least one local frame. This check ensures that if
// we get a backtrace that never makes it to the user code because it has detected a
// bug in the Rust runtime, we don't prune away every frame.
if has_local_frame {
// This is part of the logic that `std` uses to select the relevant part of a
// backtrace. But here, we only look for __rust_begin_short_backtrace, not
// __rust_end_short_backtrace because the end symbol comes from a call to the default
// panic handler.
stacktrace = stacktrace
.into_iter()
.take_while(|frame| {
let def_id = frame.instance.def_id();
let path = ecx.tcx.tcx.def_path_str(def_id);
!path.contains("__rust_begin_short_backtrace")
})
.collect::<Vec<_>>();
// After we prune frames from the bottom, there are a few left that are part of the
// Rust runtime. So we remove frames until we get to a local symbol, which should be
// main or a test.
// This len check ensures that we don't somehow remove every frame, as doing so breaks
// the primary error message.
while stacktrace.len() > 1
&& stacktrace.last().map_or(false, |e| !e.instance.def_id().is_local())
{
stacktrace.pop();
}
}
}
BacktraceStyle::Full => {}
}
let stacktrace = ecx.generate_stacktrace();
let stacktrace = prune_stacktrace(ecx, stacktrace);
e.print_backtrace();
let msg = e.to_string();
report_msg(
@ -210,7 +220,7 @@ pub fn report_error<'tcx, 'mir>(
// Include a note like `std` does for short backtraces, but since we are opt-out not opt-in, we
// do not include a note when backtraces are off.
if ecx.machine.backtrace_style == BacktraceStyle::Short && has_local_frame {
if ecx.machine.backtrace_style == BacktraceStyle::Short && has_local_frame(&stacktrace) {
ecx.tcx.sess.diagnostic().note_without_error(
"some details are omitted, run with `MIRIFLAGS=-Zmiri-backtrace=full` for a verbose backtrace",
);
@ -367,6 +377,8 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
);
}
let stacktrace = prune_stacktrace(this, stacktrace);
// Show diagnostics.
for e in diagnostics.drain(..) {
use NonHaltingDiagnostic::*;