Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make backtrace function names and spans match up #1283

Merged
merged 4 commits into from
Apr 2, 2020

Conversation

RalfJung
Copy link
Member

@RalfJung RalfJung commented Mar 30, 2020

This is the Miri side of rust-lang/rust#70590.
Fixes #521

@RalfJung RalfJung added the S-blocked-on-rust Status: Blocked on landing a Rust PR label Mar 30, 2020
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this pull request Apr 1, 2020
Miri: make backtrace function names and spans match up

Currently, Miri backtraces are a bit confusing:
```
error: Undefined Behavior: entering unreachable code
  --> tests/compile-fail/never_transmute_void.rs:10:11
   |
10 |     match v {} //~ ERROR  entering unreachable code
   |           ^ entering unreachable code
   |
   = help: this indicates a bug in the program: it performed an invalid operation, and caused Undefined Behavior
   = help: see https://doc.rust-lang.org/nightly/reference/behavior-considered-undefined.html for further information
note: inside call to `f` at tests/compile-fail/never_transmute_void.rs:17:5
  --> tests/compile-fail/never_transmute_void.rs:17:5
   |
17 |     f(v); //~ inside call to `f`
   |     ^^^^
   = note: inside call to `main` at /home/r/.rustup/toolchains/miri/lib/rustlib/src/rust/src/libstd/rt.rs:67:34
   = note: inside call to closure at /home/r/.rustup/toolchains/miri/lib/rustlib/src/rust/src/libstd/rt.rs:52:73
   = note: inside call to closure at /home/r/.rustup/toolchains/miri/lib/rustlib/src/rust/src/libstd/sys_common/backtrace.rs:130:5
```
When reading this like a normal backtrace, one would expect that e.g. the backrace involves the "main" function at "libstd/rt.rs:67:34". But that is not actually where we are in the main function, that is *where the main function is called*.

This is not how backtraces are usually rendered (including e.g. with `RUST_BACKTRACE=1`). Usually we print next to each function name where inside that function the frame is currently executing, not where the *parent* frame is executing. With this PR and the Miri side at rust-lang/miri#1283, the backtrace now looks as follows:
```
error: Undefined Behavior: entering unreachable code
  --> tests/compile-fail/never_transmute_void.rs:10:11
   |
10 |     match v {} //~ ERROR entering unreachable code
   |           ^ entering unreachable code
   |
   = help: this indicates a bug in the program: it performed an invalid operation, and caused Undefined Behavior
   = help: see https://doc.rust-lang.org/nightly/reference/behavior-considered-undefined.html for further information
   = note: inside `f` at tests/compile-fail/never_transmute_void.rs:10:11
note: inside `main` at tests/compile-fail/never_transmute_void.rs:17:5
  --> tests/compile-fail/never_transmute_void.rs:17:5
   |
17 |     f(v); //~ inside `main`
   |     ^^^^
   = note: inside closure at /home/r/src/rust/rustc/src/libstd/rt.rs:67:34
   = note: inside closure at /home/r/src/rust/rustc/src/libstd/rt.rs:52:73
   = note: inside `std::sys_common::backtrace::__rust_begin_short_backtrace::<[closure@DefId(1:6034 ~ std[87db]::rt[0]::lang_start_internal[0]::{{closure}}[0]::{{closure}}[0]) 0:&dyn std::ops::Fn() -> i32 + std::marker::Sync + std::panic::RefUnwindSafe], i32>` at /home/r/src/rust/rustc/src/libstd/sys_common/backtrace.rs:130:5
```
Now function name and printed line numbers match up in the notes.

This code is partially shared with const-eval, so the change also affects const-eval: instead of printing what is being called at some span, we print which function/constant this span is inside.

With this, we can also remove the `span` field from Miri's stack frames (which used to track the *caller span* of that frame, quite confusing), and then get of a whole lot of `span` arguments that ultimately just served to fill that field (and as a fallback for `caller_location`, which however was never actually used).

r? @oli-obk
@RalfJung
Copy link
Member Author

RalfJung commented Apr 2, 2020

@bors r+

@bors
Copy link
Collaborator

bors commented Apr 2, 2020

📌 Commit 5ce2466 has been approved by RalfJung

@bors
Copy link
Collaborator

bors commented Apr 2, 2020

⌛ Testing commit 5ce2466 with merge ed14328...

@bors
Copy link
Collaborator

bors commented Apr 2, 2020

☀️ Test successful - checks-travis, status-appveyor
Approved by: RalfJung
Pushing ed14328 to master...

@bors bors merged commit ed14328 into rust-lang:master Apr 2, 2020
@RalfJung RalfJung deleted the backtrace branch April 2, 2020 08:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-blocked-on-rust Status: Blocked on landing a Rust PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Better stack traces
2 participants