WIP: feat: add CrashpadClient::Add/RemoveAttachment() for macOS

Author: jpnurmi

client/crashpad_client.h

@@ -855,7 +855,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.
   //!

client/crashpad_client_mac.cc

@@ -35,6 +35,7 @@
 #include "util/mach/mach_extensions.h"
 #include "util/mach/mach_message.h"
 #include "util/mach/notify_server.h"
+#include "util/mach/payload_message.h"
 #include "util/misc/clock.h"
 #include "util/misc/implicit_cast.h"
 #include "util/posix/spawn_subprocess.h"
@@ -520,7 +521,7 @@ bool CrashpadClient::StartHandler(
     bool wait_for_upload,
     const base::FilePath& crash_reporter,
     const base::FilePath& crash_envelope) {
-  (void) wait_for_upload; // unused in mac (for now)
+  (void)wait_for_upload;  // unused in mac (for now)
 
   // The “restartable” behavior can only be selected on OS X 10.10 and later. In
   // previous OS versions, if the initial client were to crash while attempting
@@ -609,4 +610,13 @@ void CrashpadClient::UseSystemDefaultHandler() {
   }
 }
 
+void CrashpadClient::AddAttachment(const base::FilePath& attachment) {
+  SendPayloadMessage(exception_port_.get(), kAddAttachment, attachment.value());
+}
+
+void CrashpadClient::RemoveAttachment(const base::FilePath& attachment) {
+  SendPayloadMessage(
+      exception_port_.get(), kRemoveAttachment, attachment.value());
+}
+
 }  // namespace crashpad

handler/mac/crash_report_exception_handler.cc

@@ -16,6 +16,7 @@
 
 #include <utility>
 #include <vector>
+#include <algorithm>
 
 #include "base/apple/mach_logging.h"
 #include "base/apple/scoped_mach_port.h"
@@ -55,10 +56,15 @@ CrashReportExceptionHandler::CrashReportExceptionHandler(
     : database_(database),
       upload_thread_(upload_thread),
       process_annotations_(process_annotations),
-      attachments_(attachments),
+      attachments_(),
       user_stream_data_sources_(user_stream_data_sources),
       crash_reporter_(crash_reporter),
-      crash_envelope_(crash_envelope) {}
+      crash_envelope_(crash_envelope) {
+  if (attachments) {
+    // Copy the attachments into our owned vector.
+    attachments_ = *attachments;
+  }
+}
 
 CrashReportExceptionHandler::~CrashReportExceptionHandler() {
 }
@@ -182,7 +188,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()
@@ -206,7 +212,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);
         }
@@ -306,4 +312,40 @@ kern_return_t CrashReportExceptionHandler::CatchMachException(
   return KERN_SUCCESS;
 }
 
+void CrashReportExceptionHandler::HandlePayloadMessage(
+    const PayloadMessage& message) {
+  std::string payload(static_cast<const char*>(message.payload.address),
+                   message.payload.size - 1);
+
+  switch (message.type) {
+    case kAddAttachment:
+      AddAttachment(base::FilePath(payload));
+      break;
+    case kRemoveAttachment:
+      RemoveAttachment(base::FilePath(payload));
+      break;
+    default:
+      LOG(ERROR) << "unknown attachment message type: " << message.type;
+      break;
+  }
+}
+
+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

handler/mac/crash_report_exception_handler.h

@@ -22,6 +22,7 @@
 
 #include "client/crash_report_database.h"
 #include "handler/crash_report_upload_thread.h"
+#include "handler/mac/exception_handler_server.h"
 #include "handler/user_stream_data_source.h"
 #include "util/mach/exc_server_variants.h"
 
