[4/8] vcomp: Implement stub for _vcomp_fork.

Sebastian Lackner sebastian at fds-team.de
Wed Jul 15 08:32:02 CDT 2015


On 15.07.2015 08:08, Alexandre Julliard wrote:
> Sebastian Lackner <sebastian at fds-team.de> writes:
> 
>> +void WINAPIV _vcomp_fork(BOOL ifval, int nargs, void *wrapper, ...)
>> +{
>> +    __ms_va_list valist;
>> +
>> +    TRACE("(%d, %d, %p, ...)\n", ifval, nargs, wrapper);
>> +
>> +    __ms_va_start(valist, wrapper);
>> +    _vcomp_fork_call_wrapper(wrapper, nargs, valist);
>> +    __ms_va_end(valist);
>> +}
> 
> Using a valist for this is questionable, even more so in the next patch
> where you copy it around, and doesn't buy anything over doing it in
> assembly from the start.
> 

Hello Alexandre,

thanks for the review.

I am not sure why you think it is questionable. We have compiler support for
variadic functions, so why should we not use it? Please note that on x86_64
this does more than just grabbing a pointer to the variadic arguments, the
declaration as WINAPIV also ensures that the compiler stores the register
values ecx/edx/r8/r9 back onto the stack:

    11bf:       89 4d 10                mov    %ecx,0x10(%rbp)
    11c2:       89 55 18                mov    %edx,0x18(%rbp)
    11c5:       4c 89 45 20             mov    %r8,0x20(%rbp)
    11c9:       4c 89 4d 28             mov    %r9,0x28(%rbp)

Moreover, implementing the whole logic of _vcomp_fork in Assembler seems
unreasonable. Do you mean a second Assembler wrapper, one to grab the "valist"
pointer, and one to copy the arguments again on the target stack to execute
the function? Would be nice if you could explain a bit more detailed what kind
of solution you have in mind. We definitely need to copy the argument list,
the target function does not take a pointer, but instead a copy of all arguments.

Best regards,
Sebastian




More information about the wine-devel mailing list