From a029d98ad05a9435874bbef1f9cd97a152f5bb9a Mon Sep 17 00:00:00 2001 From: Elliott Hughes Date: Mon, 21 Jun 2021 12:36:47 -0700 Subject: [PATCH] crash_dump: avoid misleading error messages. I'm guessing that the original F crash_dump64: crash_dump.cpp:460] failed to attach to thread 1671, already traced by 0 () was probably a race, where there _was_ a tracer but they disappeared? Whatever, it doesn't seem helpful to show "already traced by nobody", and we also don't want to clobber errno in the fallthrough case (previously just where get_tracer() failed, but now also where get_tracer() returns "nobody"). Bug: http://b/188668580 Test: treehugger Change-Id: I3fa3b4f7e32531d48dfbb0ef946ff351ed5d9171 --- debuggerd/crash_dump.cpp | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/debuggerd/crash_dump.cpp b/debuggerd/crash_dump.cpp index a15274019..5bb1d75ad 100644 --- a/debuggerd/crash_dump.cpp +++ b/debuggerd/crash_dump.cpp @@ -32,6 +32,7 @@ #include #include +#include #include #include #include @@ -67,8 +68,9 @@ #include "protocol.h" #include "util.h" -using android::base::unique_fd; +using android::base::ErrnoRestorer; using android::base::StringPrintf; +using android::base::unique_fd; static bool pid_contains_tid(int pid_proc_fd, pid_t tid) { struct stat st; @@ -89,10 +91,11 @@ static pid_t get_tracer(pid_t tracee) { static bool ptrace_seize_thread(int pid_proc_fd, pid_t tid, std::string* error, int flags = 0) { if (ptrace(PTRACE_SEIZE, tid, 0, flags) != 0) { if (errno == EPERM) { - pid_t tracer = get_tracer(tid); - if (tracer != -1) { + ErrnoRestorer errno_restorer; // In case get_tracer() fails and we fall through. + pid_t tracer_pid = get_tracer(tid); + if (tracer_pid > 0) { *error = StringPrintf("failed to attach to thread %d, already traced by %d (%s)", tid, - tracer, get_process_name(tracer).c_str()); + tracer_pid, get_process_name(tracer_pid).c_str()); return false; } }