[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