From 9f03ed12a608344f275244e59051fb876a4aa06c Mon Sep 17 00:00:00 2001 From: Elliott Hughes Date: Wed, 29 Jul 2015 22:24:13 -0700 Subject: [PATCH] Stop sending SIGPIPE to debuggerd. SIGPIPE is a pretty normal way for command-line apps to die, but because we catch it and report it via debuggerd, we get a lot of bogus bugs. We could catch SIGPIPE in our tools, but that's not really legit and slightly misleading. "But", you say, "catching SIGPIPE is useful for app bugs!". Except a trawl through buganizer suggests it's misleading there too. Not least because it's usually an innocent victim that dies --- the problem is usually on the other end of the pipe (which you learn nothing about because that process already died, which is what closed the pipe). We also don't catch SIGALRM, which is another signal that will terminate your process if you don't catch it, but that one actually represents a logic error in the crashing process, so there's a stronger argument for catching that. (Except it too is not a real source of bugs.) Bug: http://b/20659371 Change-Id: I79820b36573ddaa9a7bad0561a52f23e7a8d15ac --- linker/debugger.cpp | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/linker/debugger.cpp b/linker/debugger.cpp index 46c97af91..3731c992a 100644 --- a/linker/debugger.cpp +++ b/linker/debugger.cpp @@ -135,9 +135,6 @@ static void log_signal_summary(int signum, const siginfo_t* info) { signal_name = "SIGILL"; has_address = true; break; - case SIGPIPE: - signal_name = "SIGPIPE"; - break; case SIGSEGV: signal_name = "SIGSEGV"; has_address = true; @@ -273,7 +270,7 @@ static void debuggerd_signal_handler(int signal_number, siginfo_t* info, void*) signal(signal_number, SIG_DFL); // These signals are not re-thrown when we resume. This means that - // crashing due to (say) SIGPIPE doesn't work the way you'd expect it + // crashing due to (say) SIGABRT doesn't work the way you'd expect it // to. We work around this by throwing them manually. We don't want // to do this for *all* signals because it'll screw up the si_addr for // faults like SIGSEGV. It does screw up the si_code, which is why we @@ -281,7 +278,6 @@ static void debuggerd_signal_handler(int signal_number, siginfo_t* info, void*) switch (signal_number) { case SIGABRT: case SIGFPE: - case SIGPIPE: #if defined(SIGSTKFLT) case SIGSTKFLT: #endif @@ -307,7 +303,6 @@ __LIBC_HIDDEN__ void debuggerd_init() { sigaction(SIGBUS, &action, nullptr); sigaction(SIGFPE, &action, nullptr); sigaction(SIGILL, &action, nullptr); - sigaction(SIGPIPE, &action, nullptr); sigaction(SIGSEGV, &action, nullptr); #if defined(SIGSTKFLT) sigaction(SIGSTKFLT, &action, nullptr);