[PATCH] Move the signal handling logic into sanitizer_common

Dmitry Vyukov dvyukov at google.com
Fri Jan 24 04:19:00 PST 2014



================
Comment at: interception/interception.h:89
@@ -88,1 +88,3 @@
 
+// TODO(glider): move everything but the documentation to platform-specific
+// headers.
----------------
+1
this file hurts my brain


================
Comment at: sanitizer_common/sanitizer_linux.cc:506
@@ +505,3 @@
+bool IsDeadlySignal(int signum) {
+  return (signum == SIGSEGV) && common_flags()->handle_segv;
+}
----------------
Evgeniy Stepanov wrote:
> Why are we not catching sigbus, sigill, etc here? What's the difference from sigsegv?
common_flags()->handle_segv does not belong here, it must be checked somewhere else

================
Comment at: tsan/lit_tests/null_deref.cc:1
@@ +1,2 @@
+// RUN: %clangxx_tsan -O0 %s -o %t && not %t 2>&1 | FileCheck %s --check-prefix=CHECK-%os --check-prefix=CHECK
+
----------------
why do you need --check-prefix=CHECK-%os here?

================
Comment at: tsan/rtl/tsan_interceptors.cc:1665
@@ -1675,2 +1664,3 @@
   REAL(sigfillset)(&newact.sa_mask);
-  if (act->sa_handler != SIG_IGN && act->sa_handler != SIG_DFL) {
+  if (act->sa_handler != __sanitizer::sig_ign &&
+      act->sa_handler != __sanitizer::sig_dfl) {
----------------
here and below __sanitizer:: must not be needed, we always rely on "using namespace __sanitizer", otehrwise there would be too many __sanitizer::'s

================
Comment at: tsan/rtl/tsan_rtl.cc:245
@@ -243,1 +244,3 @@
+  SetSigactionCallback(tsan_sigaction_interceptor);
+  SetSignalCallback(tsan_signal_interceptor);
   InitializeLibIgnore();
----------------
move these 3 lines into InitializeInterceptors()

================
Comment at: tsan/rtl/tsan_defs.h:167
@@ +166,3 @@
+#ifndef TSAN_GO
+int tsan_sigaction_interceptor(
+    int sig, const __sanitizer_sigaction *act, __sanitizer_sigaction *old);
----------------
delete this from this file

================
Comment at: tsan/rtl/tsan_platform.h:136
@@ +135,3 @@
+#ifndef TSAN_GO
+void TSAN_OnSIGSEGV(int, void *siginfo, void *context);
+#endif
----------------
delete

================
Comment at: tsan/rtl/tsan_platform_linux.cc:363
@@ +362,3 @@
+#ifndef TSAN_GO
+void TSAN_OnSIGSEGV(int, void *siginfo, void *context) {
+  uptr addr = (uptr)((siginfo_t*)siginfo)->si_addr;
----------------
move into tsan_interceptors.cc

================
Comment at: tsan/rtl/tsan_platform_linux.cc:363
@@ +362,3 @@
+#ifndef TSAN_GO
+void TSAN_OnSIGSEGV(int, void *siginfo, void *context) {
+  uptr addr = (uptr)((siginfo_t*)siginfo)->si_addr;
----------------
Dmitry Vyukov wrote:
> move into tsan_interceptors.cc
please don't prefix things in tsan with tsan
they all are tsan

================
Comment at: tsan/rtl/tsan_platform_linux.cc:367
@@ +366,3 @@
+  // Write the first message using the bullet-proof write.
+  if (13 != internal_write(2, "TSAN:SIGSEGV\n", 13)) Die();
+  uptr pc, sp, bp;
----------------
just don't check return value and continue, it won't be worse
probably it can return EINTR, who knows
make the message "ThreadSanitizer: SIGSEGV\n", all tsan messages start with "ThreadSanitizer: "

================
Comment at: tsan/rtl/tsan_platform_linux.cc:370
@@ +369,3 @@
+  GetPcSpBp(context, &pc, &sp, &bp);
+  //ReportSIGSEGV(pc, sp, bp, addr);
+  Die();
----------------
why is this commented out?
this is the whole point of this whole patch, OS can perfectly print "SIGSEGV" itself


http://llvm-reviews.chandlerc.com/D2605



More information about the llvm-commits mailing list