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

[Dmitry Vyukov]

PTAL


================
Comment at: lib/sanitizer_common/sanitizer_stoptheworld_linux_libcdep.cc:416
@@ +415,3 @@
+      uptr waitpid_status = internal_waitpid(tracer_pid, NULL, __WALL);
+      if (!internal_iserror(waitpid_status, &local_errno))
+        break;
----------------
earthdok wrote:
> Wrong. You're assuming that internal_waitpid() will not touch errno. This is true only on x86_64 Linux where we have our custom wrappers implemented. Other platforms (MIPS, ARM) use the libc implementation (see sanitizer_syscall_generic.inc).
I am assuming nothing. This is existing code. I just properly handle EINTR.
Is there a simple way to fix other platforms?

================
Comment at: test/asan/TestCases/Linux/leak_check_segv.cc:2
@@ +1,3 @@
+// Test that SIGSEGV during leak checking does not crash the process.
+// RUN: %clangxx_asan -O1 %s -o %t -pthread && LSAN_OPTIONS="verbosity=1" not %run %t 2>&1
+// REQUIRES: asan-64-bits
----------------
earthdok wrote:
> -pthread is unnecessary with asan
> if you want to use it at least be consistent (-pthread/-lpthread)
Done

================
Comment at: test/asan/TestCases/Linux/signal_during_stop_the_world.cc:5
@@ +4,3 @@
+
+// RUN: %clangxx_asan -O1 -lpthread %s -o %t && %run %t 2>&1 | FileCheck %s
+
----------------
earthdok wrote:
> see above
Done

================
Comment at: test/asan/TestCases/Linux/signal_during_stop_the_world.cc:37
@@ +36,3 @@
+    for (;;) {
+      kill(parent, SIGCHLD);
+      kill(parent, SIGPROF);
----------------
earthdok wrote:
> Why these two signals in particular?
no strong reason
it is reasonable if SIGCHLD unblocks waitpid
but on my system it seems that SIGCHLD does not, but SIGPROF does
added comment

================
Comment at: test/asan/TestCases/Linux/signal_during_stop_the_world.cc:43
@@ +42,3 @@
+  void *mem[1000];
+  for (int i = 0; i < 1000; i++)
+    mem[i] = malloc(1000);
----------------
earthdok wrote:
> This barely makes a difference, the time spent attaching to threads will probably be >> this.
> 
> If you want to slow down leak detection you could make a few large allocs and fill the memory with pointers to them. 
Removed this part

================
Comment at: test/asan/TestCases/Linux/signal_during_stop_the_world.cc:54
@@ +53,3 @@
+  waitpid(pid, 0, 0);
+  sleep(1);  // If the tracer thread still runs, give it time to crash.
+  for (int i = 0; i < 1000; i++)
----------------
earthdok wrote:
> There's zero guarantee that it will crash...
Episodic crashes are fine with me.
Still much better than the current absence of interesting tests.

http://reviews.llvm.org/D8032

EMAIL PREFERENCES
  http://reviews.llvm.org/settings/panel/emailpreferences/