[PATCH v2 0/4] 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? -- v2: ntdll: Check SYSCALL_HAVE_WRFSGSBASE syscall flag only for wrfsbase. ntdll: Swap %eax and %edx registers in the i386 syscall dispatcher. ntdll: Check syscall table and syscall number before saving FPU. ntdll: Use named labels for jumps in the syscall dispatcher. 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> --- 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 bb8032e8c74..82c9d699b17 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 82c9d699b17..e99f417d164 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" "leal -0x34(%ebp),%esp\n\t" "\n.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 abf96a9f740..9e4f4046d43 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, "\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 */ + "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=126566 Your paranoid android. === debian11 (32 bit report) === d3d9: 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 d3dx10_34: d3dx10: Timeout d3dx10_35: d3dx10: Timeout d3dx10_36: d3dx10: Timeout Report validation errors: d3dx10_37:d3dx10 timeout === debian11 (build log) === WineRunWineTest.pl:error: The task timed out
On Mon Nov 21 15:23:54 2022 +0000, Rémi Bernon wrote:
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. I dropped the changes for now.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/1437#note_16732
Is there something I should do differently here? -- https://gitlab.winehq.org/wine/wine/-/merge_requests/1437#note_17539
Is there something I should do differently here?
Some of these changes don't look like improvements, at least not on their own. They may be improvements when combined with your optimization patches, but it's not clear yet that we want to do it that way. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/1437#note_17730
This merge request was closed by Rémi Bernon. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/1437
participants (3)
-
Alexandre Julliard (＠julliard) -
Marvin -
Rémi Bernon