Fix fallback paths for dumping threads.
In the fallback path, if the non-main thread is the target
to be dumped, then no other threads are dumped when creating
a tombstone. Fix this and add unit tests to verify that
this all threads, including the main thread are dumped.
Bug: 234058038
Test: All unit tests pass.
Test: debuggerd -b media.swcodec process
Test: debuggerd media.swcodec process
Change-Id: Ibb75264f7b3847acdbab939a66902d986c0d0e5c
diff --git a/debuggerd/debuggerd_test.cpp b/debuggerd/debuggerd_test.cpp
index e113308..ed90ab4 100644
--- a/debuggerd/debuggerd_test.cpp
+++ b/debuggerd/debuggerd_test.cpp
@@ -1486,6 +1486,37 @@
ASSERT_BACKTRACE_FRAME(result, "abort");
}
+TEST_F(CrasherTest, seccomp_tombstone_multiple_threads_abort) {
+ int intercept_result;
+ unique_fd output_fd;
+
+ static const auto dump_type = kDebuggerdTombstone;
+ StartProcess(
+ []() {
+ std::thread a(foo);
+ std::thread b(bar);
+
+ std::this_thread::sleep_for(100ms);
+
+ std::thread abort_thread([] { abort(); });
+ abort_thread.join();
+ },
+ &seccomp_fork);
+
+ StartIntercept(&output_fd, dump_type);
+ FinishCrasher();
+ AssertDeath(SIGABRT);
+ FinishIntercept(&intercept_result);
+ ASSERT_EQ(1, intercept_result) << "tombstoned reported failure";
+
+ std::string result;
+ ConsumeFd(std::move(output_fd), &result);
+ ASSERT_BACKTRACE_FRAME(result, "abort");
+ ASSERT_BACKTRACE_FRAME(result, "foo");
+ ASSERT_BACKTRACE_FRAME(result, "bar");
+ ASSERT_BACKTRACE_FRAME(result, "main");
+}
+
TEST_F(CrasherTest, seccomp_backtrace) {
int intercept_result;
unique_fd output_fd;
@@ -1516,6 +1547,40 @@
ASSERT_BACKTRACE_FRAME(result, "bar");
}
+TEST_F(CrasherTest, seccomp_backtrace_from_thread) {
+ int intercept_result;
+ unique_fd output_fd;
+
+ static const auto dump_type = kDebuggerdNativeBacktrace;
+ StartProcess(
+ []() {
+ std::thread a(foo);
+ std::thread b(bar);
+
+ std::this_thread::sleep_for(100ms);
+
+ std::thread raise_thread([] {
+ raise_debugger_signal(dump_type);
+ _exit(0);
+ });
+ raise_thread.join();
+ },
+ &seccomp_fork);
+
+ StartIntercept(&output_fd, dump_type);
+ FinishCrasher();
+ AssertDeath(0);
+ FinishIntercept(&intercept_result);
+ ASSERT_EQ(1, intercept_result) << "tombstoned reported failure";
+
+ std::string result;
+ ConsumeFd(std::move(output_fd), &result);
+ ASSERT_BACKTRACE_FRAME(result, "raise_debugger_signal");
+ ASSERT_BACKTRACE_FRAME(result, "foo");
+ ASSERT_BACKTRACE_FRAME(result, "bar");
+ ASSERT_BACKTRACE_FRAME(result, "main");
+}
+
TEST_F(CrasherTest, seccomp_crash_logcat) {
StartProcess([]() { abort(); }, &seccomp_fork);
FinishCrasher();
diff --git a/debuggerd/handler/debuggerd_fallback.cpp b/debuggerd/handler/debuggerd_fallback.cpp
index 70e3022..4721391 100644
--- a/debuggerd/handler/debuggerd_fallback.cpp
+++ b/debuggerd/handler/debuggerd_fallback.cpp
@@ -210,7 +210,10 @@
// Send a signal to all of our siblings, asking them to dump their stack.
pid_t current_tid = gettid();
- if (!iterate_tids(current_tid, [&output_fd](pid_t tid) {
+ if (!iterate_tids(current_tid, [&output_fd, ¤t_tid](pid_t tid) {
+ if (current_tid == tid) {
+ return;
+ }
// Use a pipe, to be able to detect situations where the thread gracefully exits before
// receiving our signal.
unique_fd pipe_read, pipe_write;
diff --git a/debuggerd/libdebuggerd/tombstone.cpp b/debuggerd/libdebuggerd/tombstone.cpp
index 5ca2c00..e5b4d74 100644
--- a/debuggerd/libdebuggerd/tombstone.cpp
+++ b/debuggerd/libdebuggerd/tombstone.cpp
@@ -55,15 +55,15 @@
siginfo_t* siginfo, ucontext_t* ucontext) {
pid_t uid = getuid();
pid_t pid = getpid();
- pid_t tid = gettid();
+ pid_t target_tid = gettid();
log_t log;
- log.current_tid = tid;
- log.crashed_tid = tid;
+ log.current_tid = target_tid;
+ log.crashed_tid = target_tid;
log.tfd = tombstone_fd;
log.amfd_data = nullptr;
- std::string thread_name = get_thread_name(tid);
+ std::string thread_name = get_thread_name(target_tid);
std::vector<std::string> command_line = get_command_line(pid);
std::unique_ptr<unwindstack::Regs> regs(
@@ -73,32 +73,34 @@
android::base::ReadFileToString("/proc/self/attr/current", &selinux_label);
std::map<pid_t, ThreadInfo> threads;
- threads[tid] = ThreadInfo{
- .registers = std::move(regs), .uid = uid, .tid = tid, .thread_name = std::move(thread_name),
- .pid = pid, .command_line = std::move(command_line), .selinux_label = std::move(selinux_label),
- .siginfo = siginfo,
+ threads[target_tid] = ThreadInfo {
+ .registers = std::move(regs), .uid = uid, .tid = target_tid,
+ .thread_name = std::move(thread_name), .pid = pid, .command_line = std::move(command_line),
+ .selinux_label = std::move(selinux_label), .siginfo = siginfo,
#if defined(__aarch64__)
// Only supported on aarch64 for now.
.tagged_addr_ctrl = prctl(PR_GET_TAGGED_ADDR_CTRL, 0, 0, 0, 0),
.pac_enabled_keys = prctl(PR_PAC_GET_ENABLED_KEYS, 0, 0, 0, 0),
#endif
};
- if (pid == tid) {
- const ThreadInfo& thread = threads[pid];
- if (!iterate_tids(pid, [&threads, &thread](pid_t tid) {
- threads[tid] = ThreadInfo{
- .uid = thread.uid,
- .tid = tid,
- .pid = thread.pid,
- .command_line = thread.command_line,
- .thread_name = get_thread_name(tid),
- .tagged_addr_ctrl = thread.tagged_addr_ctrl,
- .pac_enabled_keys = thread.pac_enabled_keys,
- };
- })) {
- async_safe_format_log(ANDROID_LOG_ERROR, LOG_TAG, "failed to open /proc/%d/task: %s", pid,
- strerror(errno));
- }
+ const ThreadInfo& thread = threads[pid];
+ if (!iterate_tids(pid, [&threads, &thread, &target_tid](pid_t tid) {
+ if (target_tid == tid) {
+ return;
+ }
+ async_safe_format_log(ANDROID_LOG_ERROR, LOG_TAG, "Adding thread %d", tid);
+ threads[tid] = ThreadInfo{
+ .uid = thread.uid,
+ .tid = tid,
+ .pid = thread.pid,
+ .command_line = thread.command_line,
+ .thread_name = get_thread_name(tid),
+ .tagged_addr_ctrl = thread.tagged_addr_ctrl,
+ .pac_enabled_keys = thread.pac_enabled_keys,
+ };
+ })) {
+ async_safe_format_log(ANDROID_LOG_ERROR, LOG_TAG, "failed to open /proc/%d/task: %s", pid,
+ strerror(errno));
}
// Do not use the thread cache here because it will call pthread_key_create
@@ -116,8 +118,8 @@
ProcessInfo process_info;
process_info.abort_msg_address = abort_msg_address;
- engrave_tombstone(unique_fd(dup(tombstone_fd)), unique_fd(dup(proto_fd)), &unwinder, threads, tid,
- process_info, nullptr, nullptr);
+ engrave_tombstone(unique_fd(dup(tombstone_fd)), unique_fd(dup(proto_fd)), &unwinder, threads,
+ target_tid, process_info, nullptr, nullptr);
}
void engrave_tombstone(unique_fd output_fd, unique_fd proto_fd,
diff --git a/debuggerd/util.cpp b/debuggerd/util.cpp
index 5c6abc9..df033df 100644
--- a/debuggerd/util.cpp
+++ b/debuggerd/util.cpp
@@ -90,9 +90,7 @@
if (tid == 0) {
continue;
}
- if (pid != tid) {
- callback(tid);
- }
+ callback(tid);
}
return true;
}