Re: [Qemu-devel] [PATCH] linux-user: ppc64: don't use volatile register during safe_syscall

[Laurent Vivier]

Le 26/07/2018 à 19:15, Richard Henderson a écrit :
> On 07/25/2018 11:48 PM, Shivaprasad G Bhat wrote:
>> r11 is a volatile register on PPC as per calling conventions.
>> The safe_syscall code uses it to check if the signal_pending
>> is set during the safe_syscall. When a syscall is interrupted
>> on return from signal handling, the r11 might be corrupted
>> before we retry the syscall leading to a crash. The registers
>> r0-r13 are not to be used here as they have
>> volatile/designated/reserved usages. Change the code to use
>> r14 which is non-volatile and is appropriate for local use in
>> safe_syscall.
>>
>> Signed-off-by: Shivaprasad G Bhat <address@hidden>
>> ---
>> Steps to reproduce:
>> On PPC host, issue `qemu-ppc64le /usr/bin/cc -E -`
>> Attempt Ctrl-C, the issue is reproduced.
>>
>> Reference:
>> https://refspecs.linuxfoundation.org/ELF/ppc64/PPC-elf64abi-1.9.html#REG
>>
>>  linux-user/host/ppc64/safe-syscall.inc.S |    4 ++--
>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/linux-user/host/ppc64/safe-syscall.inc.S 
>> b/linux-user/host/ppc64/safe-syscall.inc.S
>> index d30050a67c..b0cbbe6a69 100644
>> --- a/linux-user/host/ppc64/safe-syscall.inc.S
>> +++ b/linux-user/host/ppc64/safe-syscall.inc.S
>> @@ -49,7 +49,7 @@ safe_syscall_base:
>>       *               and returns the result in r3
>>       * Shuffle everything around appropriately.
>>       */
>> -    mr      11, 3   /* signal_pending */
>> +    mr      14, 3   /* signal_pending */
> 
> I do see that I was incorrect in assuming that r11 would be unmodified.  But
> you can't simply write to a call-saved register -- you must preserve its value
> for the caller.
> 
> Saving the value requires that you find some space on, or create, a stack
> frame.  Note that there are two different conventions for _CALL_AIX and
> _CALL_ELF==2.

Can we guess the syscall ('sc') will not modify neither r11 nor r14, but
the function caller expects that r11 is not modified because it's the
environment pointer, and saves r14 because it's one of its local
variable it knows it has to preserve?

In this case, I think Shivaprasad's fix is correct.

Thanks,
Laurent