-
Notifications
You must be signed in to change notification settings - Fork 12.9k
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
Add line numbers to windows-gnu backtraces #28004
Conversation
(rust_highfive has picked a reviewer for you, use r? to override) |
@@ -15,5 +15,5 @@ pub use self::imp::*; | |||
mod imp; | |||
|
|||
#[cfg(not(any(target_os = "macos", target_os = "ios")))] | |||
#[path = "libbacktrace.rs"] | |||
#[path = "../../../gnu/libbacktrace.rs"] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can this file instead live inside of src/libstd/sys/common
? It can be #[cfg]
'd off there and just pub use
'd here
Nice! I've wondered in the past if we should just start using libbacktrace for everything instead of just symbols/filenames/line numbers, it's got a pretty good track record so far. |
Accidentally closed the PR there for a second! Anyway, I've made the changes you suggested - I'm mostly held back by how long it takes to build everything, so I've still been unable to test properly in all configurations. |
all(unix, not(any(target_os = "macos", target_os = "ios"))), | ||
all(windows, target_env = "gnu") | ||
) | ||
)] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Typically we don't format this with the newlines here, instead something like:
#[cfg(any(all(unix, not(any(target_os = "macos", target_os = "ios"))),
all(windows, target_env = "gnu")))]
Thanks! Just one minor nit and r=me after a squashing of the last 6 commits |
All done |
@bors: r+ 4b2910c99ece3dd042f558c7303d372b78395fc0 |
Er actually, just remembered, there's a test that tests for line numbers and can you be sure that it's enabled on GNU windows now? I believe the test is @bors: r- |
I've enabled the tests, and they seem to be passing. |
@bors: r+ 889fee33d47106bd51b06a9927c939fd8c396061 Thanks! |
⌛ Testing commit 889fee3 with merge 4686e32... |
💔 Test failed - auto-mac-64-opt |
@bors: r=alexcrichton |
📌 Commit b6c0d66 has been approved by |
⌛ Testing commit b6c0d66 with merge bccd8c6... |
💔 Test failed - auto-win-gnu-64-nopt-t |
@bors: r=alexcrichton |
@bors: r+ 9173b937807b8cfd749340c5ddbc573c84018a3b |
⌛ Testing commit 9173b93 with merge ab23511... |
💔 Test failed - auto-win-gnu-32-opt |
Fixing this is currently blocked by the fact that I'm unable to build rustc to stage2 under The Release backtrace only showed that it might be happening inside llvm's tail-call elimination pass. |
@Diggsey yeah I've seen that happen unfortunately with gcc 5.2.0, although I'm not sure how to downgrade... Last I checked (a few weeks ago I think) it also happened with the HEAD LLVM, and unfortunately I haven't tracked it down much farther than that :( |
9173b93
to
3aec908
Compare
Ok, I've just disabled the two backtrace tests on 32-bit windows-gnu again, since the problem seems to be unrelated to this PR (#28218). |
☔ The latest upstream changes (presumably #28200) made this pull request unmergeable. Please resolve the merge conflicts. |
Fix formatting Remove unused imports Refactor Fix msvc build Fix line lengths Formatting Enable backtrace tests Fix using directive on mac pwd info Work-around buildbot PWD bug, and fix libbacktrace configuration Use alternative to `env -u` which is not supported on bitrig Disable tests on 32-bit windows gnu
3aec908
to
d4fc3ec
Compare
Rebased |
@bors r=alexcrichton |
📌 Commit d4fc3ec has been approved by |
⌛ Testing commit d4fc3ec with merge 074c4b6... |
💔 Test failed - auto-linux-32-nopt-t |
Wat??? |
@bors: retry On Thu, Sep 3, 2015 at 10:47 PM, Diggory Blake [email protected]
|
Technically this could also be used for `windows-msvc` targets, as I believe they have *both* dwarf and pdb debug information, but I haven't enabled it there as it should really use the native windows APIs for that, instead of libbacktrace. I wasn't exactly sure where I should put "gnu" specific stuff, so tell me if I should structure things differently. This is still a WIP, and I haven't tested properly to make sure I haven't broken msvc/linux builds yet.
🎊 |
Hmm... I think this caused the
|
@jonas-schievink Neither of those tests were enabled on windows before this PR, so it's a safe bet that this caused it. It looks like for some reason libbacktrace is unable to access the dwarf debug info on the executable. Most likely causes are eiither there's no debug info being generated in the first place, or there's some new security restriction on windows 10 that's preventing it. |
Technically this could also be used for
windows-msvc
targets, as I believe they have both dwarf and pdb debug information, but I haven't enabled it there as it should really use the native windows APIs for that, instead of libbacktrace.I wasn't exactly sure where I should put "gnu" specific stuff, so tell me if I should structure things differently.
This is still a WIP, and I haven't tested properly to make sure I haven't broken msvc/linux builds yet.