From fc44a3747cf596be25ef0d6ee2c69d1f9dcad143 Mon Sep 17 00:00:00 2001 From: Joshua Peraza Date: Mon, 9 Sep 2019 14:07:42 -0700 Subject: [PATCH] linux: Allow configuring unhandled signals Change-Id: I621555f892a3064c5cba09120309bc900da237f9 Reviewed-on: https://chromium-review.googlesource.com/c/crashpad/crashpad/+/1793563 Reviewed-by: Mark Mentovai Commit-Queue: Joshua Peraza --- client/crashpad_client.h | 21 ++++++++++++++++---- client/crashpad_client_linux.cc | 35 +++++++++++++++++++-------------- util/posix/signals.cc | 16 +++++++++++---- util/posix/signals.h | 12 ++++++++--- util/posix/signals_test.cc | 7 ++++++- 5 files changed, 64 insertions(+), 27 deletions(-) diff --git a/client/crashpad_client.h b/client/crashpad_client.h index 36c9d1f8..f357ca01 100644 --- a/client/crashpad_client.h +++ b/client/crashpad_client.h @@ -16,6 +16,7 @@ #define CRASHPAD_CLIENT_CRASHPAD_CLIENT_H_ #include +#include #include #include @@ -138,7 +139,7 @@ class CrashpadClient { //! this process' ptracer. 0 indicates it is not necessary to set the //! handler as this process' ptracer. -1 indicates that the handler's //! process ID should be determined by communicating over the socket. - static bool SetHandlerSocket(ScopedFileHandle sock, pid_t pid); + bool SetHandlerSocket(ScopedFileHandle sock, pid_t pid); #endif // OS_ANDROID || OS_LINUX || DOXYGEN #if defined(OS_ANDROID) || DOXYGEN @@ -170,7 +171,7 @@ class CrashpadClient { //! specified in this parameter. //! //! \return `true` on success, `false` on failure with a message logged. - static bool StartJavaHandlerAtCrash( + bool StartJavaHandlerAtCrash( const std::string& class_name, const std::vector* env, const base::FilePath& database, @@ -251,7 +252,7 @@ class CrashpadClient { //! specified in this parameter. //! //! \return `true` on success, `false` on failure with a message logged. - static bool StartHandlerWithLinkerAtCrash( + bool StartHandlerWithLinkerAtCrash( const std::string& handler_trampoline, const std::string& handler_library, bool is_64_bit, @@ -333,7 +334,7 @@ class CrashpadClient { //! specified in this parameter. //! //! \return `true` on success, `false` on failure with a message logged. - static bool StartHandlerAtCrash( + bool StartHandlerAtCrash( const base::FilePath& handler, const base::FilePath& database, const base::FilePath& metrics_dir, @@ -413,6 +414,16 @@ class CrashpadClient { //! \param[in] handler The custom crash signal handler to install. static void SetFirstChanceExceptionHandler(FirstChanceHandler handler); + //! \brief Configures a set of signals that shouldn't have Crashpad signal + //! handlers installed. + //! + //! This method should be called before calling StartHandler(), + //! SetHandlerSocket(), or other methods that install Crashpad signal + //! handlers. + //! + //! \param[in] unhandled_signals The set of unhandled signals + void SetUnhandledSignals(const std::set& unhandled_signals); + #endif // OS_LINUX || OS_ANDROID || DOXYGEN #if defined(OS_MACOSX) || DOXYGEN @@ -604,6 +615,8 @@ class CrashpadClient { #elif defined(OS_WIN) std::wstring ipc_pipe_; ScopedKernelHANDLE handler_start_thread_; +#elif defined(OS_LINUX) + std::set unhandled_signals_; #endif // OS_MACOSX DISALLOW_COPY_AND_ASSIGN(CrashpadClient); diff --git a/client/crashpad_client_linux.cc b/client/crashpad_client_linux.cc index eb210a28..c3ebbd4c 100644 --- a/client/crashpad_client_linux.cc +++ b/client/crashpad_client_linux.cc @@ -155,11 +155,11 @@ class SignalHandler { protected: SignalHandler() = default; - bool Install() { + bool Install(const std::set* unhandled_signals) { DCHECK(!handler_); handler_ = this; return Signals::InstallCrashHandlers( - HandleOrReraiseSignal, 0, &old_actions_); + HandleOrReraiseSignal, 0, &old_actions_, unhandled_signals); } const ExceptionInformation& GetExceptionInfo() { @@ -202,7 +202,8 @@ class LaunchAtCrashHandler : public SignalHandler { } bool Initialize(std::vector* argv_in, - const std::vector* envp) { + const std::vector* envp, + const std::set* unhandled_signals) { argv_strings_.swap(*argv_in); if (envp) { @@ -215,7 +216,7 @@ class LaunchAtCrashHandler : public SignalHandler { &GetExceptionInfo())); StringVectorToCStringVector(argv_strings_, &argv_); - return Install(); + return Install(unhandled_signals); } void HandleCrashImpl() override { @@ -270,7 +271,9 @@ class RequestCrashDumpHandler : public SignalHandler { // created the namespace. // pid > 0 directly indicates what the handler's pid is expected to be, so // retrieving this information from the handler is not necessary. - bool Initialize(ScopedFileHandle sock, pid_t pid) { + bool Initialize(ScopedFileHandle sock, + pid_t pid, + const std::set* unhandled_signals) { ExceptionHandlerClient client(sock.get(), true); if (pid < 0) { ucred creds; @@ -285,7 +288,7 @@ class RequestCrashDumpHandler : public SignalHandler { } sock_to_handler_.reset(sock.release()); handler_pid_ = pid; - return Install(); + return Install(unhandled_signals); } bool GetHandlerSocket(int* sock, pid_t* pid) { @@ -368,7 +371,8 @@ bool CrashpadClient::StartHandler( } auto signal_handler = RequestCrashDumpHandler::Get(); - return signal_handler->Initialize(std::move(client_sock), -1); + return signal_handler->Initialize( + std::move(client_sock), -1, &unhandled_signals_); } #if defined(OS_ANDROID) || defined(OS_LINUX) @@ -378,16 +382,14 @@ bool CrashpadClient::GetHandlerSocket(int* sock, pid_t* pid) { return signal_handler->GetHandlerSocket(sock, pid); } -// static bool CrashpadClient::SetHandlerSocket(ScopedFileHandle sock, pid_t pid) { auto signal_handler = RequestCrashDumpHandler::Get(); - return signal_handler->Initialize(std::move(sock), pid); + return signal_handler->Initialize(std::move(sock), pid, &unhandled_signals_); } #endif // OS_ANDROID || OS_LINUX #if defined(OS_ANDROID) -// static bool CrashpadClient::StartJavaHandlerAtCrash( const std::string& class_name, const std::vector* env, @@ -405,7 +407,7 @@ bool CrashpadClient::StartJavaHandlerAtCrash( kInvalidFileHandle); auto signal_handler = LaunchAtCrashHandler::Get(); - return signal_handler->Initialize(&argv, env); + return signal_handler->Initialize(&argv, env, &unhandled_signals_); } // static @@ -423,7 +425,6 @@ bool CrashpadClient::StartJavaHandlerForClient( return DoubleForkAndExec(argv, env, socket, false, nullptr); } -// static bool CrashpadClient::StartHandlerWithLinkerAtCrash( const std::string& handler_trampoline, const std::string& handler_library, @@ -445,7 +446,7 @@ bool CrashpadClient::StartHandlerWithLinkerAtCrash( arguments, kInvalidFileHandle); auto signal_handler = LaunchAtCrashHandler::Get(); - return signal_handler->Initialize(&argv, env); + return signal_handler->Initialize(&argv, env, &unhandled_signals_); } // static @@ -475,7 +476,6 @@ bool CrashpadClient::StartHandlerWithLinkerForClient( #endif -// static bool CrashpadClient::StartHandlerAtCrash( const base::FilePath& handler, const base::FilePath& database, @@ -487,7 +487,7 @@ bool CrashpadClient::StartHandlerAtCrash( handler, database, metrics_dir, url, annotations, arguments); auto signal_handler = LaunchAtCrashHandler::Get(); - return signal_handler->Initialize(&argv, nullptr); + return signal_handler->Initialize(&argv, nullptr, &unhandled_signals_); } // static @@ -543,6 +543,11 @@ void CrashpadClient::SetFirstChanceExceptionHandler( SignalHandler::Get()->SetFirstChanceHandler(handler); } +void CrashpadClient::SetUnhandledSignals(const std::set& signals) { + DCHECK(!SignalHandler::Get()); + unhandled_signals_ = signals; +} + #if defined(OS_CHROMEOS) // static void CrashpadClient::SetCrashLoopBefore(uint64_t crash_loop_before_time) { diff --git a/util/posix/signals.cc b/util/posix/signals.cc index 1384de1e..53c10389 100644 --- a/util/posix/signals.cc +++ b/util/posix/signals.cc @@ -93,9 +93,14 @@ constexpr int kTerminateSignals[] = { bool InstallHandlers(const std::vector& signals, Signals::Handler handler, int flags, - Signals::OldActions* old_actions) { + Signals::OldActions* old_actions, + const std::set* unhandled_signals) { bool success = true; for (int sig : signals) { + if (unhandled_signals && + unhandled_signals->find(sig) != unhandled_signals->end()) { + continue; + } success &= Signals::InstallHandler( sig, handler, @@ -151,13 +156,15 @@ bool Signals::InstallDefaultHandler(int sig) { // static bool Signals::InstallCrashHandlers(Handler handler, int flags, - OldActions* old_actions) { + OldActions* old_actions, + const std::set* unhandled_signals) { return InstallHandlers( std::vector(kCrashSignals, kCrashSignals + base::size(kCrashSignals)), handler, flags, - old_actions); + old_actions, + unhandled_signals); } // static @@ -169,7 +176,8 @@ bool Signals::InstallTerminateHandlers(Handler handler, kTerminateSignals + base::size(kTerminateSignals)), handler, flags, - old_actions); + old_actions, + nullptr); } // static diff --git a/util/posix/signals.h b/util/posix/signals.h index dc550594..368161bf 100644 --- a/util/posix/signals.h +++ b/util/posix/signals.h @@ -17,6 +17,8 @@ #include +#include + #include "base/macros.h" namespace crashpad { @@ -114,15 +116,19 @@ class Signals { //! the new action. May be `nullptr` if not needed. The same \a //! old_actions object may be used for calls to both this function and //! InstallTerminateHandlers(). + //! \param[in] unhandled_signals Signal handlers will not be installed for + //! signal numbers in this set. Optional. //! //! \return `true` on success. `false` on failure with a message logged. //! //! \warning This function may not be called from a signal handler because of //! its use of logging. See RestoreHandlerAndReraiseSignalOnReturn() //! instead. - static bool InstallCrashHandlers(Handler handler, - int flags, - OldActions* old_actions); + static bool InstallCrashHandlers( + Handler handler, + int flags, + OldActions* old_actions, + const std::set* unhandled_signals = nullptr); //! \brief Installs a new signal handler for all signals associated with //! termination. diff --git a/util/posix/signals_test.cc b/util/posix/signals_test.cc index d91e3cc6..f46e331e 100644 --- a/util/posix/signals_test.cc +++ b/util/posix/signals_test.cc @@ -258,7 +258,12 @@ class SignalsTest : public Multiprocess { void MultiprocessChild() override { bool (*install_handlers)(Signals::Handler, int, Signals::OldActions*); if (Signals::IsCrashSignal(sig_)) { - install_handlers = Signals::InstallCrashHandlers; + install_handlers = [](Signals::Handler handler, + int sig, + Signals::OldActions* old_actions) { + return Signals::InstallCrashHandlers( + handler, sig, old_actions, nullptr); + }; } else if (Signals::IsTerminateSignal(sig_)) { install_handlers = Signals::InstallTerminateHandlers; } else {