[fuchsia] transition off deprecated exception APIs

Removes the remaining references to the old port-based exception APIs in
favor of the new channel-based APIs.

Bug: fuchsia:ZX-4031
Test: runtests on emulator and device
Change-Id: Ieac5b66c2f676966d1018d771cab6c8635f12a8f
Reviewed-on: https://chromium-review.googlesource.com/c/crashpad/crashpad/+/1700321
Reviewed-by: Francois Rousseau <frousseau@google.com>
Commit-Queue: Francois Rousseau <frousseau@google.com>

client/crashpad_client_fuchsia.cc

@@ -15,8 +15,8 @@
 #include "client/crashpad_client.h"
 
 #include <lib/fdio/spawn.h>
+#include <lib/zx/channel.h>
 #include <lib/zx/job.h>
-#include <lib/zx/port.h>
 #include <lib/zx/process.h>
 #include <zircon/processargs.h>
 
@@ -24,7 +24,6 @@
 #include "base/logging.h"
 #include "base/strings/stringprintf.h"
 #include "client/client_argv_handling.h"
-#include "util/fuchsia/system_exception_port_key.h"
 
 namespace crashpad {
 
@@ -44,48 +43,41 @@ bool CrashpadClient::StartHandler(
   DCHECK_EQ(restartable, false);  // Not used on Fuchsia.
   DCHECK_EQ(asynchronous_start, false);  // Not used on Fuchsia.
 
-  zx::port exception_port;
-  zx_status_t status = zx::port::create(0, &exception_port);
-  if (status != ZX_OK) {
-    ZX_LOG(ERROR, status) << "zx_port_create";
-    return false;
-  }
-
-  status = zx::job::default_job()->bind_exception_port(
-      exception_port, kSystemExceptionPortKey, 0);
-  if (status != ZX_OK) {
-    ZX_LOG(ERROR, status) << "zx_task_bind_exception_port";
-    return false;
-  }
-
   std::vector<std::string> argv_strings = BuildHandlerArgvStrings(
       handler, database, metrics_dir, url, annotations, arguments);
 
   std::vector<const char*> argv;
   StringVectorToCStringVector(argv_strings, &argv);
 
-  // Follow the same protocol as devmgr and crashlogger in Zircon (that is,
+  // Set up handles to send to the spawned process:
-  // process handle as handle 0, with type USER0, exception port handle as
+  //   0. PA_USER0 job
-  // handle 1, also with type PA_USER0) so that it's trivial to replace
+  //   1. PA_USER0 exception channel
-  // crashlogger with crashpad_handler. The exception port is passed on, so
+  //
-  // released here. Currently it is assumed that this process's default job
+  // Currently it is assumed that this process's default job handle is the
-  // handle is the exception port that should be monitored. In the future, it
+  // exception channel that should be monitored. In the future, it might be
-  // might be useful for this to be configurable by the client.
+  // useful for this to be configurable by the client.
-  constexpr size_t kActionCount = 2;
+  zx::job job;
-  fdio_spawn_action_t actions[] = {
+  zx_status_t status =
-      {.action = FDIO_SPAWN_ACTION_ADD_HANDLE,
+      zx::job::default_job()->duplicate(ZX_RIGHT_SAME_RIGHTS, &job);
-       .h = {.id = PA_HND(PA_USER0, 0), .handle = ZX_HANDLE_INVALID}},
-      {.action = FDIO_SPAWN_ACTION_ADD_HANDLE,
-       .h = {.id = PA_HND(PA_USER0, 1), .handle = ZX_HANDLE_INVALID}},
-  };
-
-  status = zx_handle_duplicate(
-      zx_job_default(), ZX_RIGHT_SAME_RIGHTS, &actions[0].h.handle);
   if (status != ZX_OK) {
     ZX_LOG(ERROR, status) << "zx_handle_duplicate";
     return false;
   }
-  actions[1].h.handle = exception_port.release();
+
+  zx::channel exception_channel;
+  status = job.create_exception_channel(0, &exception_channel);
+  if (status != ZX_OK) {
+    ZX_LOG(ERROR, status) << "zx_task_create_exception_channel";
+    return false;
+  }
+
+  constexpr size_t kActionCount = 2;
+  fdio_spawn_action_t actions[] = {
+      {.action = FDIO_SPAWN_ACTION_ADD_HANDLE,
+       .h = {.id = PA_HND(PA_USER0, 0), .handle = job.release()}},
+      {.action = FDIO_SPAWN_ACTION_ADD_HANDLE,
+       .h = {.id = PA_HND(PA_USER0, 1), .handle = exception_channel.release()}},
+  };
 
   char error_message[FDIO_SPAWN_ERR_MSG_MAX_LENGTH];
   zx::process child;

handler/fuchsia/crash_report_exception_handler.cc

@@ -29,33 +29,6 @@
 
 namespace crashpad {
 
-namespace {
-
-class ScopedThreadResumeAfterException {
- public:
-  ScopedThreadResumeAfterException(const zx::thread& thread,
-                                   const zx::unowned_port& exception_port)
-      : thread_(thread), exception_port_(exception_port) {}
-  ~ScopedThreadResumeAfterException() {
-    DCHECK(thread_->is_valid());
-    // Resuming with ZX_RESUME_TRY_NEXT chains to the next handler. In normal
-    // operation, there won't be another beyond this one, which will result in
-    // the kernel terminating the process.
-    zx_status_t status =
-        thread_->resume_from_exception(*exception_port_, ZX_RESUME_TRY_NEXT);
-    ZX_LOG_IF(ERROR, status != ZX_OK, status)
-        << "zx_task_resume_from_exception";
-  }
-
- private:
-  zx::unowned_thread thread_;
-  const zx::unowned_port& exception_port_;
-
-  DISALLOW_COPY_AND_ASSIGN(ScopedThreadResumeAfterException);
-};
-
-}  // namespace
-
 CrashReportExceptionHandler::CrashReportExceptionHandler(
     CrashReportDatabase* database,
     CrashReportUploadThread* upload_thread,
@@ -70,30 +43,6 @@ CrashReportExceptionHandler::CrashReportExceptionHandler(
 
 CrashReportExceptionHandler::~CrashReportExceptionHandler() {}
 
-bool CrashReportExceptionHandler::HandleException(
-    uint64_t process_id,
-    uint64_t thread_id,
-    const zx::unowned_port& exception_port,
-    UUID* local_report_id) {
-  // TODO(scottmg): This function needs to be instrumented with metrics calls,
-  // https://crashpad.chromium.org/bug/230.
-
-  zx::process process(GetProcessFromKoid(process_id));
-  if (!process.is_valid()) {
-    // There's no way to resume the thread if the process retrieval fails.
-    // Assume that the process has been already killed, and bail.
-    return false;
-  }
-
-  zx::thread thread(GetThreadHandleByKoid(process, thread_id));
-  if (!thread.is_valid()) {
-    return false;
-  }
-
-  ScopedThreadResumeAfterException resume(thread, exception_port);
-  return HandleException(process, thread, local_report_id);
-}
-
 bool CrashReportExceptionHandler::HandleException(const zx::process& process,
                                                   const zx::thread& thread,
                                                   UUID* local_report_id) {

handler/fuchsia/crash_report_exception_handler.h

@@ -70,25 +70,6 @@ class CrashReportExceptionHandler {
 
   ~CrashReportExceptionHandler();
 
-  //! \brief Called when the exception handler server has caught an exception
-  //!     and wants a crash dump to be taken.
-  //!
-  //! This function is expected to call `zx_task_resume_from_exception()` in
-  //! order to complete handling of the exception.
-  //!
-  //! \param[in] process_id The koid of the process which sustained the
-  //!     exception.
-  //! \param[in] thread_id The koid of the thread which sustained the exception.
-  //! \param[in] exception_port The exception port on which the exception was
-  //!     serviced. This can be used to resume the excepting thread.
-  //! \param[out] local_report_id The unique identifier for the report created
-  //!     in the local report database. Optional.
-  //! \return `true` on success, or `false` with an error logged.
-  bool HandleException(uint64_t process_id,
-                       uint64_t thread_id,
-                       const zx::unowned_port& exception_port,
-                       UUID* local_report_id = nullptr);
-
   //! \brief Called when the exception handler server has caught an exception
   //!     and wants a crash dump to be taken.
   //!

handler/fuchsia/exception_handler_server.cc

@@ -14,45 +14,75 @@
 
 #include "handler/fuchsia/exception_handler_server.h"
 
+#include <lib/zx/exception.h>
 #include <lib/zx/time.h>
-#include <zircon/syscalls/port.h>
+#include <zircon/syscalls/exception.h>
 
 #include <utility>
 
 #include "base/fuchsia/fuchsia_logging.h"
 #include "base/logging.h"
 #include "handler/fuchsia/crash_report_exception_handler.h"
-#include "util/fuchsia/system_exception_port_key.h"
 
 namespace crashpad {
 
 ExceptionHandlerServer::ExceptionHandlerServer(zx::job root_job,
-                                               zx::port exception_port)
+                                               zx::channel exception_channel)
     : root_job_(std::move(root_job)),
-      exception_port_(std::move(exception_port)) {}
+      exception_channel_(std::move(exception_channel)) {}
 
 ExceptionHandlerServer::~ExceptionHandlerServer() = default;
 
 void ExceptionHandlerServer::Run(CrashReportExceptionHandler* handler) {
   while (true) {
-    zx_port_packet_t packet;
+    zx_signals_t signals;
-    zx_status_t status = exception_port_.wait(zx::time::infinite(), &packet);
+    zx_status_t status = exception_channel_.wait_one(
+        ZX_CHANNEL_READABLE | ZX_CHANNEL_PEER_CLOSED,
+        zx::time::infinite(),
+        &signals);
     if (status != ZX_OK) {
       ZX_LOG(ERROR, status) << "zx_port_wait, aborting";
       return;
     }
 
-    if (packet.key != kSystemExceptionPortKey) {
+    if (signals & ZX_CHANNEL_READABLE) {
-      LOG(ERROR) << "unexpected packet key, ignoring";
+      zx_exception_info_t info;
-      continue;
+      zx::exception exception;
+      status = exception_channel_.read(0,
+                                       &info,
+                                       exception.reset_and_get_address(),
+                                       sizeof(info),
+                                       1,
+                                       nullptr,
+                                       nullptr);
+      if (status != ZX_OK) {
+        ZX_LOG(ERROR, status) << "zx_channel_read, aborting";
+        return;
       }
 
-    bool result = handler->HandleException(packet.exception.pid,
+      zx::process process;
-                                           packet.exception.tid,
+      status = exception.get_process(&process);
-                                           zx::unowned_port(exception_port_));
+      if (status != ZX_OK) {
+        ZX_LOG(ERROR, status) << "zx_exception_get_process, aborting";
+        return;
+      }
+
+      zx::thread thread;
+      status = exception.get_thread(&thread);
+      if (status != ZX_OK) {
+        ZX_LOG(ERROR, status) << "zx_exception_get_thread, aborting";
+        return;
+      }
+
+      bool result =
+          handler->HandleException(std::move(process), std::move(thread));
       if (!result) {
         LOG(ERROR) << "HandleException failed";
       }
+    } else {
+      // Job terminated, exit the loop.
+      return;
+    }
   }
 }
 

handler/fuchsia/exception_handler_server.h

@@ -15,8 +15,8 @@
 #ifndef CRASHPAD_HANDLER_FUCHSIA_EXCEPTION_HANDLER_SERVER_H_
 #define CRASHPAD_HANDLER_FUCHSIA_EXCEPTION_HANDLER_SERVER_H_
 
+#include <lib/zx/channel.h>
 #include <lib/zx/job.h>
-#include <lib/zx/port.h>
 
 #include "base/macros.h"
 
@@ -31,11 +31,11 @@ class ExceptionHandlerServer {
   //! \brief Constructs an ExceptionHandlerServer object.
   //!
   //! \param[in] root_job The root of the tree of processes that will be handled
-  //!     by this server. It is assumed that \a exception_port is the exception
+  //!     by this server. It is assumed that \a exception_channel is the
-  //!     port of this job.
+  //      exception channel of this job.
-  //! \param[in] exception_port The exception port that this server will
+  //! \param[in] exception_channel The exception channel that this server will
   //!     monitor.
-  ExceptionHandlerServer(zx::job root_job, zx::port exception_port);
+  ExceptionHandlerServer(zx::job root_job, zx::channel exception_channel);
   ~ExceptionHandlerServer();
 
   //! \brief Runs the exception-handling server.
@@ -46,7 +46,7 @@ class ExceptionHandlerServer {
 
  private:
   zx::job root_job_;
-  zx::port exception_port_;
+  zx::channel exception_channel_;
 
   DISALLOW_COPY_AND_ASSIGN(ExceptionHandlerServer);
 };

handler/handler_main.cc

@@ -86,6 +86,9 @@
 #include <zircon/process.h>
 #include <zircon/processargs.h>
 
+#include <lib/zx/channel.h>
+#include <lib/zx/job.h>
+
 #include "handler/fuchsia/crash_report_exception_handler.h"
 #include "handler/fuchsia/exception_handler_server.h"
 #elif defined(OS_LINUX)
@@ -982,18 +985,18 @@ int HandlerMain(int argc,
   // crashpad_handler.
   zx::job root_job(zx_take_startup_handle(PA_HND(PA_USER0, 0)));
   if (!root_job.is_valid()) {
-    LOG(ERROR) << "no process handle passed in startup handle 0";
+    LOG(ERROR) << "no job handle passed in startup handle 0";
     return EXIT_FAILURE;
   }
 
-  zx::port exception_port(zx_take_startup_handle(PA_HND(PA_USER0, 1)));
+  zx::channel exception_channel(zx_take_startup_handle(PA_HND(PA_USER0, 1)));
-  if (!exception_port.is_valid()) {
+  if (!exception_channel.is_valid()) {
-    LOG(ERROR) << "no exception port handle passed in startup handle 1";
+    LOG(ERROR) << "no exception channel handle passed in startup handle 1";
     return EXIT_FAILURE;
   }
 
   ExceptionHandlerServer exception_handler_server(std::move(root_job),
-                                                  std::move(exception_port));
+                                                  std::move(exception_channel));
 #elif defined(OS_LINUX) || defined(OS_ANDROID)
   ExceptionHandlerServer exception_handler_server;
 #endif  // OS_MACOSX

util/BUILD.gn

@@ -422,7 +422,6 @@ static_library("util") {
       "fuchsia/koid_utilities.h",
       "fuchsia/scoped_task_suspend.cc",
       "fuchsia/scoped_task_suspend.h",
-      "fuchsia/system_exception_port_key.h",
       "misc/capture_context_fuchsia.S",
       "misc/paths_fuchsia.cc",
       "process/process_memory_fuchsia.cc",

util/fuchsia/system_exception_port_key.h

@@ -1,27 +0,0 @@
-// Copyright 2018 The Crashpad Authors. All rights reserved.
-//
-// 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.
-
-#ifndef CRASHPAD_UTIL_FUCHSIA_EXCEPTION_PORT_KEY_H_
-#define CRASHPAD_UTIL_FUCHSIA_EXCEPTION_PORT_KEY_H_
-
-namespace crashpad {
-
-//! \brief The key used in `zx_task_bind_exception_port()` and packet
-//!     processing. This matches the value that Zircon's `devmgr` and
-//!     `crashlogger` use for interoperability, for now.
-constexpr uint64_t kSystemExceptionPortKey = 1166444u;
-
-}  // namespace crashpad
-
-#endif  // CRASHPAD_UTIL_FUCHSIA_EXCEPTION_PORT_KEY_H_