Merge "Fix problem with wait_for_gdb." into nyc-dev
am: a7431cfa57
* commit 'a7431cfa570c6385ecd2041d56ead2603d7b7b80':
Fix problem with wait_for_gdb.
Change-Id: I6d995c423f4ad9134965a215b1eea4cc7fd338b9
diff --git a/debuggerd/backtrace.cpp b/debuggerd/backtrace.cpp
index 32843d8..8f4a53f 100644
--- a/debuggerd/backtrace.cpp
+++ b/debuggerd/backtrace.cpp
@@ -29,6 +29,7 @@
#include <sys/ptrace.h>
#include <memory>
+#include <string>
#include <backtrace/Backtrace.h>
@@ -96,11 +97,11 @@
}
}
-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);
diff --git a/debuggerd/backtrace.h b/debuggerd/backtrace.h
index 98c433b..acd5eaa 100644
--- a/debuggerd/backtrace.h
+++ b/debuggerd/backtrace.h
@@ -20,6 +20,7 @@
#include <sys/types.h>
#include <set>
+#include <string>
#include "utility.h"
@@ -28,8 +29,8 @@
// 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);
diff --git a/debuggerd/debuggerd.cpp b/debuggerd/debuggerd.cpp
index 0a43622..d2e6c29 100644
--- a/debuggerd/debuggerd.cpp
+++ b/debuggerd/debuggerd.cpp
@@ -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>
@@ -287,6 +291,41 @@
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);
@@ -414,7 +453,7 @@
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;
@@ -432,10 +471,10 @@
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) {
@@ -459,7 +498,7 @@
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:
@@ -546,9 +585,11 @@
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;
@@ -561,11 +602,11 @@
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());
}
}
}
@@ -578,6 +619,13 @@
}
}
+ 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));
}
@@ -594,9 +642,12 @@
}
// 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 2addd5d..49f3690 100644
--- a/debuggerd/test/dump_memory_test.cpp
+++ b/debuggerd/test/dump_memory_test.cpp
@@ -125,7 +125,7 @@
}
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 96b3a7a..58d640e 100644
--- a/debuggerd/test/tombstone_test.cpp
+++ b/debuggerd/test/tombstone_test.cpp
@@ -68,7 +68,8 @@
}
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 @@
std::unique_ptr<BacktraceMock> backtrace_mock_;
log_t log_;
+ std::string amfd_data_;
};
TEST_F(TombstoneTest, single_map) {
@@ -117,6 +119,8 @@
#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 @@
#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 @@
#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 @@
#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 @@
#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 @@
#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 @@
#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 @@
#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 @@
#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 @@
<< "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 @@
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 @@
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 d802c8c..983d49e 100644
--- a/debuggerd/tombstone.cpp
+++ b/debuggerd/tombstone.cpp
@@ -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 @@
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 @@
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 @@
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 @@
}
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 7f3eebe..487d950 100644
--- a/debuggerd/tombstone.h
+++ b/debuggerd/tombstone.h
@@ -34,6 +34,6 @@
/* 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
diff --git a/debuggerd/utility.cpp b/debuggerd/utility.cpp
index cd252ce..bd06095 100644
--- a/debuggerd/utility.cpp
+++ b/debuggerd/utility.cpp
@@ -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 @@
&& 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 @@
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 ed08ddc..cd01188 100644
--- a/debuggerd/utility.h
+++ b/debuggerd/utility.h
@@ -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 @@
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