Re: [PATCH 12/13] ntdll: Introduce a new ntdll_dispatch_syscall helper.

[Zebediah Figura]

From "[PATCH 12/13] ntdll: Introduce a new ntdll_dispatch_syscall helper.":
> Wrapping stack reads in a single place so we can silent Valgrind false
> positives when reading arguments from the user stack on x86_64.

What kind of false positives?
> diff --git a/dlls/ntdll/unix/signal_i386.c b/dlls/ntdll/unix/signal_i386.c
> index d5dd77b43ff..9250f863145 100644
> --- a/dlls/ntdll/unix/signal_i386.c
> +++ b/dlls/ntdll/unix/signal_i386.c
> @@ -2499,11 +2499,6 @@ __ASM_GLOBAL_FUNC( __wine_syscall_dispatcher,
>                      __ASM_CFI(".cfi_rel_offset %edi,-0x08\n\t")
>                      __ASM_CFI(".cfi_rel_offset %esi,-0x04\n\t")
>                      __ASM_CFI(".cfi_rel_offset %ebp,-0x00\n\t")
> -
>               "leal 4(%esp),%esi\n\t"         /* first argument */
> -
>               "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 */
>                "testl $3,(%ecx)\n\t"           /* frame->syscall_flags & (SYSCALL_HAVE_XSAVE | SYSCALL_HAVE_XSAVEC) */
>                "jz 2f\n\t"
>                "movl $7,%eax\n\t"
>
> @@ -2536,21 +2531,21 @@ __ASM_GLOBAL_FUNC( __wine_syscall_dispatcher,
>                      "jmp 4f\n"
>                      "3:\tfnsave 0x40(%ecx)\n\t"
>                      "fwait\n"
> -
>               "4:\tmovl %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"
> -
>               "movl 12(%ebx),%eax\n\t"        /* table->ArgumentTable */
> -
>               "movzbl (%eax,%edx,1),%ecx\n\t"
> -
>               "movl (%ebx),%eax\n\t"          /* table->ServiceTable */
> -
>               "subl %ecx,%esp\n\t"
> -
>               "shrl $2,%ecx\n\t"
> -
>               "4:\t"
> -
> -
>               /* switch stack and dispatch syscall */
> -
>               "leal 4(%esp),%esi\n\t"         /* 1st argument */
> -
>               "movl 0x1c(%ecx),%eax\n\t"      /* frame->eax */
> -
>               "movl 0x38(%ecx),%ebx\n\t"      /* frame->syscall_table */
> -
>               "movl %ecx,%esp\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 16(%esi),%ecx\n\t"        /* 5th, ... arguments */
> -
>               "pushl %ecx\n\t"
> -
>               "pushl %esi\n\t"
> -
>               "pushl %eax\n\t"
> -
>               "pushl %ebx\n\t"
> -
>               "call ntdll_dispatch_syscall\n\t"
> -
>                "leal -0x34(%ebp),%esp\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 */

I'm concerned about the prospect of introducing another call in the 
syscall dispatcher. Some syscalls are in a very hot path. Maybe another 
direct call doesn't make things worse, but we're also in a place where 
we are counting individual instructions... do we know that this doesn't 
make a difference?