Le 30/07/2018 à 14:44, Richard Henderson a écrit : > On 07/30/2018 06:09 AM, 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. >> Use SP+16 which is a slot for LR, for save/restore of previous value >> of r14. SP+16 can be used, as LR is preserved across the syscall. >> >> Steps to reproduce: >> On PPC host, issue `qemu-x86_64 /usr/bin/cc -E -` >> Attempt Ctrl-C, the issue is reproduced. >> >> Reference: >> https://refspecs.linuxfoundation.org/ELF/ppc64/PPC-elf64abi-1.9.html#REG >> https://openpowerfoundation.org/wp-content/uploads/2016/03/ABI64BitOpenPOWERv1.1_16July2015_pub4.pdf >> >> Signed-off-by: Shivaprasad G Bhat <[email protected]> > > This does fix the bug, so > Tested-by: Richard Henderson <[email protected]> > > But we can do slightly better: > >> @@ -49,7 +49,8 @@ safe_syscall_base: >> * and returns the result in r3 >> * Shuffle everything around appropriately. >> */ >> - mr 11, 3 /* signal_pending */ >> + std 14, 16(1) /* Preserve r14 in SP+16 */ > > Above this context, we have a .cfi_startproc directive, which indicates we are > providing unwind information for this function (trivial up to this point). > > To preserve that, you need to add > > .cfi_offset 14, 16 > > right here, indicating r14 is saved 16 bytes "up" the stack frame. > > Since this portion of the stack frame is allocated by the caller, this saved > value remains valid through the end of the function, so we do not need to do > anything at the points where the value is restored. > >> safe_syscall_end: >> + ld 14, 16(1) /* restore r14 to its original value */ >> /* code path when we did execute the syscall */ > > Swap these two lines. > > With those two changes, > Reviewed-by: Richard Henderson <[email protected]>
Tested-by: Laurent Vivier <[email protected]> Reviewed-by: Laurent Vivier <[email protected]> I think this patch should go into the next -rc because it fixes a lot of failures in the LTP on ppc64 host (hundreds of failure...). David, if you want, I can take it through the linux-user tree. Thanks, Laurent
