Add a fallback for stacktrace printing for older Windows versions. #50526

moxian · 2018-05-08T07:24:38Z

Some time ago we 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.

Tested on my Windows 7 - both msvc and gnu versions produce a readable stacktrace.

Fixes #50138

rust-highfive · 2018-05-08T07:24:41Z

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @Kimundi (or someone else) soon.

If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. Due to the way GitHub handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes.

Please see the contribution instructions for more information.

Zoxc · 2018-05-10T14:53:39Z

I'd like to see an enum in BacktraceContext which stores the set of functions which are going to be used instead of checking for them in each function. Also you should split the functions into two variants instead of having large matches. The best would be to make these functions generic so you could keep a single one.

moxian · 2018-05-11T07:03:05Z

Good points. Will do.

Zoxc · 2018-05-14T17:13:37Z

src/libstd/sys/windows/backtrace/mod.rs

+    }
+}
+
+fn set_frames_ex(


You should be able to make this generic by passing the STACKFRAME type, the init_frame function and a closure which calls StackWalk and returns the relevant information.

Done, thank you very much!

This is my first real foray into rust generics, so please let me know if I did anything particularly wrong here.

Zoxc · 2018-05-14T17:15:51Z

src/libstd/sys/windows/backtrace/mod.rs

+            ) == c::TRUE
+        {
+            let addr = frame.AddrPC.Offset;
+            if addr == frame.AddrReturn.Offset || addr == 0 || frame.AddrReturn.Offset == 0


This logic shouldn't be used. set_frames_64 and set_frames_ex should walk the stack in the same way.

Removed the cumbersome logic.

moxian · 2018-05-18T12:45:39Z

Genericalized.
Sorry that this is taking so long.

Zoxc · 2018-05-22T00:30:11Z

It's better now. I would prefer a solution without the extra traits though.

So fn set_frames<W: StackWalker>(StackWalk: W, frames: &mut [Frame]) -> io::Result<usize> could become

fn set_frames<
  Frame,
  Walk: FnMut(c::DWORD, c::HANDLE, c::HANDLE, &mut Frame, &mut c::CONTEXT)
  Addr: Fn(&mut Frame) -> *const u8,
>(frames: &mut [Frame], walk: Walk, addr: Addr) -> io::Result<usize>

And something similar could be done to remove the other added traits.

shepmaster · 2018-06-02T14:18:39Z

Ping from triage, @moxian ! Will you have time to revisit this soon?

moxian · 2018-06-04T11:20:58Z

@ triage: Sorry this is taking so long.
I picked up the bug because I wanted it fixed, not because I wanted to fix it myself. I hope me having this PR open does not prevent sufficiently motivated others from fixing it in the meantime (IOW: feel free to steal this from me any time)

@ Zoxc: I removed the extra traits in printing functions, but couldn't figure out how to do the stack walking without the trait.
The proposed closure wrapping StackWalkExFn/StackWalk64Fn cannot be of type FnMut(c::DWORD, c::HANDLE, c::HANDLE, &mut Frame, &mut c::CONTEXT)[1] since the wrapped function accepts concrete c::STACKFRAME64/c::STACKFRAME_EX type which is "erased", if you will, by that time. In other words the above type signature requires closure instance be polymorphic over the type of STACKFRAME which is impossible.
One option would be to pass in the rust's Frame instead of windows' c::STACKFRAME(64|_EX), but that would require moving the logic of populating the Frame from the STACKFRAME to be moved out of set_frame() and into the closure body , which necessarily duplicates said logic across both of the variants, and we are effectively back at commit 1.

[1] I assumed by Frame here you meant an abstraction encompassing both c::STACKFRAME64 and c::STACKFRAME_EX. If not, and you meant the sys_common::backtrace::Frame, then see the second paragraph.

Zoxc · 2018-06-10T19:40:24Z

I've pushed a commit which abstracts over stalk walking using a macro. A macro allows avoiding duplicating the setup of the stack frame structure, so I just used that for the entire thing.

I've yet to test that this code works on Windows 7/10.

That should be done and then we can get @alexcrichton to rubber stamp this.

moxian · 2018-06-14T05:59:10Z

I can confirm that this works on Windows 7 - both msvc and gnu variants.
I also tested it with dbghelp version 10.0.17134.12, and it still works, so should be good for Windows 10 as well (I don't have access to the real Win 10 system, so this is the best approximation I can get).

pietroalbini · 2018-06-25T08:21:22Z

r? @alexcrichton

alexcrichton · 2018-06-28T02:56:01Z

@bors: r+

Looks great to me!

bors · 2018-06-28T02:56:02Z

📌 Commit cfabe858a681de1227b054df0ab59d73f9c53227 has been approved by alexcrichton

bors · 2018-06-28T03:57:24Z

🔒 Merge conflict

This pull request and the master branch diverged in a way that cannot be automatically merged. Please rebase on top of the latest master branch, and let the reviewer approve again.

How do I rebase?

Assuming self is your fork and upstream is this repository, you can resolve the conflict following these steps:

git checkout just-fix (switch to your branch)
git fetch upstream master (retrieve the latest master)
git rebase upstream/master -p (rebase on top of it)
Follow the on-screen instruction to resolve conflicts (check git status if you got lost).
git push self just-fix --force-with-lease (update this PR)

You may also read Git Rebasing to Resolve Conflicts by Drew Blessing for a short tutorial.

Please avoid the "Resolve conflicts" button on GitHub. It uses git merge instead of git rebase which makes the PR commit history more difficult to read.

Sometimes step 4 will complete without asking for resolution. This is usually due to difference between how Cargo.lock conflict is handled during merge and rebase. This is normal, and you should still perform step 5 to update this PR.

Error message

CONFLICT (file/directory): There is a directory with name src/libbacktrace in heads/homu-tmp. Adding src/libbacktrace as src/libbacktrace~HEAD
Automatic merge failed; fix conflicts and then commit the result.

moxian · 2018-06-28T11:08:19Z

@pietroalbini Will do today (or maybe tomorrow - depending on your timezone).

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

.. rather than having them be one giant match statement.

.. and pass them around in BacktraceContext.

moxian · 2018-06-29T00:03:37Z

Done.

alexcrichton · 2018-06-29T02:14:56Z

@bors: r+

bors · 2018-06-29T02:14:57Z

📌 Commit be7f619 has been approved by alexcrichton

bors · 2018-06-29T06:15:56Z

⌛ Testing commit be7f619 with merge 090a7110e08eee03f71a6fb95a1c9baa2b61733e...

bors · 2018-06-29T07:34:08Z

💔 Test failed - status-travis

rust-highfive · 2018-06-29T07:34:10Z

The job dist-x86_64-linux of your PR failed on Travis (raw log). Through arcane magic we have determined that the following fragments from the build log may contain information about the problem.

Click to expand the log.

[01:10:30] [TIMING] Analysis { compiler: Compiler { stage: 2, host: "x86_64-unknown-linux-gnu" }, target: "x86_64-unknown-linux-gnu" } -- 3.058
[01:10:30] Dist src
[01:10:43] [TIMING] Src -- 12.640
[01:10:43] Create plain source tarball
[01:12:52] curl: (6) Couldn't resolve host 's3-us-west-1.amazonaws.com'
[01:14:44] curl: (6) Couldn't resolve host 's3-us-west-1.amazonaws.com'
[01:16:37] curl: (6) Couldn't resolve host 's3-us-west-1.amazonaws.com'
[01:16:37] thread 'main' panicked at 'failed to download openssl source: exit code: 6', bootstrap/native.rs:589:17
[01:16:37] failed to run: /checkout/obj/build/bootstrap/debug/bootstrap dist --host x86_64-unknown-linux-gnu --target x86_64-unknown-linux-gnu
[01:16:37] Build completed unsuccessfully in 1:11:00
travis_time:end:0de3483f:start=1530253039615388785,finish=1530257636929458900,duration=4597314070115

---
travis_time:end:08d5687b:start=1530257638293772449,finish=1530257638303225967,duration=9453518
travis_fold:end:after_failure.3
travis_fold:start:after_failure.4
travis_time:start:1dc8c752
$ head -30 ./obj/build/x86_64-unknown-linux-gnu/native/asan/build/lib/asan/clang_rt.asan-dynamic-i386.vers || true
global:
global:
  _ZdaPv;
  _ZdaPvRKSt9nothrow_t;
  _ZdaPvSt11align_val_t;
  _ZdaPvSt11align_val_tRKSt9nothrow_t;
  _ZdaPvj;
  _ZdaPvjSt11align_val_t;
  _ZdlPv;
  _ZdlPvRKSt9nothrow_t;
  _ZdlPvSt11align_val_t;
  _ZdlPvSt11align_val_tRKSt9nothrow_t;
  _ZdlPvj;
  _ZdlPvjSt11align_val_t;
  _Znaj;
  _ZnajRKSt9nothrow_t;
  _ZnajSt11align_val_t;
  _ZnajSt11align_val_tRKSt9nothrow_t;
  _Znwj;
  _ZnwjRKSt9nothrow_t;
  _ZnwjSt11align_val_t;
  _ZnwjSt11align_val_tRKSt9nothrow_t;
  __asan_*;
  __cxa_atexit;
  __cxa_throw;
  __fprintf_chk;
  __getdelim;
  __interceptor___cxa_atexit;
  __interceptor___cxa_throw;
  __interceptor___fprintf_chk;
travis_fold:end:after_failure.4
travis_fold:start:after_failure.5
travis_time:start:0060de60
$ dmesg | grep -i kill

I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact @TimNN. (Feature Requests)

rust-highfive · 2018-06-29T07:34:15Z

The job dist-x86_64-linux of your PR failed on Travis (raw log). Through arcane magic we have determined that the following fragments from the build log may contain information about the problem.

Click to expand the log.

[01:10:30] [TIMING] Analysis { compiler: Compiler { stage: 2, host: "x86_64-unknown-linux-gnu" }, target: "x86_64-unknown-linux-gnu" } -- 3.058
[01:10:30] Dist src
[01:10:43] [TIMING] Src -- 12.640
[01:10:43] Create plain source tarball
[01:12:52] curl: (6) Couldn't resolve host 's3-us-west-1.amazonaws.com'
[01:14:44] curl: (6) Couldn't resolve host 's3-us-west-1.amazonaws.com'
[01:16:37] curl: (6) Couldn't resolve host 's3-us-west-1.amazonaws.com'
[01:16:37] thread 'main' panicked at 'failed to download openssl source: exit code: 6', bootstrap/native.rs:589:17
[01:16:37] failed to run: /checkout/obj/build/bootstrap/debug/bootstrap dist --host x86_64-unknown-linux-gnu --target x86_64-unknown-linux-gnu
[01:16:37] Build completed unsuccessfully in 1:11:00
travis_time:end:0de3483f:start=1530253039615388785,finish=1530257636929458900,duration=4597314070115

---
travis_time:end:08d5687b:start=1530257638293772449,finish=1530257638303225967,duration=9453518
travis_fold:end:after_failure.3
travis_fold:start:after_failure.4
travis_time:start:1dc8c752
$ head -30 ./obj/build/x86_64-unknown-linux-gnu/native/asan/build/lib/asan/clang_rt.asan-dynamic-i386.vers || true
global:
global:
  _ZdaPv;
  _ZdaPvRKSt9nothrow_t;
  _ZdaPvSt11align_val_t;
  _ZdaPvSt11align_val_tRKSt9nothrow_t;
  _ZdaPvj;
  _ZdaPvjSt11align_val_t;
  _ZdlPv;
  _ZdlPvRKSt9nothrow_t;
  _ZdlPvSt11align_val_t;
  _ZdlPvSt11align_val_tRKSt9nothrow_t;
  _ZdlPvj;
  _ZdlPvjSt11align_val_t;
  _Znaj;
  _ZnajRKSt9nothrow_t;
  _ZnajSt11align_val_t;
  _ZnajSt11align_val_tRKSt9nothrow_t;
  _Znwj;
  _ZnwjRKSt9nothrow_t;
  _ZnwjSt11align_val_t;
  _ZnwjSt11align_val_tRKSt9nothrow_t;
  __asan_*;
  __cxa_atexit;
  __cxa_throw;
  __fprintf_chk;
  __getdelim;
  __interceptor___cxa_atexit;
  __interceptor___cxa_throw;
  __interceptor___fprintf_chk;
travis_fold:end:after_failure.4
travis_fold:start:after_failure.5
travis_time:start:0060de60
$ dmesg | grep -i kill

I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact @TimNN. (Feature Requests)

kennytm · 2018-06-29T08:31:35Z

@bors retry travis-ci/travis-ci#9696

bors · 2018-06-29T09:42:46Z

⌛ Testing commit be7f619 with merge 6e5e63a...

Add a fallback for stacktrace printing for older Windows versions. Some time ago we 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. Tested on my Windows 7 - both msvc and gnu versions produce a readable stacktrace. Fixes #50138

bors · 2018-06-29T11:45:59Z

☀️ Test successful - status-appveyor, status-travis
Approved by: alexcrichton
Pushing 6e5e63a to master...

@alexcrichton

Fix backtraces for inlined functions on Windows Fixes an regression introduced in rust-lang#50526 r? @alexcrichton

@alexcrichton

Fix backtraces for inlined functions on Windows Fixes an regression introduced in rust-lang#50526 r? @alexcrichton

rust-highfive assigned Kimundi May 8, 2018

rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label May 8, 2018

Zoxc reviewed May 14, 2018

View reviewed changes

pietroalbini assigned Zoxc and unassigned Kimundi May 28, 2018

pietroalbini 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-review Status: Awaiting review from the assignee but also interested parties. labels May 28, 2018

Zoxc force-pushed the just-fix branch from d5b1f20 to 2a7fcc4 Compare June 10, 2018 19:37

Zoxc force-pushed the just-fix branch from 2a7fcc4 to cfabe85 Compare June 10, 2018 19:41

pietroalbini 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 Jun 25, 2018

rust-highfive assigned alexcrichton and unassigned Zoxc Jun 25, 2018

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 Jun 28, 2018

bors removed the S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. label Jun 28, 2018

moxian added 6 commits June 28, 2018 21:56

Split separate stackwalk variants into their own functions

3245a47

.. rather than having them be one giant match statement.

Load backtrace-related functions only once

c0b280f

.. and pass them around in BacktraceContext.

Make stackwalking generic instead of matching on enum variants.

a0b1501

Make msvc symbol extraction/printing functions generic.

9d426ac

Change traits to bare FnMut where possible.

be7f619

moxian force-pushed the just-fix branch from cfabe85 to be7f619 Compare June 29, 2018 00:03

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-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jun 29, 2018

bors added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Jun 29, 2018

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 Jun 29, 2018

bors merged commit be7f619 into rust-lang:master Jun 29, 2018

Zoxc mentioned this pull request Dec 31, 2018

Fix backtraces for inlined functions on Windows #57238

Merged

Mark-Simulacrum added a commit to Mark-Simulacrum/rust that referenced this pull request Jan 3, 2019

Rollup merge of rust-lang#57238 - Zoxc:bt-fix, r=alexcrichton

400d453

Fix backtraces for inlined functions on Windows Fixes an regression introduced in rust-lang#50526 r? @alexcrichton

kennytm added a commit to kennytm/rust that referenced this pull request Jan 5, 2019

Rollup merge of rust-lang#57238 - Zoxc:bt-fix, r=alexcrichton

dd3e273

Fix backtraces for inlined functions on Windows Fixes an regression introduced in rust-lang#50526 r? @alexcrichton

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add a fallback for stacktrace printing for older Windows versions. #50526

Add a fallback for stacktrace printing for older Windows versions. #50526

moxian commented May 8, 2018

rust-highfive commented May 8, 2018

Zoxc commented May 10, 2018 •

edited

Loading

moxian commented May 11, 2018

Zoxc May 14, 2018

moxian May 18, 2018

Zoxc May 14, 2018

moxian May 18, 2018

moxian commented May 18, 2018

Zoxc commented May 22, 2018

shepmaster commented Jun 2, 2018

moxian commented Jun 4, 2018 •

edited

Loading

Zoxc commented Jun 10, 2018

moxian commented Jun 14, 2018 •

edited

Loading

pietroalbini commented Jun 25, 2018

alexcrichton commented Jun 28, 2018

bors commented Jun 28, 2018

bors commented Jun 28, 2018

moxian commented Jun 28, 2018

moxian commented Jun 29, 2018

alexcrichton commented Jun 29, 2018

bors commented Jun 29, 2018

bors commented Jun 29, 2018

bors commented Jun 29, 2018

rust-highfive commented Jun 29, 2018

rust-highfive commented Jun 29, 2018

kennytm commented Jun 29, 2018

bors commented Jun 29, 2018

bors commented Jun 29, 2018

Add a fallback for stacktrace printing for older Windows versions. #50526

Add a fallback for stacktrace printing for older Windows versions. #50526

Conversation

moxian commented May 8, 2018

rust-highfive commented May 8, 2018

Zoxc commented May 10, 2018 • edited Loading

moxian commented May 11, 2018

Zoxc May 14, 2018

Choose a reason for hiding this comment

moxian May 18, 2018

Choose a reason for hiding this comment

Zoxc May 14, 2018

Choose a reason for hiding this comment

moxian May 18, 2018

Choose a reason for hiding this comment

moxian commented May 18, 2018

Zoxc commented May 22, 2018

shepmaster commented Jun 2, 2018

moxian commented Jun 4, 2018 • edited Loading

Zoxc commented Jun 10, 2018

moxian commented Jun 14, 2018 • edited Loading

pietroalbini commented Jun 25, 2018

alexcrichton commented Jun 28, 2018

bors commented Jun 28, 2018

bors commented Jun 28, 2018

moxian commented Jun 28, 2018

moxian commented Jun 29, 2018

alexcrichton commented Jun 29, 2018

bors commented Jun 29, 2018

bors commented Jun 29, 2018

bors commented Jun 29, 2018

rust-highfive commented Jun 29, 2018

rust-highfive commented Jun 29, 2018

kennytm commented Jun 29, 2018

bors commented Jun 29, 2018

bors commented Jun 29, 2018

Zoxc commented May 10, 2018 •

edited

Loading

moxian commented Jun 4, 2018 •

edited

Loading

moxian commented Jun 14, 2018 •

edited

Loading