diff --git a/debuggerd/backtrace.cpp b/debuggerd/backtrace.cpp index 32843d860..8f4a53f57 100644 --- a/debuggerd/backtrace.cpp +++ b/debuggerd/backtrace.cpp @@ -29,6 +29,7 @@ #include #include +#include #include @@ -96,11 +97,11 @@ static void dump_thread(log_t* log, BacktraceMap* map, pid_t pid, pid_t tid) { } } -void dump_backtrace(int fd, int amfd, BacktraceMap* map, pid_t pid, pid_t tid, - const std::set& siblings) { +void dump_backtrace(int fd, BacktraceMap* map, pid_t pid, pid_t tid, + const std::set& siblings, std::string* amfd_data) { log_t log; log.tfd = fd; - log.amfd = amfd; + log.amfd_data = amfd_data; dump_process_header(&log, pid); dump_thread(&log, map, pid, tid); diff --git a/debuggerd/backtrace.h b/debuggerd/backtrace.h index 98c433b6b..acd5eaac6 100644 --- a/debuggerd/backtrace.h +++ b/debuggerd/backtrace.h @@ -20,6 +20,7 @@ #include #include +#include #include "utility.h" @@ -28,8 +29,8 @@ class BacktraceMap; // Dumps a backtrace using a format similar to what Dalvik uses so that the result // can be intermixed in a bug report. -void dump_backtrace(int fd, int amfd, BacktraceMap* map, pid_t pid, pid_t tid, - const std::set& siblings); +void dump_backtrace(int fd, BacktraceMap* map, pid_t pid, pid_t tid, + const std::set& siblings, std::string* amfd_data); /* Dumps the backtrace in the backtrace data structure to the log. */ void dump_backtrace_to_log(Backtrace* backtrace, log_t* log, const char* prefix); diff --git a/debuggerd/debuggerd.cpp b/debuggerd/debuggerd.cpp index 7f0e5bb19..b90a5e00c 100644 --- a/debuggerd/debuggerd.cpp +++ b/debuggerd/debuggerd.cpp @@ -14,6 +14,7 @@ * limitations under the License. */ +#include #include #include #include @@ -32,12 +33,15 @@ #include #include +#include #include +#include #include #include +#include #include #include #include @@ -288,6 +292,41 @@ static int activity_manager_connect() { return amfd.release(); } +static void activity_manager_write(int pid, int signal, int amfd, const std::string& amfd_data) { + if (amfd == -1) { + return; + } + + // Activity Manager protocol: binary 32-bit network-byte-order ints for the + // pid and signal number, followed by the raw text of the dump, culminating + // in a zero byte that marks end-of-data. + uint32_t datum = htonl(pid); + if (!android::base::WriteFully(amfd, &datum, 4)) { + ALOGE("AM pid write failed: %s\n", strerror(errno)); + return; + } + datum = htonl(signal); + if (!android::base::WriteFully(amfd, &datum, 4)) { + ALOGE("AM signal write failed: %s\n", strerror(errno)); + return; + } + + if (!android::base::WriteFully(amfd, amfd_data.c_str(), amfd_data.size())) { + ALOGE("AM data write failed: %s\n", strerror(errno)); + return; + } + + // Send EOD to the Activity Manager, then wait for its ack to avoid racing + // ahead and killing the target out from under it. + uint8_t eodMarker = 0; + if (!android::base::WriteFully(amfd, &eodMarker, 1)) { + ALOGE("AM eod write failed: %s\n", strerror(errno)); + return; + } + // 3 sec timeout reading the ack; we're fine if the read fails. + android::base::ReadFully(amfd, &eodMarker, 1); +} + static bool should_attach_gdb(const debugger_request_t& request) { if (request.action == DEBUGGER_ACTION_CRASH) { return property_get_bool("debug.debuggerd.wait_for_gdb", false); @@ -415,7 +454,7 @@ static void ptrace_siblings(pid_t pid, pid_t main_tid, std::set& tids) { static bool perform_dump(const debugger_request_t& request, int fd, int tombstone_fd, BacktraceMap* backtrace_map, const std::set& siblings, - int* crash_signal, int amfd) { + int* crash_signal, std::string* amfd_data) { if (TEMP_FAILURE_RETRY(write(fd, "\0", 1)) != 1) { ALOGE("debuggerd: failed to respond to client: %s\n", strerror(errno)); return false; @@ -433,10 +472,10 @@ static bool perform_dump(const debugger_request_t& request, int fd, int tombston if (request.action == DEBUGGER_ACTION_DUMP_TOMBSTONE) { ALOGV("debuggerd: stopped -- dumping to tombstone"); engrave_tombstone(tombstone_fd, backtrace_map, request.pid, request.tid, siblings, signal, - request.original_si_code, request.abort_msg_address, amfd); + request.original_si_code, request.abort_msg_address, amfd_data); } else if (request.action == DEBUGGER_ACTION_DUMP_BACKTRACE) { ALOGV("debuggerd: stopped -- dumping to fd"); - dump_backtrace(fd, -1, backtrace_map, request.pid, request.tid, siblings); + dump_backtrace(fd, backtrace_map, request.pid, request.tid, siblings, nullptr); } else { ALOGV("debuggerd: stopped -- continuing"); if (ptrace(PTRACE_CONT, request.tid, 0, 0) != 0) { @@ -460,7 +499,7 @@ static bool perform_dump(const debugger_request_t& request, int fd, int tombston ALOGV("stopped -- fatal signal\n"); *crash_signal = signal; engrave_tombstone(tombstone_fd, backtrace_map, request.pid, request.tid, siblings, signal, - request.original_si_code, request.abort_msg_address, amfd); + request.original_si_code, request.abort_msg_address, amfd_data); break; default: @@ -547,9 +586,11 @@ static void worker_process(int fd, debugger_request_t& request) { std::unique_ptr backtrace_map(BacktraceMap::Create(request.pid)); int amfd = -1; + std::unique_ptr amfd_data; if (request.action == DEBUGGER_ACTION_CRASH) { // Connect to the activity manager before dropping privileges. amfd = activity_manager_connect(); + amfd_data.reset(new std::string); } bool succeeded = false; @@ -562,11 +603,11 @@ static void worker_process(int fd, debugger_request_t& request) { int crash_signal = SIGKILL; succeeded = perform_dump(request, fd, tombstone_fd, backtrace_map.get(), siblings, - &crash_signal, amfd); + &crash_signal, amfd_data.get()); if (succeeded) { if (request.action == DEBUGGER_ACTION_DUMP_TOMBSTONE) { if (!tombstone_path.empty()) { - write(fd, tombstone_path.c_str(), tombstone_path.length()); + android::base::WriteFully(fd, tombstone_path.c_str(), tombstone_path.length()); } } } @@ -579,6 +620,13 @@ static void worker_process(int fd, debugger_request_t& request) { } } + if (!attach_gdb) { + // Tell the Activity Manager about the crashing process. If we are + // waiting for gdb to attach, do not send this or Activity Manager + // might kill the process before anyone can attach. + activity_manager_write(request.pid, crash_signal, amfd, *amfd_data.get()); + } + if (ptrace(PTRACE_DETACH, request.tid, 0, 0) != 0) { ALOGE("debuggerd: ptrace detach from %d failed: %s", request.tid, strerror(errno)); } @@ -595,9 +643,12 @@ static void worker_process(int fd, debugger_request_t& request) { } // Wait for gdb, if requested. - if (attach_gdb && succeeded) { + if (attach_gdb) { wait_for_user_action(request); + // Now tell the activity manager about this process. + activity_manager_write(request.pid, crash_signal, amfd, *amfd_data.get()); + // Tell the signal process to send SIGCONT to the target. if (!send_signal(request.pid, 0, SIGCONT)) { ALOGE("debuggerd: failed to resume process %d: %s", request.pid, strerror(errno)); diff --git a/debuggerd/test/dump_memory_test.cpp b/debuggerd/test/dump_memory_test.cpp index 2addd5dd6..49f369018 100644 --- a/debuggerd/test/dump_memory_test.cpp +++ b/debuggerd/test/dump_memory_test.cpp @@ -125,7 +125,7 @@ class DumpMemoryTest : public ::testing::Test { } log_.tfd = tombstone_fd; - log_.amfd = -1; + log_.amfd_data = nullptr; log_.crashed_tid = 12; log_.current_tid = 12; log_.should_retrieve_logcat = false; diff --git a/debuggerd/test/tombstone_test.cpp b/debuggerd/test/tombstone_test.cpp index 96b3a7aae..58d640e08 100644 --- a/debuggerd/test/tombstone_test.cpp +++ b/debuggerd/test/tombstone_test.cpp @@ -68,7 +68,8 @@ class TombstoneTest : public ::testing::Test { } log_.tfd = tombstone_fd; - log_.amfd = -1; + amfd_data_.clear(); + log_.amfd_data = &amfd_data_; log_.crashed_tid = 12; log_.current_tid = 12; log_.should_retrieve_logcat = false; @@ -90,6 +91,7 @@ class TombstoneTest : public ::testing::Test { std::unique_ptr backtrace_mock_; log_t log_; + std::string amfd_data_; }; TEST_F(TombstoneTest, single_map) { @@ -117,6 +119,8 @@ TEST_F(TombstoneTest, single_map) { #endif ASSERT_STREQ(expected_dump, tombstone_contents.c_str()); + ASSERT_STREQ("", amfd_data_.c_str()); + // Verify that the log buf is empty, and no error messages. ASSERT_STREQ("", getFakeLogBuf().c_str()); ASSERT_STREQ("", getFakeLogPrint().c_str()); @@ -150,6 +154,8 @@ TEST_F(TombstoneTest, single_map_elf_build_id) { #endif ASSERT_STREQ(expected_dump, tombstone_contents.c_str()); + ASSERT_STREQ("", amfd_data_.c_str()); + // Verify that the log buf is empty, and no error messages. ASSERT_STREQ("", getFakeLogBuf().c_str()); ASSERT_STREQ("", getFakeLogPrint().c_str()); @@ -189,6 +195,8 @@ TEST_F(TombstoneTest, single_map_no_build_id) { #endif ASSERT_STREQ(expected_dump, tombstone_contents.c_str()); + ASSERT_STREQ("", amfd_data_.c_str()); + // Verify that the log buf is empty, and no error messages. ASSERT_STREQ("", getFakeLogBuf().c_str()); ASSERT_STREQ("", getFakeLogPrint().c_str()); @@ -251,6 +259,8 @@ TEST_F(TombstoneTest, multiple_maps) { #endif ASSERT_STREQ(expected_dump, tombstone_contents.c_str()); + ASSERT_STREQ("", amfd_data_.c_str()); + // Verify that the log buf is empty, and no error messages. ASSERT_STREQ("", getFakeLogBuf().c_str()); ASSERT_STREQ("", getFakeLogPrint().c_str()); @@ -305,6 +315,8 @@ TEST_F(TombstoneTest, multiple_maps_fault_address_before) { #endif ASSERT_STREQ(expected_dump, tombstone_contents.c_str()); + ASSERT_STREQ("", amfd_data_.c_str()); + // Verify that the log buf is empty, and no error messages. ASSERT_STREQ("", getFakeLogBuf().c_str()); ASSERT_STREQ("", getFakeLogPrint().c_str()); @@ -359,6 +371,8 @@ TEST_F(TombstoneTest, multiple_maps_fault_address_between) { #endif ASSERT_STREQ(expected_dump, tombstone_contents.c_str()); + ASSERT_STREQ("", amfd_data_.c_str()); + // Verify that the log buf is empty, and no error messages. ASSERT_STREQ("", getFakeLogBuf().c_str()); ASSERT_STREQ("", getFakeLogPrint().c_str()); @@ -411,6 +425,8 @@ TEST_F(TombstoneTest, multiple_maps_fault_address_in_map) { #endif ASSERT_STREQ(expected_dump, tombstone_contents.c_str()); + ASSERT_STREQ("", amfd_data_.c_str()); + // Verify that the log buf is empty, and no error messages. ASSERT_STREQ("", getFakeLogBuf().c_str()); ASSERT_STREQ("", getFakeLogPrint().c_str()); @@ -469,6 +485,8 @@ TEST_F(TombstoneTest, multiple_maps_fault_address_after) { #endif ASSERT_STREQ(expected_dump, tombstone_contents.c_str()); + ASSERT_STREQ("", amfd_data_.c_str()); + // Verify that the log buf is empty, and no error messages. ASSERT_STREQ("", getFakeLogBuf().c_str()); ASSERT_STREQ("", getFakeLogPrint().c_str()); @@ -501,6 +519,8 @@ TEST_F(TombstoneTest, multiple_maps_getsiginfo_fail) { #endif ASSERT_STREQ(expected_dump, tombstone_contents.c_str()); + ASSERT_STREQ("", amfd_data_.c_str()); + // Verify that the log buf is empty, and no error messages. ASSERT_STREQ("", getFakeLogBuf().c_str()); ASSERT_STREQ("6 DEBUG Cannot get siginfo for 100: Bad address\n\n", getFakeLogPrint().c_str()); @@ -562,6 +582,8 @@ TEST_F(TombstoneTest, multiple_maps_check_signal_has_si_addr) { << "Signal " << si.si_signo << " is not expected to include an address."; } + ASSERT_STREQ("", amfd_data_.c_str()); + // Verify that the log buf is empty, and no error messages. ASSERT_STREQ("", getFakeLogBuf().c_str()); ASSERT_STREQ("", getFakeLogPrint().c_str()); @@ -582,6 +604,8 @@ TEST_F(TombstoneTest, dump_signal_info_error) { ASSERT_STREQ("", getFakeLogBuf().c_str()); ASSERT_STREQ("6 DEBUG cannot get siginfo: Bad address\n\n", getFakeLogPrint().c_str()); + + ASSERT_STREQ("", amfd_data_.c_str()); } TEST_F(TombstoneTest, dump_log_file_error) { @@ -596,4 +620,14 @@ TEST_F(TombstoneTest, dump_log_file_error) { ASSERT_STREQ("", getFakeLogBuf().c_str()); ASSERT_STREQ("6 DEBUG Unable to open /fake/filename: Permission denied\n\n", getFakeLogPrint().c_str()); + + ASSERT_STREQ("", amfd_data_.c_str()); +} + +TEST_F(TombstoneTest, dump_header_info) { + dump_header_info(&log_); + + std::string expected = "Build fingerprint: 'unknown'\nRevision: 'unknown'\n"; + expected += android::base::StringPrintf("ABI: '%s'\n", ABI_STRING); + ASSERT_STREQ(expected.c_str(), amfd_data_.c_str()); } diff --git a/debuggerd/tombstone.cpp b/debuggerd/tombstone.cpp index d802c8c07..983d49e17 100644 --- a/debuggerd/tombstone.cpp +++ b/debuggerd/tombstone.cpp @@ -16,7 +16,6 @@ #define LOG_TAG "DEBUG" -#include #include #include #include @@ -613,16 +612,6 @@ static void dump_crash(log_t* log, BacktraceMap* map, pid_t pid, pid_t tid, property_get("ro.debuggable", value, "0"); bool want_logs = (value[0] == '1'); - if (log->amfd >= 0) { - // Activity Manager protocol: binary 32-bit network-byte-order ints for the - // pid and signal number, followed by the raw text of the dump, culminating - // in a zero byte that marks end-of-data. - uint32_t datum = htonl(pid); - TEMP_FAILURE_RETRY( write(log->amfd, &datum, 4) ); - datum = htonl(signal); - TEMP_FAILURE_RETRY( write(log->amfd, &datum, 4) ); - } - _LOG(log, logtype::HEADER, "*** *** *** *** *** *** *** *** *** *** *** *** *** *** *** ***\n"); dump_header_info(log); @@ -640,17 +629,6 @@ static void dump_crash(log_t* log, BacktraceMap* map, pid_t pid, pid_t tid, if (want_logs) { dump_logs(log, pid, 0); } - - // send EOD to the Activity Manager, then wait for its ack to avoid racing ahead - // and killing the target out from under it - if (log->amfd >= 0) { - uint8_t eodMarker = 0; - TEMP_FAILURE_RETRY( write(log->amfd, &eodMarker, 1) ); - // 3 sec timeout reading the ack; we're fine if that happens - TEMP_FAILURE_RETRY( read(log->amfd, &eodMarker, 1) ); - } - - return; } // open_tombstone - find an available tombstone slot, if any, of the @@ -708,7 +686,7 @@ int open_tombstone(std::string* out_path) { void engrave_tombstone(int tombstone_fd, BacktraceMap* map, pid_t pid, pid_t tid, const std::set& siblings, int signal, int original_si_code, - uintptr_t abort_msg_address, int amfd) { + uintptr_t abort_msg_address, std::string* amfd_data) { log_t log; log.current_tid = tid; log.crashed_tid = tid; @@ -719,8 +697,6 @@ void engrave_tombstone(int tombstone_fd, BacktraceMap* map, pid_t pid, pid_t tid } log.tfd = tombstone_fd; - // Preserve amfd since it can be modified through the calls below without - // being closed. - log.amfd = amfd; + log.amfd_data = amfd_data; dump_crash(&log, map, pid, tid, siblings, signal, original_si_code, abort_msg_address); } diff --git a/debuggerd/tombstone.h b/debuggerd/tombstone.h index 7f3eebe0b..487d95058 100644 --- a/debuggerd/tombstone.h +++ b/debuggerd/tombstone.h @@ -34,6 +34,6 @@ int open_tombstone(std::string* path); /* Creates a tombstone file and writes the crash dump to it. */ void engrave_tombstone(int tombstone_fd, BacktraceMap* map, pid_t pid, pid_t tid, const std::set& siblings, int signal, int original_si_code, - uintptr_t abort_msg_address, int amfd); + uintptr_t abort_msg_address, std::string* amfd_data); #endif // _DEBUGGERD_TOMBSTONE_H diff --git a/debuggerd/utility.cpp b/debuggerd/utility.cpp index cd252ce72..bd060954e 100644 --- a/debuggerd/utility.cpp +++ b/debuggerd/utility.cpp @@ -25,7 +25,8 @@ #include #include -#include +#include + #include #include #include @@ -49,7 +50,6 @@ void _LOG(log_t* log, enum logtype ltype, const char* fmt, ...) { && log->crashed_tid != -1 && log->current_tid != -1 && (log->crashed_tid == log->current_tid); - bool write_to_activitymanager = (log->amfd != -1); char buf[512]; va_list ap; @@ -68,12 +68,8 @@ void _LOG(log_t* log, enum logtype ltype, const char* fmt, ...) { if (write_to_logcat) { __android_log_buf_write(LOG_ID_CRASH, ANDROID_LOG_FATAL, LOG_TAG, buf); - if (write_to_activitymanager) { - if (!android::base::WriteFully(log->amfd, buf, len)) { - // timeout or other failure on write; stop informing the activity manager - ALOGE("AM write failed: %s", strerror(errno)); - log->amfd = -1; - } + if (log->amfd_data != nullptr) { + *log->amfd_data += buf; } } } diff --git a/debuggerd/utility.h b/debuggerd/utility.h index ed08ddcbb..cd01188c3 100644 --- a/debuggerd/utility.h +++ b/debuggerd/utility.h @@ -21,6 +21,8 @@ #include #include +#include + #include // Figure out the abi based on defined macros. @@ -42,10 +44,10 @@ struct log_t{ - /* tombstone file descriptor */ + // Tombstone file descriptor. int tfd; - /* Activity Manager socket file descriptor */ - int amfd; + // Data to be sent to the Activity Manager. + std::string* amfd_data; // The tid of the thread that crashed. pid_t crashed_tid; // The tid of the thread we are currently working with. @@ -54,7 +56,8 @@ struct log_t{ bool should_retrieve_logcat; log_t() - : tfd(-1), amfd(-1), crashed_tid(-1), current_tid(-1), should_retrieve_logcat(true) {} + : tfd(-1), amfd_data(nullptr), crashed_tid(-1), current_tid(-1), + should_retrieve_logcat(true) {} }; // List of types of logs to simplify the logging decision in _LOG