Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=52213 Signed-off-by: Rémi Bernon rbernon@codeweavers.com ---
Something like that seems to let pthread_exit unwinding work and call its cleanup handlers.
Unwinding also seems to work fine when adding .cfi metadata to point to the right PE frames, although I didn't look exactly how it handles that. Maybe it only works in the context of builtin libraries.
As we don't want pthread to do anything with the PE code I'm keeping track of unix address only here instead, which I'm assuming makes sure it only unwinds syscall frames.
Somehow libunwind only cares about %rip .cfi directives, although I've also added .cfi for %rsp here (resp. %eip, %esp on i386), I'm not completely sure why or which option is best.
dlls/ntdll/unix/signal_i386.c | 5 +++-- dlls/ntdll/unix/signal_x86_64.c | 5 +++-- 2 files changed, 6 insertions(+), 4 deletions(-)
diff --git a/dlls/ntdll/unix/signal_i386.c b/dlls/ntdll/unix/signal_i386.c index 6bb5649e2b5..0e3b1daf51a 100644 --- a/dlls/ntdll/unix/signal_i386.c +++ b/dlls/ntdll/unix/signal_i386.c @@ -1613,8 +1613,9 @@ NTSTATUS WINAPI KeUserModeCallback( ULONG id, const void *args, ULONG len, void NTSTATUS WINAPI NtCallbackReturn( void *ret_ptr, ULONG ret_len, NTSTATUS status ) { struct user_callback_frame *frame = (struct user_callback_frame *)x86_thread_data()->syscall_frame; + void *exit_frame = x86_thread_data()->exit_frame;
- if (!frame->frame.prev_frame) return STATUS_NO_CALLBACK_ACTIVE; + if (frame->frame.prev_frame == exit_frame) return STATUS_NO_CALLBACK_ACTIVE;
*frame->ret_ptr = ret_ptr; *frame->ret_len = ret_len; @@ -2399,7 +2400,7 @@ void DECLSPEC_HIDDEN call_init_thunk( LPTHREAD_START_ROUTINE entry, void *arg, B *(--stack) = 0xdeadbabe; frame->esp = (DWORD)stack; frame->eip = (DWORD)pLdrInitializeThunk; - frame->prev_frame = NULL; + frame->prev_frame = thread_data->exit_frame; frame->syscall_flags = syscall_flags; frame->syscall_table = KeServiceDescriptorTable; frame->restore_flags |= LOWORD(CONTEXT_INTEGER); diff --git a/dlls/ntdll/unix/signal_x86_64.c b/dlls/ntdll/unix/signal_x86_64.c index 68855dccacf..da89b958665 100644 --- a/dlls/ntdll/unix/signal_x86_64.c +++ b/dlls/ntdll/unix/signal_x86_64.c @@ -2364,8 +2364,9 @@ NTSTATUS WINAPI KeUserModeCallback( ULONG id, const void *args, ULONG len, void NTSTATUS WINAPI NtCallbackReturn( void *ret_ptr, ULONG ret_len, NTSTATUS status ) { struct user_callback_frame *frame = (struct user_callback_frame *)amd64_thread_data()->syscall_frame; + void *exit_frame = amd64_thread_data()->exit_frame;
- if (!frame->frame.prev_frame) return STATUS_NO_CALLBACK_ACTIVE; + if (frame->frame.prev_frame == exit_frame) return STATUS_NO_CALLBACK_ACTIVE;
*frame->ret_ptr = ret_ptr; *frame->ret_len = ret_len; @@ -3072,7 +3073,7 @@ void DECLSPEC_HIDDEN call_init_thunk( LPTHREAD_START_ROUTINE entry, void *arg, B frame->rsp = (ULONG64)ctx - 8; frame->rip = (ULONG64)pLdrInitializeThunk; frame->rcx = (ULONG64)ctx; - frame->prev_frame = NULL; + frame->prev_frame = thread_data->exit_frame; frame->restore_flags |= CONTEXT_INTEGER; frame->syscall_flags = syscall_flags; frame->syscall_table = KeServiceDescriptorTable;
For pthread_exit unwinding purposes, as libunwind needs to retrieve %rip and we don't want to let it unwind the PE frames, we need to keep track of a unix-only return address chain.
Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=52213 Signed-off-by: Rémi Bernon rbernon@codeweavers.com --- dlls/ntdll/unix/signal_i386.c | 7 +++++-- dlls/ntdll/unix/signal_x86_64.c | 5 ++++- 2 files changed, 9 insertions(+), 3 deletions(-)
diff --git a/dlls/ntdll/unix/signal_i386.c b/dlls/ntdll/unix/signal_i386.c index 0e3b1daf51a..d98a3b1d4bb 100644 --- a/dlls/ntdll/unix/signal_i386.c +++ b/dlls/ntdll/unix/signal_i386.c @@ -463,9 +463,10 @@ struct syscall_frame * 32-bit mode, but some processors fault if they're not in writable memory. */ DECLSPEC_ALIGN(64) XSTATE xstate; /* 240 */ + DWORD unwind_eip; /* 380 */ };
-C_ASSERT( sizeof(struct syscall_frame) == 0x380 ); +C_ASSERT( sizeof(struct syscall_frame) == 0x3c0 );
struct x86_thread_data { @@ -2432,7 +2433,7 @@ __ASM_GLOBAL_FUNC( signal_start_thread, "movl 0x1f8(%ecx),%eax\n\t" /* x86_thread_data()->syscall_frame */ "orl %eax,%eax\n\t" "jnz 1f\n\t" - "leal -0x380(%esp),%eax\n\t" /* sizeof(struct syscall_frame) */ + "leal -0x3c0(%esp),%eax\n\t" /* sizeof(struct syscall_frame) */ "andl $~63,%eax\n\t" "movl %eax,0x1f8(%ecx)\n" /* x86_thread_data()->syscall_frame */ "1:\tmovl %eax,%esp\n\t" @@ -2591,6 +2592,8 @@ __ASM_GLOBAL_FUNC( __wine_syscall_dispatcher, "6:\tmovl $0xc000000d,%eax\n\t" /* STATUS_INVALID_PARAMETER */ "jmp 5b\n" __ASM_NAME("__wine_syscall_dispatcher_return") ":\n\t" + "movl 0(%esp),%eax\n\t" + "movl %eax,0x380(%esp)\n\t" /* frame->unwind_eip */ "movl 8(%esp),%eax\n\t" "movl 4(%esp),%esp\n\t" "jmp 5b" ) diff --git a/dlls/ntdll/unix/signal_x86_64.c b/dlls/ntdll/unix/signal_x86_64.c index da89b958665..f8cddd15569 100644 --- a/dlls/ntdll/unix/signal_x86_64.c +++ b/dlls/ntdll/unix/signal_x86_64.c @@ -349,7 +349,8 @@ struct syscall_frame struct syscall_frame *prev_frame; /* 00a0 */ SYSTEM_SERVICE_TABLE *syscall_table; /* 00a8 */ DWORD syscall_flags; /* 00b0 */ - DWORD align[3]; /* 00b4 */ + DWORD align; /* 00b4 */ + ULONG64 unwind_rip; /* 00b8 */ XMM_SAVE_AREA32 xsave; /* 00c0 */ DECLSPEC_ALIGN(64) XSTATE xstate; /* 02c0 */ }; @@ -3278,6 +3279,8 @@ __ASM_GLOBAL_FUNC( __wine_syscall_dispatcher, "5:\tmovl $0xc000000d,%edx\n\t" /* STATUS_INVALID_PARAMETER */ "movq %rsp,%rcx\n" __ASM_NAME("__wine_syscall_dispatcher_return") ":\n\t" + "movq 0(%rsp),%r14\n\t" + "movq %r14,0xb8(%rcx)\n\t" /* frame->unwind_rip */ "movl 0xb0(%rcx),%r14d\n\t" /* frame->syscall_flags */ "movq %rdx,%rax\n\t" "jmp 2b" )
On 2/8/22 04:05, Rémi Bernon wrote:
For pthread_exit unwinding purposes, as libunwind needs to retrieve %rip and we don't want to let it unwind the PE frames, we need to keep track of a unix-only return address chain.
Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=52213 Signed-off-by: Rémi Bernon rbernon@codeweavers.com
dlls/ntdll/unix/signal_i386.c | 7 +++++-- dlls/ntdll/unix/signal_x86_64.c | 5 ++++- 2 files changed, 9 insertions(+), 3 deletions(-)
diff --git a/dlls/ntdll/unix/signal_i386.c b/dlls/ntdll/unix/signal_i386.c index 0e3b1daf51a..d98a3b1d4bb 100644 --- a/dlls/ntdll/unix/signal_i386.c +++ b/dlls/ntdll/unix/signal_i386.c @@ -463,9 +463,10 @@ struct syscall_frame * 32-bit mode, but some processors fault if they're not in writable memory. */ DECLSPEC_ALIGN(64) XSTATE xstate; /* 240 */
- DWORD unwind_eip; /* 380 */
};
-C_ASSERT( sizeof(struct syscall_frame) == 0x380 ); +C_ASSERT( sizeof(struct syscall_frame) == 0x3c0 );
struct x86_thread_data { @@ -2432,7 +2433,7 @@ __ASM_GLOBAL_FUNC( signal_start_thread, "movl 0x1f8(%ecx),%eax\n\t" /* x86_thread_data()->syscall_frame */ "orl %eax,%eax\n\t" "jnz 1f\n\t"
"leal -0x380(%esp),%eax\n\t" /* sizeof(struct syscall_frame) */
"leal -0x3c0(%esp),%eax\n\t" /* sizeof(struct syscall_frame) */ "andl $~63,%eax\n\t" "movl %eax,0x1f8(%ecx)\n" /* x86_thread_data()->syscall_frame */ "1:\tmovl %eax,%esp\n\t"@@ -2591,6 +2592,8 @@ __ASM_GLOBAL_FUNC( __wine_syscall_dispatcher, "6:\tmovl $0xc000000d,%eax\n\t" /* STATUS_INVALID_PARAMETER */ "jmp 5b\n" __ASM_NAME("__wine_syscall_dispatcher_return") ":\n\t"
"movl 0(%esp),%eax\n\t""movl %eax,0x380(%esp)\n\t" /* frame->unwind_eip */
We can't unwind to the caller without also saving the caller's nonvolatile registers. Also, KeUserModeCallback is one of the callers of __wine_syscall_dispatcher_return, and returning to it would not be useful since it is usually called from PE modules anyway. Maybe just use prev_frame directly, and set unwind_eip to an hardcoded address? (Speaking of which, we could use .cfi_val_encoded_addr here, but it's a GNU-only extension AFAIK.)
"movl 8(%esp),%eax\n\t" "movl 4(%esp),%esp\n\t" "jmp 5b" )diff --git a/dlls/ntdll/unix/signal_x86_64.c b/dlls/ntdll/unix/signal_x86_64.c index da89b958665..f8cddd15569 100644 --- a/dlls/ntdll/unix/signal_x86_64.c +++ b/dlls/ntdll/unix/signal_x86_64.c @@ -349,7 +349,8 @@ struct syscall_frame struct syscall_frame *prev_frame; /* 00a0 */ SYSTEM_SERVICE_TABLE *syscall_table; /* 00a8 */ DWORD syscall_flags; /* 00b0 */
- DWORD align[3]; /* 00b4 */
- DWORD align; /* 00b4 */
- ULONG64 unwind_rip; /* 00b8 */ XMM_SAVE_AREA32 xsave; /* 00c0 */ DECLSPEC_ALIGN(64) XSTATE xstate; /* 02c0 */
}; @@ -3278,6 +3279,8 @@ __ASM_GLOBAL_FUNC( __wine_syscall_dispatcher, "5:\tmovl $0xc000000d,%edx\n\t" /* STATUS_INVALID_PARAMETER */ "movq %rsp,%rcx\n" __ASM_NAME("__wine_syscall_dispatcher_return") ":\n\t"
"movq 0(%rsp),%r14\n\t""movq %r14,0xb8(%rcx)\n\t" /* frame->unwind_rip */
Ditto.
"movl 0xb0(%rcx),%r14d\n\t" /* frame->syscall_flags */ "movq %rdx,%rax\n\t" "jmp 2b" )
On 2/9/22 17:34, Jinoh Kang wrote:
On 2/8/22 04:05, Rémi Bernon wrote:
For pthread_exit unwinding purposes, as libunwind needs to retrieve %rip and we don't want to let it unwind the PE frames, we need to keep track of a unix-only return address chain.
Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=52213 Signed-off-by: Rémi Bernon rbernon@codeweavers.com
dlls/ntdll/unix/signal_i386.c | 7 +++++-- dlls/ntdll/unix/signal_x86_64.c | 5 ++++- 2 files changed, 9 insertions(+), 3 deletions(-)
diff --git a/dlls/ntdll/unix/signal_i386.c b/dlls/ntdll/unix/signal_i386.c index 0e3b1daf51a..d98a3b1d4bb 100644 --- a/dlls/ntdll/unix/signal_i386.c +++ b/dlls/ntdll/unix/signal_i386.c @@ -463,9 +463,10 @@ struct syscall_frame * 32-bit mode, but some processors fault if they're not in writable memory. */ DECLSPEC_ALIGN(64) XSTATE xstate; /* 240 */
- DWORD unwind_eip; /* 380 */ };
-C_ASSERT( sizeof(struct syscall_frame) == 0x380 ); +C_ASSERT( sizeof(struct syscall_frame) == 0x3c0 );
struct x86_thread_data { @@ -2432,7 +2433,7 @@ __ASM_GLOBAL_FUNC( signal_start_thread, "movl 0x1f8(%ecx),%eax\n\t" /* x86_thread_data()->syscall_frame */ "orl %eax,%eax\n\t" "jnz 1f\n\t"
"leal -0x380(%esp),%eax\n\t" /* sizeof(struct syscall_frame) */
"leal -0x3c0(%esp),%eax\n\t" /* sizeof(struct syscall_frame) */ "andl $~63,%eax\n\t" "movl %eax,0x1f8(%ecx)\n" /* x86_thread_data()->syscall_frame */ "1:\tmovl %eax,%esp\n\t"@@ -2591,6 +2592,8 @@ __ASM_GLOBAL_FUNC( __wine_syscall_dispatcher, "6:\tmovl $0xc000000d,%eax\n\t" /* STATUS_INVALID_PARAMETER */ "jmp 5b\n" __ASM_NAME("__wine_syscall_dispatcher_return") ":\n\t"
"movl 0(%esp),%eax\n\t""movl %eax,0x380(%esp)\n\t" /* frame->unwind_eip */We can't unwind to the caller without also saving the caller's nonvolatile registers. Also, KeUserModeCallback is one of the callers of __wine_syscall_dispatcher_return, and returning to it would not be useful since it is usually called from PE modules anyway. Maybe just use prev_frame directly, and set unwind_eip to an hardcoded address? (Speaking of which, we could use .cfi_val_encoded_addr here, but it's a GNU-only extension AFAIK.)
Hmm, I'm pretty sure that KeUserModeCallback is meant to be called from the unix side (although it's a Windows kernel API, it is meant to be called from kernel mode, which is our unix world), and would switch back to the PE stack, pushing a new syscall frame on the unix stack.
What I meant to do here, is to keep track of the unix caller (so either call_init_thunk or KeUserModeCallback), so that we will later link to it when a new syscall will be done, to the exit frame, as if we were returning to call_init_thunk and wherever the thread was started from, when the thread exits.
Regarding the registers, I think it's what __wine_setjmpex does, no? Otherwise it would have trouble already. Maybe call_init_thunk should do it too, although it's never supposed to returning to it normally (only through unwinding in pthread_exit).
On 2/10/22 03:13, Rémi Bernon wrote:
On 2/9/22 17:34, Jinoh Kang wrote:
On 2/8/22 04:05, Rémi Bernon wrote:
For pthread_exit unwinding purposes, as libunwind needs to retrieve %rip and we don't want to let it unwind the PE frames, we need to keep track of a unix-only return address chain.
Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=52213 Signed-off-by: Rémi Bernon rbernon@codeweavers.com
dlls/ntdll/unix/signal_i386.c | 7 +++++-- dlls/ntdll/unix/signal_x86_64.c | 5 ++++- 2 files changed, 9 insertions(+), 3 deletions(-)
diff --git a/dlls/ntdll/unix/signal_i386.c b/dlls/ntdll/unix/signal_i386.c index 0e3b1daf51a..d98a3b1d4bb 100644 --- a/dlls/ntdll/unix/signal_i386.c +++ b/dlls/ntdll/unix/signal_i386.c @@ -463,9 +463,10 @@ struct syscall_frame * 32-bit mode, but some processors fault if they're not in writable memory. */ DECLSPEC_ALIGN(64) XSTATE xstate; /* 240 */ + DWORD unwind_eip; /* 380 */ }; -C_ASSERT( sizeof(struct syscall_frame) == 0x380 ); +C_ASSERT( sizeof(struct syscall_frame) == 0x3c0 ); struct x86_thread_data { @@ -2432,7 +2433,7 @@ __ASM_GLOBAL_FUNC( signal_start_thread, "movl 0x1f8(%ecx),%eax\n\t" /* x86_thread_data()->syscall_frame */ "orl %eax,%eax\n\t" "jnz 1f\n\t" - "leal -0x380(%esp),%eax\n\t" /* sizeof(struct syscall_frame) */ + "leal -0x3c0(%esp),%eax\n\t" /* sizeof(struct syscall_frame) */ "andl $~63,%eax\n\t" "movl %eax,0x1f8(%ecx)\n" /* x86_thread_data()->syscall_frame */ "1:\tmovl %eax,%esp\n\t" @@ -2591,6 +2592,8 @@ __ASM_GLOBAL_FUNC( __wine_syscall_dispatcher, "6:\tmovl $0xc000000d,%eax\n\t" /* STATUS_INVALID_PARAMETER */ "jmp 5b\n" __ASM_NAME("__wine_syscall_dispatcher_return") ":\n\t" + "movl 0(%esp),%eax\n\t" + "movl %eax,0x380(%esp)\n\t" /* frame->unwind_eip */
We can't unwind to the caller without also saving the caller's nonvolatile registers. Also, KeUserModeCallback is one of the callers of __wine_syscall_dispatcher_return, and returning to it would not be useful since it is usually called from PE modules anyway. Maybe just use prev_frame directly, and set unwind_eip to an hardcoded address? (Speaking of which, we could use .cfi_val_encoded_addr here, but it's a GNU-only extension AFAIK.)
Hmm, I'm pretty sure that KeUserModeCallback is meant to be called from the unix side (although it's a Windows kernel API, it is meant to be called from kernel mode, which is our unix world), and would switch back to the PE stack, pushing a new syscall frame on the unix stack.
Yeah, that sounds about right. Sorry for the noise.
My point of not using the return address and using the prev_frame/exit_frame chain still stands. No functions expect __wine_syscall_dispatcher_return to return.
What I meant to do here, is to keep track of the unix caller (so either call_init_thunk or KeUserModeCallback), so that we will later link to it when a new syscall will be done, to the exit frame, as if we were returning to call_init_thunk and wherever the thread was started from, when the thread exits.
Regarding the registers, I think it's what __wine_setjmpex does, no?
Yes, but simply unwinding to the caller does not mean we're using the jmp_buf.
Otherwise it would have trouble already.
Maybe it does not immediately result in crash because CFI instructions gradually recover nonvolatile registers along the frames and mask the clobbering. We should not rely on this behavior though.
Maybe call_init_thunk should do it too, although it's never supposed to returning to it normally (only through unwinding in pthread_exit).
Making sure stack pointer points to previous syscall / exit frame before entering a syscall, and restoring the PE frame information on return.
Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=52213 Signed-off-by: Rémi Bernon rbernon@codeweavers.com --- dlls/ntdll/unix/signal_i386.c | 9 +++++++++ dlls/ntdll/unix/signal_x86_64.c | 9 +++++++++ 2 files changed, 18 insertions(+)
diff --git a/dlls/ntdll/unix/signal_i386.c b/dlls/ntdll/unix/signal_i386.c index d98a3b1d4bb..2f6e2fd4153 100644 --- a/dlls/ntdll/unix/signal_i386.c +++ b/dlls/ntdll/unix/signal_i386.c @@ -2492,6 +2492,9 @@ __ASM_GLOBAL_FUNC( __wine_syscall_dispatcher, "movl %esi,0x30(%ecx)\n\t" "movl %ebp,0x34(%ecx)\n\t" "leal 0x34(%ecx),%ebp\n\t" + __ASM_CFI(".cfi_def_cfa %ebp,0\n\t") + __ASM_CFI(".cfi_rel_offset %eip,-0x2c\n\t") + __ASM_CFI(".cfi_rel_offset %esp,-0x28\n\t") "leal 4(%esp),%esi\n\t" /* first argument */ "movl %eax,%ebx\n\t" "shrl $8,%ebx\n\t" @@ -2530,6 +2533,9 @@ __ASM_GLOBAL_FUNC( __wine_syscall_dispatcher, "3:\tfnsave 0x40(%ecx)\n\t" "fwait\n" "4:\tmovl %ecx,%esp\n\t" + __ASM_CFI(".cfi_def_cfa %ebp,0\n\t") + __ASM_CFI(".cfi_rel_offset %eip,0x34c\n\t") /* frame->unwind_rip */ + __ASM_CFI(".cfi_rel_offset %esp,0x08\n\t") /* frame->prev_frame */ "movl 0x1c(%esp),%edx\n\t" /* frame->eax */ "andl $0xfff,%edx\n\t" /* syscall number */ "cmpl 8(%ebx),%edx\n\t" /* table->ServiceLimit */ @@ -2545,6 +2551,9 @@ __ASM_GLOBAL_FUNC( __wine_syscall_dispatcher, "rep; movsl\n\t" "call *(%eax,%edx,4)\n\t" "leal -0x34(%ebp),%esp\n" + __ASM_CFI(".cfi_def_cfa %ebp,0\n\t") + __ASM_CFI(".cfi_rel_offset %eip,-0x2c\n\t") + __ASM_CFI(".cfi_rel_offset %esp,-0x28\n\t") "5:\tmovl 0(%esp),%ecx\n\t" /* frame->syscall_flags + (frame->restore_flags << 16) */ "testl $0x68 << 16,%ecx\n\t" /* CONTEXT_FLOATING_POINT | CONTEXT_EXTENDED_REGISTERS | CONTEXT_XSAVE */ "jz 3f\n\t" diff --git a/dlls/ntdll/unix/signal_x86_64.c b/dlls/ntdll/unix/signal_x86_64.c index f8cddd15569..ee2723cdb24 100644 --- a/dlls/ntdll/unix/signal_x86_64.c +++ b/dlls/ntdll/unix/signal_x86_64.c @@ -3166,6 +3166,9 @@ __ASM_GLOBAL_FUNC( __wine_syscall_dispatcher, "movw %ss,0x90(%rcx)\n\t" "movw %gs,0x92(%rcx)\n\t" "movq %rbp,0x98(%rcx)\n\t" + __ASM_CFI(".cfi_def_cfa %rcx,0\n\t") + __ASM_CFI(".cfi_rel_offset %rip,0x70\n\t") + __ASM_CFI(".cfi_rel_offset %rsp,0x88\n\t") /* Legends of Runeterra hooks the first system call return instruction, and * depends on us returning to it. Adjust the return address accordingly. */ "subq $0xb,0x70(%rcx)\n\t" @@ -3206,6 +3209,9 @@ __ASM_GLOBAL_FUNC( __wine_syscall_dispatcher, #endif "leaq 0x28(%rsp),%rsi\n\t" /* first argument */ "movq %rcx,%rsp\n\t" + __ASM_CFI(".cfi_def_cfa %rbp,0\n\t") + __ASM_CFI(".cfi_rel_offset %rip,0x20\n\t") /* frame->unwind_rip */ + __ASM_CFI(".cfi_rel_offset %rsp,0x08\n\t") /* frame->prev_frame */ "movq 0x00(%rcx),%rax\n\t" "movq 0x18(%rcx),%rdx\n\t" "movl %eax,%ebx\n\t" @@ -3231,6 +3237,9 @@ __ASM_GLOBAL_FUNC( __wine_syscall_dispatcher, "movq (%rbx),%r10\n\t" /* table->ServiceTable */ "callq *(%r10,%rax,8)\n\t" "leaq -0x98(%rbp),%rcx\n" + __ASM_CFI(".cfi_def_cfa %rcx,0\n\t") + __ASM_CFI(".cfi_rel_offset %rip,0x70\n\t") + __ASM_CFI(".cfi_rel_offset %rsp,0x88\n\t") "2:\tmovl 0x94(%rcx),%edx\n\t" /* frame->restore_flags */ #ifdef __linux__ "testl $12,%r14d\n\t" /* SYSCALL_HAVE_PTHREAD_TEB | SYSCALL_HAVE_WRFSGSBASE */
On 2/8/22 04:05, Rémi Bernon wrote:
Making sure stack pointer points to previous syscall / exit frame before entering a syscall, and restoring the PE frame information on return.
Would this unwind into the PE side? I don't thunk libunwind would be able to handle SEH unwind infos; perhaps what we want to do instead is to unwind into exit_frame?
If what we indeed want is to unwind to PE, I have a much accurate (albeit ugly) version for the CFI expressions.
It supports: - dual fxsave / xsave handling (the bulk of the complexity) - can unwind from every point inside the syscall dispatcher - works with GDB (attach to wine via normal GDB, and it will trace through the PE up to the initial frame)
--- dlls/ntdll/unix/inline_dwarf.h | 59 +++++++++ dlls/ntdll/unix/signal_i386.c | 213 +++++++++++++++++++++++++++++++-- 2 files changed, 260 insertions(+), 12 deletions(-) create mode 100644 dlls/ntdll/unix/inline_dwarf.h
diff --git a/dlls/ntdll/unix/inline_dwarf.h b/dlls/ntdll/unix/inline_dwarf.h new file mode 100644 index 00000000000..0027164a9f3 --- /dev/null +++ b/dlls/ntdll/unix/inline_dwarf.h @@ -0,0 +1,59 @@ +#ifndef __WINE_INLINE_DWARF_H +#define __WINE_INLINE_DWARF_H + +#define PP_ARG_64TH( \ + _1, _2, _3, _4, _5, _6, _7, _8, _9, _10, \ + _11, _12, _13, _14, _15, _16, _17, _18, _19, _20, \ + _21, _22, _23, _24, _25, _26, _27, _28, _29, _30, \ + _31, _32, _33, _34, _35, _36, _37, _38, _39, _40, \ + _41, _42, _43, _44, _45, _46, _47, _48, _49, _50, \ + _51, _52, _53, _54, _55, _56, _57, _58, _59, _60, \ + _61, _62, _63, N, ...) N + +#define PP_NARG(...) PP_ARG_64TH(__VA_ARGS__, \ + 63, 62, 61, 60, \ + 59, 58, 57, 56, 55, 54, 53, 52, 51, 50, \ + 49, 48, 47, 46, 45, 44, 43, 42, 41, 40, \ + 39, 38, 37, 36, 35, 34, 33, 32, 31, 30, \ + 29, 28, 27, 26, 25, 24, 23, 22, 21, 20, \ + 19, 18, 17, 16, 15, 14, 13, 12, 11, 10, \ + 9, 8, 7, 6, 5, 4, 3, 2, 1, 0) + +#define IDW_STRINGIFY_(...) #__VA_ARGS__ +#define IDW_STRINGIFY(...) IDW_STRINGIFY_(__VA_ARGS__) +#define IDW_EXPAND(...) __VA_ARGS__ + +#define I_DW_CFA_expression 0x10 +#define I_DW_CFA_val_expression 0x16 + +#define I_DW_OP_deref 0x06 +#define I_DW_OP_const1u 0x08 +#define I_DW_OP_dup 0x12 +#define I_DW_OP_swap 0x16 +#define I_DW_OP_and 0x1a +#define I_DW_OP_neg 0x1f +#define I_DW_OP_not 0x20 +#define I_DW_OP_or 0x21 +#define I_DW_OP_plus 0x22 +#define I_DW_OP_plus_uconst 0x23 +#define I_DW_OP_bra 0x28 +#define I_DW_OP_eq 0x29 +#define I_DW_OP_lit0 0x30 +#define I_DW_OP_reg0 0x50 +#define I_DW_OP_deref_size 0x94 +#define I_DW_OP_call_frame_cfa 0x9c + +#define I_MAX_DW_OP_LITERAL 0x1f + +#define IDW_const1u(x) I_DW_OP_const1u, x + +#define I_DW_BLOCK(...) \ + IDW_STRINGIFY(PP_NARG(__VA_ARGS__)) ", " IDW_STRINGIFY(__VA_ARGS__) + +#define I_DW_then_else(t, f) \ + I_DW_OP_lit0, I_DW_OP_eq, I_DW_OP_neg, \ + I_DW_OP_dup, IDW_EXPAND f, \ + I_DW_OP_and, I_DW_OP_swap, I_DW_OP_not, IDW_EXPAND t, \ + I_DW_OP_and, I_DW_OP_or + +#endif diff --git a/dlls/ntdll/unix/signal_i386.c b/dlls/ntdll/unix/signal_i386.c index 6bb5649e2b5..3bda41de0c6 100644 --- a/dlls/ntdll/unix/signal_i386.c +++ b/dlls/ntdll/unix/signal_i386.c @@ -2466,6 +2466,133 @@ __ASM_GLOBAL_FUNC( signal_exit_thread, "pushl %eax\n\t" "call *%ecx" )
+#define SYSCALL_FRAME_SYSCALL_FLAGS_OFFSET 0 +#define SYSCALL_FRAME_RESTORE_FLAGS_OFFSET 2 +#define I_SYSCALL_HAVE_FXSAVE_OR_LATER 7 +#define I_CONTEXT_INTEGER 2 + +typedef char syscall_frame_syscall_flags_offset[( + offsetof(struct syscall_frame, syscall_flags) == SYSCALL_FRAME_SYSCALL_FLAGS_OFFSET ? 1 : -1 +)]; +typedef char syscall_frame_restore_flags_offset[( + offsetof(struct syscall_frame, restore_flags) == SYSCALL_FRAME_RESTORE_FLAGS_OFFSET +)]; +typedef char syscall_have_fxsave_or_later[( + I_SYSCALL_HAVE_FXSAVE_OR_LATER == (SYSCALL_HAVE_XSAVE | SYSCALL_HAVE_XSAVEC | + SYSCALL_HAVE_FXSAVE) ? 1 : -1 +)]; +typedef char i_context_integer[(CONTEXT_i386 | I_CONTEXT_INTEGER) == CONTEXT_INTEGER ? 1 : -1]; + +#include "inline_dwarf.h" + +#if SYSCALL_FRAME_SYSCALL_FLAGS_OFFSET != 0 +#error please modify ASM_CFI_XSAVE_OR_FSAVE to add offset before dup +#endif + +#if SYSCALL_FRAME_RESTORE_FLAGS_OFFSET <= 0x7f +#define I_DW_OP_plus_off_restore_flags \ + I_DW_OP_plus_uconst, SYSCALL_FRAME_RESTORE_FLAGS_OFFSET +#else +#error please define I_DW_OP_plus_off_restore_flags to hold syscall_frame::restore_flags offset +#endif + +#if I_SYSCALL_HAVE_FXSAVE_OR_LATER <= I_MAX_DW_OP_LITERAL +#define I_DW_OP_lit_SYSCALL_HAVE_FXSAVE_OR_LATER I_DW_OP_lit0 + I_SYSCALL_HAVE_FXSAVE_OR_LATER +#else +#error please define I_DW_OP_lit_SYSCALL_HAVE_FXSAVE_OR_LATER to hold I_SYSCALL_HAVE_FXSAVE_OR_LATER value +#endif + +#if I_CONTEXT_INTEGER <= I_MAX_DW_OP_LITERAL +#define I_DW_OP_lit_CONTEXT_INTEGER I_DW_OP_lit0 + I_CONTEXT_INTEGER +#else +#error please define I_DW_OP_lit_CONTEXT_INTEGER to hold I_CONTEXT_INTEGER value +#endif + +#define ASM_CFI_XSAVE_OR_FSAVE(reg_seq, off_xsave, off_fsave) \ + ".cfi_escape " IDW_STRINGIFY(I_DW_CFA_expression) ", " reg_seq ", " \ + I_DW_BLOCK( \ + I_DW_OP_dup, I_DW_OP_deref_size, 0x02, \ + I_DW_OP_lit_SYSCALL_HAVE_FXSAVE_OR_LATER, I_DW_OP_and, \ + I_DW_then_else((off_xsave), (off_fsave)), \ + I_DW_OP_plus \ + ) + +#define ASM_CFI_SYSCALL_FRAME_FLTSAVE() \ + ASM_CFI_XSAVE_OR_FSAVE("11" /* ST0 */, IDW_const1u(0x60), IDW_const1u(0x5c)) "\n\t" \ + ASM_CFI_XSAVE_OR_FSAVE("12" /* ST1 */, IDW_const1u(0x70), IDW_const1u(0x66)) "\n\t" \ + ASM_CFI_XSAVE_OR_FSAVE("13" /* ST2 */, IDW_const1u(0x80), IDW_const1u(0x70)) "\n\t" \ + ASM_CFI_XSAVE_OR_FSAVE("14" /* ST3 */, IDW_const1u(0x90), IDW_const1u(0x7a)) "\n\t" \ + ASM_CFI_XSAVE_OR_FSAVE("15" /* ST4 */, IDW_const1u(0xa0), IDW_const1u(0x84)) "\n\t" \ + ASM_CFI_XSAVE_OR_FSAVE("16" /* ST5 */, IDW_const1u(0xb0), IDW_const1u(0x8e)) "\n\t" \ + ASM_CFI_XSAVE_OR_FSAVE("17" /* ST6 */, IDW_const1u(0xc0), IDW_const1u(0x98)) "\n\t" \ + ASM_CFI_XSAVE_OR_FSAVE("18" /* ST7 */, IDW_const1u(0xd0), IDW_const1u(0xa2)) "\n\t" \ + ".cfi_offset %xmm0,0xe0\n\t" \ + ".cfi_offset %xmm1,0xf0\n\t" \ + ".cfi_offset %xmm2,0x100\n\t" \ + ".cfi_offset %xmm3,0x110\n\t" \ + ".cfi_offset %xmm4,0x120\n\t" \ + ".cfi_offset %xmm5,0x130\n\t" \ + ".cfi_offset %xmm6,0x140\n\t" \ + ".cfi_offset %xmm7,0x150\n\t" \ + ".cfi_offset 39,0x18" /* MXCSR */ + +#define ASM_CFI_FLT_SAME_VALUE() \ + ".cfi_same_value 11\n\t" /* ST0 */ \ + ".cfi_same_value 12\n\t" /* ST1 */ \ + ".cfi_same_value 13\n\t" /* ST2 */ \ + ".cfi_same_value 14\n\t" /* ST3 */ \ + ".cfi_same_value 15\n\t" /* ST4 */ \ + ".cfi_same_value 16\n\t" /* ST5 */ \ + ".cfi_same_value 17\n\t" /* ST6 */ \ + ".cfi_same_value 18\n\t" /* ST7 */ \ + ".cfi_same_value %xmm0\n\t" \ + ".cfi_same_value %xmm1\n\t" \ + ".cfi_same_value %xmm2\n\t" \ + ".cfi_same_value %xmm3\n\t" \ + ".cfi_same_value %xmm4\n\t" \ + ".cfi_same_value %xmm5\n\t" \ + ".cfi_same_value %xmm6\n\t" \ + ".cfi_same_value %xmm7\n\t" \ + ".cfi_same_value 39" /* MXCSR */ + +#define SYSCALL_DISPATCHER_POSTPROLOG_CFI(basereg) \ + ".cfi_signal_frame\n\t" \ + ".cfi_def_cfa " basereg ",0\n\t" \ + ".cfi_offset %eip,0x08\n\t" \ + ".cfi_offset %eflags,0x04\n\t" + +#define SYSCALL_DISPATCHER_FOREACH_SAVE_REG(func, basereg) \ + func(l, "%esp", 4, 0x0c, basereg) /* frame->esp */ \ + func(w, "%cs" , 41, 0x10, basereg) \ + func(w, "%ss" , 42, 0x12, basereg) \ + func(w, "%ds" , 43, 0x14, basereg) \ + func(w, "%es" , 40, 0x16, basereg) \ + func(w, "%fs" , 44, 0x18, basereg) \ + func(w, "%gs" , 45, 0x1a, basereg) \ + func(l, "%eax", 0, 0x1c, basereg) \ + func(l, "%ebx", 3, 0x20, basereg) \ + func(l, "%edi", 7, 0x2c, basereg) \ + func(l, "%esi", 6, 0x30, basereg) \ + func(l, "%ebp", 5, 0x34, basereg) \ + +#define SAVE_REG_CFI_l(reg, regno, off, basereg) \ + __ASM_CFI(".cfi_offset " #regno "," #off "\n\t") + +#define SAVE_REG_CFI_w(reg, regno, off, basereg) \ + __ASM_CFI(".cfi_escape " IDW_STRINGIFY(I_DW_CFA_val_expression) ", " #regno ", " \ + I_DW_BLOCK(I_DW_OP_plus_uconst, off, I_DW_OP_deref_size, 0x02) "\n\t") \ + +#define SAVE_REG_AND_CFI(prefix, reg, regno, off, basereg) \ + "mov" #prefix " " reg "," #off "(" basereg ")\n\t" \ + SAVE_REG_CFI_##prefix(reg, regno, off, basereg) + +#define SAVE_REG_CFI_ONLY(prefix, reg, regno, off, basereg) \ + SAVE_REG_CFI_##prefix(reg, regno, off, basereg) + +#define SYSCALL_DISPATCHER_OTHER_START(basereg) \ + __ASM_CFI(SYSCALL_DISPATCHER_POSTPROLOG_CFI(basereg)) \ + SYSCALL_DISPATCHER_FOREACH_SAVE_REG(SAVE_REG_CFI_ONLY, basereg) \ + __ASM_CFI(ASM_CFI_SYSCALL_FRAME_FLTSAVE())
/*********************************************************************** * __wine_syscall_dispatcher @@ -2474,22 +2601,20 @@ __ASM_GLOBAL_FUNC( __wine_syscall_dispatcher, "movl %fs:0x1f8,%ecx\n\t" /* x86_thread_data()->syscall_frame */ "movw $0,0x02(%ecx)\n\t" /* frame->restore_flags */ "popl 0x08(%ecx)\n\t" /* frame->eip */ + __ASM_CFI(".cfi_adjust_cfa_offset -4\n\t") + __ASM_CFI(".cfi_escape 0x10,0x08,0x02,0x71,0x08\n\t") /* eip, DW_op_breg1 + 0x08 */ "pushfl\n\t" + __ASM_CFI(".cfi_adjust_cfa_offset 4\n\t") + __ASM_CFI(".cfi_rel_offset %eflags,0\n\t") "popl 0x04(%ecx)\n" /* frame->eflags */ + __ASM_CFI("\t.cfi_endproc\n") __ASM_NAME("__wine_syscall_dispatcher_prolog_end") ":\n\t" - "movl %esp,0x0c(%ecx)\n\t" /* frame->esp */ - "movw %cs,0x10(%ecx)\n\t" - "movw %ss,0x12(%ecx)\n\t" - "movw %ds,0x14(%ecx)\n\t" - "movw %es,0x16(%ecx)\n\t" - "movw %fs,0x18(%ecx)\n\t" - "movw %gs,0x1a(%ecx)\n\t" - "movl %eax,0x1c(%ecx)\n\t" - "movl %ebx,0x20(%ecx)\n\t" - "movl %edi,0x2c(%ecx)\n\t" - "movl %esi,0x30(%ecx)\n\t" - "movl %ebp,0x34(%ecx)\n\t" + __ASM_CFI(".cfi_startproc simple\n\t") /* don't emit default frame instructions */ + __ASM_CFI(SYSCALL_DISPATCHER_POSTPROLOG_CFI("%ecx")) + __ASM_CFI(".cfi_same_value %esp\n\t") + SYSCALL_DISPATCHER_FOREACH_SAVE_REG(SAVE_REG_AND_CFI, "%ecx") "leal 0x34(%ecx),%ebp\n\t" + __ASM_CFI(".cfi_def_cfa %ebp, -0x34\n\t") "leal 4(%esp),%esi\n\t" /* first argument */ "movl %eax,%ebx\n\t" "shrl $8,%ebx\n\t" @@ -2518,14 +2643,24 @@ __ASM_GLOBAL_FUNC( __wine_syscall_dispatcher, "movl %edx,0x278(%ecx)\n\t" "movl %edx,0x27c(%ecx)\n\t" "xsavec 0x40(%ecx)\n\t" + __ASM_CFI(ASM_CFI_SYSCALL_FRAME_FLTSAVE() "\n\t") "jmp 4f\n" + __ASM_CFI("\t.cfi_remember_state\n") + __ASM_CFI("\t" ASM_CFI_FLT_SAME_VALUE() "\n") "1:\txsave 0x40(%ecx)\n\t" + __ASM_CFI(".cfi_restore_state\n\t") "jmp 4f\n" + __ASM_CFI("\t.cfi_remember_state\n") + __ASM_CFI("\t" ASM_CFI_FLT_SAME_VALUE() "\n") "2:\ttestl $4,(%ecx)\n\t" /* frame->syscall_flags & SYSCALL_HAVE_FXSAVE */ "jz 3f\n\t" "fxsave 0x40(%ecx)\n\t" + __ASM_CFI(".cfi_restore_state\n\t") "jmp 4f\n" + __ASM_CFI("\t.cfi_remember_state\n") + __ASM_CFI("\t" ASM_CFI_FLT_SAME_VALUE() "\n") "3:\tfnsave 0x40(%ecx)\n\t" + __ASM_CFI(".cfi_restore_state\n\t") "fwait\n" "4:\tmovl %ecx,%esp\n\t" "movl 0x1c(%esp),%edx\n\t" /* frame->eax */ @@ -2543,6 +2678,7 @@ __ASM_GLOBAL_FUNC( __wine_syscall_dispatcher, "rep; movsl\n\t" "call *(%eax,%edx,4)\n\t" "leal -0x34(%ebp),%esp\n" + __ASM_CFI("\t.cfi_def_cfa %esp, 0\n") "5:\tmovl 0(%esp),%ecx\n\t" /* frame->syscall_flags + (frame->restore_flags << 16) */ "testl $0x68 << 16,%ecx\n\t" /* CONTEXT_FLOATING_POINT | CONTEXT_EXTENDED_REGISTERS | CONTEXT_XSAVE */ "jz 3f\n\t" @@ -2552,46 +2688,99 @@ __ASM_GLOBAL_FUNC( __wine_syscall_dispatcher, "movl $7,%eax\n\t" "xorl %edx,%edx\n\t" "xrstor 0x40(%esp)\n\t" + __ASM_CFI(".cfi_remember_state\n\t") + __ASM_CFI(ASM_CFI_FLT_SAME_VALUE() "\n\t") "movl %esi,%eax\n\t" "jmp 3f\n" + __ASM_CFI("\t.cfi_restore_state\n") "1:\ttestl $4,%ecx\n\t" /* SYSCALL_HAVE_FXSAVE */ "jz 2f\n\t" "fxrstor 0x40(%esp)\n\t" + __ASM_CFI(".cfi_remember_state\n\t") + __ASM_CFI(ASM_CFI_FLT_SAME_VALUE() "\n\t") "jmp 3f\n" + __ASM_CFI("\t.cfi_restore_state\n") "2:\tfrstor 0x40(%esp)\n\t" + __ASM_CFI(ASM_CFI_FLT_SAME_VALUE() "\n\t") "fwait\n" "3:\tmovl 0x2c(%esp),%edi\n\t" + __ASM_CFI(".cfi_same_value %edi\n\t") "movl 0x30(%esp),%esi\n\t" + __ASM_CFI(".cfi_same_value %esi\n\t") "movl 0x34(%esp),%ebp\n\t" + __ASM_CFI(".cfi_same_value %ebp\n\t") "testl $0x7 << 16,%ecx\n\t" /* CONTEXT_CONTROL | CONTEXT_SEGMENTS | CONTEXT_INTEGER */ "jnz 1f\n\t" "movl 0x20(%esp),%ebx\n\t" + __ASM_CFI(".cfi_remember_state\n\t") + __ASM_CFI(".cfi_same_value %ebx\n\t") "movl 0x08(%esp),%ecx\n\t" /* frame->eip */ + __ASM_CFI(".cfi_register %eip, %ecx\n\t") "movl 0x0c(%esp),%esp\n\t" /* frame->esp */ + __ASM_CFI(".cfi_same_value %esp\n\t") "jmpl *%ecx\n" + __ASM_CFI("\t.cfi_restore_state\n") "1:\ttestl $0x2 << 16,%ecx\n\t" /* CONTEXT_INTEGER */ "jz 1f\n\t" "movl 0x1c(%esp),%eax\n\t" + __ASM_CFI(".cfi_same_value %eax\n\t") "movl 0x24(%esp),%ecx\n\t" + __ASM_CFI(".cfi_same_value %ecx\n\t") "movl 0x28(%esp),%edx\n" + __ASM_CFI("\t.cfi_same_value %edx\n") "1:\tmovl 0x0c(%esp),%ebx\n\t" /* frame->esp */ + __ASM_CFI(".cfi_register %esp, %ebx\n\t") "movw 0x12(%esp),%ss\n\t" + __ASM_CFI(".cfi_same_value %ss\n\t") "xchgl %ebx,%esp\n\t" + __ASM_CFI(".cfi_endproc\n\t") + __ASM_CFI(".cfi_startproc simple\n\t") + __ASM_CFI(".cfi_def_cfa %esp,0\n\t") + __ASM_CFI(".cfi_escape 0x10,0x09,0x02,0x71,0x04\n\t") /* eflags, DW_op_breg1 + 0x04 */ + __ASM_CFI(".cfi_escape 0x10,0x29,0x02,0x71,0x10\n\t") /* cs, DW_op_breg1 + 0x10 */ + __ASM_CFI(".cfi_escape 0x10,0x08,0x02,0x71,0x08\n\t") /* eip, DW_op_breg1 + 0x08 */ + __ASM_CFI(".cfi_escape 0x10,0x2b,0x02,0x71,0x14\n\t") /* ds, DW_op_breg1 + 0x14 */ + __ASM_CFI(".cfi_escape 0x10,0x28,0x02,0x71,0x16\n\t") /* es, DW_op_breg1 + 0x16 */ + __ASM_CFI(".cfi_escape 0x10,0x2c,0x02,0x71,0x18\n\t") /* fs, DW_op_breg1 + 0x18 */ + __ASM_CFI(".cfi_escape 0x10,0x2d,0x02,0x71,0x1a\n\t") /* gs, DW_op_breg1 + 0x1a */ + __ASM_CFI(".cfi_escape 0x10,0x03,0x02,0x71,0x20\n\t") /* ebx, DW_op_breg1 + 0x20 */ "pushl 0x04(%ebx)\n\t" /* frame->eflags */ + __ASM_CFI(".cfi_adjust_cfa_offset 4\n\t") + __ASM_CFI(".cfi_rel_offset %eflags, 0\n\t") "pushl 0x10(%ebx)\n\t" /* frame->cs */ + __ASM_CFI(".cfi_adjust_cfa_offset 4\n\t") + __ASM_CFI(".cfi_rel_offset %cs, 0\n\t") "pushl 0x08(%ebx)\n\t" /* frame->eip */ + __ASM_CFI(".cfi_adjust_cfa_offset 4\n\t") + __ASM_CFI(".cfi_rel_offset %eip, 0\n\t") "pushl 0x14(%ebx)\n\t" /* frame->ds */ + __ASM_CFI(".cfi_adjust_cfa_offset 4\n\t") + __ASM_CFI(".cfi_rel_offset %ds, 0\n\t") "movw 0x16(%ebx),%es\n\t" + __ASM_CFI(".cfi_same_value %es\n\t") "movw 0x18(%ebx),%fs\n\t" + __ASM_CFI(".cfi_same_value %fs\n\t") "movw 0x1a(%ebx),%gs\n\t" + __ASM_CFI(".cfi_same_value %gs\n\t") "movl 0x20(%ebx),%ebx\n\t" + __ASM_CFI(".cfi_same_value %ebx\n\t") "popl %ds\n\t" + __ASM_CFI(".cfi_same_value %ds\n\t") + __ASM_CFI(".cfi_adjust_cfa_offset -4\n\t") "iret\n" + __ASM_CFI("\t.cfi_endproc\n") + __ASM_CFI("\t.cfi_startproc simple\n") + "\t" SYSCALL_DISPATCHER_OTHER_START("%esp") "\n" "6:\tmovl $0xc000000d,%eax\n\t" /* STATUS_INVALID_PARAMETER */ "jmp 5b\n" + __ASM_CFI("\t.cfi_endproc\n") __ASM_NAME("__wine_syscall_dispatcher_return") ":\n\t" + __ASM_CFI(".cfi_startproc\n\t") "movl 8(%esp),%eax\n\t" "movl 4(%esp),%esp\n\t" + __ASM_CFI(".cfi_endproc\n\t") + __ASM_CFI(".cfi_startproc simple\n\t") + SYSCALL_DISPATCHER_OTHER_START("%esp") "\n\t" "jmp 5b" )
On 2/9/22 14:29, Jinoh Kang wrote:
On 2/8/22 04:05, Rémi Bernon wrote:
Making sure stack pointer points to previous syscall / exit frame before entering a syscall, and restoring the PE frame information on return.
Would this unwind into the PE side? I don't thunk libunwind would be able to handle SEH unwind infos; perhaps what we want to do instead is to unwind into exit_frame?
If what we indeed want is to unwind to PE, I have a much accurate (albeit ugly) version for the CFI expressions.
As far as I understand and from Alexandre feedback we don't want to let pthread / libunwind unwind the PE frames at all.
As we current don't care too much what happens on thread exit (I'm not sure what is supposed to happen on the PE side of the threads), I'm only trying to unwind the unix frames here.
So, the syscall frames, any unix-side call frames that could be there, and the eventual nested syscall frames, up to the exit frame.
It supports:
- dual fxsave / xsave handling (the bulk of the complexity)
- can unwind from every point inside the syscall dispatcher
- works with GDB (attach to wine via normal GDB, and it will trace through the PE up to the initial frame)
That's nice, I've been able to have GDB cross the syscall dispatcher with a much simpler version, basically doing the same as here but pointing to the PE frame instead.
Then maybe it doesn't work on every instruction of the dispatcher, and it also isn't compatible with this patch ofc.
Cheers,
On 2/8/22 04:05, Rémi Bernon wrote:
Making sure stack pointer points to previous syscall / exit frame before entering a syscall, and restoring the PE frame information on return.
Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=52213 Signed-off-by: Rémi Bernon rbernon@codeweavers.com
dlls/ntdll/unix/signal_i386.c | 9 +++++++++ dlls/ntdll/unix/signal_x86_64.c | 9 +++++++++ 2 files changed, 18 insertions(+)
diff --git a/dlls/ntdll/unix/signal_i386.c b/dlls/ntdll/unix/signal_i386.c index d98a3b1d4bb..2f6e2fd4153 100644 --- a/dlls/ntdll/unix/signal_i386.c +++ b/dlls/ntdll/unix/signal_i386.c @@ -2492,6 +2492,9 @@ __ASM_GLOBAL_FUNC( __wine_syscall_dispatcher, "movl %esi,0x30(%ecx)\n\t" "movl %ebp,0x34(%ecx)\n\t" "leal 0x34(%ecx),%ebp\n\t"
__ASM_CFI(".cfi_def_cfa %ebp,0\n\t")
This changes the value of CFA. By definition, the actual value of CFA (not the CFA register) may never change within the context of a subroutine activation [1]. If we desire to switch CFA to a different frame anyway (with EIP overriden), we must end the current FDE with ".cfi_endproc" and start another FDE with ".cfi_startproc simple". See [2] and [3] for how glibc achieves this.
__ASM_CFI(".cfi_rel_offset %eip,-0x2c\n\t")
This is the system call return address, which would be in a PE module. I don't think this is very useful, since we will later switch to the exit frame anyway.
__ASM_CFI(".cfi_rel_offset %esp,-0x28\n\t")
This makes GDB unhappy: "previous frame inner to this frame (corrupt stack)?" [4]. This is caused by jumping from one stack (the thread stack) to another (the kernel/syscall stack). This usually never happens unless we're calling __morestack or handling a signal. The solution would be to have .cfi_signal_frame at the start of FDE; not sure if it is worth it...
"leal 4(%esp),%esi\n\t" /* first argument */ "movl %eax,%ebx\n\t" "shrl $8,%ebx\n\t"@@ -2530,6 +2533,9 @@ __ASM_GLOBAL_FUNC( __wine_syscall_dispatcher, "3:\tfnsave 0x40(%ecx)\n\t" "fwait\n" "4:\tmovl %ecx,%esp\n\t"
__ASM_CFI(".cfi_def_cfa %ebp,0\n\t")
This is unnecessary sine we have already configured the CFA expression previously (unless we're going to start another FDE, of course). Also, changing SP does not mean it is an unwind pivot by itself.
__ASM_CFI(".cfi_rel_offset %eip,0x34c\n\t") /* frame->unwind_rip */__ASM_CFI(".cfi_rel_offset %esp,0x08\n\t") /* frame->prev_frame */ "movl 0x1c(%esp),%edx\n\t" /* frame->eax */ "andl $0xfff,%edx\n\t" /* syscall number */ "cmpl 8(%ebx),%edx\n\t" /* table->ServiceLimit */@@ -2545,6 +2551,9 @@ __ASM_GLOBAL_FUNC( __wine_syscall_dispatcher, "rep; movsl\n\t" "call *(%eax,%edx,4)\n\t" "leal -0x34(%ebp),%esp\n"
__ASM_CFI(".cfi_def_cfa %ebp,0\n\t")
Ditto.
__ASM_CFI(".cfi_rel_offset %eip,-0x2c\n\t")__ASM_CFI(".cfi_rel_offset %esp,-0x28\n\t") "5:\tmovl 0(%esp),%ecx\n\t" /* frame->syscall_flags + (frame->restore_flags << 16) */ "testl $0x68 << 16,%ecx\n\t" /* CONTEXT_FLOATING_POINT | CONTEXT_EXTENDED_REGISTERS | CONTEXT_XSAVE */ "jz 3f\n\t"diff --git a/dlls/ntdll/unix/signal_x86_64.c b/dlls/ntdll/unix/signal_x86_64.c index f8cddd15569..ee2723cdb24 100644 --- a/dlls/ntdll/unix/signal_x86_64.c +++ b/dlls/ntdll/unix/signal_x86_64.c @@ -3166,6 +3166,9 @@ __ASM_GLOBAL_FUNC( __wine_syscall_dispatcher, "movw %ss,0x90(%rcx)\n\t" "movw %gs,0x92(%rcx)\n\t" "movq %rbp,0x98(%rcx)\n\t"
__ASM_CFI(".cfi_def_cfa %rcx,0\n\t")__ASM_CFI(".cfi_rel_offset %rip,0x70\n\t")__ASM_CFI(".cfi_rel_offset %rsp,0x88\n\t")
Ditto for x86_64.
/* Legends of Runeterra hooks the first system call return instruction, and * depends on us returning to it. Adjust the return address accordingly. */ "subq $0xb,0x70(%rcx)\n\t"@@ -3206,6 +3209,9 @@ __ASM_GLOBAL_FUNC( __wine_syscall_dispatcher, #endif "leaq 0x28(%rsp),%rsi\n\t" /* first argument */ "movq %rcx,%rsp\n\t"
__ASM_CFI(".cfi_def_cfa %rbp,0\n\t")__ASM_CFI(".cfi_rel_offset %rip,0x20\n\t") /* frame->unwind_rip */__ASM_CFI(".cfi_rel_offset %rsp,0x08\n\t") /* frame->prev_frame */ "movq 0x00(%rcx),%rax\n\t" "movq 0x18(%rcx),%rdx\n\t" "movl %eax,%ebx\n\t"@@ -3231,6 +3237,9 @@ __ASM_GLOBAL_FUNC( __wine_syscall_dispatcher, "movq (%rbx),%r10\n\t" /* table->ServiceTable */ "callq *(%r10,%rax,8)\n\t" "leaq -0x98(%rbp),%rcx\n"
__ASM_CFI(".cfi_def_cfa %rcx,0\n\t")__ASM_CFI(".cfi_rel_offset %rip,0x70\n\t")__ASM_CFI(".cfi_rel_offset %rsp,0x88\n\t") "2:\tmovl 0x94(%rcx),%edx\n\t" /* frame->restore_flags */#ifdef __linux__ "testl $12,%r14d\n\t" /* SYSCALL_HAVE_PTHREAD_TEB | SYSCALL_HAVE_WRFSGSBASE */
Also, it might be a good idea to apply this to the ARM side as well (although I'm not sure).
[1] DWARF Debugging Information Format Version 5, page 172, line 4 (§6.4 Call Frame Information), https://dwarfstd.org/doc/DWARF5.pdf ("(By definition, the CFA value does not change.)") [2] https://elixir.bootlin.com/glibc/glibc-2.35.9000/source/sysdeps/unix/sysv/li... [3] https://elixir.bootlin.com/glibc/glibc-2.35.9000/source/sysdeps/unix/sysv/li...
On 2/10/22 01:00, Jinoh Kang wrote:
On 2/8/22 04:05, Rémi Bernon wrote:
Making sure stack pointer points to previous syscall / exit frame before entering a syscall, and restoring the PE frame information on return.
Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=52213 Signed-off-by: Rémi Bernon rbernon@codeweavers.com
dlls/ntdll/unix/signal_i386.c | 9 +++++++++ dlls/ntdll/unix/signal_x86_64.c | 9 +++++++++ 2 files changed, 18 insertions(+)
diff --git a/dlls/ntdll/unix/signal_i386.c b/dlls/ntdll/unix/signal_i386.c index d98a3b1d4bb..2f6e2fd4153 100644 --- a/dlls/ntdll/unix/signal_i386.c +++ b/dlls/ntdll/unix/signal_i386.c @@ -2492,6 +2492,9 @@ __ASM_GLOBAL_FUNC( __wine_syscall_dispatcher, "movl %esi,0x30(%ecx)\n\t" "movl %ebp,0x34(%ecx)\n\t" "leal 0x34(%ecx),%ebp\n\t"
__ASM_CFI(".cfi_def_cfa %ebp,0\n\t")This changes the value of CFA. By definition, the actual value of CFA (not the CFA register) may never change within the context of a subroutine activation [1]. If we desire to switch CFA to a different frame anyway (with EIP overriden), we must end the current FDE with ".cfi_endproc" and start another FDE with ".cfi_startproc simple". See [2] and [3] for how glibc achieves this.
__ASM_CFI(".cfi_rel_offset %eip,-0x2c\n\t")This is the system call return address, which would be in a PE module. I don't think this is very useful, since we will later switch to the exit frame anyway.
__ASM_CFI(".cfi_rel_offset %esp,-0x28\n\t")This makes GDB unhappy: "previous frame inner to this frame (corrupt stack)?" [4].
Oops, I forgot the link. It's at https://stackoverflow.com/questions/52518857/stackful-coroutines-gdb-previou.... My apologies.
On 2/9/22 17:00, Jinoh Kang wrote:
On 2/8/22 04:05, Rémi Bernon wrote:
Making sure stack pointer points to previous syscall / exit frame before entering a syscall, and restoring the PE frame information on return.
Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=52213 Signed-off-by: Rémi Bernon rbernon@codeweavers.com
dlls/ntdll/unix/signal_i386.c | 9 +++++++++ dlls/ntdll/unix/signal_x86_64.c | 9 +++++++++ 2 files changed, 18 insertions(+)
diff --git a/dlls/ntdll/unix/signal_i386.c b/dlls/ntdll/unix/signal_i386.c index d98a3b1d4bb..2f6e2fd4153 100644 --- a/dlls/ntdll/unix/signal_i386.c +++ b/dlls/ntdll/unix/signal_i386.c @@ -2492,6 +2492,9 @@ __ASM_GLOBAL_FUNC( __wine_syscall_dispatcher, "movl %esi,0x30(%ecx)\n\t" "movl %ebp,0x34(%ecx)\n\t" "leal 0x34(%ecx),%ebp\n\t"
__ASM_CFI(".cfi_def_cfa %ebp,0\n\t")This changes the value of CFA. By definition, the actual value of CFA (not the CFA register) may never change within the context of a subroutine activation [1]. If we desire to switch CFA to a different frame anyway (with EIP overriden), we must end the current FDE with ".cfi_endproc" and start another FDE with ".cfi_startproc simple". See [2] and [3] for how glibc achieves this.
__ASM_CFI(".cfi_rel_offset %eip,-0x2c\n\t")This is the system call return address, which would be in a PE module. I don't think this is very useful, since we will later switch to the exit frame anyway.
Yes, I added this here only because there's the same thing after the stack change and call, right after we restore the stack pointer. The latter is needed I think, as I understand the CFI are valid from their location up to the next cfi_endproc, and we want to restore the PE unwinding info as soon as we're off the unix stack.
I guess then this should instead be done in the same way as glibc does for it to be correct, with cfi_endproc + cfi_startproc inserted on the boundaries where we're switching stacks.
__ASM_CFI(".cfi_rel_offset %esp,-0x28\n\t")This makes GDB unhappy: "previous frame inner to this frame (corrupt stack)?" [4]. This is caused by jumping from one stack (the thread stack) to another (the kernel/syscall stack). This usually never happens unless we're calling __morestack or handling a signal. The solution would be to have .cfi_signal_frame at the start of FDE; not sure if it is worth it...
Yes, GDB isn't happy about the syscall frame being above the PE stack, it then believes they are on the same stack but that the syscall frame frame is inner. For some reason, it now manages to unwind in my local setup, though I may just have hacked out the heuristic in a local GDB build because it was annoying.
In any case if we manage to link the unix frames together it should then happily unwind the syscall frames up to the exit frame and skip all the PE ones. It is a bit unfortunate for debugging purposes, but there's not much to do about it, and this will fix this error.
/* Legends of Runeterra hooks the first system call return instruction, and * depends on us returning to it. Adjust the return address accordingly. */ "subq $0xb,0x70(%rcx)\n\t"@@ -3206,6 +3209,9 @@ __ASM_GLOBAL_FUNC( __wine_syscall_dispatcher, #endif "leaq 0x28(%rsp),%rsi\n\t" /* first argument */ "movq %rcx,%rsp\n\t"
__ASM_CFI(".cfi_def_cfa %rbp,0\n\t")__ASM_CFI(".cfi_rel_offset %rip,0x20\n\t") /* frame->unwind_rip */__ASM_CFI(".cfi_rel_offset %rsp,0x08\n\t") /* frame->prev_frame */ "movq 0x00(%rcx),%rax\n\t" "movq 0x18(%rcx),%rdx\n\t" "movl %eax,%ebx\n\t"@@ -3231,6 +3237,9 @@ __ASM_GLOBAL_FUNC( __wine_syscall_dispatcher, "movq (%rbx),%r10\n\t" /* table->ServiceTable */ "callq *(%r10,%rax,8)\n\t" "leaq -0x98(%rbp),%rcx\n"
__ASM_CFI(".cfi_def_cfa %rcx,0\n\t")__ASM_CFI(".cfi_rel_offset %rip,0x70\n\t")__ASM_CFI(".cfi_rel_offset %rsp,0x88\n\t") "2:\tmovl 0x94(%rcx),%edx\n\t" /* frame->restore_flags */Also, it might be a good idea to apply this to the ARM side as well (although I'm not sure).
Yeah, I didn't do it because I have no idea which registers are needed there.
[1] DWARF Debugging Information Format Version 5, page 172, line 4 (§6.4 Call Frame Information), https://dwarfstd.org/doc/DWARF5.pdf ("(By definition, the CFA value does not change.)") [2] https://elixir.bootlin.com/glibc/glibc-2.35.9000/source/sysdeps/unix/sysv/li... [3] https://elixir.bootlin.com/glibc/glibc-2.35.9000/source/sysdeps/unix/sysv/li...
Thanks for the help!
On Thu, Feb 10, 2022, 2:54 AM Rémi Bernon rbernon@codeweavers.com wrote:
On 2/9/22 17:00, Jinoh Kang wrote:
On 2/8/22 04:05, Rémi Bernon wrote:
Making sure stack pointer points to previous syscall / exit frame before entering a syscall, and restoring the PE frame information on return.
Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=52213 Signed-off-by: Rémi Bernon rbernon@codeweavers.com
dlls/ntdll/unix/signal_i386.c | 9 +++++++++ dlls/ntdll/unix/signal_x86_64.c | 9 +++++++++ 2 files changed, 18 insertions(+)
diff --git a/dlls/ntdll/unix/signal_i386.c
b/dlls/ntdll/unix/signal_i386.c
index d98a3b1d4bb..2f6e2fd4153 100644 --- a/dlls/ntdll/unix/signal_i386.c +++ b/dlls/ntdll/unix/signal_i386.c @@ -2492,6 +2492,9 @@ __ASM_GLOBAL_FUNC( __wine_syscall_dispatcher, "movl %esi,0x30(%ecx)\n\t" "movl %ebp,0x34(%ecx)\n\t" "leal 0x34(%ecx),%ebp\n\t"
__ASM_CFI(".cfi_def_cfa %ebp,0\n\t")This changes the value of CFA. By definition, the actual value of CFA
(not the CFA register) may never change within the context of a subroutine activation [1].
If we desire to switch CFA to a different frame anyway (with EIP
overriden), we must end the current FDE with ".cfi_endproc" and start another FDE with ".cfi_startproc simple".
See [2] and [3] for how glibc achieves this.
__ASM_CFI(".cfi_rel_offset %eip,-0x2c\n\t")This is the system call return address, which would be in a PE module. I don't think this is very useful, since we will later switch to the
exit frame anyway.
Yes, I added this here only because there's the same thing after the stack change and call, right after we restore the stack pointer. The latter is needed I think, as I understand the CFI are valid from their location up to the next cfi_endproc, and we want to restore the PE unwinding info as soon as we're off the unix stack.
I think what we want to do here is to mark the end of the call stack with ".cfi_undefined %eip". If we do want to return to the PE side, we may as well restore all the registers; a partial (not full) unwind would just make debugging things harder.
I guess then this should instead be done in the same way as glibc does for it to be correct, with cfi_endproc + cfi_startproc inserted on the boundaries where we're switching stacks.
Sounds great.
Also, please keep in mind that glibc places its CFI pivot points after SP switch only because the old stack is no longer valid (clone) or needed (setcontext); as long as the CFA expression resolves to the same address value and the current frame is valid, it's ok to leave the CFI state unchanged.
__ASM_CFI(".cfi_rel_offset %esp,-0x28\n\t")This makes GDB unhappy: "previous frame inner to this frame (corrupt
stack)?" [4].
This is caused by jumping from one stack (the thread stack) to another
(the kernel/syscall stack). This usually never happens unless we're calling __morestack or handling a signal.
The solution would be to have .cfi_signal_frame at the start of FDE; not
sure if it is worth it...
Yes, GDB isn't happy about the syscall frame being above the PE stack, it then believes they are on the same stack but that the syscall frame frame is inner. For some reason, it now manages to unwind in my local setup, though I may just have hacked out the heuristic in a local GDB build because it was annoying.
In any case if we manage to link the unix frames together it should then happily unwind the syscall frames up to the exit frame and skip all the PE ones. It is a bit unfortunate for debugging purposes, but there's not much to do about it, and this will fix this error.
For debugging purposes I think the user can find some way to inject dynamic unwind info, so that shouldn't be a concern.
/* Legends of Runeterra hooks the first systemcall return instruction, and
* depends on us returning to it. Adjust thereturn address accordingly. */
"subq $0xb,0x70(%rcx)\n\t"@@ -3206,6 +3209,9 @@ __ASM_GLOBAL_FUNC( __wine_syscall_dispatcher, #endif "leaq 0x28(%rsp),%rsi\n\t" /* first argument
*/
"movq %rcx,%rsp\n\t"
__ASM_CFI(".cfi_def_cfa %rbp,0\n\t")__ASM_CFI(".cfi_rel_offset %rip,0x20\n\t") /*frame->unwind_rip */
__ASM_CFI(".cfi_rel_offset %rsp,0x08\n\t") /*frame->prev_frame */
"movq 0x00(%rcx),%rax\n\t" "movq 0x18(%rcx),%rdx\n\t" "movl %eax,%ebx\n\t"@@ -3231,6 +3237,9 @@ __ASM_GLOBAL_FUNC( __wine_syscall_dispatcher, "movq (%rbx),%r10\n\t" /*
table->ServiceTable */
"callq *(%r10,%rax,8)\n\t" "leaq -0x98(%rbp),%rcx\n"
__ASM_CFI(".cfi_def_cfa %rcx,0\n\t")__ASM_CFI(".cfi_rel_offset %rip,0x70\n\t")__ASM_CFI(".cfi_rel_offset %rsp,0x88\n\t") "2:\tmovl 0x94(%rcx),%edx\n\t" /*frame->restore_flags */
#ifdef __linux__ "testl $12,%r14d\n\t" /*
SYSCALL_HAVE_PTHREAD_TEB | SYSCALL_HAVE_WRFSGSBASE */
Also, it might be a good idea to apply this to the ARM side as well
(although I'm not sure).
Yeah, I didn't do it because I have no idea which registers are needed there.
I suppose we can start with SP and LR.
[1] DWARF Debugging Information Format Version 5, page 172, line 4 (§6.4
Call Frame Information), https://dwarfstd.org/doc/DWARF5.pdf ("(By definition, the CFA value does not change.)")
[2]
https://elixir.bootlin.com/glibc/glibc-2.35.9000/source/sysdeps/unix/sysv/li...
[3]
https://elixir.bootlin.com/glibc/glibc-2.35.9000/source/sysdeps/unix/sysv/li...
Thanks for the help!
Rémi Bernon rbernon@codeweavers.com
Let pthread_exit unwind the syscall and exit frames properly, and call its pthread_cleanup handlers.
Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=52213 Signed-off-by: Rémi Bernon rbernon@codeweavers.com --- dlls/ntdll/unix/signal_i386.c | 19 +------------------ dlls/ntdll/unix/signal_x86_64.c | 17 +---------------- 2 files changed, 2 insertions(+), 34 deletions(-)
diff --git a/dlls/ntdll/unix/signal_i386.c b/dlls/ntdll/unix/signal_i386.c index 2f6e2fd4153..1fcee4182c9 100644 --- a/dlls/ntdll/unix/signal_i386.c +++ b/dlls/ntdll/unix/signal_i386.c @@ -2449,24 +2449,7 @@ __ASM_GLOBAL_FUNC( signal_start_thread, */ __ASM_GLOBAL_FUNC( signal_exit_thread, "movl 8(%esp),%ecx\n\t" - "movl 12(%esp),%esi\n\t" - "xorl %edx,%edx\n\t" - /* fetch exit frame */ - "xchgl %edx,0x1f4(%esi)\n\t" /* x86_thread_data()->exit_frame */ - "testl %edx,%edx\n\t" - "jnz 1f\n\t" - "jmp *%ecx\n\t" - /* switch to exit frame stack */ - "1:\tmovl 4(%esp),%eax\n\t" - "movl %edx,%ebp\n\t" - __ASM_CFI(".cfi_def_cfa %ebp,4\n\t") - __ASM_CFI(".cfi_rel_offset %ebp,0\n\t") - __ASM_CFI(".cfi_rel_offset %ebx,-4\n\t") - __ASM_CFI(".cfi_rel_offset %esi,-8\n\t") - __ASM_CFI(".cfi_rel_offset %edi,-12\n\t") - "leal -20(%ebp),%esp\n\t" - "pushl %eax\n\t" - "call *%ecx" ) + "jmp *%ecx\n\t" )
/*********************************************************************** diff --git a/dlls/ntdll/unix/signal_x86_64.c b/dlls/ntdll/unix/signal_x86_64.c index ee2723cdb24..bef956bf7fc 100644 --- a/dlls/ntdll/unix/signal_x86_64.c +++ b/dlls/ntdll/unix/signal_x86_64.c @@ -3121,22 +3121,7 @@ __ASM_GLOBAL_FUNC( signal_start_thread, * signal_exit_thread */ __ASM_GLOBAL_FUNC( signal_exit_thread, - /* fetch exit frame */ - "xorl %ecx,%ecx\n\t" - "xchgq %rcx,0x320(%rdx)\n\t" /* amd64_thread_data()->exit_frame */ - "testq %rcx,%rcx\n\t" - "jnz 1f\n\t" - "jmp *%rsi\n" - /* switch to exit frame stack */ - "1:\tmovq %rcx,%rsp\n\t" - __ASM_CFI(".cfi_adjust_cfa_offset 56\n\t") - __ASM_CFI(".cfi_rel_offset %rbp,48\n\t") - __ASM_CFI(".cfi_rel_offset %rbx,40\n\t") - __ASM_CFI(".cfi_rel_offset %r12,32\n\t") - __ASM_CFI(".cfi_rel_offset %r13,24\n\t") - __ASM_CFI(".cfi_rel_offset %r14,16\n\t") - __ASM_CFI(".cfi_rel_offset %r15,8\n\t") - "call *%rsi" ) + "jmp *%rsi\n" )
/*********************************************************************** * __wine_syscall_dispatcher
-
Jin-oh Kang -
Jinoh Kang
-
Rémi Bernon