Fix problem with wait_for_gdb.

When someone enables wait_for_gdb, activity manager will kill the
stopped process before a developer can attach to the process. To
allow debugging in this case, change the code to only contact the
activity manager right before continuing the process that is
crashing.

Also, modify the conditions under which to do a gdb attach. The previous
code did a partial attach if perform_dump failed. The new version simply
allows an attach regardless of whether perform_dump passes or fails.

Bug: 28409358
Change-Id: I4fe8d390c212d47e007cbe6543c2d114c3c7aeff
This commit is contained in:
Christopher Ferris 2016-05-03 16:32:13 -07:00
parent f2049163a4
commit 9818bd2bbe
9 changed files with 115 additions and 53 deletions

View File

@ -29,6 +29,7 @@
#include <sys/ptrace.h>
#include <memory>
#include <string>
#include <backtrace/Backtrace.h>
@ -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<pid_t>& siblings) {
void dump_backtrace(int fd, BacktraceMap* map, pid_t pid, pid_t tid,
const std::set<pid_t>& 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);

View File

@ -20,6 +20,7 @@
#include <sys/types.h>
#include <set>
#include <string>
#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<pid_t>& siblings);
void dump_backtrace(int fd, BacktraceMap* map, pid_t pid, pid_t tid,
const std::set<pid_t>& 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);

View File

@ -14,6 +14,7 @@
* limitations under the License.
*/
#include <arpa/inet.h>
#include <dirent.h>
#include <elf.h>
#include <errno.h>
@ -32,12 +33,15 @@
#include <sys/un.h>
#include <time.h>
#include <memory>
#include <set>
#include <string>
#include <selinux/android.h>
#include <log/logger.h>
#include <android-base/file.h>
#include <android-base/unique_fd.h>
#include <cutils/debugger.h>
#include <cutils/properties.h>
@ -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<pid_t>& tids) {
static bool perform_dump(const debugger_request_t& request, int fd, int tombstone_fd,
BacktraceMap* backtrace_map, const std::set<pid_t>& 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<BacktraceMap> backtrace_map(BacktraceMap::Create(request.pid));
int amfd = -1;
std::unique_ptr<std::string> 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));

View File

@ -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;

View File

@ -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<BacktraceMock> 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());
}

View File

@ -16,7 +16,6 @@
#define LOG_TAG "DEBUG"
#include <arpa/inet.h>
#include <dirent.h>
#include <errno.h>
#include <fcntl.h>
@ -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<pid_t>& 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);
}

View File

@ -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<pid_t>& 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

View File

@ -25,7 +25,8 @@
#include <sys/ptrace.h>
#include <sys/wait.h>
#include <android-base/file.h>
#include <string>
#include <android-base/stringprintf.h>
#include <backtrace/Backtrace.h>
#include <log/log.h>
@ -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;
}
}
}

View File

@ -21,6 +21,8 @@
#include <stdbool.h>
#include <sys/types.h>
#include <string>
#include <backtrace/Backtrace.h>
// 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