From 36e473fa8afd7e1cff16016885f5df2d20784936 Mon Sep 17 00:00:00 2001 From: J-P Nurmi Date: Thu, 7 May 2026 11:43:49 +0200 Subject: [PATCH] feat: implement CrashpadClient::Add/RemoveAttachment() for macOS Add macOS add/remove attachment control messages using the existing `ClientToServerMessage` protocol added in #147. This mirrors the dynamic attachment support added for Windows and Linux in #124. Co-Authored-By: Codex --- client/crashpad_client.h | 2 +- client/crashpad_client_mac.cc | 12 +++++++ handler/mac/crash_report_exception_handler.cc | 35 ++++++++++++++++--- handler/mac/crash_report_exception_handler.h | 6 ++-- handler/mac/exception_handler_server.cc | 6 ++++ handler/mac/exception_handler_server.h | 9 +++++ util/mach/exception_handler_protocol.cc | 8 +++++ util/mach/exception_handler_protocol.h | 11 ++++++ 8 files changed, 82 insertions(+), 7 deletions(-) diff --git a/client/crashpad_client.h b/client/crashpad_client.h index 6f8351872..8067c4e93 100644 --- a/client/crashpad_client.h +++ b/client/crashpad_client.h @@ -858,7 +858,7 @@ class CrashpadClient { static void SetCrashLoopBefore(uint64_t crash_loop_before_time); #endif -#if BUILDFLAG(IS_WIN) || BUILDFLAG(IS_LINUX) || DOXYGEN +#if BUILDFLAG(IS_WIN) || BUILDFLAG(IS_LINUX) || BUILDFLAG(IS_MAC) || DOXYGEN //! \brief Adds a file to the list of files to be attached to the crash //! report. //! diff --git a/client/crashpad_client_mac.cc b/client/crashpad_client_mac.cc index 010c71b68..d9993d4bd 100644 --- a/client/crashpad_client_mac.cc +++ b/client/crashpad_client_mac.cc @@ -629,6 +629,18 @@ void CrashpadClient::RequestRetry() { ClientToServerMessage::kRequestRetry); } +void CrashpadClient::AddAttachment(const base::FilePath& attachment) { + SendClientToServerMessage(exception_port_.get(), + ClientToServerMessage::kAddAttachment, + attachment.value()); +} + +void CrashpadClient::RemoveAttachment(const base::FilePath& attachment) { + SendClientToServerMessage(exception_port_.get(), + ClientToServerMessage::kRemoveAttachment, + attachment.value()); +} + // static void CrashpadClient::UseSystemDefaultHandler() { base::apple::ScopedMachSendRight system_crash_reporter_handler( diff --git a/handler/mac/crash_report_exception_handler.cc b/handler/mac/crash_report_exception_handler.cc index e97d54010..a6341abc8 100644 --- a/handler/mac/crash_report_exception_handler.cc +++ b/handler/mac/crash_report_exception_handler.cc @@ -14,6 +14,7 @@ #include "handler/mac/crash_report_exception_handler.h" +#include #include #include @@ -57,12 +58,16 @@ CrashReportExceptionHandler::CrashReportExceptionHandler( : database_(database), upload_thread_(upload_thread), process_annotations_(process_annotations), - attachments_(attachments), + attachments_(), user_stream_data_sources_(user_stream_data_sources), wait_for_upload_(wait_for_upload), crash_reporter_(crash_reporter), crash_envelope_(crash_envelope), - report_id_(report_id) {} + report_id_(report_id) { + if (attachments) { + attachments_ = *attachments; + } +} CrashReportExceptionHandler::~CrashReportExceptionHandler() { } @@ -186,7 +191,7 @@ kern_return_t CrashReportExceptionHandler::CatchMachException( return KERN_FAILURE; } - 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() @@ -210,7 +215,7 @@ kern_return_t CrashReportExceptionHandler::CatchMachException( if (has_crash_reporter) { CrashReportDatabase::Envelope envelope(new_report->ReportID()); if (envelope.Initialize(*crash_envelope_)) { - envelope.AddAttachments(*attachments_); + envelope.AddAttachments(attachments_); if (auto reader = new_report->Reader()) { envelope.AddMinidump(reader); } @@ -320,4 +325,26 @@ void CrashReportExceptionHandler::RequestRetry() { } } +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); +} + } // namespace crashpad diff --git a/handler/mac/crash_report_exception_handler.h b/handler/mac/crash_report_exception_handler.h index d4ae04443..2fe4ba039 100644 --- a/handler/mac/crash_report_exception_handler.h +++ b/handler/mac/crash_report_exception_handler.h @@ -19,6 +19,7 @@ #include #include +#include #include "client/crash_report_database.h" #include "handler/crash_report_upload_thread.h" @@ -97,13 +98,14 @@ class CrashReportExceptionHandler final // ExceptionHandlerServer::Delegate: void RequestRetry() override; + void AddAttachment(const base::FilePath& attachment) override; + void RemoveAttachment(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 UserStreamDataSources* user_stream_data_sources_; // weak bool wait_for_upload_; const base::FilePath* crash_reporter_; // weak diff --git a/handler/mac/exception_handler_server.cc b/handler/mac/exception_handler_server.cc index b538c5388..6cb9428bf 100644 --- a/handler/mac/exception_handler_server.cc +++ b/handler/mac/exception_handler_server.cc @@ -57,6 +57,12 @@ class ClientToServerMessageServer : public MachMessageServer::Interface { case ClientToServerMessage::kRequestRetry: delegate_->RequestRetry(); break; + case ClientToServerMessage::kAddAttachment: + delegate_->AddAttachment(base::FilePath(message->Payload())); + break; + case ClientToServerMessage::kRemoveAttachment: + delegate_->RemoveAttachment(base::FilePath(message->Payload())); + break; } return true; } diff --git a/handler/mac/exception_handler_server.h b/handler/mac/exception_handler_server.h index c83d98f27..e97d9ab87 100644 --- a/handler/mac/exception_handler_server.h +++ b/handler/mac/exception_handler_server.h @@ -18,6 +18,7 @@ #include #include "base/apple/scoped_mach_port.h" +#include "base/files/file_path.h" #include "util/mach/exc_server_variants.h" namespace crashpad { @@ -35,6 +36,14 @@ class ExceptionHandlerServer { //! \brief Called when the server has received a request to retry pending //! report uploads. virtual void RequestRetry() = 0; + + //! \brief Called when the server has received a request to add a file to + //! the list of files attached to crash reports. + virtual void AddAttachment(const base::FilePath& attachment) = 0; + + //! \brief Called when the server has received a request to remove a file + //! from the list of files attached to crash reports. + virtual void RemoveAttachment(const base::FilePath& attachment) = 0; }; //! \brief Constructs an ExceptionHandlerServer object. diff --git a/util/mach/exception_handler_protocol.cc b/util/mach/exception_handler_protocol.cc index e9cc6c49d..8fa80d100 100644 --- a/util/mach/exception_handler_protocol.cc +++ b/util/mach/exception_handler_protocol.cc @@ -22,6 +22,14 @@ namespace crashpad { +std::string ClientToServerMessage::Payload() const { + const char* payload_data = static_cast(payload.address); + if (!payload_data || payload.size == 0) { + return std::string(); + } + return std::string(payload_data, static_cast(payload.size - 1)); +} + bool SendClientToServerMessage(mach_port_t port, ClientToServerMessage::Type type, const std::string& payload) { diff --git a/util/mach/exception_handler_protocol.h b/util/mach/exception_handler_protocol.h index 145f1550a..cdd3af1d1 100644 --- a/util/mach/exception_handler_protocol.h +++ b/util/mach/exception_handler_protocol.h @@ -41,6 +41,14 @@ struct ClientToServerMessage { //! \brief Ask the handler to retry pending report uploads. Carries no //! meaningful payload. kRequestRetry = 1, + + //! \brief Add a file to the list of files attached to crash reports. The + //! payload contains the attachment path. + kAddAttachment = 2, + + //! \brief Remove a file from the list of files attached to crash reports. + //! The payload contains the attachment path. + kRemoveAttachment = 3, }; mach_msg_header_t header; @@ -48,6 +56,9 @@ struct ClientToServerMessage { mach_msg_ool_descriptor_t payload; NDR_record_t ndr; Type type; + + //! \brief Returns the optional string payload. + std::string Payload() const; }; //! \brief Sends a ClientToServerMessage to the handler.