@@ -30,7 +31,8 @@ namespace crashpad {
 //! \brief An exception handler that writes crash reports for exception messages
 //!     to a CrashReportDatabase.
 class CrashReportExceptionHandler final
-    : public UniversalMachExcServer::Interface {
+    : public UniversalMachExcServer::Interface,
+      public PayloadMessageHandler {
  public:
   //! \brief Creates a new object that will store crash reports in \a database.
   //!
@@ -89,11 +91,27 @@ class CrashReportExceptionHandler final
       const mach_msg_trailer_t* trailer,
       bool* destroy_complex_request) override;
 
+  // PayloadMessageHandler:
+
+  //! \brief Processes a payload message by adding or removing an attachment
+  //!     to or from the attachments vector.
+  void HandlePayloadMessage(const PayloadMessage& message) override;
+
+  //! \brief Adds an attachment to the attachments list.
+  //!
+  //! \param[in] path The path of the attachment to add.
+  void AddAttachment(const base::FilePath& path);
+
+  //! \brief Removes an attachment from the attachments list.
+  //!
+  //! \param[in] path The path of the attachment to remove.
+  void RemoveAttachment(const base::FilePath& path);
+
  private:
   CrashReportDatabase* database_;  // weak
   CrashReportUploadThread* upload_thread_;  // weak
   const std::map<std::string, std::string>* process_annotations_;  // weak
-  const std::vector<base::FilePath>* attachments_;  // weak
+  std::vector<base::FilePath> attachments_;  // owned
   const UserStreamDataSources* user_stream_data_sources_;  // weak
   const base::FilePath* crash_reporter_;  // weak
   const base::FilePath* crash_envelope_;  // weak

handler/mac/exception_handler_server.cc

@@ -19,6 +19,7 @@
 #include "base/apple/mach_logging.h"
 #include "base/check.h"
 #include "base/logging.h"
+#include "handler/mac/crash_report_exception_handler.h"
 #include "util/mach/composite_mach_message_server.h"
 #include "util/mach/mach_extensions.h"
 #include "util/mach/mach_message.h"
@@ -29,6 +30,49 @@ namespace crashpad {
 
 namespace {
 
+// Custom server for handling payload messages
+class PayloadMessageServer : public MachMessageServer::Interface {
+ public:
+  PayloadMessageServer(PayloadMessageHandler* handler) : handler_(handler) {}
+
+  PayloadMessageServer(const PayloadMessageServer&) = delete;
+  PayloadMessageServer& operator=(const PayloadMessageServer&) = delete;
+
+  // MachMessageServer::Interface:
+
+  // Processes an incoming message and dispatches it to the appropriate handler.
+  bool MachMessageServerFunction(const mach_msg_header_t* in_header,
+                                 mach_msg_header_t* out_header,
+                                 bool* destroy_complex_request) override {
+    const PayloadMessage* message =
+        ReceivePayloadMessage(in_header, out_header);
+    if (message) {
+      if (handler_) {
+        handler_->HandlePayloadMessage(*message);
+      }
+      *destroy_complex_request = true;
+      return true;
+    }
+
+    return false;
+  }
+
+  std::set<mach_msg_id_t> MachMessageServerRequestIDs() override {
+    return {kPayloadMessageID};
+  }
+
+  mach_msg_size_t MachMessageServerRequestSize() override {
+    return sizeof(PayloadMessage);
+  }
+
+  mach_msg_size_t MachMessageServerReplySize() override {
+    return sizeof(mig_reply_error_t);  // Minimal size for a proper Mach reply message
+  }
+
+ private:
+  PayloadMessageHandler* handler_;  // weak
+};
+
 class ExceptionHandlerServerRun : public UniversalMachExcServer::Interface,
                                   public NotifyServer::DefaultInterface {
  public:
@@ -41,6 +85,7 @@ class ExceptionHandlerServerRun : public UniversalMachExcServer::Interface,
         NotifyServer::DefaultInterface(),
         mach_exc_server_(this),
         notify_server_(this),
+        payload_message_server_(dynamic_cast<CrashReportExceptionHandler*>(exception_interface)),
         composite_mach_message_server_(),
         exception_interface_(exception_interface),
         exception_port_(exception_port),
@@ -49,6 +94,7 @@ class ExceptionHandlerServerRun : public UniversalMachExcServer::Interface,
         launchd_(launchd) {
     composite_mach_message_server_.AddHandler(&mach_exc_server_);
     composite_mach_message_server_.AddHandler(&notify_server_);
+    composite_mach_message_server_.AddHandler(&payload_message_server_);
   }
 
   ExceptionHandlerServerRun(const ExceptionHandlerServerRun&) = delete;
@@ -182,6 +228,7 @@ class ExceptionHandlerServerRun : public UniversalMachExcServer::Interface,
  private:
   UniversalMachExcServer mach_exc_server_;
   NotifyServer notify_server_;
+  PayloadMessageServer payload_message_server_;
   CompositeMachMessageServer composite_mach_message_server_;
   UniversalMachExcServer::Interface* exception_interface_;  // weak
   mach_port_t exception_port_;  // weak

handler/mac/exception_handler_server.h

@@ -19,9 +19,21 @@
 
 #include "base/apple/scoped_mach_port.h"
 #include "util/mach/exc_server_variants.h"
+#include "util/mach/payload_message.h"
 
 namespace crashpad {
 
+//! \brief Interface for handling payload messages
+class PayloadMessageHandler {
+ public:
+  virtual ~PayloadMessageHandler() {}
+
+  //! \brief Processes a payload message
+  //!
+  //! \param[in] message The message to process
+  virtual void HandlePayloadMessage(const PayloadMessage& message) = 0;
+};
+
 //! \brief Runs the main exception-handling server in Crashpad’s handler
 //!     process.
 class ExceptionHandlerServer {

util/CMakeLists.txt

@@ -193,6 +193,8 @@ if(APPLE)
             mach/exception_types.h
             mach/notify_server.cc
             mach/notify_server.h
+            mach/payload_message.cc
+            mach/payload_message.h
             mach/scoped_task_suspend.cc
             mach/scoped_task_suspend.h
             mach/task_for_pid.cc

util/mach/payload_message.cc

@@ -0,0 +1,77 @@
+// Copyright 2014 The Crashpad Authors
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+#include "util/mach/payload_message.h"
+
+#include "base/apple/mach_logging.h"
+#include "base/logging.h"
+#include "util/mach/mach_message.h"
+
+namespace crashpad {
+
+bool SendPayloadMessage(mach_port_t port,
+                        PayloadMessageType type,
+                        const std::string& payload) {
+  if (port == MACH_PORT_NULL) {
+    DLOG(ERROR) << "Cannot send attachment message, no exception port";
+    return false;
+  }
+
+  PayloadMessage message = {};
+  message.header.msgh_bits =
+      MACH_MSGH_BITS(MACH_MSG_TYPE_COPY_SEND, 0) | MACH_MSGH_BITS_COMPLEX;
+  message.header.msgh_size = sizeof(message);
+  message.header.msgh_remote_port = port;
+  message.header.msgh_local_port = MACH_PORT_NULL;
+  message.header.msgh_id = kPayloadMessageID;
+
+  message.body.msgh_descriptor_count = 1;
+
+  message.payload.address = const_cast<char*>(payload.c_str());
+  message.payload.size = payload.size() + 1;
+  message.payload.deallocate = FALSE;
+  message.payload.copy = MACH_MSG_VIRTUAL_COPY;
+  message.payload.type = MACH_MSG_OOL_DESCRIPTOR;
+
+  message.ndr = NDR_record;
+  message.type = type;
+
+  kern_return_t kr = mach_msg(&message.header,
+                              MACH_SEND_MSG,
+                              sizeof(message),
+                              0,
+                              MACH_PORT_NULL,
+                              MACH_MSG_TIMEOUT_NONE,
+                              MACH_PORT_NULL);
+
+  if (kr != KERN_SUCCESS) {
+    MACH_LOG(ERROR, kr) << "mach_msg";
+    return false;
+  }
+
+  return true;
+}
+
+const PayloadMessage* ReceivePayloadMessage(const mach_msg_header_t* in_header,
+                                            mach_msg_header_t* out_header) {
+  if (in_header->msgh_id != kPayloadMessageID) {
+    return nullptr;
+  }
+
+  PrepareMIGReplyFromRequest(in_header, out_header);
+  SetMIGReplyError(out_header, MIG_NO_REPLY);
+  return reinterpret_cast<const PayloadMessage*>(in_header);
+}
+
+}  // namespace crashpad