[compiler-rt] r229832 - tsan: fix PTRACE_ATTACH handling during stop-the-world

[Dmitry Vyukov]

Author: dvyukov
Date: Thu Feb 19 03:02:29 2015
New Revision: 229832

URL: http://llvm.org/viewvc/llvm-project?rev=229832&view=rev
Log:
tsan: fix PTRACE_ATTACH handling during stop-the-world

If the thread receives a signal concurrently with PTRACE_ATTACH,
we can get notification about the signal before notification about stop.
In such case we need to forward the signal to the thread, otherwise
the signal will be missed (as we do PTRACE_DETACH with arg=0) and
any logic relying on signals will break. After forwarding we need to
continue to wait for stopping, because the thread is not stopped yet.
We do ignore delivery of SIGSTOP, because we want to make stop-the-world
as invisible as possible.

http://reviews.llvm.org/D7723

 --This line, and those below, will be ignored--
M    lib/sanitizer_common/sanitizer_stoptheworld_linux_libcdep.cc
M    test/tsan/signal_segv_handler.cc

Modified:
    compiler-rt/trunk/lib/sanitizer_common/sanitizer_stoptheworld_linux_libcdep.cc
    compiler-rt/trunk/test/tsan/signal_segv_handler.cc

Modified: compiler-rt/trunk/lib/sanitizer_common/sanitizer_stoptheworld_linux_libcdep.cc
URL: http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/lib/sanitizer_common/sanitizer_stoptheworld_linux_libcdep.cc?rev=229832&r1=229831&r2=229832&view=diff
==============================================================================
--- compiler-rt/trunk/lib/sanitizer_common/sanitizer_stoptheworld_linux_libcdep.cc (original)
+++ compiler-rt/trunk/lib/sanitizer_common/sanitizer_stoptheworld_linux_libcdep.cc Thu Feb 19 03:02:29 2015
@@ -89,36 +89,50 @@ class ThreadSuspender {
   bool SuspendThread(SuspendedThreadID thread_id);
 };
 
-bool ThreadSuspender::SuspendThread(SuspendedThreadID thread_id) {
+bool ThreadSuspender::SuspendThread(SuspendedThreadID tid) {
   // Are we already attached to this thread?
   // Currently this check takes linear time, however the number of threads is
   // usually small.
-  if (suspended_threads_list_.Contains(thread_id))
+  if (suspended_threads_list_.Contains(tid))
     return false;
   int pterrno;
-  if (internal_iserror(internal_ptrace(PTRACE_ATTACH, thread_id, NULL, NULL),
+  if (internal_iserror(internal_ptrace(PTRACE_ATTACH, tid, NULL, NULL),
                        &pterrno)) {
     // Either the thread is dead, or something prevented us from attaching.
     // Log this event and move on.
-    VReport(1, "Could not attach to thread %d (errno %d).\n", thread_id,
-            pterrno);
+    VReport(1, "Could not attach to thread %d (errno %d).\n", tid, pterrno);
     return false;
   } else {
-    VReport(1, "Attached to thread %d.\n", thread_id);
+    VReport(1, "Attached to thread %d.\n", tid);
     // The thread is not guaranteed to stop before ptrace returns, so we must
-    // wait on it.
-    uptr waitpid_status;
-    HANDLE_EINTR(waitpid_status, internal_waitpid(thread_id, NULL, __WALL));
-    int wperrno;
-    if (internal_iserror(waitpid_status, &wperrno)) {
-      // Got a ECHILD error. I don't think this situation is possible, but it
-      // doesn't hurt to report it.
-      VReport(1, "Waiting on thread %d failed, detaching (errno %d).\n",
-              thread_id, wperrno);
-      internal_ptrace(PTRACE_DETACH, thread_id, NULL, NULL);
-      return false;
+    // wait on it. Note: if the thread receives a signal concurrently,
+    // we can get notification about the signal before notification about stop.
+    // In such case we need to forward the signal to the thread, otherwise
+    // the signal will be missed (as we do PTRACE_DETACH with arg=0) and
+    // any logic relying on signals will break. After forwarding we need to
+    // continue to wait for stopping, because the thread is not stopped yet.
+    // We do ignore delivery of SIGSTOP, because we want to make stop-the-world
+    // as invisible as possible.
+    for (;;) {
+      int status;
+      uptr waitpid_status;
+      HANDLE_EINTR(waitpid_status, internal_waitpid(tid, &status, __WALL));
+      int wperrno;
+      if (internal_iserror(waitpid_status, &wperrno)) {
+        // Got a ECHILD error. I don't think this situation is possible, but it
+        // doesn't hurt to report it.
+        VReport(1, "Waiting on thread %d failed, detaching (errno %d).\n",
+                tid, wperrno);
+        internal_ptrace(PTRACE_DETACH, tid, NULL, NULL);
+        return false;
+      }
+      if (WIFSTOPPED(status) && WSTOPSIG(status) != SIGSTOP) {
+        internal_ptrace(PTRACE_CONT, tid, 0, (void*)(uptr)WSTOPSIG(status));
+        continue;
+      }
+      break;
     }
-    suspended_threads_list_.Append(thread_id);
+    suspended_threads_list_.Append(tid);
     return true;
   }
 }

Modified: compiler-rt/trunk/test/tsan/signal_segv_handler.cc
URL: http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/test/tsan/signal_segv_handler.cc?rev=229832&r1=229831&r2=229832&view=diff
==============================================================================
--- compiler-rt/trunk/test/tsan/signal_segv_handler.cc (original)
+++ compiler-rt/trunk/test/tsan/signal_segv_handler.cc Thu Feb 19 03:02:29 2015
@@ -24,9 +24,6 @@ void handler(int signo, siginfo_t *info,
 }
 
 int main() {
-  // Disabled for now, because there is another issue in ptrace.
-  // It will be addressed in a subsequent change.
-  goto done;
   struct sigaction a;
   a.sa_sigaction = handler;
   a.sa_flags = SA_SIGINFO;
@@ -36,7 +33,6 @@ int main() {
     mprotect(guard, 4096, PROT_NONE);
     *(int*)guard = 1;
   }
- done:
   fprintf(stderr, "DONE\n");
 }