From 1f1021b6447072208788d408ca1b9deccc08f2e2 Mon Sep 17 00:00:00 2001 From: J-P Nurmi Date: Fri, 25 Apr 2025 12:30:25 +0200 Subject: [PATCH 1/2] feat: add CrashpadClient::Add/RemoveAttachment() for Windows --- client/crashpad_client.h | 14 +++++++++++ client/crashpad_client_win.cc | 18 ++++++++++++++ handler/win/crash_report_exception_handler.cc | 24 +++++++++++++++++-- handler/win/crash_report_exception_handler.h | 6 ++++- util/win/exception_handler_server.cc | 20 ++++++++++++++++ util/win/exception_handler_server.h | 14 +++++++++++ util/win/registration_protocol_win_structs.h | 13 ++++++++++ 7 files changed, 106 insertions(+), 3 deletions(-) diff --git a/client/crashpad_client.h b/client/crashpad_client.h index e9eed9cc42..a91fc41112 100644 --- a/client/crashpad_client.h +++ b/client/crashpad_client.h @@ -840,6 +840,20 @@ class CrashpadClient { static void SetCrashLoopBefore(uint64_t crash_loop_before_time); #endif +#if BUILDFLAG(IS_WIN) || DOXYGEN + //! \brief Adds a file to the list of files to be attached to the crash + //! report. + //! + //! \param[in] attachment The path to the file to be added. + void AddAttachment(const base::FilePath& attachment); + + //! \brief Removes a file from the list of files to be attached to the crash + //! report. + //! + //! \param[in] attachment The path to the file to be removed. + void RemoveAttachment(const base::FilePath& attachment); +#endif + private: #if BUILDFLAG(IS_WIN) //! \brief Registers process handlers for the client. diff --git a/client/crashpad_client_win.cc b/client/crashpad_client_win.cc index 7282449327..28e8388cb1 100644 --- a/client/crashpad_client_win.cc +++ b/client/crashpad_client_win.cc @@ -1183,4 +1183,22 @@ void CrashpadClient::SetFirstChanceExceptionHandler( first_chance_handler_ = handler; } +void CrashpadClient::AddAttachment(const base::FilePath& attachment) { + ClientToServerMessage message = {}; + message.type = ClientToServerMessage::kAddAttachment; + swprintf_s( + message.attachment.path, MAX_PATH, L"%ls", attachment.value().c_str()); + ServerToClientMessage response = {}; + SendToCrashHandlerServer(ipc_pipe_, message, &response); +} + +void CrashpadClient::RemoveAttachment(const base::FilePath& attachment) { + ClientToServerMessage message = {}; + message.type = ClientToServerMessage::kRemoveAttachment; + swprintf_s( + message.attachment.path, MAX_PATH, L"%ls", attachment.value().c_str()); + ServerToClientMessage response = {}; + SendToCrashHandlerServer(ipc_pipe_, message, &response); +} + } // namespace crashpad diff --git a/handler/win/crash_report_exception_handler.cc b/handler/win/crash_report_exception_handler.cc index 2f8a731e48..968500402e 100644 --- a/handler/win/crash_report_exception_handler.cc +++ b/handler/win/crash_report_exception_handler.cc @@ -45,7 +45,7 @@ CrashReportExceptionHandler::CrashReportExceptionHandler( : database_(database), upload_thread_(upload_thread), process_annotations_(process_annotations), - attachments_(attachments), + attachments_(*attachments), screenshot_(screenshot), wait_for_upload_(wait_for_upload), user_stream_data_sources_(user_stream_data_sources) {} @@ -117,7 +117,7 @@ unsigned int CrashReportExceptionHandler::ExceptionHandlerServerException( return termination_code; } - for (const auto& attachment : (*attachments_)) { + for (const auto& attachment : attachments_) { FileReader file_reader; if (!file_reader.Open(attachment)) { LOG(ERROR) << "attachment " << attachment @@ -175,4 +175,24 @@ unsigned int CrashReportExceptionHandler::ExceptionHandlerServerException( return termination_code; } +void CrashReportExceptionHandler::ExceptionHandlerServerAttachmentAdded( + const base::FilePath& attachment) { + auto it = std::find(attachments_.begin(), attachments_.end(), attachment); + if (it != attachments_.end()) { + LOG(WARNING) << "ignoring duplicate attachment " << attachment; + return; + } + attachments_.push_back(attachment); +} + +void CrashReportExceptionHandler::ExceptionHandlerServerAttachmentRemoved( + const base::FilePath& attachment) { + auto it = std::find(attachments_.begin(), attachments_.end(), attachment); + if (it == attachments_.end()) { + LOG(WARNING) << "ignoring non-existent attachment " << attachment; + return; + } + attachments_.erase(it); +} + } // namespace crashpad diff --git a/handler/win/crash_report_exception_handler.h b/handler/win/crash_report_exception_handler.h index d200fa7d4c..d0197da837 100644 --- a/handler/win/crash_report_exception_handler.h +++ b/handler/win/crash_report_exception_handler.h @@ -79,12 +79,16 @@ class CrashReportExceptionHandler final HANDLE process, WinVMAddress exception_information_address, WinVMAddress debug_critical_section_address) override; + void ExceptionHandlerServerAttachmentAdded( + const base::FilePath& attachment) override; + void ExceptionHandlerServerAttachmentRemoved( + const base::FilePath& attachment) override; private: CrashReportDatabase* database_; // weak CrashReportUploadThread* upload_thread_; // weak const std::map* process_annotations_; // weak - const std::vector* attachments_; // weak + std::vector attachments_; const base::FilePath* screenshot_; // weak const bool wait_for_upload_; const UserStreamDataSources* user_stream_data_sources_; // weak diff --git a/util/win/exception_handler_server.cc b/util/win/exception_handler_server.cc index bb5b90a1dc..43cdaabc4b 100644 --- a/util/win/exception_handler_server.cc +++ b/util/win/exception_handler_server.cc @@ -455,6 +455,26 @@ bool ExceptionHandlerServer::ServiceClientConnection( // Handled below. break; + case ClientToServerMessage::kAddAttachment: { + ServerToClientMessage shutdown_response = {}; + service_context.delegate()->ExceptionHandlerServerAttachmentAdded( + base::FilePath(message.attachment.path)); + LoggingWriteFile(service_context.pipe(), + &shutdown_response, + sizeof(shutdown_response)); + return false; + } + + case ClientToServerMessage::kRemoveAttachment: { + ServerToClientMessage shutdown_response = {}; + service_context.delegate()->ExceptionHandlerServerAttachmentRemoved( + base::FilePath(message.attachment.path)); + LoggingWriteFile(service_context.pipe(), + &shutdown_response, + sizeof(shutdown_response)); + return false; + } + default: LOG(ERROR) << "unhandled message type: " << message.type; return false; diff --git a/util/win/exception_handler_server.h b/util/win/exception_handler_server.h index 6f67ce8adc..df1ce87d1d 100644 --- a/util/win/exception_handler_server.h +++ b/util/win/exception_handler_server.h @@ -58,6 +58,20 @@ class ExceptionHandlerServer { WinVMAddress exception_information_address, WinVMAddress debug_critical_section_address) = 0; + //! \brief Called when the server has received a request to add an + //! attachment. + //! + //! \param[in] attachment The path of the attachment. + virtual void ExceptionHandlerServerAttachmentAdded( + const base::FilePath& attachment) = 0; + + //! \brief Called when the server has received a request to remove an + //! attachment. + //! + //! \param[in] attachment The path of the attachment. + virtual void ExceptionHandlerServerAttachmentRemoved( + const base::FilePath& attachment) = 0; + protected: ~Delegate(); }; diff --git a/util/win/registration_protocol_win_structs.h b/util/win/registration_protocol_win_structs.h index f4ad4fd5ae..fae7488606 100644 --- a/util/win/registration_protocol_win_structs.h +++ b/util/win/registration_protocol_win_structs.h @@ -116,6 +116,12 @@ struct ShutdownRequest { uint64_t token; }; +//! \brief A file attachment request. +struct AttachmentRequest { + //! \brief The path of the attachment. + wchar_t path[MAX_PATH]; +}; + //! \brief The message passed from client to server by //! SendToCrashHandlerServer(). struct ClientToServerMessage { @@ -127,6 +133,12 @@ struct ClientToServerMessage { //! \brief For ShutdownRequest. kShutdown, + //! \brief For AttachmentRequest. + kAddAttachment, + + //! \brief For AttachmentRequest. + kRemoveAttachment, + //! \brief An empty message sent by the initial client in asynchronous mode. //! No data is required, this just confirms that the server is ready to //! accept client registrations. @@ -136,6 +148,7 @@ struct ClientToServerMessage { union { RegistrationRequest registration; ShutdownRequest shutdown; + AttachmentRequest attachment; }; }; From b97109c0cbda479d2f3e21419466b0228ce38e08 Mon Sep 17 00:00:00 2001 From: J-P Nurmi Date: Fri, 25 Apr 2025 16:29:01 +0200 Subject: [PATCH 2/2] feat: add CrashpadClient::Add/RemoveAttachment() for Linux --- client/crashpad_client.h | 2 +- client/crashpad_client_linux.cc | 20 ++++++++++++++++ .../linux/crash_report_exception_handler.cc | 24 +++++++++++++++++-- .../linux/crash_report_exception_handler.h | 6 ++++- handler/linux/exception_handler_server.cc | 8 +++++++ handler/linux/exception_handler_server.h | 6 +++++ util/linux/exception_handler_client.cc | 19 +++++++++++++++ util/linux/exception_handler_client.h | 6 +++++ util/linux/exception_handler_protocol.h | 16 ++++++++++++- 9 files changed, 102 insertions(+), 5 deletions(-) diff --git a/client/crashpad_client.h b/client/crashpad_client.h index a91fc41112..ffb14c8e73 100644 --- a/client/crashpad_client.h +++ b/client/crashpad_client.h @@ -840,7 +840,7 @@ class CrashpadClient { static void SetCrashLoopBefore(uint64_t crash_loop_before_time); #endif -#if BUILDFLAG(IS_WIN) || DOXYGEN +#if BUILDFLAG(IS_WIN) || BUILDFLAG(IS_LINUX) || DOXYGEN //! \brief Adds a file to the list of files to be attached to the crash //! report. //! diff --git a/client/crashpad_client_linux.cc b/client/crashpad_client_linux.cc index a7e495f129..84aab5bfbd 100644 --- a/client/crashpad_client_linux.cc +++ b/client/crashpad_client_linux.cc @@ -436,6 +436,16 @@ class RequestCrashDumpHandler : public SignalHandler { } #endif + void AddAttachment(const base::FilePath& attachment) { + ExceptionHandlerClient client(sock_to_handler_.get(), true); + client.AddAttachment(attachment); + } + + void RemoveAttachment(const base::FilePath& attachment) { + ExceptionHandlerClient client(sock_to_handler_.get(), true); + client.RemoveAttachment(attachment); + } + private: RequestCrashDumpHandler() = default; @@ -807,4 +817,14 @@ void CrashpadClient::SetCrashLoopBefore(uint64_t crash_loop_before_time) { } #endif +void CrashpadClient::AddAttachment(const base::FilePath& attachment) { + auto signal_handler = RequestCrashDumpHandler::Get(); + signal_handler->AddAttachment(attachment); +} + +void CrashpadClient::RemoveAttachment(const base::FilePath& attachment) { + auto signal_handler = RequestCrashDumpHandler::Get(); + signal_handler->RemoveAttachment(attachment); +} + } // namespace crashpad diff --git a/handler/linux/crash_report_exception_handler.cc b/handler/linux/crash_report_exception_handler.cc index aa2330396e..15eac12517 100644 --- a/handler/linux/crash_report_exception_handler.cc +++ b/handler/linux/crash_report_exception_handler.cc @@ -111,7 +111,7 @@ CrashReportExceptionHandler::CrashReportExceptionHandler( : database_(database), upload_thread_(upload_thread), process_annotations_(process_annotations), - attachments_(attachments), + attachments_(*attachments), write_minidump_to_database_(write_minidump_to_database), write_minidump_to_log_(write_minidump_to_log), user_stream_data_sources_(user_stream_data_sources), @@ -210,6 +210,26 @@ bool CrashReportExceptionHandler::HandleExceptionWithConnection( return result; } +void CrashReportExceptionHandler::AddAttachment( + const base::FilePath& attachment) { + auto it = std::find(attachments_.begin(), attachments_.end(), attachment); + if (it != attachments_.end()) { + LOG(WARNING) << "ignoring duplicate attachment " << attachment; + return; + } + attachments_.push_back(attachment); +} + +void CrashReportExceptionHandler::RemoveAttachment( + const base::FilePath& attachment) { + auto it = std::find(attachments_.begin(), attachments_.end(), attachment); + if (it == attachments_.end()) { + LOG(WARNING) << "ignoring non-existent attachment " << attachment; + return; + } + attachments_.erase(it); +} + bool CrashReportExceptionHandler::WriteMinidumpToDatabase( ProcessSnapshotLinux* process_snapshot, ProcessSnapshotSanitized* sanitized_snapshot, @@ -252,7 +272,7 @@ bool CrashReportExceptionHandler::WriteMinidumpToDatabase( } } - for (const auto& attachment : (*attachments_)) { + for (const auto& attachment : attachments_) { FileReader file_reader; if (!file_reader.Open(attachment)) { LOG(ERROR) << "attachment " << attachment.value().c_str() diff --git a/handler/linux/crash_report_exception_handler.h b/handler/linux/crash_report_exception_handler.h index 0ee6423e03..07db486a36 100644 --- a/handler/linux/crash_report_exception_handler.h +++ b/handler/linux/crash_report_exception_handler.h @@ -18,6 +18,7 @@ #include #include +#include "base/files/file_path.h" #include "client/crash_report_database.h" #include "handler/crash_report_upload_thread.h" #include "handler/linux/exception_handler_server.h" @@ -94,6 +95,9 @@ class CrashReportExceptionHandler : public ExceptionHandlerServer::Delegate { int broker_sock, UUID* local_report_id = nullptr) override; + void AddAttachment(const base::FilePath& attachment) override; + void RemoveAttachment(const base::FilePath& attachment) override; + private: bool HandleExceptionWithConnection( PtraceConnection* connection, @@ -119,7 +123,7 @@ class CrashReportExceptionHandler : public ExceptionHandlerServer::Delegate { CrashReportDatabase* database_; // weak CrashReportUploadThread* upload_thread_; // weak const std::map* process_annotations_; // weak - const std::vector* attachments_; // weak + std::vector attachments_; bool write_minidump_to_database_; bool write_minidump_to_log_; const UserStreamDataSources* user_stream_data_sources_; // weak diff --git a/handler/linux/exception_handler_server.cc b/handler/linux/exception_handler_server.cc index 00b076469b..69c3c319a7 100644 --- a/handler/linux/exception_handler_server.cc +++ b/handler/linux/exception_handler_server.cc @@ -431,6 +431,14 @@ bool ExceptionHandlerServer::ReceiveClientMessage(Event* event) { message.requesting_thread_stack_address, event->fd.get(), event->type == Event::Type::kSharedSocketMessage); + + case ExceptionHandlerProtocol::ClientToServerMessage::kTypeAddAttachment: + delegate_->AddAttachment(base::FilePath(message.attachment_info.path)); + return true; + + case ExceptionHandlerProtocol::ClientToServerMessage::kTypeRemoveAttachment: + delegate_->RemoveAttachment(base::FilePath(message.attachment_info.path)); + return true; } DCHECK(false); diff --git a/handler/linux/exception_handler_server.h b/handler/linux/exception_handler_server.h index 738881ad3e..df9749ef31 100644 --- a/handler/linux/exception_handler_server.h +++ b/handler/linux/exception_handler_server.h @@ -111,6 +111,12 @@ class ExceptionHandlerServer { int broker_sock, UUID* local_report_id = nullptr) = 0; + //! \brief Called to add an attachment to the crash report. + virtual void AddAttachment(const base::FilePath& attachment) = 0; + + //! \brief Called to remove an attachment from the crash report. + virtual void RemoveAttachment(const base::FilePath& attachment) = 0; + virtual ~Delegate() {} }; diff --git a/util/linux/exception_handler_client.cc b/util/linux/exception_handler_client.cc index 68dc67eff9..60d72257d1 100644 --- a/util/linux/exception_handler_client.cc +++ b/util/linux/exception_handler_client.cc @@ -225,4 +225,23 @@ int ExceptionHandlerClient::WaitForCrashDumpComplete() { return errno; } +void ExceptionHandlerClient::AddAttachment(const base::FilePath& attachment) { + ExceptionHandlerProtocol::ClientToServerMessage message; + message.type = + ExceptionHandlerProtocol::ClientToServerMessage::kTypeAddAttachment; + snprintf( + message.attachment_info.path, PATH_MAX, "%s", attachment.value().c_str()); + UnixCredentialSocket::SendMsg(server_sock_, &message, sizeof(message)); +} + +void ExceptionHandlerClient::RemoveAttachment( + const base::FilePath& attachment) { + ExceptionHandlerProtocol::ClientToServerMessage message; + message.type = + ExceptionHandlerProtocol::ClientToServerMessage::kTypeRemoveAttachment; + snprintf( + message.attachment_info.path, PATH_MAX, "%s", attachment.value().c_str()); + UnixCredentialSocket::SendMsg(server_sock_, &message, sizeof(message)); +} + } // namespace crashpad diff --git a/util/linux/exception_handler_client.h b/util/linux/exception_handler_client.h index 2212e2d15d..30c706a0c2 100644 --- a/util/linux/exception_handler_client.h +++ b/util/linux/exception_handler_client.h @@ -68,6 +68,12 @@ class ExceptionHandlerClient { //! \param[in] can_set_ptracer Whether SetPtracer should be enabled. void SetCanSetPtracer(bool can_set_ptracer); + //! \brief Adds an attachment to the crash report. + void AddAttachment(const base::FilePath& attachment); + + //! \brief Removes an attachment from the crash report. + void RemoveAttachment(const base::FilePath& attachment); + private: int SendCrashDumpRequest( const ExceptionHandlerProtocol::ClientInformation& info, diff --git a/util/linux/exception_handler_protocol.h b/util/linux/exception_handler_protocol.h index 14fd863314..07773a0010 100644 --- a/util/linux/exception_handler_protocol.h +++ b/util/linux/exception_handler_protocol.h @@ -16,6 +16,7 @@ #define CRASHPAD_UTIL_LINUX_EXCEPTION_HANDLER_PROTOCOL_H_ #include +#include #include #include #include @@ -59,6 +60,10 @@ class ExceptionHandlerProtocol { #endif }; + struct AttachmentInformation { + char path[PATH_MAX]; + }; + //! \brief The signal used to indicate a crash dump is complete. //! //! When multiple clients share a single socket connection with the handler, @@ -81,7 +86,13 @@ class ExceptionHandlerProtocol { kTypeCheckCredentials, //! \brief Used to request a crash dump for the sending client. - kTypeCrashDumpRequest + kTypeCrashDumpRequest, + + //! \brief Request that the server add an attachment. + kTypeAddAttachment, + + //! \brief Request that the server remove an attachment. + kTypeRemoveAttachment, }; Type type; @@ -92,6 +103,9 @@ class ExceptionHandlerProtocol { union { //! \brief Valid for type == kCrashDumpRequest ClientInformation client_info; + + //! \brief Valid for type == kAddAttachment || type == kRemoveAttachment + AttachmentInformation attachment_info; }; };