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.

Sign up for GitHub

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

Jump to bottom

Print inlined functions on Windows #47252

Merged
merged 1 commit into from
Jan 26, 2018
Merged

Print inlined functions on Windows #47252

merged 1 commit into from
Jan 26, 2018

Conversation

Zoxc
Copy link
Contributor

@Zoxc Zoxc commented Jan 7, 2018

Split from #45637

r? @alexcrichton

alexcrichton
alexcrichton reviewed Jan 8, 2018
View reviewed changes
src/libstd/sys_common/backtrace.rs Outdated
let skipped_before = 0;
// Look for the first occurence of a panic entry point
// Skip all frames before that
let skipped_before = frames.iter().position(|frame| {
Copy link
Member

Choose a reason for hiding this comment

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

Did these leak in by accident? I think tidy failed to compile due to these maybe?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I forgot to commit my code...

@alexcrichton
Copy link
Member

Can this test also be updated?

@kennytm kennytm added the S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. label Jan 8, 2018
@alexcrichton
Copy link
Member

Was it possible to update the relevant test to avoid the special-casing of msvc?

@Zoxc
Copy link
Contributor Author

Zoxc commented Jan 8, 2018

It looks like location information disappeared from x86-32. I'll have to look into that.

@kennytm
Copy link
Member

kennytm commented Jan 17, 2018

Hi @Zoxc! Have you checked the issue of 32-bit x86?

@kennytm kennytm mentioned this pull request Jan 17, 2018
Closed
@Zoxc
Copy link
Contributor Author

Zoxc commented Jan 17, 2018

I checked that similar code does work for C++. There might be some issue with either the FFI or debug information Rust generates. I'll try to use the C++ code to generate a stack trace for Rust code.

@Zoxc
Copy link
Contributor Author

Zoxc commented Jan 25, 2018 •
edited
Loading

I used a pointer instead of a u64 in SymGetLineFromInlineContext, which messed up the ABI on 32-bit platforms.

@Zoxc Zoxc force-pushed the backtrace-win branch 2 times, most recently from e059019 to 5253bab Compare January 25, 2018 08:29
@Zoxc
Copy link
Contributor Author

Zoxc commented Jan 25, 2018

This is now ready to be merged. The backtrace test still doesn't pass because the debug information we emit is not entirely correct.

@kennytm kennytm added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jan 25, 2018
@alexcrichton
Copy link
Member

@bors: r+

Ok, thanks for investigating!

@bors
Copy link
Contributor

bors commented Jan 25, 2018

📌 Commit 5253bab has been approved by alexcrichton

@kennytm kennytm 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 Jan 25, 2018
@bors
Copy link
Contributor

bors commented Jan 25, 2018

⌛ Testing commit 5253bab with merge 2b6205e...

bors added a commit that referenced this pull request Jan 25, 2018
@bors
Auto merge of #47252 - Zoxc:backtrace-win, r=alexcrichton
2b6205e 
Print inlined functions on Windows

Split from #45637

r? @alexcrichton
@bors
Copy link
Contributor

bors commented Jan 25, 2018

💔 Test failed - status-travis

@kennytm
Copy link
Member

kennytm commented Jan 25, 2018 •
edited
Loading

dist-various-2 failed, cannot build on CloudABI, legit.

Just a missing field.

[00:47:16] warning: dropping unsupported crate type `dylib` for target `x86_64-unknown-cloudabi`
[00:47:16] 
[00:47:19] error[E0063]: missing field `inline_context` in initializer of `sys_common::backtrace::Frame`
[00:47:19]   --> libstd/sys/cloudabi/backtrace.rs:77:29
[00:47:19]    |
[00:47:19] 77 |         cx.frames[cx.idx] = Frame {
[00:47:19]    |                             ^^^^^ missing `inline_context`
[00:47:19] 
[00:47:21] error: aborting due to previous error
[00:47:21] 
[00:47:21] error: Could not compile `std`.

@kennytm kennytm added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Jan 25, 2018
@Zoxc
Copy link
Contributor Author

Zoxc commented Jan 26, 2018

@bors r=alexcrichton

@bors
Copy link
Contributor

bors commented Jan 26, 2018

📌 Commit 634f8cc has been approved by alexcrichton

@kennytm kennytm 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-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jan 26, 2018
@bors
Copy link
Contributor

bors commented Jan 26, 2018

⌛ Testing commit 634f8cc with merge a97cd17...

bors added a commit that referenced this pull request Jan 26, 2018
@bors
Auto merge of #47252 - Zoxc:backtrace-win, r=alexcrichton
a97cd17 
Print inlined functions on Windows

Split from #45637

r? @alexcrichton
@bors
Copy link
Contributor

bors commented Jan 26, 2018

💔 Test failed - status-appveyor

@alexcrichton alexcrichton merged commit 634f8cc into rust-lang:master Jan 26, 2018
@alexcrichton
Copy link
Member

Gonna merge anyway since all tests passed and it's just the appveyor timeout.

@pravic pravic mentioned this pull request Apr 24, 2018
Closed
moxian added a commit to moxian/rust that referenced this pull request May 8, 2018
@moxian
Add a fallback for stacktrace printing for older Windows versions.
b5bd130 
PR rust-lang#47252 switched stack inspection functions of dbghelp.dll
to their newer alternatives that also capture inlined context.
Unfortunately, said new alternatives are not present in older
dbghelp.dll versions.
In particular Windows 7 at the time of writing has dbghelp.dll
version 6.1.7601 from 2010, that lacks StackWalkEx and friends.

Fixes rust-lang#50138
bors pushed a commit that referenced this pull request Jun 29, 2018
@moxian
Add a fallback for stacktrace printing for older Windows versions.
d39c66b 
PR #47252 switched stack inspection functions of dbghelp.dll
to their newer alternatives that also capture inlined context.
Unfortunately, said new alternatives are not present in older
dbghelp.dll versions.
In particular Windows 7 at the time of writing has dbghelp.dll
version 6.1.7601 from 2010, that lacks StackWalkEx and friends.

Fixes #50138
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@alexcrichton alexcrichton alexcrichton left review comments

Assignees

@alexcrichton alexcrichton

Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@Zoxc @alexcrichton @kennytm @bors