[PATCH] tsan: fix PTRACE_ATTACH handling during stop-the-world

[Dmitry Vyukov]

removed done label


http://reviews.llvm.org/D7723

Files:
  lib/sanitizer_common/sanitizer_stoptheworld_linux_libcdep.cc
  test/tsan/signal_segv_handler.cc

Index: lib/sanitizer_common/sanitizer_stoptheworld_linux_libcdep.cc
===================================================================
--- lib/sanitizer_common/sanitizer_stoptheworld_linux_libcdep.cc
+++ lib/sanitizer_common/sanitizer_stoptheworld_linux_libcdep.cc
@@ -89,36 +89,50 @@
   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;
   }
 }
Index: test/tsan/signal_segv_handler.cc
===================================================================
--- test/tsan/signal_segv_handler.cc
+++ test/tsan/signal_segv_handler.cc
@@ -24,9 +24,6 @@
 }
 
 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 @@
     mprotect(guard, 4096, PROT_NONE);
     *(int*)guard = 1;
   }
- done:
   fprintf(stderr, "DONE\n");
 }

EMAIL PREFERENCES
  http://reviews.llvm.org/settings/panel/emailpreferences/
-------------- next part --------------
A non-text attachment was scrubbed...
Name: D7723.20199.patch
Type: text/x-patch
Size: 3921 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20150218/7f88b818/attachment.bin>