-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
Save and restore frame pointer across Iextcall on ARM64 #13079
Conversation
31768a7
to
64e7cdf
Compare
More context on how this is working and why I ended up with this implementation. By default macOS has frame pointers enabled for clang/Xcode on ARM64, except for leaf functions. I haven't found a quotable documentation link for this but it can be determined experimentally by either compiling a simple C program or running lldb on the initial C parts of the runtime (startup_nat.c). Given this backtrace: (lldb) br s -n caml_program
Breakpoint 3: where = fib.exe`caml_program, address = 0x0000000100001790
(lldb) bt
* thread #1, queue = 'com.apple.main-thread', stop reason = breakpoint 1.1
* frame #0: 0x0000000100001790 fib.exe`caml_program
frame #1: 0x00000001000640b4 fib.exe`caml_start_program + 132
frame #2: 0x0000000100063948 fib.exe`caml_startup_common(argv=0x000000016fdff340, pooling=<unavailable>) at startup_nat.c:128:9 [opt]
frame #3: 0x00000001000639ac fib.exe`caml_main [inlined] caml_startup_exn(argv=<unavailable>) at startup_nat.c:135:10 [opt]
frame #4: 0x00000001000639a4 fib.exe`caml_main [inlined] caml_startup(argv=<unavailable>) at startup_nat.c:140:15 [opt]
frame #5: 0x00000001000639a4 fib.exe`caml_main(argv=<unavailable>) at startup_nat.c:147:3 [opt]
frame #6: 0x000000010004e640 fib.exe`main(argc=<unavailable>, argv=<unavailable>) at main.c:37:3 [opt]
frame #7: 0x0000000197c860e0 dyld`start + 2360 What is happening in mov x29, sp
ldr x16, [x28, #0x40]
mov sp, x16
bl 0x100064440 ; symbol stub for: caml_system__code_end + 168
mov sp, x29 At this point lldb unwinding breaks because it can't find the previous x29 value corresponding to frame th1/fr3 CFA is 0x140030010: Register fp (29) contents are 0x140030000, offset is 16
th1/fr3 supplying caller's saved pc (32)'s location using arm64-apple-darwin default unwind plan UnwindPlan
th1/fr3 supplying caller's register pc (32) from the stack, saved at CFA plus offset -8 [saved at 0x140030008]
th1/fr3 trying to unwind from this function with the UnwindPlan 'arm64-apple-darwin default unwind plan' because UnwindPlan 'EmulateInstructionARM64' failed.
th1/fr3 supplying caller's saved fp (29)'s location using arm64-apple-darwin default unwind plan UnwindPlan
th1/fr3 supplying caller's register fp (29) from the stack, saved at CFA plus offset -16 [saved at 0x140030000]
th1/fr3 supplying caller's saved pc (32)'s location, cached
th1/fr3 supplying caller's saved pc (32)'s location, cached
th1/fr4 pc = 0x100002fdc
th1/fr3 supplying caller's saved fp (29)'s location, cached
th1/fr4 fp = 0x0
th1/fr3 supplying caller's saved sp (31)'s location using arm64-apple-darwin default unwind plan UnwindPlan
th1/fr3 did not supply reg location for sp (31) because it is volatile
th1/fr4 with pc value of 0x100002fdc, symbol name is 'caml_program'
th1/fr4 Backing up the pc value of 0x100002fdc by 1 and re-doing symbol lookup; old symbol was caml_program
th1/fr4 Symbol is now caml_program
th1/fr4 Using full unwind plan 'eh_frame CFI'
th1/fr4 active row: 0x0000000100002fd4: CFA=sp+16 => lr=[CFA-8]
th1/fr3 supplying caller's saved sp (31)'s location using arm64-apple-darwin default unwind plan UnwindPlan
th1/fr3 did not supply reg location for sp (31) because it is volatile
th1/fr4 failed to get cfa
Frame 4 invalid RegisterContext for this frame, stopping stack walk
th1/fr4 no save location for fp (29) via 'eh_frame CFI'
th1/fr3 supplying caller's saved fp (29)'s location, cached
th1/fr4 requested caller's saved PC but this UnwindPlan uses a RA reg; getting lr (30) instead
th1/fr4 supplying caller's saved lr (30)'s location using eh_frame CFI UnwindPlan
th1/fr4 supplying caller's register lr (30) from the stack, saved at CFA plus offset -8 [saved at 0x1400300a8]
th1/fr4 requested caller's saved PC but this UnwindPlan uses a RA reg; getting lr (30) instead
th1/fr4 supplying caller's saved lr (30)'s location using eh_frame CFI UnwindPlan
th1/fr4 supplying caller's register lr (30) from the stack, saved at CFA plus offset -8 [saved at 0x1400300a8]
th1/fr5 pc = 0x0
th1/fr4 no save location for fp (29) via 'eh_frame CFI'
th1/fr3 supplying caller's saved fp (29)'s location, cached
th1/fr5 fp = 0x0
th1/fr4 supplying caller's saved sp (31)'s location, cached
th1/fr5 sp = 0x1400300b0
th1/fr5 this frame has a pc of 0x0
Frame 5 invalid RegisterContext for this frame, stopping stack walk
th1 Unwind of this thread is complete.
* thread #1, queue = 'com.apple.main-thread', stop reason = instruction step over
* frame #0: 0x0000000100064440 fib.exe`caml_system__code_end + 168
frame #1: 0x0000000100006f34 fib.exe`camlCamlinternalFormatBasics.entry + 44
frame #2: 0x0000000100002fdc fib.exe`caml_program + 28
frame #3: 0x00000001000640b4 fib.exe`caml_start_program + 132
frame #4: 0x0000000100002fdc fib.exe`caml_program + 28 At this point I am not 100% certain which Unwind plan is failing. But you can clearly see |
I had a conversation with @tmcgilchrist on a different channel. I learnt a few things about this PR, which others may appreciate. This work is part of the larger effort to improve lldb debugging support on macOS Arm64 platform. Note that gdb does not work on macOS Arm64 and lldb is the only native debugger. The experience of lldb on macOS Arm64 is considerably worse than debugging with gdb on Linux. Given that macOS with Apple Silicon is a popular developer machine, it is useful to put effort into improving the lldb debugging support on macOS Arm64 backend. Reading the PR message, which mentions the frame pointer register, I assumed that this PR improves backtraces on Arm64 in frame pointer mode. In fact, Arm64 does not yet have support for frame pointers and the PR improves backtraces without them. What is interesting is that, unlike Amd64 backend, on arm64, the frame pointer register This PR saves and restores What I still find surprising is that, lldb is able to produce better backtraces now for the nested recursive calls of
On trunk, I see
I suspect that |
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.
I believe the change is correct. Marking caml_start_program
as a signal frame seems fine to me for consistency of backtraces across platforms.
Not clang documentation, but this seems to align with https://developer.apple.com/documentation/xcode/writing-arm64-code-for-apple-platforms and this in particular:
|
@tmcgilchrist can you rebase the changes against the latest updates on trunk? |
00ef6be
to
3158a07
Compare
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.
This needs to be fixed: https://github.com/ocaml/ocaml/pull/13079/files#r1558891974
There is an unrelated assertion failure in the CI: https://github.com/ocaml/ocaml/actions/runs/8623935626/job/23638056562?pr=13079#step:7:77. I'll open a separate issue to track this. |
3158a07
to
7714f4d
Compare
Restores full backtraces on macOS ARM64 where the ABI expects the frame pointer register (x29) to always address a valid frame record.
7714f4d
to
de0df97
Compare
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.
LGTM now. Thanks @tmcgilchrist.
Should this be considered for 5.2? |
Some context on the general state of macOS LLDB debugging to help make an informed decision. With this change you have backtraces plus you can set breakpoints based on addresses in lldb. If you combine this change with #13050 then you get proper backtraces, setting breakpoints based on addresses and the ability to set breakpoints via symbols. eg Setting breakpoints based on source file and line number is still broken but I am working on it. |
Three remarks on this change:
|
If someone can document in this discussion that the performance impact is negligible, it sounds like a good idea to cherry-pick the change to 5.2 as a "bugfix for a feature we didn't know we nearly had". |
Is it useful to link to the recommendation in #13079 (comment)? Not saving and restoring
IINM, we generally don't have coverage in the testsuite for native debugger behaviours. For |
Save and restore frame pointer across Iextcall on ARM64 (cherry picked from commit c39c3f9)
Cherry-picked to 5.2.0 as a small bug fix yielding noticeable developer experience improvement in 5f767ec . |
This PR restores full backtraces on macOS ARM64 with lldb.
In OCaml 5.1.1 lldb looses the full backtrace once it reaches OCaml code as shown in this lldb session using this example program:
With this change the full backtrace is present:
Additionally on linux arm64 using gdb, adding
CFI_SIGNAL_FRAME
tocaml_start_program
replaces that frame with<signal handler called>
:This matches the backtraces generated on linux x86_64 but is not necessary to provide a full backtrace. Linux LLDB is unaffected by this change, it already had working backtraces.