[PATCH 0/6] MR1437: ntdll: Cleanup and refactor the x86_64 and i386 syscall dispatchers.
In preparation for https://gitlab.winehq.org/wine/wine/-/merge_requests/1324. This also begins preparation for a slightly different route than what the MR currently takes, with syscall flags eventually stored in the CounterTable rather than overusing syscall number unused bits. To do that we're checking the syscall number and loading the syscall table (keeping it in %rbx/%ebx) earlier. This assumes that %rbx isn't modified in between, for instance by the eventual `SYS_arch_prctl` syscall, but I believe it is the case? -- https://gitlab.winehq.org/wine/wine/-/merge_requests/1437
From: Rémi Bernon <rbernon(a)codeweavers.com> --- dlls/ntdll/unix/signal_i386.c | 105 ++++++++++++++++++++------------ dlls/ntdll/unix/signal_x86_64.c | 103 +++++++++++++++++++------------ 2 files changed, 129 insertions(+), 79 deletions(-) diff --git a/dlls/ntdll/unix/signal_i386.c b/dlls/ntdll/unix/signal_i386.c index cc8605c2a4f..bb8032e8c74 100644 --- a/dlls/ntdll/unix/signal_i386.c +++ b/dlls/ntdll/unix/signal_i386.c @@ -2537,8 +2537,9 @@ __ASM_GLOBAL_FUNC( __wine_syscall_dispatcher, __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" + + ".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) @@ -2570,8 +2571,10 @@ __ASM_GLOBAL_FUNC( __wine_syscall_dispatcher, "shrl $8,%ebx\n\t" "andl $0x30,%ebx\n\t" /* syscall table number */ "addl 0x38(%ecx),%ebx\n\t" /* frame->syscall_table */ + + "\n.L__wine_syscall_dispatcher_save_fpu:\n\t" "testl $3,(%ecx)\n\t" /* frame->syscall_flags & (SYSCALL_HAVE_XSAVE | SYSCALL_HAVE_XSAVEC) */ - "jz 2f\n\t" + "jz .L__wine_syscall_dispatcher_no_xsave\n\t" "movl $7,%eax\n\t" "xorl %edx,%edx\n\t" "movl %edx,0x240(%ecx)\n\t" @@ -2581,7 +2584,7 @@ __ASM_GLOBAL_FUNC( __wine_syscall_dispatcher, "movl %edx,0x250(%ecx)\n\t" "movl %edx,0x254(%ecx)\n\t" "testl $2,(%ecx)\n\t" /* frame->syscall_flags & SYSCALL_HAVE_XSAVEC */ - "jz 1f\n\t" + "jz .L__wine_syscall_dispatcher_no_xsavec\n\t" "movl %edx,0x258(%ecx)\n\t" "movl %edx,0x25c(%ecx)\n\t" "movl %edx,0x260(%ecx)\n\t" @@ -2593,20 +2596,25 @@ __ASM_GLOBAL_FUNC( __wine_syscall_dispatcher, "movl %edx,0x278(%ecx)\n\t" "movl %edx,0x27c(%ecx)\n\t" "xsavec 0x40(%ecx)\n\t" - "jmp 4f\n" - "1:\txsave 0x40(%ecx)\n\t" - "jmp 4f\n" - "2:\ttestl $4,(%ecx)\n\t" /* frame->syscall_flags & SYSCALL_HAVE_FXSAVE */ - "jz 3f\n\t" + "jmp .L__wine_syscall_dispatcher_fpu_saved\n\t" + "\n.L__wine_syscall_dispatcher_no_xsavec:\n\t" + "xsave 0x40(%ecx)\n\t" + "jmp .L__wine_syscall_dispatcher_fpu_saved\n\t" + "\n.L__wine_syscall_dispatcher_no_xsave:\n\t" + "testl $4,(%ecx)\n\t" /* frame->syscall_flags & SYSCALL_HAVE_FXSAVE */ + "jz .L__wine_syscall_dispatcher_no_fxsave\n\t" "fxsave 0x40(%ecx)\n\t" - "jmp 4f\n" - "3:\tfnsave 0x40(%ecx)\n\t" - "fwait\n" - "4:\tmovl %ecx,%esp\n\t" + "jmp .L__wine_syscall_dispatcher_fpu_saved\n\t" + "\n.L__wine_syscall_dispatcher_no_fxsave:\n\t" + "fnsave 0x40(%ecx)\n\t" + "fwait\n\t" + + "\n.L__wine_syscall_dispatcher_fpu_saved:\n\t" + "movl %ecx,%esp\n\t" "movl 0x1c(%esp),%edx\n\t" /* frame->eax */ "andl $0xfff,%edx\n\t" /* syscall number */ "cmpl 8(%ebx),%edx\n\t" /* table->ServiceLimit */ - "jae 6f\n\t" + "jae .L__wine_syscall_dispatcher_invalid_arg\n\t" "movl 12(%ebx),%eax\n\t" /* table->ArgumentTable */ "movzbl (%eax,%edx,1),%ecx\n\t" "movl (%ebx),%eax\n\t" /* table->ServiceTable */ @@ -2617,8 +2625,9 @@ __ASM_GLOBAL_FUNC( __wine_syscall_dispatcher, "cld\n\t" "rep; movsl\n\t" "call *(%eax,%edx,4)\n\t" - "leal -0x34(%ebp),%esp\n" - "5:\t" + "leal -0x34(%ebp),%esp\n\t" + + "\n.L__wine_syscall_dispatcher_restore:\n\t" __ASM_CFI_CFA_IS_AT1(esp, 0x0c) __ASM_CFI_REG_IS_AT1(esp, esp, 0x0c) __ASM_CFI_REG_IS_AT1(eip, esp, 0x08) @@ -2628,22 +2637,29 @@ __ASM_GLOBAL_FUNC( __wine_syscall_dispatcher, __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" + "jz .L__wine_syscall_dispatcher_fpu_restored\n\t" + + "\n.L__wine_syscall_dispatcher_restore_fpu:\n\t" "testl $3,%ecx\n\t" /* SYSCALL_HAVE_XSAVE | SYSCALL_HAVE_XSAVEC */ - "jz 1f\n\t" + "jz .L__wine_syscall_dispatcher_no_xrstor\n\t" "movl %eax,%esi\n\t" "movl $7,%eax\n\t" "xorl %edx,%edx\n\t" "xrstor 0x40(%esp)\n\t" "movl %esi,%eax\n\t" - "jmp 3f\n" - "1:\ttestl $4,%ecx\n\t" /* SYSCALL_HAVE_FXSAVE */ - "jz 2f\n\t" + "jmp .L__wine_syscall_dispatcher_fpu_restored\n\t" + "\n.L__wine_syscall_dispatcher_no_xrstor:\n\t" + "testl $4,%ecx\n\t" /* SYSCALL_HAVE_FXSAVE */ + "jz .L__wine_syscall_dispatcher_no_fxrstor\n\t" "fxrstor 0x40(%esp)\n\t" - "jmp 3f\n" - "2:\tfrstor 0x40(%esp)\n\t" - "fwait\n" - "3:\tmovl 0x2c(%esp),%edi\n\t" + "jmp .L__wine_syscall_dispatcher_fpu_restored\n\t" + "\n.L__wine_syscall_dispatcher_no_fxrstor:\n\t" + "frstor 0x40(%esp)\n\t" + "fwait\n\t" + + "\n.L__wine_syscall_dispatcher_fpu_restored:\n\t" + "movl 0x2c(%esp),%edi\n\t" + /* remember state when $esp is pointing to "frame" */ __ASM_CFI(".cfi_remember_state\n\t") __ASM_CFI(".cfi_same_value %edi\n\t") "movl 0x30(%esp),%esi\n\t" @@ -2651,22 +2667,27 @@ __ASM_GLOBAL_FUNC( __wine_syscall_dispatcher, "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" + "jnz .L__wine_syscall_dispatcher_restore_integer\n\t" "movl 0x20(%esp),%ebx\n\t" + /* remember state when $esp is pointing to partially restored "frame" */ __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" + "jmpl *%ecx\n\t" + + "\n.L__wine_syscall_dispatcher_restore_integer:\n\t" + /* $esp is now pointing to partially restored "frame" again */ + __ASM_CFI("\t.cfi_restore_state\n\t") + "testl $0x2 << 16,%ecx\n\t" /* CONTEXT_INTEGER */ + "jz .L__wine_syscall_dispatcher_restore_control\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 */ + "movl 0x28(%esp),%edx\n\t" + "\n.L__wine_syscall_dispatcher_restore_control:\n\t" + "movl 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" @@ -2690,12 +2711,17 @@ __ASM_GLOBAL_FUNC( __wine_syscall_dispatcher, __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" + "iret\n\t" + + "\n.L__wine_syscall_dispatcher_invalid_arg:\n\t" + /* $esp is now pointing to "frame" again */ + __ASM_CFI("\t.cfi_restore_state\n\t") + "movl $0xc000000d,%eax\n\t" /* STATUS_INVALID_PARAMETER */ + "jmp .L__wine_syscall_dispatcher_restore\n\t" + + ".globl " __ASM_NAME("__wine_syscall_dispatcher_return") "\n" + __ASM_NAME("__wine_syscall_dispatcher_return") ":\n\t" + /* remember state when $esp is pointing to "frame" */ __ASM_CFI(".cfi_remember_state\n\t") __ASM_CFI(".cfi_def_cfa %esp, 4\n\t") __ASM_CFI(".cfi_restore %esp\n\t") @@ -2706,8 +2732,9 @@ __ASM_GLOBAL_FUNC( __wine_syscall_dispatcher, __ASM_CFI(".cfi_restore %ebp\n\t") "movl 8(%esp),%eax\n\t" "movl 4(%esp),%esp\n\t" + /* $esp is now pointing to "frame" again */ __ASM_CFI(".cfi_restore_state\n\t") - "jmp 5b" ) + "jmp .L__wine_syscall_dispatcher_restore\n\t" ) /*********************************************************************** diff --git a/dlls/ntdll/unix/signal_x86_64.c b/dlls/ntdll/unix/signal_x86_64.c index cc070dda5ae..6af8aa1a31d 100644 --- a/dlls/ntdll/unix/signal_x86_64.c +++ b/dlls/ntdll/unix/signal_x86_64.c @@ -2615,8 +2615,9 @@ __ASM_GLOBAL_FUNC( __wine_syscall_dispatcher, "popq 0x80(%rcx)\n\t" __ASM_CFI(".cfi_adjust_cfa_offset -8\n\t") "movl $0,0x94(%rcx)\n\t" /* frame->restore_flags */ - ".globl " __ASM_NAME("__wine_syscall_dispatcher_prolog_end") "\n" - __ASM_NAME("__wine_syscall_dispatcher_prolog_end") ":\n\t" + + ".globl " __ASM_NAME("__wine_syscall_dispatcher_prolog_end") "\n" + __ASM_NAME("__wine_syscall_dispatcher_prolog_end") ":\n\t" "movq %rax,0x00(%rcx)\n\t" "movq %rbx,0x08(%rcx)\n\t" __ASM_CFI_REG_IS_AT1(rbx, rcx, 0x08) @@ -2648,28 +2649,34 @@ __ASM_GLOBAL_FUNC( __wine_syscall_dispatcher, * depends on us returning to it. Adjust the return address accordingly. */ "subq $0xb,0x70(%rcx)\n\t" "movl 0xb0(%rcx),%r14d\n\t" /* frame->syscall_flags */ + + "\n.L__wine_syscall_dispatcher_save_fpu:\n\t" "testl $3,%r14d\n\t" /* SYSCALL_HAVE_XSAVE | SYSCALL_HAVE_XSAVEC */ - "jz 2f\n\t" + "jz .L__wine_syscall_dispatcher_no_xsave\n\t" "movl $7,%eax\n\t" "xorl %edx,%edx\n\t" "movq %rdx,0x2c0(%rcx)\n\t" "movq %rdx,0x2c8(%rcx)\n\t" "movq %rdx,0x2d0(%rcx)\n\t" "testl $2,%r14d\n\t" /* SYSCALL_HAVE_XSAVEC */ - "jz 1f\n\t" + "jz .L__wine_syscall_dispatcher_no_xsavec\n\t" "movq %rdx,0x2d8(%rcx)\n\t" "movq %rdx,0x2e0(%rcx)\n\t" "movq %rdx,0x2e8(%rcx)\n\t" "movq %rdx,0x2f0(%rcx)\n\t" "movq %rdx,0x2f8(%rcx)\n\t" "xsavec64 0xc0(%rcx)\n\t" - "jmp 3f\n" - "1:\txsave64 0xc0(%rcx)\n\t" - "jmp 3f\n" - "2:\tfxsave64 0xc0(%rcx)\n" + "jmp .L__wine_syscall_dispatcher_fpu_saved\n\t" + "\n.L__wine_syscall_dispatcher_no_xsavec:\n\t" + "xsave64 0xc0(%rcx)\n\t" + "jmp .L__wine_syscall_dispatcher_fpu_saved\n\t" + "\n.L__wine_syscall_dispatcher_no_xsave:\n\t" + "fxsave64 0xc0(%rcx)\n\t" + + "\n.L__wine_syscall_dispatcher_fpu_saved:\n\t" /* remember state when $rcx is pointing to "frame" */ __ASM_CFI(".cfi_remember_state\n\t") - "3:\tleaq 0x98(%rcx),%rbp\n\t" + "leaq 0x98(%rcx),%rbp\n\t" __ASM_CFI_CFA_IS_AT1(rbp, 0x70) __ASM_CFI_REG_IS_AT1(rsp, rbp, 0x70) __ASM_CFI_REG_IS_AT1(rip, rbp, 0x58) @@ -2683,17 +2690,18 @@ __ASM_GLOBAL_FUNC( __wine_syscall_dispatcher, __ASM_CFI_REG_IS_AT1(rbp, rbp, 0x00) #ifdef __linux__ "testl $12,%r14d\n\t" /* SYSCALL_HAVE_PTHREAD_TEB | SYSCALL_HAVE_WRFSGSBASE */ - "jz 2f\n\t" + "jz .L__wine_syscall_dispatcher_fsgs_swapped\n\t" "movq %gs:0x330,%rsi\n\t" /* amd64_thread_data()->pthread_teb */ "testl $8,%r14d\n\t" /* SYSCALL_HAVE_WRFSGSBASE */ - "jz 1f\n\t" + "jz .L__wine_syscall_dispatcher_no_wrfsbase\n\t" "wrfsbase %rsi\n\t" - "jmp 2f\n" - "1:\tmov $0x1002,%edi\n\t" /* ARCH_SET_FS */ + "jmp .L__wine_syscall_dispatcher_fsgs_swapped\n\t" + "\n.L__wine_syscall_dispatcher_no_wrfsbase:\n\t" + "mov $0x1002,%edi\n\t" /* ARCH_SET_FS */ "mov $158,%eax\n\t" /* SYS_arch_prctl */ "syscall\n\t" - "leaq -0x98(%rbp),%rcx\n" - "2:\n\t" + "leaq -0x98(%rbp),%rcx\n\t" + "\n.L__wine_syscall_dispatcher_fsgs_swapped:\n\t" #endif "leaq 0x28(%rsp),%rsi\n\t" /* first argument */ "movq %rcx,%rsp\n\t" @@ -2706,44 +2714,52 @@ __ASM_GLOBAL_FUNC( __wine_syscall_dispatcher, "leaq (%rcx,%rbx,2),%rbx\n\t" "andl $0xfff,%eax\n\t" /* syscall number */ "cmpq 16(%rbx),%rax\n\t" /* table->ServiceLimit */ - "jae 5f\n\t" + "jae .L__wine_syscall_dispatcher_invalid_arg\n\t" "movq 24(%rbx),%rcx\n\t" /* table->ArgumentTable */ "movzbl (%rcx,%rax),%ecx\n\t" "subq $0x20,%rcx\n\t" - "jbe 1f\n\t" + "jbe .L__wine_syscall_dispatcher_args_copied\n\t" "subq %rcx,%rsp\n\t" "shrq $3,%rcx\n\t" "andq $~15,%rsp\n\t" "movq %rsp,%rdi\n\t" "cld\n\t" - "rep; movsq\n" - "1:\tmovq %r10,%rcx\n\t" + "rep; movsq\n\t" + "\n.L__wine_syscall_dispatcher_args_copied:\n\t" + "movq %r10,%rcx\n\t" "subq $0x20,%rsp\n\t" "movq (%rbx),%r10\n\t" /* table->ServiceTable */ "callq *(%r10,%rax,8)\n\t" - "leaq -0x98(%rbp),%rcx\n" + "leaq -0x98(%rbp),%rcx\n\t" /* $rcx is now pointing to "frame" again */ __ASM_CFI(".cfi_restore_state\n\t") - "2:\tmovl 0x94(%rcx),%edx\n\t" /* frame->restore_flags */ + + "\n.L__wine_syscall_dispatcher_restore:\n\t" + "movl 0x94(%rcx),%edx\n\t" /* frame->restore_flags */ #ifdef __linux__ "testl $12,%r14d\n\t" /* SYSCALL_HAVE_PTHREAD_TEB | SYSCALL_HAVE_WRFSGSBASE */ - "jz 1f\n\t" - "movw 0x7e(%rcx),%fs\n" - "1:\n\t" + "jz .L__wine_syscall_dispatcher_fsgs_restored\n\t" + "movw 0x7e(%rcx),%fs\n\t" + "\n.L__wine_syscall_dispatcher_fsgs_restored:\n\t" #endif "testl $0x48,%edx\n\t" /* CONTEXT_FLOATING_POINT | CONTEXT_XSTATE */ - "jz 4f\n\t" + "jz .L__wine_syscall_dispatcher_fpu_restored\n\t" + + "\n.L__wine_syscall_dispatcher_restore_fpu:\n\t" "testl $3,%r14d\n\t" /* SYSCALL_HAVE_XSAVE | SYSCALL_HAVE_XSAVEC */ - "jz 3f\n\t" + "jz .L__wine_syscall_dispatcher_no_xrstor\n\t" "movq %rax,%r11\n\t" "movl $7,%eax\n\t" "xorl %edx,%edx\n\t" "xrstor64 0xc0(%rcx)\n\t" "movq %r11,%rax\n\t" "movl 0x94(%rcx),%edx\n\t" - "jmp 4f\n" - "3:\tfxrstor64 0xc0(%rcx)\n" - "4:\tmovq 0x98(%rcx),%rbp\n\t" + "jmp .L__wine_syscall_dispatcher_fpu_restored\n\t" + "\n.L__wine_syscall_dispatcher_no_xrstor:\n\t" + "fxrstor64 0xc0(%rcx)\n\t" + + "\n.L__wine_syscall_dispatcher_fpu_restored:\n\t" + "movq 0x98(%rcx),%rbp\n\t" __ASM_CFI(".cfi_same_value rbp\n\t") "movq 0x68(%rcx),%r15\n\t" __ASM_CFI(".cfi_same_value r15\n\t") @@ -2760,7 +2776,7 @@ __ASM_GLOBAL_FUNC( __wine_syscall_dispatcher, "movq 0x08(%rcx),%rbx\n\t" __ASM_CFI(".cfi_same_value rbx\n\t") "testl $0x3,%edx\n\t" /* CONTEXT_CONTROL | CONTEXT_INTEGER */ - "jnz 1f\n\t" + "jnz .L__wine_syscall_dispatcher_restore_control\n\t" __ASM_CFI(".cfi_remember_state\n\t") "movq 0x80(%rcx),%r11\n\t" /* frame->eflags */ "pushq %r11\n\t" @@ -2775,25 +2791,29 @@ __ASM_GLOBAL_FUNC( __wine_syscall_dispatcher, __ASM_CFI(".cfi_restore_state\n\t") /* remember state when $rcx is pointing to "frame" */ __ASM_CFI(".cfi_remember_state\n\t") - "1:\tleaq 0x70(%rcx),%rsp\n\t" + + "\n.L__wine_syscall_dispatcher_restore_control:\n\t" + "leaq 0x70(%rcx),%rsp\n\t" __ASM_CFI_CFA_IS_AT1(rsp, 0x18) __ASM_CFI_REG_IS_AT1(rsp, rsp, 0x18) __ASM_CFI_REG_IS_AT1(rip, rsp, 0x00) "testl $0x2,%edx\n\t" /* CONTEXT_INTEGER */ - "jnz 1f\n\t" + "jnz .L__wine_syscall_dispatcher_restore_integer\n\t" "movq 0x10(%rsp),%r11\n\t" /* frame->eflags */ "movq (%rsp),%rcx\n\t" /* frame->rip */ __ASM_CFI(".cfi_register rip, rcx\n\t") - "iretq\n" + "iretq\n\t" __ASM_CFI_REG_IS_AT1(rip, rsp, 0x00) - "1:\tmovq 0x00(%rcx),%rax\n\t" + + "\n.L__wine_syscall_dispatcher_restore_integer:\n\t" + "movq 0x00(%rcx),%rax\n\t" "movq 0x18(%rcx),%rdx\n\t" "movq 0x30(%rcx),%r8\n\t" "movq 0x38(%rcx),%r9\n\t" "movq 0x40(%rcx),%r10\n\t" "movq 0x48(%rcx),%r11\n\t" - "movq 0x10(%rcx),%rcx\n" - "iretq\n" + "movq 0x10(%rcx),%rcx\n\t" + "iretq\n\t" __ASM_CFI_CFA_IS_AT1(rbp, 0x70) __ASM_CFI_REG_IS_AT1(rsp, rbp, 0x70) __ASM_CFI_REG_IS_AT1(rip, rbp, 0x58) @@ -2805,15 +2825,18 @@ __ASM_GLOBAL_FUNC( __wine_syscall_dispatcher, __ASM_CFI_REG_IS_AT1(r14, rbp, 0x48) __ASM_CFI_REG_IS_AT1(r15, rbp, 0x50) __ASM_CFI_REG_IS_AT1(rbp, rbp, 0x00) - "5:\tmovl $0xc000000d,%edx\n\t" /* STATUS_INVALID_PARAMETER */ + + "\n.L__wine_syscall_dispatcher_invalid_arg:\n\t" + "movl $0xc000000d,%edx\n\t" /* STATUS_INVALID_PARAMETER */ "movq %rsp,%rcx\n\t" /* $rcx is now pointing to "frame" again */ __ASM_CFI(".cfi_restore_state\n\t") - ".globl " __ASM_NAME("__wine_syscall_dispatcher_return") "\n" - __ASM_NAME("__wine_syscall_dispatcher_return") ":\n\t" + + ".globl " __ASM_NAME("__wine_syscall_dispatcher_return") "\n" + __ASM_NAME("__wine_syscall_dispatcher_return") ":\n\t" "movl 0xb0(%rcx),%r14d\n\t" /* frame->syscall_flags */ "movq %rdx,%rax\n\t" - "jmp 2b" ) + "jmp .L__wine_syscall_dispatcher_restore\n\t" ) /*********************************************************************** -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/1437
From: Rémi Bernon <rbernon(a)codeweavers.com> We jump to .L__wine_syscall_dispatcher_invalid_arg after changing %esp, then we return to .L__wine_syscall_dispatcher_restore without restoring it from -0x34(%ebp), this is incorrect. Move %esp restoration to the restore path instead and update %ebp accordingly for the __wine_syscall_dispatcher_return entry path. --- dlls/ntdll/unix/signal_i386.c | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/dlls/ntdll/unix/signal_i386.c b/dlls/ntdll/unix/signal_i386.c index bb8032e8c74..48832199eb3 100644 --- a/dlls/ntdll/unix/signal_i386.c +++ b/dlls/ntdll/unix/signal_i386.c @@ -2625,9 +2625,11 @@ __ASM_GLOBAL_FUNC( __wine_syscall_dispatcher, "cld\n\t" "rep; movsl\n\t" "call *(%eax,%edx,4)\n\t" - "leal -0x34(%ebp),%esp\n\t" "\n.L__wine_syscall_dispatcher_restore:\n\t" + "leal -0x34(%ebp),%esp\n\t" + /* remember state when $ebp is pointing to "frame" */ + __ASM_CFI(".cfi_remember_state\n\t") __ASM_CFI_CFA_IS_AT1(esp, 0x0c) __ASM_CFI_REG_IS_AT1(esp, esp, 0x0c) __ASM_CFI_REG_IS_AT1(eip, esp, 0x08) @@ -2659,8 +2661,6 @@ __ASM_GLOBAL_FUNC( __wine_syscall_dispatcher, "\n.L__wine_syscall_dispatcher_fpu_restored:\n\t" "movl 0x2c(%esp),%edi\n\t" - /* remember state when $esp is pointing to "frame" */ - __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") @@ -2714,14 +2714,14 @@ __ASM_GLOBAL_FUNC( __wine_syscall_dispatcher, "iret\n\t" "\n.L__wine_syscall_dispatcher_invalid_arg:\n\t" - /* $esp is now pointing to "frame" again */ + /* $ebp is now pointing to "frame" again */ __ASM_CFI("\t.cfi_restore_state\n\t") "movl $0xc000000d,%eax\n\t" /* STATUS_INVALID_PARAMETER */ "jmp .L__wine_syscall_dispatcher_restore\n\t" ".globl " __ASM_NAME("__wine_syscall_dispatcher_return") "\n" __ASM_NAME("__wine_syscall_dispatcher_return") ":\n\t" - /* remember state when $esp is pointing to "frame" */ + /* remember state when $ebp is pointing to "frame" */ __ASM_CFI(".cfi_remember_state\n\t") __ASM_CFI(".cfi_def_cfa %esp, 4\n\t") __ASM_CFI(".cfi_restore %esp\n\t") @@ -2732,7 +2732,8 @@ __ASM_GLOBAL_FUNC( __wine_syscall_dispatcher, __ASM_CFI(".cfi_restore %ebp\n\t") "movl 8(%esp),%eax\n\t" "movl 4(%esp),%esp\n\t" - /* $esp is now pointing to "frame" again */ + "leal 0x34(%esp),%ebp\n\t" + /* $ebp is now pointing to "frame" again */ __ASM_CFI(".cfi_restore_state\n\t") "jmp .L__wine_syscall_dispatcher_restore\n\t" ) -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/1437
From: Rémi Bernon <rbernon(a)codeweavers.com> --- dlls/ntdll/unix/signal_i386.c | 10 +++++++--- dlls/ntdll/unix/signal_x86_64.c | 33 ++++++++++----------------------- 2 files changed, 17 insertions(+), 26 deletions(-) diff --git a/dlls/ntdll/unix/signal_i386.c b/dlls/ntdll/unix/signal_i386.c index 48832199eb3..57da9efd608 100644 --- a/dlls/ntdll/unix/signal_i386.c +++ b/dlls/ntdll/unix/signal_i386.c @@ -2566,11 +2566,16 @@ __ASM_GLOBAL_FUNC( __wine_syscall_dispatcher, __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 */ + + "\n.L__wine_syscall_dispatcher_check_syscall:\n\t" "movl %eax,%ebx\n\t" "shrl $8,%ebx\n\t" "andl $0x30,%ebx\n\t" /* syscall table number */ "addl 0x38(%ecx),%ebx\n\t" /* frame->syscall_table */ + "movl 0x1c(%ecx),%eax\n\t" /* frame->eax */ + "andl $0xfff,%eax\n\t" /* syscall number */ + "cmpl 8(%ebx),%eax\n\t" /* table->ServiceLimit */ + "jae .L__wine_syscall_dispatcher_invalid_arg\n\t" "\n.L__wine_syscall_dispatcher_save_fpu:\n\t" "testl $3,(%ecx)\n\t" /* frame->syscall_flags & (SYSCALL_HAVE_XSAVE | SYSCALL_HAVE_XSAVEC) */ @@ -2610,11 +2615,10 @@ __ASM_GLOBAL_FUNC( __wine_syscall_dispatcher, "fwait\n\t" "\n.L__wine_syscall_dispatcher_fpu_saved:\n\t" + "leal 4(%esp),%esi\n\t" /* first argument */ "movl %ecx,%esp\n\t" "movl 0x1c(%esp),%edx\n\t" /* frame->eax */ "andl $0xfff,%edx\n\t" /* syscall number */ - "cmpl 8(%ebx),%edx\n\t" /* table->ServiceLimit */ - "jae .L__wine_syscall_dispatcher_invalid_arg\n\t" "movl 12(%ebx),%eax\n\t" /* table->ArgumentTable */ "movzbl (%eax,%edx,1),%ecx\n\t" "movl (%ebx),%eax\n\t" /* table->ServiceTable */ diff --git a/dlls/ntdll/unix/signal_x86_64.c b/dlls/ntdll/unix/signal_x86_64.c index 6af8aa1a31d..abf96a9f740 100644 --- a/dlls/ntdll/unix/signal_x86_64.c +++ b/dlls/ntdll/unix/signal_x86_64.c @@ -2648,7 +2648,17 @@ __ASM_GLOBAL_FUNC( __wine_syscall_dispatcher, /* 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" + + "\n.L__wine_syscall_dispatcher_check_syscall:\n\t" + "movl %eax,%ebx\n\t" "movl 0xb0(%rcx),%r14d\n\t" /* frame->syscall_flags */ + "shrl $7,%ebx\n\t" + "andl $0x60,%ebx\n\t" /* syscall table number */ + "movq 0xa8(%rcx),%rdx\n\t" /* frame->syscall_table */ + "leaq (%rdx,%rbx),%rbx\n\t" + "andl $0xfff,%eax\n\t" /* syscall number */ + "cmpq 16(%rbx),%rax\n\t" /* table->ServiceLimit */ + "jae .L__wine_syscall_dispatcher_invalid_arg\n\t" "\n.L__wine_syscall_dispatcher_save_fpu:\n\t" "testl $3,%r14d\n\t" /* SYSCALL_HAVE_XSAVE | SYSCALL_HAVE_XSAVEC */ @@ -2707,14 +2717,7 @@ __ASM_GLOBAL_FUNC( __wine_syscall_dispatcher, "movq %rcx,%rsp\n\t" "movq 0x00(%rcx),%rax\n\t" "movq 0x18(%rcx),%rdx\n\t" - "movl %eax,%ebx\n\t" - "shrl $8,%ebx\n\t" - "andl $0x30,%ebx\n\t" /* syscall table number */ - "movq 0xa8(%rcx),%rcx\n\t" /* frame->syscall_table */ - "leaq (%rcx,%rbx,2),%rbx\n\t" "andl $0xfff,%eax\n\t" /* syscall number */ - "cmpq 16(%rbx),%rax\n\t" /* table->ServiceLimit */ - "jae .L__wine_syscall_dispatcher_invalid_arg\n\t" "movq 24(%rbx),%rcx\n\t" /* table->ArgumentTable */ "movzbl (%rcx,%rax),%ecx\n\t" "subq $0x20,%rcx\n\t" @@ -2789,8 +2792,6 @@ __ASM_GLOBAL_FUNC( __wine_syscall_dispatcher, "jmpq *%rcx\n\t" /* $rcx is now pointing to "frame" again */ __ASM_CFI(".cfi_restore_state\n\t") - /* remember state when $rcx is pointing to "frame" */ - __ASM_CFI(".cfi_remember_state\n\t") "\n.L__wine_syscall_dispatcher_restore_control:\n\t" "leaq 0x70(%rcx),%rsp\n\t" @@ -2814,23 +2815,9 @@ __ASM_GLOBAL_FUNC( __wine_syscall_dispatcher, "movq 0x48(%rcx),%r11\n\t" "movq 0x10(%rcx),%rcx\n\t" "iretq\n\t" - __ASM_CFI_CFA_IS_AT1(rbp, 0x70) - __ASM_CFI_REG_IS_AT1(rsp, rbp, 0x70) - __ASM_CFI_REG_IS_AT1(rip, rbp, 0x58) - __ASM_CFI_REG_IS_AT2(rbx, rbp, 0xf0, 0x7e) - __ASM_CFI_REG_IS_AT2(rsi, rbp, 0x88, 0x7f) - __ASM_CFI_REG_IS_AT2(rdi, rbp, 0x90, 0x7f) - __ASM_CFI_REG_IS_AT2(r12, rbp, 0xb8, 0x7f) - __ASM_CFI_REG_IS_AT1(r13, rbp, 0x40) - __ASM_CFI_REG_IS_AT1(r14, rbp, 0x48) - __ASM_CFI_REG_IS_AT1(r15, rbp, 0x50) - __ASM_CFI_REG_IS_AT1(rbp, rbp, 0x00) "\n.L__wine_syscall_dispatcher_invalid_arg:\n\t" "movl $0xc000000d,%edx\n\t" /* STATUS_INVALID_PARAMETER */ - "movq %rsp,%rcx\n\t" - /* $rcx is now pointing to "frame" again */ - __ASM_CFI(".cfi_restore_state\n\t") ".globl " __ASM_NAME("__wine_syscall_dispatcher_return") "\n" __ASM_NAME("__wine_syscall_dispatcher_return") ":\n\t" -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/1437
From: Rémi Bernon <rbernon(a)codeweavers.com> Using %eax more consistently as the syscall number. --- dlls/ntdll/unix/signal_i386.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/dlls/ntdll/unix/signal_i386.c b/dlls/ntdll/unix/signal_i386.c index 57da9efd608..b63d82e7e2e 100644 --- a/dlls/ntdll/unix/signal_i386.c +++ b/dlls/ntdll/unix/signal_i386.c @@ -2617,18 +2617,18 @@ __ASM_GLOBAL_FUNC( __wine_syscall_dispatcher, "\n.L__wine_syscall_dispatcher_fpu_saved:\n\t" "leal 4(%esp),%esi\n\t" /* first argument */ "movl %ecx,%esp\n\t" - "movl 0x1c(%esp),%edx\n\t" /* frame->eax */ - "andl $0xfff,%edx\n\t" /* syscall number */ - "movl 12(%ebx),%eax\n\t" /* table->ArgumentTable */ - "movzbl (%eax,%edx,1),%ecx\n\t" - "movl (%ebx),%eax\n\t" /* table->ServiceTable */ + "movl 0x1c(%esp),%eax\n\t" /* frame->eax */ + "andl $0xfff,%eax\n\t" /* syscall number */ + "movl 12(%ebx),%edx\n\t" /* table->ArgumentTable */ + "movzbl (%edx,%eax,1),%ecx\n\t" + "movl (%ebx),%edx\n\t" /* table->ServiceTable */ "subl %ecx,%esp\n\t" "shrl $2,%ecx\n\t" "andl $~15,%esp\n\t" "movl %esp,%edi\n\t" "cld\n\t" "rep; movsl\n\t" - "call *(%eax,%edx,4)\n\t" + "call *(%edx,%eax,4)\n\t" "\n.L__wine_syscall_dispatcher_restore:\n\t" "leal -0x34(%ebp),%esp\n\t" -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/1437
From: Rémi Bernon <rbernon(a)codeweavers.com> Making it a bit more consistent with the i386 dispatcher. --- dlls/ntdll/unix/signal_x86_64.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/dlls/ntdll/unix/signal_x86_64.c b/dlls/ntdll/unix/signal_x86_64.c index abf96a9f740..81e5e4d2fdc 100644 --- a/dlls/ntdll/unix/signal_x86_64.c +++ b/dlls/ntdll/unix/signal_x86_64.c @@ -2733,11 +2733,11 @@ __ASM_GLOBAL_FUNC( __wine_syscall_dispatcher, "subq $0x20,%rsp\n\t" "movq (%rbx),%r10\n\t" /* table->ServiceTable */ "callq *(%r10,%rax,8)\n\t" + + "\n.L__wine_syscall_dispatcher_restore:\n\t" "leaq -0x98(%rbp),%rcx\n\t" /* $rcx is now pointing to "frame" again */ __ASM_CFI(".cfi_restore_state\n\t") - - "\n.L__wine_syscall_dispatcher_restore:\n\t" "movl 0x94(%rcx),%edx\n\t" /* frame->restore_flags */ #ifdef __linux__ "testl $12,%r14d\n\t" /* SYSCALL_HAVE_PTHREAD_TEB | SYSCALL_HAVE_WRFSGSBASE */ @@ -2823,6 +2823,7 @@ __ASM_GLOBAL_FUNC( __wine_syscall_dispatcher, __ASM_NAME("__wine_syscall_dispatcher_return") ":\n\t" "movl 0xb0(%rcx),%r14d\n\t" /* frame->syscall_flags */ "movq %rdx,%rax\n\t" + "leaq 0x98(%rcx),%rbp\n\t" "jmp .L__wine_syscall_dispatcher_restore\n\t" ) -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/1437
From: Rémi Bernon <rbernon(a)codeweavers.com> SYSCALL_HAVE_PTHREAD_TEB is always set when SYSCALL_HAVE_WRFSGSBASE is, there is no point testing both flags at once. --- dlls/ntdll/unix/signal_x86_64.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/dlls/ntdll/unix/signal_x86_64.c b/dlls/ntdll/unix/signal_x86_64.c index 81e5e4d2fdc..90dc5152d31 100644 --- a/dlls/ntdll/unix/signal_x86_64.c +++ b/dlls/ntdll/unix/signal_x86_64.c @@ -1613,7 +1613,7 @@ __ASM_GLOBAL_FUNC( call_user_mode_callback, "movq %r10,0xa0(%rsp)\n\t" /* frame->prev_frame */ "movq %rsp,0x328(%r11)\n\t" /* amd64_thread_data()->syscall_frame */ #ifdef __linux__ - "testl $12,%r14d\n\t" /* SYSCALL_HAVE_PTHREAD_TEB | SYSCALL_HAVE_WRFSGSBASE */ + "testl $4,%r14d\n\t" /* SYSCALL_HAVE_PTHREAD_TEB */ "jz 1f\n\t" "movw 0x338(%r11),%fs\n" /* amd64_thread_data()->fs */ "1:\n\t" @@ -2699,7 +2699,7 @@ __ASM_GLOBAL_FUNC( __wine_syscall_dispatcher, __ASM_CFI_REG_IS_AT1(r15, rbp, 0x50) __ASM_CFI_REG_IS_AT1(rbp, rbp, 0x00) #ifdef __linux__ - "testl $12,%r14d\n\t" /* SYSCALL_HAVE_PTHREAD_TEB | SYSCALL_HAVE_WRFSGSBASE */ + "testl $4,%r14d\n\t" /* SYSCALL_HAVE_PTHREAD_TEB */ "jz .L__wine_syscall_dispatcher_fsgs_swapped\n\t" "movq %gs:0x330,%rsi\n\t" /* amd64_thread_data()->pthread_teb */ "testl $8,%r14d\n\t" /* SYSCALL_HAVE_WRFSGSBASE */ @@ -2740,7 +2740,7 @@ __ASM_GLOBAL_FUNC( __wine_syscall_dispatcher, __ASM_CFI(".cfi_restore_state\n\t") "movl 0x94(%rcx),%edx\n\t" /* frame->restore_flags */ #ifdef __linux__ - "testl $12,%r14d\n\t" /* SYSCALL_HAVE_PTHREAD_TEB | SYSCALL_HAVE_WRFSGSBASE */ + "testl $4,%r14d\n\t" /* SYSCALL_HAVE_PTHREAD_TEB */ "jz .L__wine_syscall_dispatcher_fsgs_restored\n\t" "movw 0x7e(%rcx),%fs\n\t" "\n.L__wine_syscall_dispatcher_fsgs_restored:\n\t" -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/1437
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 tests also ran into some preexisting test failures. If you know how to fix them that would be helpful. See the TestBot job for the details: The full results can be found at: https://testbot.winehq.org/JobDetails.pl?Key=126351 Your paranoid android. === debian11 (32 bit report) === d3d8: stateblock: Timeout visual: Timeout d3d9: d3d9ex: Timeout device: Timeout stateblock: Timeout visual: Timeout d3dcompiler_43: asm: Timeout blob: Timeout hlsl_d3d11: Timeout hlsl_d3d9: Timeout reflection: Timeout d3dcompiler_46: asm: Timeout blob: Timeout hlsl_d3d11: Timeout hlsl_d3d9: Timeout reflection: Timeout d3dcompiler_47: asm: Timeout blob: Timeout hlsl_d3d11: Timeout hlsl_d3d9: Timeout reflection: Timeout d3drm: d3drm: Timeout vector: Timeout Report validation errors: d3dx10: Timeout === debian11 (build log) === WineRunWineTest.pl:error: The task timed out
Alexandre Julliard (@julliard) commented about dlls/ntdll/unix/signal_i386.c:
"movzbl (%eax,%edx,1),%ecx\n\t" "movl (%ebx),%eax\n\t" /* table->ServiceTable */ "subl %ecx,%esp\n\t" "shrl $2,%ecx\n\t" "andl $~15,%esp\n\t" "movl %esp,%edi\n\t" "cld\n\t" "rep; movsl\n\t" "call *(%eax,%edx,4)\n\t" - "leal -0x34(%ebp),%esp\n\t"
"\n.L__wine_syscall_dispatcher_restore:\n\t" + "leal -0x34(%ebp),%esp\n\t" + /* remember state when $ebp is pointing to "frame" */ + __ASM_CFI(".cfi_remember_state\n\t") __ASM_CFI_CFA_IS_AT1(esp, 0x0c)
I don't understand why you are changing this. The frame is pointed to by %esp, we don't use %ebp at all, except as a way to get %esp back to the syscall frame after calling the entry point. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/1437#note_16720
On Mon Nov 21 15:08:57 2022 +0000, Alexandre Julliard wrote:
I don't understand why you are changing this. The frame is pointed to by %esp, we don't use %ebp at all, except as a way to get %esp back to the syscall frame after calling the entry point. Hmm... I probably got lost somewhere. So the commit comment is wrong and this is unnecessary.
Then I think it would make it easier to have the `lea` instruction in the return path to simplify an eventual future change like https://gitlab.winehq.org/wine/wine/-/merge_requests/1324/diffs?commit_id=20.... That's more the case for the x86_64 dispatcher which otherwise needs more complicated CFI instructions (like using `cfi_remember_state` twice as in 20dadb19fda2555eafbbb70278af7096d5d890aa), which is also why I was replicating this change, in 4040d4d4821810b82a348fca06538a2d12b9da67 in this MR. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/1437#note_16723
participants (3)
-
Alexandre Julliard (＠julliard) -
Marvin -
Rémi Bernon