[PATCH v4 0/1] MR8466: ntdll: Also restore rbp before calling user mode callback.
If wine dlls are built with frame pointers enabled, the frame pointer will be used during unwinding. If we don't restore frame pointer before calling the user mode callback, then later when the unwinder encounters the user mode callback frame, it will set the frame pointer to something unexpected (depends on what it was during `call_user_mode_callback`). Then for the subsequent frame it adjusts the stack pointer based on the frame pointer, thus derailing the unwinding process. -- v4: ntdll: Also restore rbp before calling user mode callback. https://gitlab.winehq.org/wine/wine/-/merge_requests/8466
From: Yuxuan Shui <yshui(a)codeweavers.com> If wine dlls are built with frame pointers enabled, the frame pointer will be used during unwinding. If we don't restore frame pointer before calling the user mode callback, then later when the unwinder encounters the user mode callback frame, it will set the frame pointer to something unexpected (depends on what it was during `call_user_mode_callback`). Then for the subsequent frame it adjusts the stack pointer based on the frame pointer, thus derailing the unwinding process. --- dlls/ntdll/unix/signal_x86_64.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/dlls/ntdll/unix/signal_x86_64.c b/dlls/ntdll/unix/signal_x86_64.c index 6ec662b9850..624ca111c43 100644 --- a/dlls/ntdll/unix/signal_x86_64.c +++ b/dlls/ntdll/unix/signal_x86_64.c @@ -1646,6 +1646,7 @@ __ASM_GLOBAL_FUNC( call_user_mode_callback, "movq %r10,0xa0(%rsp)\n\t" /* frame->prev_frame */ "movq %rsp,0x378(%r13)\n\t" /* thread_data->syscall_frame */ "testl $1,0x380(%r13)\n\t" /* thread_data->syscall_trace */ + "movq %r10,%r14\n\t" /* user_frame (previous thread_data->syscall_frame) */ "jz 1f\n\t" "movq %rdi,%r12\n\t" /* user_rsp */ "movl 0x2c(%r12),%edi\n\t" /* id */ @@ -1658,6 +1659,7 @@ __ASM_GLOBAL_FUNC( call_user_mode_callback, "movq %r15,%rcx\n" /* func */ /* switch to user stack */ "1:\tmovq %rdi,%rsp\n\t" /* user_rsp */ + "movq 0x98(%r14),%rbp\n\t" /* user_frame->Rbp */ #ifdef __linux__ "movw 0x338(%r13),%ax\n" /* amd64_thread_data()->fs */ "testw %ax,%ax\n\t" -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/8466
On Sat Jul 5 00:39:30 2025 +0000, Paul Gofman wrote:
That would also allow to see the test in full and look for potential caveats, what if some details lead to such result. Also unclear why you have rbx as `deadbeaf` on Wine if you saved all non-volatile registers. I updated the patch to use rbp from frame.
test: https://gitlab.winehq.org/-/snippets/25 -- https://gitlab.winehq.org/wine/wine/-/merge_requests/8466#note_108864
Paul Gofman (@gofman) commented about dlls/ntdll/unix/signal_x86_64.c:
"movq %r10,0xa0(%rsp)\n\t" /* frame->prev_frame */ "movq %rsp,0x378(%r13)\n\t" /* thread_data->syscall_frame */ "testl $1,0x380(%r13)\n\t" /* thread_data->syscall_trace */ + "movq %r10,%r14\n\t" /* user_frame (previous thread_data->syscall_frame) */
You can probably use r14 instead of r10 at once and thus avoid adding extra transfer (performance for one extra mov doesn't matter but longer and less obvious code does). -- https://gitlab.winehq.org/wine/wine/-/merge_requests/8466#note_108865
A test confirming that only rbp is saved among non-volatile registers still would be nice. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/8466#note_108866
On Sat Jul 5 01:15:08 2025 +0000, Paul Gofman wrote:
A test confirming that only rbp is saved among non-volatile registers still would be nice. how do you confirm that other regs are _not_ saved? it's possible for them to equal by chance.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/8466#note_108868
On Sat Jul 5 01:15:08 2025 +0000, Yuxuan Shui wrote:
how do you confirm that other regs are _not_ saved? it's possible for them to equal by chance. then the test can randomly fail. is there a way to test this that's no probabilistic?
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/8466#note_108869
participants (3)
-
Paul Gofman (＠gofman) -
Yuxuan Shui
-
Yuxuan Shui (＠yshui)