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

[backtraces]: look for the begin symbol only after seeing end #87602

Conversation

wesleywiser
Copy link
Member

On x86_64-pc-windows-msvc, we often get backtraces which look like
this:

```
10:     0x7ff77e0e9be5 - std::panicking::rust_panic_with_hook
11:     0x7ff77e0e11b4 - std::sys_common::backtrace::__rust_begin_short_backtrace::h5769736bdb11136c
12:     0x7ff77e0e116f - std::sys_common::backtrace::__rust_end_short_backtrace::h61c7ecb1b55338ae
13:     0x7ff77e0f89dd - std::panicking::begin_panic::h8e60ef9f82a41805
14:     0x7ff77e0e108c - d
15:     0x7ff77e0e1069 - c
16:     0x7ff77e0e1059 - b
17:     0x7ff77e0e1049 - a
18:     0x7ff77e0e1039 - core::ptr::drop_in_place<std::rt::lang_start<()>::{{closure}}>::h1bfcd14d5e15ba81
19:     0x7ff77e0e1186 - std::sys_common::backtrace::__rust_begin_short_backtrace::h5769736bdb11136c
20:     0x7ff77e0e100c - std::rt::lang_start::{{closure}}::ha054184bbf9921e3
```

Notice that __rust_begin_short_backtrace appears on frame 11 before
__rust_end_short_backtrace on frame 12. This is because in typical
release binaries without debug symbols, dbghelp.dll, which we use to walk
and symbolize the stack, does not know where CGU internal functions
start or end and so the closure invoked by __rust_end_short_backtrace
is incorrectly described as __rust_begin_short_backtrace because it
happens to be near that symbol.

While that can obviously change, this has been happening quite
consistently since #75048. Since this is a very small change to the std
and the change makes sense by itself, I think this is worth doing.

This doesn't completely resolve the situation for release binaries on
Windows, since without debug symbols, the stack printed can still show
incorrect symbol names (this is why the test uses #[no_mangle]) but it
does slightly improve the situation in that you see the same backtrace
you would see with RUST_BACKTRACE=full or in a debugger (without the
uninteresting bits at the top and bottom).

Fixes part of #87481

On `x86_64-pc-windows-msvc`, we often get backtraces which look like
this:

```
10:     0x7ff77e0e9be5 - std::panicking::rust_panic_with_hook
11:     0x7ff77e0e11b4 - std::sys_common::backtrace::__rust_begin_short_backtrace::h5769736bdb11136c
12:     0x7ff77e0e116f - std::sys_common::backtrace::__rust_end_short_backtrace::h61c7ecb1b55338ae
13:     0x7ff77e0f89dd - std::panicking::begin_panic::h8e60ef9f82a41805
14:     0x7ff77e0e108c - d
15:     0x7ff77e0e1069 - c
16:     0x7ff77e0e1059 - b
17:     0x7ff77e0e1049 - a
18:     0x7ff77e0e1039 - core::ptr::drop_in_place<std::rt::lang_start<()>::{{closure}}>::h1bfcd14d5e15ba81
19:     0x7ff77e0e1186 - std::sys_common::backtrace::__rust_begin_short_backtrace::h5769736bdb11136c
20:     0x7ff77e0e100c - std::rt::lang_start::{{closure}}::ha054184bbf9921e3
```

Notice that `__rust_begin_short_backtrace` appears on frame 11 before
`__rust_end_short_backtrace` on frame 12. This is because in typical
release binaries without debug symbols, dbghelp.dll, which we use to walk
and symbolize the stack, does not know where CGU internal functions
start or end and so the closure invoked by `__rust_end_short_backtrace`
is incorrectly described as `__rust_begin_short_backtrace` because it
happens to be near that symbol.

While that can obviously change, this has been happening quite
consistently since rust-lang#75048. Since this is a very small change to the std
and the change makes sense by itself, I think this is worth doing.

This doesn't completely resolve the situation for release binaries on
Windows, since without debug symbols, the stack printed can still show
incorrect symbol names (this is why the test uses `#[no_mangle]`) but it
does slightly improve the situation in that you see the same backtrace
you would see with `RUST_BACKTRACE=full` or in a debugger (without the
uninteresting bits at the top and bottom).
@rust-highfive
Copy link
Collaborator

r? @dtolnay

(rust-highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jul 29, 2021
Copy link
Member

@dtolnay dtolnay left a comment

Choose a reason for hiding this comment

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

Nice!

@dtolnay
Copy link
Member

dtolnay commented Jul 29, 2021

@bors r+

@bors
Copy link
Contributor

bors commented Jul 29, 2021

📌 Commit 286cdc8 has been approved by dtolnay

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jul 29, 2021
bors added a commit to rust-lang-ci/rust that referenced this pull request Jul 30, 2021
Rollup of 10 pull requests

Successful merges:

 - rust-lang#87052 (Optimize fmt::PadAdapter::wrap)
 - rust-lang#87522 (Fix assert in diy_float)
 - rust-lang#87553 (Fix typo in rustc_driver::version)
 - rust-lang#87554 (2229: Discr should be read when PatKind is Range)
 - rust-lang#87564 (min_type_alias_impl_trait is going to be removed in 1.56)
 - rust-lang#87574 (Update the examples in `String` and `VecDeque::retain`)
 - rust-lang#87583 (Refactor compression cache in v0 symbol mangler)
 - rust-lang#87585 (Add missing links for core::char types)
 - rust-lang#87594 (fs File get_path procfs usage for netbsd same as linux.)
 - rust-lang#87602 ([backtraces]: look for the `begin` symbol only after seeing `end`)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 84e1882 into rust-lang:master Jul 30, 2021
@rustbot rustbot added this to the 1.56.0 milestone Jul 30, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants