Skip to content

Conversation

saethlin
Copy link
Member

In #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'

@oli-obk
Copy link
Contributor

oli-obk commented Feb 26, 2022

@bors r+

@bors
Copy link
Contributor

bors commented Feb 26, 2022

📌 Commit d2bb231 has been approved by oli-obk

@bors
Copy link
Contributor

bors commented Feb 26, 2022

⌛ Testing commit d2bb231 with merge 144d565...

@bors
Copy link
Contributor

bors commented Feb 26, 2022

☀️ Test successful - checks-actions
Approved by: oli-obk
Pushing 144d565 to master...

@bors bors merged commit 144d565 into rust-lang:master Feb 26, 2022
// 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) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That seems very fragile, since it relies on a detail inside another function.

I think either prune_stacktrace should return a bool to indicate whether anything was pruned, or we just always print this because it's not a big deal.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hm, I agree. Then here we could only check if the trace was pruned.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah. Would you like to submit a PR for that?

bors added a commit that referenced this pull request Feb 26, 2022
Only print the pruning note if the trace was definitely pruned

Per #1987 (comment)

The only problem with this is that it looks like we can't emit the note about `-Zmiri-backtrace=full` if we also emit a tag-tracking diagnostic. The final note about the setting simply doesn't appear on the final error if it is printed before. I feel like we're running up against some diagnostic deduplication logic?
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants