diff --git a/debuggerd/backtrace.cpp b/debuggerd/backtrace.cpp index c4a2143cf..e7919e9ec 100644 --- a/debuggerd/backtrace.cpp +++ b/debuggerd/backtrace.cpp @@ -88,7 +88,9 @@ static void dump_thread( return; } - wait_for_stop(tid, total_sleep_time_usec); + if (!attached && wait_for_sigstop(tid, total_sleep_time_usec, detach_failed) == -1) { + return; + } UniquePtr backtrace(Backtrace::Create(tid, BACKTRACE_CURRENT_THREAD)); if (backtrace->Unwind(0)) { diff --git a/debuggerd/debuggerd.cpp b/debuggerd/debuggerd.cpp index 03d7e490b..318b22414 100644 --- a/debuggerd/debuggerd.cpp +++ b/debuggerd/debuggerd.cpp @@ -79,7 +79,7 @@ static void wait_for_user_action(const debugger_request_t &request) { "*\n" "* Wait for gdb to start, then press the VOLUME DOWN key\n" "* to let the process continue crashing.\n" - "********************************************************\n", + "********************************************************", request.pid, exe, request.tid); // Wait for VOLUME DOWN. @@ -178,11 +178,11 @@ static int read_request(int fd, debugger_request_t* out_request) { socklen_t len = sizeof(cr); int status = getsockopt(fd, SOL_SOCKET, SO_PEERCRED, &cr, &len); if (status != 0) { - ALOGE("cannot get credentials\n"); + ALOGE("cannot get credentials"); return -1; } - ALOGV("reading tid\n"); + ALOGV("reading tid"); fcntl(fd, F_SETFL, O_NONBLOCK); pollfd pollfds[1]; @@ -279,6 +279,7 @@ static void handle_request(int fd) { ALOGE("ptrace attach failed: %s\n", strerror(errno)); } else { bool detach_failed = false; + bool tid_unresponsive = false; bool attach_gdb = should_attach_gdb(&request); if (TEMP_FAILURE_RETRY(write(fd, "\0", 1)) != 1) { ALOGE("failed responding to client: %s\n", strerror(errno)); @@ -292,8 +293,9 @@ static void handle_request(int fd) { int total_sleep_time_usec = 0; for (;;) { - int signal = wait_for_signal(request.tid, &total_sleep_time_usec); - if (signal < 0) { + int signal = wait_for_sigstop(request.tid, &total_sleep_time_usec, &detach_failed); + if (signal == -1) { + tid_unresponsive = true; break; } @@ -360,27 +362,21 @@ static void handle_request(int fd) { free(tombstone_path); } - ALOGV("detaching\n"); - if (attach_gdb) { - // stop the process so we can debug - kill(request.pid, SIGSTOP); - - // detach so we can attach gdbserver - if (ptrace(PTRACE_DETACH, request.tid, 0, 0)) { - ALOGE("ptrace detach from %d failed: %s\n", request.tid, strerror(errno)); - detach_failed = true; + if (!tid_unresponsive) { + ALOGV("detaching"); + if (attach_gdb) { + // stop the process so we can debug + kill(request.pid, SIGSTOP); } - - // if debug.db.uid is set, its value indicates if we should wait - // for user action for the crashing process. - // in this case, we log a message and turn the debug LED on - // waiting for a gdb connection (for instance) - wait_for_user_action(request); - } else { - // just detach if (ptrace(PTRACE_DETACH, request.tid, 0, 0)) { - ALOGE("ptrace detach from %d failed: %s\n", request.tid, strerror(errno)); + ALOGE("ptrace detach from %d failed: %s", request.tid, strerror(errno)); detach_failed = true; + } else if (attach_gdb) { + // if debug.db.uid is set, its value indicates if we should wait + // for user action for the crashing process. + // in this case, we log a message and turn the debug LED on + // waiting for a gdb connection (for instance) + wait_for_user_action(request); } } diff --git a/debuggerd/tombstone.cpp b/debuggerd/tombstone.cpp index 32f0eca08..5b111abc9 100755 --- a/debuggerd/tombstone.cpp +++ b/debuggerd/tombstone.cpp @@ -374,11 +374,7 @@ static void dump_nearby_maps(BacktraceMap* map, log_t* log, pid_t tid) { } } -static void dump_thread( - Backtrace* backtrace, log_t* log, int* total_sleep_time_usec) { - - wait_for_stop(backtrace->Tid(), total_sleep_time_usec); - +static void dump_thread(Backtrace* backtrace, log_t* log) { dump_registers(log, backtrace->Tid()); dump_backtrace_and_stack(backtrace, log); @@ -421,13 +417,17 @@ static bool dump_sibling_thread_report( continue; } + if (wait_for_sigstop(new_tid, total_sleep_time_usec, &detach_failed) == -1) { + continue; + } + log->current_tid = new_tid; _LOG(log, logtype::THREAD, "--- --- --- --- --- --- --- --- --- --- --- --- --- --- --- ---\n"); dump_thread_info(log, pid, new_tid); UniquePtr backtrace(Backtrace::Create(pid, new_tid, map)); if (backtrace->Unwind(0)) { - dump_thread(backtrace.get(), log, total_sleep_time_usec); + dump_thread(backtrace.get(), log); } log->current_tid = log->crashed_tid; @@ -628,7 +628,7 @@ static bool dump_crash(log_t* log, pid_t pid, pid_t tid, int signal, int si_code UniquePtr backtrace(Backtrace::Create(pid, tid, map.get())); if (backtrace->Unwind(0)) { dump_abort_message(backtrace.get(), log, abort_msg_address); - dump_thread(backtrace.get(), log, total_sleep_time_usec); + dump_thread(backtrace.get(), log); } if (want_logs) { diff --git a/debuggerd/utility.cpp b/debuggerd/utility.cpp index 9a30fe3de..2baf9de19 100644 --- a/debuggerd/utility.cpp +++ b/debuggerd/utility.cpp @@ -28,8 +28,8 @@ #include #include -const int sleep_time_usec = 50000; // 0.05 seconds -const int max_total_sleep_usec = 10000000; // 10 seconds +const int SLEEP_TIME_USEC = 50000; // 0.05 seconds +const int MAX_TOTAL_SLEEP_USEC = 10000000; // 10 seconds static int write_to_am(int fd, const char* buf, int len) { int to_write = len; @@ -91,48 +91,44 @@ void _LOG(log_t* log, enum logtype ltype, const char* fmt, ...) { } } -int wait_for_signal(pid_t tid, int* total_sleep_time_usec) { +int wait_for_sigstop(pid_t tid, int* total_sleep_time_usec, bool* detach_failed) { + bool allow_dead_tid = false; for (;;) { int status; - pid_t n = waitpid(tid, &status, __WALL | WNOHANG); - if (n < 0) { - if (errno == EAGAIN) - continue; - ALOGE("waitpid failed: %s\n", strerror(errno)); - return -1; - } else if (n > 0) { - ALOGV("waitpid: n=%d status=%08x\n", n, status); + pid_t n = TEMP_FAILURE_RETRY(waitpid(tid, &status, __WALL | WNOHANG)); + if (n == -1) { + ALOGE("waitpid failed: tid %d, %s", tid, strerror(errno)); + break; + } else if (n == tid) { if (WIFSTOPPED(status)) { return WSTOPSIG(status); } else { ALOGE("unexpected waitpid response: n=%d, status=%08x\n", n, status); - return -1; + // This is the only circumstance under which we can allow a detach + // to fail with ESRCH, which indicates the tid has exited. + allow_dead_tid = true; + break; } } - if (*total_sleep_time_usec > max_total_sleep_usec) { - ALOGE("timed out waiting for tid=%d to die\n", tid); - return -1; - } - - // not ready yet - ALOGV("not ready yet\n"); - usleep(sleep_time_usec); - *total_sleep_time_usec += sleep_time_usec; - } -} - -void wait_for_stop(pid_t tid, int* total_sleep_time_usec) { - siginfo_t si; - while (TEMP_FAILURE_RETRY(ptrace(PTRACE_GETSIGINFO, tid, 0, &si)) < 0 && errno == ESRCH) { - if (*total_sleep_time_usec > max_total_sleep_usec) { - ALOGE("timed out waiting for tid=%d to stop\n", tid); + if (*total_sleep_time_usec > MAX_TOTAL_SLEEP_USEC) { + ALOGE("timed out waiting for stop signal: tid=%d", tid); break; } - usleep(sleep_time_usec); - *total_sleep_time_usec += sleep_time_usec; + usleep(SLEEP_TIME_USEC); + *total_sleep_time_usec += SLEEP_TIME_USEC; } + + if (ptrace(PTRACE_DETACH, tid, 0, 0) != 0) { + if (allow_dead_tid && errno == ESRCH) { + ALOGE("tid exited before attach completed: tid %d", tid); + } else { + *detach_failed = true; + ALOGE("detach failed: tid %d, %s", tid, strerror(errno)); + } + } + return -1; } #if defined (__mips__) diff --git a/debuggerd/utility.h b/debuggerd/utility.h index 82413b876..58a882c2b 100644 --- a/debuggerd/utility.h +++ b/debuggerd/utility.h @@ -67,12 +67,11 @@ enum logtype { LOGS }; -/* Log information onto the tombstone. */ +// Log information onto the tombstone. void _LOG(log_t* log, logtype ltype, const char *fmt, ...) __attribute__ ((format(printf, 3, 4))); -int wait_for_signal(pid_t tid, int* total_sleep_time_usec); -void wait_for_stop(pid_t tid, int* total_sleep_time_usec); +int wait_for_sigstop(pid_t, int*, bool*); void dump_memory(log_t* log, pid_t tid, uintptr_t addr);