-
-
Notifications
You must be signed in to change notification settings - Fork 265
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
Fix libunwind backtraces & Make defaulttraceinfo more generic. #4639
Fix libunwind backtraces & Make defaulttraceinfo more generic. #4639
Conversation
…acktrace()`). When execinfo is not supported, it falls back to manual unwinding, which is the same fallback as when `backtrace()` is not successful.
…nd thus confusing!) line numbers in the backtrace. For now, do not use libunwind for Musl. This does mean that musl will use the fallback path in DefaultTraceInfo which uses manual unwinding, i.e. requiring `-frame-pointer=all`.
@Geod24 The libunwind implementation needs a rework. The testcase that shows broken behavior clearly is Edit: I now have a proper fix for libunwind aswell. Will need to iron out an issue with importC and |
if it works here for LDC CI, I'll submit it upstream. |
I first had it working with importC, but the LLVM header of libunwind is pretty small so let's use the existing hard-coded binaries (easy to check that they are correct, and the additions needed are arch independent). |
Nice that you're hunting down the musl issues! 👍 - How's it looking, many failures still remaining or close to the finish line? Just wrt. v1.38.0 final that I'd like to release this weekend, unless you are close and require a bit more time. |
Let's not wait for the musl work (yesterday I ran stdlib tests for the first time, many succeed, but e.g. fiber tests hung and I have not looked into that at all yet). I'm hoping to reach the endpoint with one Github Action that builds using an alpine container (quick to set up in linux), such that we have a fully statically linked musl release. |
Indeed, thanks for doing this! I haven't personally needed Musl for a few years so never got around to fixing the more tedious issues, but I do remember backtraces not being working fully... |
PR is fully finished. Feel free to squash and merge, or postpone until after 1.38 if deemed too risky. |
there is still something wrong with backtraces. One bug I know (need to adjust backtrace address to point to call instruction instead of right after call instruction), but then there is still something buggy giving slightly incorrect backtrace on throwing an exception. |
To see current status: |
… of right after the caller.
…tead of dladdr)" This reverts commit 10a2b02.
This is fixed now (needed to adjust IP to point to the call instruction, rather than after the call). |
It turns out we can use this code fragment for libunwind aswell: ldc/runtime/druntime/src/core/runtime.d Lines 673 to 722 in c497e0a
So we can completely remove |
…20758) Based on: * ldc-developers/ldc#4639 * ldc-developers/ldc#4691 Co-authored-by: Martin Kinkelin <[email protected]>
Libunwind-based druntime implementation for backtraces results in wrong line numbers (very confusing backtrace).
Stop using libunwind for musl.Fix libunwind implementation