restarting SYSCALL system call on amd64 loses arguments

[Konstantin Belousov]

On Mon, Sep 23, 2013 at 10:26:13PM +0200, Tijl Coosemans wrote:
> Has anyone taken a look at this PR yet?
> 
> http://www.freebsd.org/cgi/query-pr.cgi?pr=182161

This looks like a valid bug, but probably not a valid testcase.

Let me elaborate.  When a signal is delivered, return from the signal
handler is performed by the sigreturn(2), which reloads the whole
register file when crossing kernel->user boundary due to sys_sigreturn(9)
setting PCB_FULL_IRET flag.  As result, the whole trap frame at the
time of the syscall entry is restored, and ERESTART return is not
exercised.

I was not able to reproduce the issue with the supplied test program
on HEAD.  I suspect that the program actually exposed the bug in the
signal delivery in the threaded processes, which I introduced for 9.1
and fixed in r251047 & r251365.

On the other hand, I agree that at least arguments should be restored
for ERESTART case, but in fact it is easier and probably less error-prone
to restore whole register file again.  Please try the patch below for
your Go code.

diff --git a/sys/amd64/amd64/vm_machdep.c b/sys/amd64/amd64/vm_machdep.c
index b7c2b67..1e3d8f5 100644
--- a/sys/amd64/amd64/vm_machdep.c
+++ b/sys/amd64/amd64/vm_machdep.c
@@ -400,9 +400,13 @@ cpu_set_syscall_retval(struct thread *td, int error)
 		 * for the next iteration.
 		 * %r10 restore is only required for freebsd/amd64 processes,
 		 * but shall be innocent for any ia32 ABI.
+		 *
+		 * Require full context restore to get the arguments
+		 * in the registers reloaded at return to usermode.
 		 */
 		td->td_frame->tf_rip -= td->td_frame->tf_err;
 		td->td_frame->tf_r10 = td->td_frame->tf_rcx;
+		set_pcb_flags(td->td_pcb, PCB_FULL_IRET);
 		break;
 
 	case EJUSTRETURN:
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 834 bytes
Desc: not available
URL: <http://lists.freebsd.org/pipermail/freebsd-current/attachments/20130924/7daec25e/attachment.sig>