From a48b41bcb8629c14210aa3102669e03267353274 Mon Sep 17 00:00:00 2001 From: Josh Gao Date: Fri, 13 Dec 2019 14:11:04 -0800 Subject: [PATCH] debuggerd: switch to using platform headers for DEBUGGER_SIGNAL. Test: treehugger Change-Id: Ie9736c4a077dba1029d2352bd94d47ce07323aec --- debuggerd/Android.bp | 35 +++++++++++++++++++++--- debuggerd/client/debuggerd_client.cpp | 3 +- debuggerd/crash_dump.cpp | 9 +++--- debuggerd/crasher/Android.bp | 2 ++ debuggerd/debuggerd_test.cpp | 9 +++--- debuggerd/handler/debuggerd_fallback.cpp | 5 ++-- debuggerd/handler/debuggerd_handler.cpp | 13 +++++---- debuggerd/include/debuggerd/handler.h | 7 +++-- debuggerd/libdebuggerd/tombstone.cpp | 3 -- debuggerd/libdebuggerd/utility.cpp | 4 ++- 10 files changed, 62 insertions(+), 28 deletions(-) diff --git a/debuggerd/Android.bp b/debuggerd/Android.bp index 0602e0a87..780e48d2f 100644 --- a/debuggerd/Android.bp +++ b/debuggerd/Android.bp @@ -74,6 +74,7 @@ cc_library_static { header_libs: [ "libbase_headers", "libdebuggerd_common_headers", + "bionic_libc_platform_headers", ], whole_static_libs: [ @@ -92,6 +93,9 @@ cc_library_static { defaults: ["debuggerd_defaults"], srcs: ["handler/debuggerd_fallback_nop.cpp"], + header_libs: ["bionic_libc_platform_headers"], + export_header_lib_headers: ["bionic_libc_platform_headers"], + whole_static_libs: [ "libdebuggerd_handler_core", ], @@ -119,6 +123,10 @@ cc_library_static { "liblzma", "libcutils", ], + + header_libs: ["bionic_libc_platform_headers"], + export_header_lib_headers: ["bionic_libc_platform_headers"], + target: { recovery: { exclude_static_libs: [ @@ -138,15 +146,21 @@ cc_library { "util.cpp", ], - header_libs: ["libdebuggerd_common_headers"], - shared_libs: [ "libbase", "libcutils", "libprocinfo", ], - export_header_lib_headers: ["libdebuggerd_common_headers"], + header_libs: [ + "libdebuggerd_common_headers", + "bionic_libc_platform_headers", + ], + export_header_lib_headers: [ + "libdebuggerd_common_headers", + "bionic_libc_platform_headers", + ], + export_include_dirs: ["include"], } @@ -167,6 +181,7 @@ cc_library_static { // Needed for private/bionic_fdsan.h include_dirs: ["bionic/libc"], + header_libs: ["bionic_libc_platform_headers"], static_libs: [ "libdexfile_support_static", // libunwindstack dependency @@ -176,6 +191,7 @@ cc_library_static { "libcutils", "liblog", ], + target: { recovery: { exclude_static_libs: [ @@ -232,6 +248,10 @@ cc_test { "libdebuggerd", ], + header_libs: [ + "bionic_libc_platform_headers", + ], + local_include_dirs: [ "libdebuggerd", ], @@ -277,6 +297,10 @@ cc_binary { }, }, + header_libs: [ + "bionic_libc_platform_headers", + ], + static_libs: [ "libtombstoned_client_static", "libdebuggerd", @@ -317,7 +341,10 @@ cc_binary { ], defaults: ["debuggerd_defaults"], - header_libs: ["libdebuggerd_common_headers"], + header_libs: [ + "bionic_libc_platform_headers", + "libdebuggerd_common_headers" + ], static_libs: [ "libbase", diff --git a/debuggerd/client/debuggerd_client.cpp b/debuggerd/client/debuggerd_client.cpp index 7e35a2f00..5c027387c 100644 --- a/debuggerd/client/debuggerd_client.cpp +++ b/debuggerd/client/debuggerd_client.cpp @@ -35,6 +35,7 @@ #include #include #include +#include #include #include @@ -50,7 +51,7 @@ using android::base::unique_fd; using android::base::WriteStringToFd; static bool send_signal(pid_t pid, const DebuggerdDumpType dump_type) { - const int signal = (dump_type == kDebuggerdJavaBacktrace) ? SIGQUIT : DEBUGGER_SIGNAL; + const int signal = (dump_type == kDebuggerdJavaBacktrace) ? SIGQUIT : BIONIC_SIGNAL_DEBUGGER; sigval val; val.sival_int = (dump_type == kDebuggerdNativeBacktrace) ? 1 : 0; diff --git a/debuggerd/crash_dump.cpp b/debuggerd/crash_dump.cpp index cb55745f1..e8f366fbe 100644 --- a/debuggerd/crash_dump.cpp +++ b/debuggerd/crash_dump.cpp @@ -40,6 +40,7 @@ #include #include #include +#include #include #include #include @@ -511,13 +512,13 @@ int main(int argc, char** argv) { // Defer the message until later, for readability. bool wait_for_gdb = android::base::GetBoolProperty("debug.debuggerd.wait_for_gdb", false); - if (siginfo.si_signo == DEBUGGER_SIGNAL) { + if (siginfo.si_signo == BIONIC_SIGNAL_DEBUGGER) { wait_for_gdb = false; } // Detach from all of our attached threads before resuming. for (const auto& [tid, thread] : thread_info) { - int resume_signal = thread.signo == DEBUGGER_SIGNAL ? 0 : thread.signo; + int resume_signal = thread.signo == BIONIC_SIGNAL_DEBUGGER ? 0 : thread.signo; if (wait_for_gdb) { resume_signal = 0; if (tgkill(target_process, tid, SIGSTOP) != 0) { @@ -555,10 +556,10 @@ int main(int argc, char** argv) { << " (target tid = " << g_target_thread << ")"; int signo = siginfo.si_signo; - bool fatal_signal = signo != DEBUGGER_SIGNAL; + bool fatal_signal = signo != BIONIC_SIGNAL_DEBUGGER; bool backtrace = false; - // si_value is special when used with DEBUGGER_SIGNAL. + // si_value is special when used with BIONIC_SIGNAL_DEBUGGER. // 0: dump tombstone // 1: dump backtrace if (!fatal_signal) { diff --git a/debuggerd/crasher/Android.bp b/debuggerd/crasher/Android.bp index 7bec470fb..e86f499bd 100644 --- a/debuggerd/crasher/Android.bp +++ b/debuggerd/crasher/Android.bp @@ -44,6 +44,7 @@ cc_binary { name: "crasher", defaults: ["crasher-defaults"], + header_libs: ["bionic_libc_platform_headers"], shared_libs: [ "libbase", "liblog", @@ -65,6 +66,7 @@ cc_binary { defaults: ["crasher-defaults"], cppflags: ["-DSTATIC_CRASHER"], static_executable: true, + header_libs: ["bionic_libc_platform_headers"], static_libs: [ "libdebuggerd_handler", "libbase", diff --git a/debuggerd/debuggerd_test.cpp b/debuggerd/debuggerd_test.cpp index 99729dcd9..6a8cc563d 100644 --- a/debuggerd/debuggerd_test.cpp +++ b/debuggerd/debuggerd_test.cpp @@ -31,6 +31,7 @@ #include #include +#include #include #include @@ -398,7 +399,7 @@ TEST_F(CrasherTest, abort_message_backtrace) { unique_fd output_fd; StartProcess([]() { android_set_abort_message("not actually aborting"); - raise(DEBUGGER_SIGNAL); + raise(BIONIC_SIGNAL_DEBUGGER); exit(0); }); StartIntercept(&output_fd); @@ -466,7 +467,7 @@ TEST_F(CrasherTest, backtrace) { sigval val; val.sival_int = 1; - ASSERT_EQ(0, sigqueue(crasher_pid, DEBUGGER_SIGNAL, val)) << strerror(errno); + ASSERT_EQ(0, sigqueue(crasher_pid, BIONIC_SIGNAL_DEBUGGER, val)) << strerror(errno); FinishIntercept(&intercept_result); ASSERT_EQ(1, intercept_result) << "tombstoned reported failure"; ConsumeFd(std::move(output_fd), &result); @@ -734,7 +735,7 @@ __attribute__((noinline)) extern "C" bool raise_debugger_signal(DebuggerdDumpTyp siginfo.si_value.sival_int = dump_type == kDebuggerdNativeBacktrace; - if (syscall(__NR_rt_tgsigqueueinfo, getpid(), gettid(), DEBUGGER_SIGNAL, &siginfo) != 0) { + if (syscall(__NR_rt_tgsigqueueinfo, getpid(), gettid(), BIONIC_SIGNAL_DEBUGGER, &siginfo) != 0) { PLOG(ERROR) << "libdebuggerd_client: failed to send signal to self"; return false; } @@ -887,7 +888,7 @@ TEST(crash_dump, zombie) { errx(2, "first waitpid returned %d (%s), expected failure with ECHILD", rc, strerror(errno)); } - raise(DEBUGGER_SIGNAL); + raise(BIONIC_SIGNAL_DEBUGGER); errno = 0; rc = TEMP_FAILURE_RETRY(waitpid(-1, &status, __WALL | __WNOTHREAD)); diff --git a/debuggerd/handler/debuggerd_fallback.cpp b/debuggerd/handler/debuggerd_fallback.cpp index bbec612a8..9bcbdb36a 100644 --- a/debuggerd/handler/debuggerd_fallback.cpp +++ b/debuggerd/handler/debuggerd_fallback.cpp @@ -42,6 +42,7 @@ #include #include #include +#include #include #include #include @@ -272,7 +273,7 @@ static void trace_handler(siginfo_t* info, ucontext_t* ucontext) { siginfo.si_pid = getpid(); siginfo.si_uid = getuid(); - if (syscall(__NR_rt_tgsigqueueinfo, getpid(), tid, DEBUGGER_SIGNAL, &siginfo) != 0) { + if (syscall(__NR_rt_tgsigqueueinfo, getpid(), tid, BIONIC_SIGNAL_DEBUGGER, &siginfo) != 0) { async_safe_format_log(ANDROID_LOG_ERROR, "libc", "failed to send trace signal to %d: %s", tid, strerror(errno)); return false; @@ -340,7 +341,7 @@ static void crash_handler(siginfo_t* info, ucontext_t* ucontext, void* abort_mes extern "C" void debuggerd_fallback_handler(siginfo_t* info, ucontext_t* ucontext, void* abort_message) { - if (info->si_signo == DEBUGGER_SIGNAL && info->si_value.sival_ptr != nullptr) { + if (info->si_signo == BIONIC_SIGNAL_DEBUGGER && info->si_value.sival_ptr != nullptr) { return trace_handler(info, ucontext); } else { return crash_handler(info, ucontext, abort_message); diff --git a/debuggerd/handler/debuggerd_handler.cpp b/debuggerd/handler/debuggerd_handler.cpp index b90ca80b4..6e0128993 100644 --- a/debuggerd/handler/debuggerd_handler.cpp +++ b/debuggerd/handler/debuggerd_handler.cpp @@ -51,6 +51,7 @@ #include #include +#include #include #include @@ -175,7 +176,7 @@ static void log_signal_summary(const siginfo_t* info) { thread_name[MAX_TASK_NAME_LEN] = 0; } - if (info->si_signo == DEBUGGER_SIGNAL) { + if (info->si_signo == BIONIC_SIGNAL_DEBUGGER) { async_safe_format_log(ANDROID_LOG_INFO, "libc", "Requested dump for tid %d (%s)", __gettid(), thread_name); return; @@ -307,7 +308,7 @@ struct debugger_thread_info { static void* pseudothread_stack; static DebuggerdDumpType get_dump_type(const debugger_thread_info* thread_info) { - if (thread_info->siginfo->si_signo == DEBUGGER_SIGNAL && + if (thread_info->siginfo->si_signo == BIONIC_SIGNAL_DEBUGGER && thread_info->siginfo->si_value.sival_int) { return kDebuggerdNativeBacktrace; } @@ -429,7 +430,7 @@ static int debuggerd_dispatch_pseudothread(void* arg) { async_safe_format_log(ANDROID_LOG_FATAL, "libc", "crash_dump helper crashed or stopped"); } - if (thread_info->siginfo->si_signo != DEBUGGER_SIGNAL) { + if (thread_info->siginfo->si_signo != BIONIC_SIGNAL_DEBUGGER) { // For crashes, we don't need to minimize pause latency. // Wait for the dump to complete before having the process exit, to avoid being murdered by // ActivityManager or init. @@ -446,7 +447,7 @@ static void resend_signal(siginfo_t* info) { // exited with the correct exit status (e.g. so that sh will report // "Segmentation fault" instead of "Killed"). For this to work, we need // to deregister our signal handler for that signal before continuing. - if (info->si_signo != DEBUGGER_SIGNAL) { + if (info->si_signo != BIONIC_SIGNAL_DEBUGGER) { signal(info->si_signo, SIG_DFL); int rc = syscall(SYS_rt_tgsigqueueinfo, __getpid(), __gettid(), info->si_signo, info); if (rc != 0) { @@ -485,7 +486,7 @@ static void debuggerd_signal_handler(int signal_number, siginfo_t* info, void* c void* abort_message = nullptr; uintptr_t si_val = reinterpret_cast(info->si_ptr); - if (signal_number == DEBUGGER_SIGNAL) { + if (signal_number == BIONIC_SIGNAL_DEBUGGER) { if (info->si_code == SI_QUEUE && info->si_pid == __getpid()) { // Allow for the abort message to be explicitly specified via the sigqueue value. // Keep the bottom bit intact for representing whether we want a backtrace or a tombstone. @@ -576,7 +577,7 @@ static void debuggerd_signal_handler(int signal_number, siginfo_t* info, void* c fatal_errno("failed to restore traceable"); } - if (info->si_signo == DEBUGGER_SIGNAL) { + if (info->si_signo == BIONIC_SIGNAL_DEBUGGER) { // If the signal is fatal, don't unlock the mutex to prevent other crashing threads from // starting to dump right before our death. pthread_mutex_unlock(&crash_mutex); diff --git a/debuggerd/include/debuggerd/handler.h b/debuggerd/include/debuggerd/handler.h index 7196e0ad4..cd6fc0510 100644 --- a/debuggerd/include/debuggerd/handler.h +++ b/debuggerd/include/debuggerd/handler.h @@ -16,6 +16,7 @@ #pragma once +#include #include #include #include @@ -33,11 +34,11 @@ typedef struct { void debuggerd_init(debuggerd_callbacks_t* callbacks); // DEBUGGER_ACTION_DUMP_TOMBSTONE and DEBUGGER_ACTION_DUMP_BACKTRACE are both -// triggered via DEBUGGER_SIGNAL. The debugger_action_t is sent via si_value +// triggered via BIONIC_SIGNAL_DEBUGGER. The debugger_action_t is sent via si_value // using sigqueue(2) or equivalent. If no si_value is specified (e.g. if the // signal is sent by kill(2)), the default behavior is to print the backtrace // to the log. -#define DEBUGGER_SIGNAL (__SIGRTMIN + 3) +#define DEBUGGER_SIGNAL BIONIC_SIGNAL_DEBUGGER static void __attribute__((__unused__)) debuggerd_register_handlers(struct sigaction* action) { sigaction(SIGABRT, action, nullptr); @@ -50,7 +51,7 @@ static void __attribute__((__unused__)) debuggerd_register_handlers(struct sigac #endif sigaction(SIGSYS, action, nullptr); sigaction(SIGTRAP, action, nullptr); - sigaction(DEBUGGER_SIGNAL, action, nullptr); + sigaction(BIONIC_SIGNAL_DEBUGGER, action, nullptr); } __END_DECLS diff --git a/debuggerd/libdebuggerd/tombstone.cpp b/debuggerd/libdebuggerd/tombstone.cpp index 236fcf7a0..b64e260a9 100644 --- a/debuggerd/libdebuggerd/tombstone.cpp +++ b/debuggerd/libdebuggerd/tombstone.cpp @@ -52,9 +52,6 @@ #include #include -// Needed to get DEBUGGER_SIGNAL. -#include "debuggerd/handler.h" - #include "libdebuggerd/backtrace.h" #include "libdebuggerd/open_files_list.h" #include "libdebuggerd/utility.h" diff --git a/debuggerd/libdebuggerd/utility.cpp b/debuggerd/libdebuggerd/utility.cpp index 5ce26fcde..0a1d2a4da 100644 --- a/debuggerd/libdebuggerd/utility.cpp +++ b/debuggerd/libdebuggerd/utility.cpp @@ -35,6 +35,7 @@ #include #include #include +#include #include #include #include @@ -296,7 +297,8 @@ const char* get_signame(const siginfo_t* si) { case SIGSTOP: return "SIGSTOP"; case SIGSYS: return "SIGSYS"; case SIGTRAP: return "SIGTRAP"; - case DEBUGGER_SIGNAL: return ""; + case BIONIC_SIGNAL_DEBUGGER: + return ""; default: return "?"; } }