PERFORCE change 152343 for review

Peter Wemm peter at FreeBSD.org
Sat Nov 1 19:56:51 PDT 2008


http://perforce.freebsd.org/chv.cgi?CH=152343

Change 152343 by peter at peter_overcee on 2008/11/02 02:56:32

	Hopefully close the signal reception vs unmasked syscalls race that was
	unique to the FreeBSD async syscall handler.  On the other valgrind platforms
	the results of the async syscall are written directly into the guest state.
	The problem is that the 'carry' return can't be done like that as it isn't
	something we can write to in asm.  Instead export it and track its progress
	through the stack and fix it up when needed.

Affected files ...

.. //depot/projects/valgrind/coregrind/m_syswrap/syscall-amd64-freebsd.S#5 edit
.. //depot/projects/valgrind/coregrind/m_syswrap/syscall-x86-freebsd.S#7 edit
.. //depot/projects/valgrind/coregrind/m_syswrap/syswrap-main.c#12 edit

Differences ...

==== //depot/projects/valgrind/coregrind/m_syswrap/syscall-amd64-freebsd.S#5 (text+ko) ====

@@ -159,8 +159,6 @@
 	movq	%rax, OFFSET_amd64_RAX(%rsi)	/* save back to RAX */
 	movq	%rdx, OFFSET_amd64_RDX(%rsi)	/* save back to RDX */
 
-	/* QQQ Race here.  see syscall-x86-freebsd.S comment */
-
 4:	/* Re-block signals.  If eip is in [4,5), then the syscall 
 	   is complete and we needn't worry about it. */
 
@@ -178,15 +176,9 @@
 
 5:	/* now safe from signals */
 
-	PUSH_di_si_dx_cx_8
 	/* Export carry state */
-	movq	%r15,%rdi
-	andq	$1, %rdi
-	/* rsi still --> VexGuestAMD64State * */
-	call	LibVEX_GuestAMD64_put_rflag_c
-	POP_di_si_dx_cx_8
-
-	movq	$0, %rax	/* SUCCESS */
+	movq	%r15,%rax
+	andq	$1, %rax	/* SUCCESS */
 	popq	%r15
 	popq	%r14
 	popq	%r13

==== //depot/projects/valgrind/coregrind/m_syswrap/syscall-x86-freebsd.S#7 (text+ko) ====

@@ -153,6 +153,7 @@
 
 4:	/* Re-block signals.  If eip is in [4,5), then the syscall is complete and 
 	   we needn't worry about it. */
+	/* QQQ: However, on FreeBSD, the trap handler has to export just carry */
 	movl	$__NR_sigprocmask, %eax
 	movl	$VKI_SIG_SETMASK, %ecx
 	movl	%ecx, 4(%esp)
@@ -166,14 +167,8 @@
 5:	/* now safe from signals */
 
 	/* Export carry state */
-	movl	4+FSZ(%esp), %ebx
-	pushl	%ebx	/* guest state * */
-	andl	$1, %edi
-	pushl	%edi	/* carry flag */
-	call	LibVEX_GuestX86_put_eflag_c
-	addl	$8, %esp
-
-	movl	$0, %eax	/* SUCCESS */
+	movl	%edi, %eax
+	andl	$1, %eax	/* SUCCESS */
 	addl	$(9*4), %esp	/* args + fake return address */
 	popl	%ebp
 	popl	%ebx

==== //depot/projects/valgrind/coregrind/m_syswrap/syswrap-main.c#12 (text+ko) ====

@@ -264,6 +264,22 @@
 #          endif
            , args
         );
+#if defined(VGP_x86_freebsd)
+   /* On FreeBSD, the success/fail status is returned */
+   if (err == 1) {
+      LibVEX_Guestx86_put_eflag_c(1, &tst->arch.vex);
+      err = 0;
+   } else {
+      LibVEX_Guestx86_put_eflag_c(0, &tst->arch.vex);
+   }
+#elif defined(VGP_amd64_freebsd)
+   if (err == 1) {
+      LibVEX_GuestAMD64_put_rflag_c(1, &tst->arch.vex);
+      err = 0;
+   } else {
+      LibVEX_GuestAMD64_put_rflag_c(0, &tst->arch.vex);
+   }
+#endif
    vg_assert2(
       err == 0,
       "ML_(do_syscall_for_client_WRK): sigprocmask error %d",
@@ -1614,6 +1630,23 @@
       /* Result committed, but the signal mask has not been restored;
          we expect our caller (the signal handler) will have fixed
          this up. */
+#if defined(VGP_x86_freebsd)
+      /* On FreeBSD, the success/fail status is returned to the caller
+	 and still has to be fixed up here. */
+      if (!(sci->flags & SfNoWriteResult)) {
+        if (sres.isError)
+           LibVEX_Guestx86_put_eflag_c(1, &th_regs->vex);
+        else
+           LibVEX_Guestx86_put_eflag_c(0, &th_regs->vex);
+      }
+#elif defined(VGP_amd64_freebsd)
+      if (!(sci->flags & SfNoWriteResult)) {
+        if (sres.isError)
+           LibVEX_GuestAMD64_put_rflag_c(1, &th_regs->vex);
+        else
+           LibVEX_GuestAMD64_put_rflag_c(0, &th_regs->vex);
+      }
+#endif
       if (debug)
          VG_(printf)("  all done\n");
       VG_(post_syscall)(tid);


More information about the p4-projects mailing list