sys_tgkill: hand the syscall to the kernel in the standard way, rather

than doing it inline.  Doing it inline screws up on ppc32-linux if
we're sending an async signal to ourselves (the same thread) because
the kernel immediately hands the signal to async_sighandler() which
then dies at the assertion that this thread's state is VgTs_WaitSys.
From which I conclude this wrapper has always had a race against the
kernel which did not show up on x86 or amd64.  (and/or that I don't
understand this stuff too well)

git-svn-id: svn://svn.valgrind.org/valgrind/trunk@5169

diff --git a/coregrind/m_syswrap/syswrap-linux.c b/coregrind/m_syswrap/syswrap-linux.c
index 6c236d5a04a31d9ced5b1e71c7969d85cbf60e4a..faa6335a350b89aa5e955309c1e2e38983976abd 100644 (file)
--- a/coregrind/m_syswrap/syswrap-linux.c
+++ b/coregrind/m_syswrap/syswrap-linux.c
@@ -895,18 +895,30 @@ PRE(sys_tgkill)
       return;
    }
    
+   /* Check to see if this kill gave us a pending signal */
+   *flags |= SfPollAfter;
+
+   if (VG_(clo_trace_signals))
+      VG_(message)(Vg_DebugMsg, "tgkill: sending signal %d to pid %d/%d",
+                  ARG3, ARG1, ARG2);
+
    /* If we're sending SIGKILL, check to see if the target is one of
       our threads and handle it specially. */
-   if (ARG3 == VKI_SIGKILL && ML_(do_sigkill)(ARG2, ARG1))
+   if (ARG3 == VKI_SIGKILL && ML_(do_sigkill)(ARG2, ARG1)) {
       SET_STATUS_Success(0);
-   else
-      SET_STATUS_from_SysRes(VG_(do_syscall3)(SYSNO, ARG1, ARG2, ARG3));
+      return;
+   }
 
-   if (VG_(clo_trace_signals))
-      VG_(message)(Vg_DebugMsg, "tgkill: sent signal %d to pid %d/%d",
-                  ARG3, ARG1, ARG2);
-   /* Check to see if this kill gave us a pending signal */
-   *flags |= SfPollAfter;
+   /* Ask to handle this syscall via the slow route, since that's the
+      only one that sets tst->status to VgTs_WaitSys.  If the result
+      of doing the syscall is an immediate run of
+      async_signalhandler() in m_signals, then we need the thread to
+      be properly tidied away.  I have the impression the previous
+      version of this wrapper worked on x86/amd64 only because the
+      kernel did not immediately deliver the async signal to this
+      thread (on ppc it did, which broke the assertion re tst->status
+      at the top of async_signalhandler()). */
+   *flags |= SfMayBlock;
 }
 POST(sys_tgkill)
 {

diff --git a/coregrind/m_syswrap/syswrap-main.c b/coregrind/m_syswrap/syswrap-main.c
index 469eb4aa981608345661f9e7ae09b84098b753dc..de85593c77808a0fbfef38b9fd9450d1502304ac 100644 (file)
--- a/coregrind/m_syswrap/syswrap-main.c
+++ b/coregrind/m_syswrap/syswrap-main.c
@@ -713,9 +713,9 @@ void VG_(client_syscall) ( ThreadId tid )
          by doing it directly in this thread, which is a lot
          simpler. */
 
-      /* Check that the given flags are allowable: MayBlock and
-         PostOnFail are ok. */
-      vg_assert(0 == (sci->flags & ~(SfMayBlock | SfPostOnFail)));
+      /* Check that the given flags are allowable: MayBlock, PollAfter
+         and PostOnFail are ok. */
+      vg_assert(0 == (sci->flags & ~(SfMayBlock | SfPostOnFail | SfPollAfter)));
 
       if (sci->flags & SfMayBlock) {