[PATCH v3 0/2] MR1182: ntdll: Add CFI unwind info to __wine_syscall_dispatcher (i386).
This change is adding DWARF (CFI) unwind information to the hand-written assembly of the `__wine_syscall_dispatcher` function. This enables unwinding through the dispatcher from the Linux stack into (and through) the Windows stack. The general idea is that the `syscall_frame` struct contains the content of the callee-save registers before the function call (in particular the stack pointer and the return address). At any point of the execution, we have a pointer into the `syscall_frame` in $ebx, $ecx, $ebp, or $esp. For the CFI codes the general idea is that we are defining the computations of the callee-save registers based on the `syscall_frame` using DWARF’s `breg` instruction, rather than relative to CFA. cc/ @florian-kuebler -- v3: ntdll: Add CFI unwind info to __wine_syscall_dispatcher (i386). ntdll: Move CFI helper macros out of signal_x86_64.c. https://gitlab.winehq.org/wine/wine/-/merge_requests/1182
Uh, thanks for taking care of that! So I don't need to deal with computing the offsets and cfi instructions :smile: -- https://gitlab.winehq.org/wine/wine/-/merge_requests/1182#note_12720
From: Jinoh Kang <jinoh.kang.kr(a)gmail.com> This change is adding DWARF (CFI) unwind information to the hand-written assembly of the `__wine_syscall_dispatcher` function. This enables unwinding through the dispatcher from the Linux stack into (and through) the Windows stack. The general idea is that the `syscall_frame` struct contains the content of the callee-save registers before the function call (in particular the stack pointer and the return address). At any point of the execution, we have a pointer into the `syscall_frame` in $ebx, $ecx, $ebp, or $esp. For the CFI codes the general idea is that we are defining the computations of the callee-save registers based on the `syscall_frame` using DWARF’s `breg` instruction, rather than relative to CFA. --- dlls/ntdll/unix/signal_i386.c | 74 ++++++++++++++++++++++++++++++++++- 1 file changed, 73 insertions(+), 1 deletion(-) diff --git a/dlls/ntdll/unix/signal_i386.c b/dlls/ntdll/unix/signal_i386.c index 2dfce706394..67efe7b544b 100644 --- a/dlls/ntdll/unix/signal_i386.c +++ b/dlls/ntdll/unix/signal_i386.c @@ -59,6 +59,7 @@ #include "wine/asm.h" #include "unix_private.h" #include "wine/debug.h" +#include "dwarf.h" WINE_DEFAULT_DEBUG_CHANNEL(seh); @@ -2493,6 +2494,21 @@ __ASM_GLOBAL_FUNC( signal_exit_thread, "call *%ecx" ) +#define DW_OP_ecx DW_OP_breg1 +#define DW_OP_ebx DW_OP_breg3 +#define DW_OP_esp DW_OP_breg4 +#define DW_OP_ebp DW_OP_breg5 + +#define DW_REG_eax 0x00 +#define DW_REG_ecx 0x01 +#define DW_REG_edx 0x02 +#define DW_REG_ebx 0x03 +#define DW_REG_esp 0x04 +#define DW_REG_ebp 0x05 +#define DW_REG_esi 0x06 +#define DW_REG_edi 0x07 +#define DW_REG_eip 0x08 + /*********************************************************************** * __wine_syscall_dispatcher */ @@ -2500,11 +2516,17 @@ __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_REG_IS_AT1(eip, ecx, 0x08) "pushfl\n\t" + __ASM_CFI(".cfi_adjust_cfa_offset 4\n\t") "popl 0x04(%ecx)\n\t" /* frame->eflags */ + __ASM_CFI(".cfi_adjust_cfa_offset -4\n\t") ".globl " __ASM_NAME("__wine_syscall_dispatcher_prolog_end") "\n" __ASM_NAME("__wine_syscall_dispatcher_prolog_end") ":\n\t" "movl %esp,0x0c(%ecx)\n\t" /* frame->esp */ + __ASM_CFI_CFA_IS_AT1(ecx, 0x0c) + __ASM_CFI_REG_IS_AT1(esp, ecx, 0x0c) "movw %cs,0x10(%ecx)\n\t" "movw %ss,0x12(%ecx)\n\t" "movw %ds,0x14(%ecx)\n\t" @@ -2513,10 +2535,21 @@ __ASM_GLOBAL_FUNC( __wine_syscall_dispatcher, "movw %gs,0x1a(%ecx)\n\t" "movl %eax,0x1c(%ecx)\n\t" "movl %ebx,0x20(%ecx)\n\t" + __ASM_CFI_REG_IS_AT1(ebx, ecx, 0x20) "movl %edi,0x2c(%ecx)\n\t" + __ASM_CFI_REG_IS_AT1(edi, ecx, 0x2c) "movl %esi,0x30(%ecx)\n\t" + __ASM_CFI_REG_IS_AT1(esi, ecx, 0x30) "movl %ebp,0x34(%ecx)\n\t" + __ASM_CFI_REG_IS_AT1(ebp, ecx, 0x34) "leal 0x34(%ecx),%ebp\n\t" + __ASM_CFI_CFA_IS_AT1(ebp, 0x58) + __ASM_CFI_REG_IS_AT1(esp, ebp, 0x58) + __ASM_CFI_REG_IS_AT1(eip, ebp, 0x54) + __ASM_CFI_REG_IS_AT1(ebx, ebp, 0x6c) + __ASM_CFI_REG_IS_AT1(edi, ebp, 0x78) + __ASM_CFI_REG_IS_AT1(esi, ebp, 0x7c) + __ASM_CFI_REG_IS_AT1(ebp, ebp, 0x00) "leal 4(%esp),%esi\n\t" /* first argument */ "movl %eax,%ebx\n\t" "shrl $8,%ebx\n\t" @@ -2570,7 +2603,15 @@ __ASM_GLOBAL_FUNC( __wine_syscall_dispatcher, "rep; movsl\n\t" "call *(%eax,%edx,4)\n\t" "leal -0x34(%ebp),%esp\n" - "5:\tmovl 0(%esp),%ecx\n\t" /* frame->syscall_flags + (frame->restore_flags << 16) */ + "5:\t" + __ASM_CFI_CFA_IS_AT1(esp, 0x0c) + __ASM_CFI_REG_IS_AT1(esp, esp, 0x0c) + __ASM_CFI_REG_IS_AT1(eip, esp, 0x08) + __ASM_CFI_REG_IS_AT1(ebx, esp, 0x20) + __ASM_CFI_REG_IS_AT1(edi, esp, 0x2c) + __ASM_CFI_REG_IS_AT1(esi, esp, 0x30) + __ASM_CFI_REG_IS_AT1(ebp, esp, 0x34) + "movl 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" "testl $3,%ecx\n\t" /* SYSCALL_HAVE_XSAVE | SYSCALL_HAVE_XSAVEC */ @@ -2588,38 +2629,69 @@ __ASM_GLOBAL_FUNC( __wine_syscall_dispatcher, "2:\tfrstor 0x40(%esp)\n\t" "fwait\n" "3:\tmovl 0x2c(%esp),%edi\n\t" + __ASM_CFI(".cfi_remember_state\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" "movl 0x24(%esp),%ecx\n\t" "movl 0x28(%esp),%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" "xchgl %ebx,%esp\n\t" + __ASM_CFI(".cfi_def_cfa %esp, 0\n\t") + __ASM_CFI(".cfi_same_value %esp\n\t") + __ASM_CFI_REG_IS_AT1(eip, ebx, 0x08) + __ASM_CFI_REG_IS_AT1(ebx, ebx, 0x20) "pushl 0x04(%ebx)\n\t" /* frame->eflags */ + __ASM_CFI(".cfi_adjust_cfa_offset 4\n\t") "pushl 0x10(%ebx)\n\t" /* frame->cs */ + __ASM_CFI(".cfi_adjust_cfa_offset 4\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") "movw 0x16(%ebx),%es\n\t" "movw 0x18(%ebx),%fs\n\t" "movw 0x1a(%ebx),%gs\n\t" "movl 0x20(%ebx),%ebx\n\t" + __ASM_CFI(".cfi_same_value %ebx\n\t") "popl %ds\n\t" + __ASM_CFI(".cfi_adjust_cfa_offset -4\n\t") "iret\n" + __ASM_CFI("\t.cfi_restore_state\n") "6:\tmovl $0xc000000d,%eax\n\t" /* STATUS_INVALID_PARAMETER */ "jmp 5b\n\t" ".globl " __ASM_NAME("__wine_syscall_dispatcher_return") "\n" __ASM_NAME("__wine_syscall_dispatcher_return") ":\n\t" + __ASM_CFI(".cfi_remember_state\n\t") + __ASM_CFI(".cfi_def_cfa %esp, 4\n\t") + __ASM_CFI(".cfi_restore %esp\n\t") + __ASM_CFI(".cfi_restore %eip\n\t") + __ASM_CFI(".cfi_restore %ebx\n\t") + __ASM_CFI(".cfi_restore %edi\n\t") + __ASM_CFI(".cfi_restore %esi\n\t") + __ASM_CFI(".cfi_restore %ebp\n\t") "movl 8(%esp),%eax\n\t" "movl 4(%esp),%esp\n\t" + __ASM_CFI(".cfi_restore_state\n\t") "jmp 5b" ) -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/1182
Hi, It looks like your patch introduced the new failures shown below. Please investigate and fix them before resubmitting your patch. If they are not new, fixing them anyway would help a lot. Otherwise please ask for the known failures list to be updated. The full results can be found at: https://testbot.winehq.org/JobDetails.pl?Key=125602 Your paranoid android. === debian11 (build log) === ../wine/include/wine/debug.h:475:58: error: ���__wine_dbch___default��� undeclared (first use in this function) ../wine/dlls/ntdll/unix/dwarf.h:385:2: error: #error Unsupported architecture ../wine/dlls/ntdll/unix/dwarf.h:394:25: error: ���NB_FRAME_REGS��� undeclared here (not in a function) ../wine/include/wine/debug.h:475:58: error: ���__wine_dbch___default��� undeclared (first use in this function) ../wine/include/wine/debug.h:463:58: error: ���__wine_dbch___default��� undeclared (first use in this function) ../wine/include/wine/debug.h:475:58: error: ���__wine_dbch___default��� undeclared (first use in this function) Task: The win32 Wine build failed === debian11b (build log) === ../wine/include/wine/debug.h:475:58: error: ���__wine_dbch___default��� undeclared (first use in this function) ../wine/dlls/ntdll/unix/dwarf.h:385:2: error: #error Unsupported architecture ../wine/dlls/ntdll/unix/dwarf.h:394:25: error: ���NB_FRAME_REGS��� undeclared here (not in a function) ../wine/include/wine/debug.h:475:58: error: ���__wine_dbch___default��� undeclared (first use in this function) ../wine/include/wine/debug.h:463:58: error: ���__wine_dbch___default��� undeclared (first use in this function) ../wine/include/wine/debug.h:475:58: error: ���__wine_dbch___default��� undeclared (first use in this function) Task: The wow32 Wine build failed
From: Jinoh Kang <jinoh.kang.kr(a)gmail.com> --- dlls/ntdll/unix/dwarf.h | 12 ++++++++++++ dlls/ntdll/unix/signal_x86_64.c | 12 ------------ 2 files changed, 12 insertions(+), 12 deletions(-) diff --git a/dlls/ntdll/unix/dwarf.h b/dlls/ntdll/unix/dwarf.h index cae7d6288ee..7a61662ba36 100644 --- a/dlls/ntdll/unix/dwarf.h +++ b/dlls/ntdll/unix/dwarf.h @@ -1010,4 +1010,16 @@ static void apply_frame_state( CONTEXT *context, struct frame_state *state, *context = new_context; } +#define __ASM_CFI_STR(...) #__VA_ARGS__ +#define __ASM_CFI_ESC(...) \ + __ASM_CFI(".cfi_escape " __ASM_CFI_STR(__VA_ARGS__) "\n\t") +#define __ASM_CFI_CFA_IS_AT1(base, offset) \ + __ASM_CFI_ESC(DW_CFA_def_cfa_expression, 0x03, DW_OP_ ## base, offset, DW_OP_deref) +#define __ASM_CFI_REG_IS_AT1(reg, base, offset) \ + __ASM_CFI_ESC(DW_CFA_expression, DW_REG_ ## reg, 0x02, DW_OP_ ## base, offset) +#define __ASM_CFI_CFA_IS_AT2(base, lo, hi) \ + __ASM_CFI_ESC(DW_CFA_def_cfa_expression, 0x04, DW_OP_ ## base, lo, hi, DW_OP_deref) +#define __ASM_CFI_REG_IS_AT2(reg, base, lo, hi) \ + __ASM_CFI_ESC(DW_CFA_expression, DW_REG_ ## reg, 0x03, DW_OP_ ## base, lo, hi) + #endif /* __NTDLL_DWARF_H */ diff --git a/dlls/ntdll/unix/signal_x86_64.c b/dlls/ntdll/unix/signal_x86_64.c index 41f860ac4f4..1043e9bda2f 100644 --- a/dlls/ntdll/unix/signal_x86_64.c +++ b/dlls/ntdll/unix/signal_x86_64.c @@ -2535,18 +2535,6 @@ __ASM_GLOBAL_FUNC( signal_exit_thread, #define DW_REG_r15 0x0f #define DW_REG_rip 0x10 -#define __ASM_CFI_STR(...) #__VA_ARGS__ -#define __ASM_CFI_ESC(...) \ - __ASM_CFI(".cfi_escape " __ASM_CFI_STR(__VA_ARGS__) "\n\t") -#define __ASM_CFI_CFA_IS_AT1(base, offset) \ - __ASM_CFI_ESC(DW_CFA_def_cfa_expression, 0x03, DW_OP_ ## base, offset, DW_OP_deref) -#define __ASM_CFI_REG_IS_AT1(reg, base, offset) \ - __ASM_CFI_ESC(DW_CFA_expression, DW_REG_ ## reg, 0x02, DW_OP_ ## base, offset) -#define __ASM_CFI_CFA_IS_AT2(base, lo, hi) \ - __ASM_CFI_ESC(DW_CFA_def_cfa_expression, 0x04, DW_OP_ ## base, lo, hi, DW_OP_deref) -#define __ASM_CFI_REG_IS_AT2(reg, base, lo, hi) \ - __ASM_CFI_ESC(DW_CFA_expression, DW_REG_ ## reg, 0x03, DW_OP_ ## base, lo, hi) - /*********************************************************************** * __wine_syscall_dispatcher */ -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/1182
On Tue Nov 1 11:03:07 2022 +0000, Rémi Bernon wrote:
Could this go above the label? No sure to see why it's been split there. To ensure that the listing (gcc -S) is formatted properly
It's ``` leal -0x34(%ebp),%esp .cfi_escape 0x... .cfi_escape 0x... 5: movl 0(%esp),%ecx ``` versus ``` leal -0x34(%ebp),%esp 5: .cfi_escape 0x... .cfi_escape 0x... movl 0(%esp),%ecx ``` -- https://gitlab.winehq.org/wine/wine/-/merge_requests/1182#note_12631
On Mon Oct 31 23:03:23 2022 +0000, Rémi Bernon wrote:
It's now been merged so you could rebase this one. Done
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/1182#note_12627
On Tue Nov 1 11:22:42 2022 +0000, Jinoh Kang wrote:
changed this line in [version 3 of the diff](/wine/wine/-/merge_requests/1182/diffs?diff_id=16204&start_sha=ddce80385d26bfba23519e5c01fbf775722664ea#fdd8c54a2afb459527e98d3e3ccbbc6ca451c5b5_2516_2516) I'll leave that to you, since you have already done that in !1074.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/1182#note_12628
participants (4)
-
Florian Kübler -
Jinoh Kang
-
Jinoh Kang (＠iamahuman)
-
Marvin