ldc icon indicating copy to clipboard operation
ldc copied to clipboard

Fix libunwind backtraces & Make defaulttraceinfo more generic.

Open JohanEngelen opened this issue 1 year ago • 10 comments

Libunwind-based druntime implementation for backtraces results in wrong line numbers (very confusing backtrace).

  1. Make DefaultTraceInfo smart enough to not strictly require execinfo.
  2. ~~Stop using libunwind for musl.~~ Fix libunwind implementation

JohanEngelen avatar May 03 '24 15:05 JohanEngelen

@Geod24 The libunwind implementation needs a rework. The testcase that shows broken behavior clearly is dmd/runnable/test17559.d. On line 70, frame.address += pip.start_ip; is where the backtrace address for symbol:line resolution is stored. But frame.address is zero there, so it will be loaded with the function's entry address. Not so useful. Instead we should use something like unw_get_reg(&cursor, UNW_REG_IP, &ip); to get the address of the return address (after the function call instruction).

Edit: I now have a proper fix for libunwind aswell. Will need to iron out an issue with importC and @nogc nothrow.

JohanEngelen avatar May 03 '24 15:05 JohanEngelen

if it works here for LDC CI, I'll submit it upstream.

JohanEngelen avatar May 03 '24 15:05 JohanEngelen

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).

JohanEngelen avatar May 03 '24 21:05 JohanEngelen

Nice that you're hunting down the musl issues! :+1: - 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.

kinke avatar May 04 '24 01:05 kinke

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.

JohanEngelen avatar May 04 '24 10:05 JohanEngelen

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...

Geod24 avatar May 04 '24 13:05 Geod24

PR is fully finished. Feel free to squash and merge, or postpone until after 1.38 if deemed too risky.

JohanEngelen avatar May 04 '24 22:05 JohanEngelen

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.

JohanEngelen avatar May 06 '24 23:05 JohanEngelen

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.

To see current status: https://github.com/ldc-developers/ldc/actions/runs/8976987555/job/24654919738

JohanEngelen avatar May 06 '24 23:05 JohanEngelen

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.

This is fixed now (needed to adjust IP to point to the call instruction, rather than after the call).

JohanEngelen avatar May 07 '24 20:05 JohanEngelen

It turns out we can use this code fragment for libunwind aswell: https://github.com/ldc-developers/ldc/blob/c497e0aed8b5956f10cfc3ac6600af788c51ffab/runtime/druntime/src/core/runtime.d#L673-L722

So we can completely remove backtrace/handler.d. ~To be~ submitted in new PR: https://github.com/ldc-developers/ldc/pull/4691

JohanEngelen avatar May 24 '24 19:05 JohanEngelen