crash_dump: set a watchdog timer.
PTRACE_DETACH is only necessary if the process is in group-stop state,
the tracer exiting is sufficient to detach and resume tracees.
Using this, set a 5 second timer with alarm(2) that just kills us, to
avoid leaving processes stopped.
Bug: http://b/34472671
Test: debuggerd_test
Test: crasher + manually inserting a 10 second sleep into crash_dump
Change-Id: Iacaa796f79037aa1585f3f2159abe45ef0069311
diff --git a/debuggerd/crash_dump.cpp b/debuggerd/crash_dump.cpp
index 1af00b4..cf6a715 100644
--- a/debuggerd/crash_dump.cpp
+++ b/debuggerd/crash_dump.cpp
@@ -257,6 +257,9 @@
exit(0);
}
+ // Die if we take too long.
+ alarm(20);
+
check_process(target_proc_fd, target);
std::string attach_error;
@@ -316,10 +319,9 @@
// Now that we have the signal that kicked things off, attach all of the
// sibling threads, and then proceed.
bool fatal_signal = signo != DEBUGGER_SIGNAL;
- int resume_signal = fatal_signal ? signo : 0;
std::set<pid_t> siblings;
std::set<pid_t> attached_siblings;
- if (resume_signal == 0) {
+ if (fatal_signal) {
if (!android::procinfo::GetProcessTids(target, &siblings)) {
PLOG(FATAL) << "failed to get process siblings";
}
@@ -352,40 +354,39 @@
attached_siblings, abort_address, fatal_signal ? &amfd_data : nullptr);
}
+ // We don't actually need to PTRACE_DETACH, as long as our tracees aren't in
+ // group-stop state, which is true as long as no stopping signals are sent.
+
bool wait_for_gdb = android::base::GetBoolProperty("debug.debuggerd.wait_for_gdb", false);
- if (wait_for_gdb) {
+ if (!fatal_signal || siginfo.si_code == SI_USER) {
// Don't wait_for_gdb when the process didn't actually crash.
- if (!fatal_signal) {
- wait_for_gdb = false;
- } else {
- // Use ALOGI to line up with output from engrave_tombstone.
- ALOGI(
- "***********************************************************\n"
- "* Process %d has been suspended while crashing.\n"
- "* To attach gdbserver and start gdb, run this on the host:\n"
- "*\n"
- "* gdbclient.py -p %d\n"
- "*\n"
- "***********************************************************",
- target, main_tid);
- }
+ wait_for_gdb = false;
}
- for (pid_t tid : attached_siblings) {
- // Don't send the signal to sibling threads.
- if (ptrace(PTRACE_DETACH, tid, 0, wait_for_gdb ? SIGSTOP : 0) != 0) {
- PLOG(ERROR) << "ptrace detach from " << tid << " failed";
+ // If the process crashed or we need to send it SIGSTOP for wait_for_gdb,
+ // get it in a state where it can receive signals, and then send the relevant
+ // signal.
+ if (wait_for_gdb || fatal_signal) {
+ if (ptrace(PTRACE_INTERRUPT, main_tid, 0, 0) != 0) {
+ PLOG(ERROR) << "failed to use PTRACE_INTERRUPT on " << main_tid;
}
- }
- if (ptrace(PTRACE_DETACH, main_tid, 0, wait_for_gdb ? SIGSTOP : resume_signal)) {
- PLOG(ERROR) << "ptrace detach from main thread " << main_tid << " failed";
+ if (tgkill(target, main_tid, wait_for_gdb ? SIGSTOP : signo) != 0) {
+ PLOG(ERROR) << "failed to resend signal " << signo << " to " << main_tid;
+ }
}
if (wait_for_gdb) {
- if (tgkill(target, main_tid, resume_signal) != 0) {
- PLOG(ERROR) << "failed to resend signal to process " << target;
- }
+ // Use ALOGI to line up with output from engrave_tombstone.
+ ALOGI(
+ "***********************************************************\n"
+ "* Process %d has been suspended while crashing.\n"
+ "* To attach gdbserver and start gdb, run this on the host:\n"
+ "*\n"
+ "* gdbclient.py -p %d\n"
+ "*\n"
+ "***********************************************************",
+ target, main_tid);
}
if (fatal_signal) {
diff --git a/debuggerd/debuggerd_test.cpp b/debuggerd/debuggerd_test.cpp
index f51b5f2..3b24193 100644
--- a/debuggerd/debuggerd_test.cpp
+++ b/debuggerd/debuggerd_test.cpp
@@ -340,24 +340,15 @@
AssertDeath(SIGABRT);
}
+// wait_for_gdb shouldn't trigger on manually sent signals.
TEST_F(CrasherTest, wait_for_gdb_signal) {
if (!android::base::SetProperty(kWaitForGdbKey, "1")) {
FAIL() << "failed to enable wait_for_gdb";
}
StartCrasher("abort");
- ASSERT_EQ(0, kill(crasher_pid, SIGABRT)) << strerror(errno);
-
- std::this_thread::sleep_for(500ms);
-
- int status;
- ASSERT_EQ(crasher_pid, (TIMEOUT(1, waitpid(crasher_pid, &status, WUNTRACED))));
- ASSERT_TRUE(WIFSTOPPED(status));
- ASSERT_EQ(SIGSTOP, WSTOPSIG(status));
-
- ASSERT_EQ(0, kill(crasher_pid, SIGCONT)) << strerror(errno);
-
- AssertDeath(SIGABRT);
+ ASSERT_EQ(0, kill(crasher_pid, SIGSEGV)) << strerror(errno);
+ AssertDeath(SIGSEGV);
}
TEST_F(CrasherTest, backtrace) {