Re: [PATCH 0/4] s390 improvements

[Richard Henderson <rth@redhat.com>]

On 12/19/2014 09:06 AM, Ulrich Weigand wrote:
> Richard Henderson wrote:
> 
>> This is relative to Dominik's patch from the 16th.  The complete
>> tree can be found at
>>
>> 	git://github.com/rth7680/libffi.git s390
>>
>> Mostly relevant is patch 3, which converts the s390 port to the
>> more modern arrangement where there's no callback into ffi_prep_args.
> 
> This is a bit confusing to me.  The assembler routine now does:
> 
>         lg      %r15,120(%r2)                   # Set up outgoing stack
> 
> without ever restoring the initial stack pointer before returning
> to its caller.  This probably works right now since the value loaded
> here is determined like that:
> 
>   /* Pass the outgoing stack frame in the r15 save slot.  */
>   frame->gpr_save[8] = (unsigned long)(stack - sizeof(struct call_frame));
> 
> and since "stack" was allocated via alloca and ffi_call_int does not
> require an argument save area when calling any of its subroutines,
> the stack pointer value computed here should always in fact be
> identical to the value %r15 already has at the above location.
> 
> Using the 160 bytes below "stack" as register save area for use of the
> target function called by ffi_call_SYSV is also only safe if those bytes
> are in fact the register save area ffi_call_int provides for its caller,
> e.g. again if the value is already identical to %r15.  (If this were
> any other value, we might clobber parts of ffi_call_int's stack frame
> that it conceivably might still access.)
> 
> However, if the procedure only works if the "lg" is a nop, why is it
> even done?   Also, the whole setup seems a bit fragile since changes
> to ffi_call_int might cause it to need an argument save area ...

The stack frame we install is created with alloca, and so we know for a fact
that ffi_call_int must be using a frame pointer to hold its own frame.  Since
we do not adjust %r15 on the way out of ffi_call_SYSV, we leave the stack frame
chain intact.  If there were another function for ffi_call_int to call after
ffi_call_SYSV (but there's not), the outgoing 160 byte save area would be present.

It's true that the load of %r15 is now a nop.  It hadn't been at one point in
my development; ffi_prep_args had had more than 5 parameters, and so there was
extra stack allocated.  I suppose if ffi_prep_args were inlined, one could be
certain of this (since there will be no function calls) and document it as such.


r~