ios: Move and update ObjcExceptionPreprocessor.

More cleanly integration the ObjExceptionPreprocessor with the
Crashpad client and in process handler, to record bought
'caught' and 'uncaught' NSExceptions.

Bug: crashpad: 31
Change-Id: I77a77ca6d893cdc74da476c1888d9bcb338339d8
Reviewed-on: https://chromium-review.googlesource.com/c/crashpad/crashpad/+/2920851
Commit-Queue: Justin Cohen <justincohen@chromium.org>
Reviewed-by: Mark Mentovai <mark@chromium.org>

client/BUILD.gn

@@ -33,6 +33,8 @@ crashpad_static_library("client") {
   if (crashpad_is_ios) {
     sources += [
       "crashpad_client_ios.cc",
+      "ios_handler/exception_processor.h",
+      "ios_handler/exception_processor.mm",
       "simulate_crash_ios.h",
     ]
   }
@@ -160,7 +162,10 @@ source_set("client_test") {
   }
 
   if (crashpad_is_ios) {
-    sources += [ "crashpad_client_ios_test.mm" ]
+    sources += [
+      "crashpad_client_ios_test.mm",
+      "ios_handler/exception_processor_test.mm",
+    ]
     sources -= [
       "annotation_list_test.cc",
       "annotation_test.cc",

client/crashpad_client_ios.cc

@@ -22,7 +22,7 @@
 #include "base/logging.h"
 #include "base/mac/mach_logging.h"
 #include "base/mac/scoped_mach_port.h"
-#include "util/ios/exception_processor.h"
+#include "client/ios_handler/exception_processor.h"
 #include "util/ios/ios_system_data_collector.h"
 #include "util/mach/exc_server_variants.h"
 #include "util/mach/exception_ports.h"
@@ -38,7 +38,9 @@ namespace crashpad {
 namespace {
 
 // A base class for signal handler and Mach exception server.
-class CrashHandler : public Thread, public UniversalMachExcServer::Interface {
+class CrashHandler : public Thread,
+                     public UniversalMachExcServer::Interface,
+                     public ObjcExceptionDelegate {
  public:
   static CrashHandler* Get() {
     static CrashHandler* instance = new CrashHandler();
@@ -188,6 +190,26 @@ class CrashHandler : public Thread, public UniversalMachExcServer::Interface {
     Signals::RestoreHandlerAndReraiseSignalOnReturn(siginfo, &old_action_);
   }
 
+  void HandleUncaughtNSException(const uint64_t* frames,
+                                 const size_t num_frames) override {
+    // TODO(justincohen): Call into in_process_handler.
+
+    // After uncaught exceptions are reported, the system immediately triggers a
+    // call to std::terminate()/abort(). Remove the abort handler so a second
+    // dump isn't generated.
+    CHECK(Signals::InstallDefaultHandler(SIGABRT));
+  }
+
+  void HandleUncaughtNSExceptionWithContext(
+      NativeCPUContext* context) override {
+    // TODO(justincohen): Call into in_process_handler.
+
+    // After uncaught exceptions are reported, the system immediately triggers a
+    // call to std::terminate()/abort(). Remove the abort handler so a second
+    // dump isn't generated.
+    CHECK(Signals::InstallDefaultHandler(SIGABRT));
+  }
+
   base::mac::ScopedMachReceiveRight exception_port_;
   ExceptionPorts::ExceptionHandlerVector original_handlers_;
   struct sigaction old_action_ = {};
@@ -208,10 +230,10 @@ void CrashpadClient::StartCrashpadInProcessHandler(
     const base::FilePath& database,
     const std::string& url,
     const std::map<std::string, std::string>& annotations) {
-  InstallObjcExceptionPreprocessor();
 
   CrashHandler* crash_handler = CrashHandler::Get();
   DCHECK(crash_handler);
+  InstallObjcExceptionPreprocessor(crash_handler);
   crash_handler->Initialize();
 }
 

client/ios_handler/exception_processor.h

@@ -15,8 +15,35 @@
 #ifndef CRASHPAD_UTIL_IOS_EXCEPTION_PROCESSOR_H_
 #define CRASHPAD_UTIL_IOS_EXCEPTION_PROCESSOR_H_
 
+#include <vector>
+
+#include "util/misc/capture_context.h"
+
 namespace crashpad {
 
+//! \brief An interface for notifying the CrashpadClient of NSExceptions.
+class ObjcExceptionDelegate {
+ public:
+  //! \brief The exception processor detected an exception as it was thrown and
+  //!     captured the cpu context.
+  //!
+  //! \param context The cpu context of the thread throwing an exception.
+  virtual void HandleUncaughtNSExceptionWithContext(
+      NativeCPUContext* context) = 0;
+
+  //! \brief The exception processor did not detect the exception as it was
+  //!     thrown, and instead caught the exception via the
+  //!     NSUncaughtExceptionHandler.
+  //!
+  //! \param frames An array of call stack frame addresses.
+  //! \param num_frames The number of frames in |frames|.
+  virtual void HandleUncaughtNSException(const uint64_t* frames,
+                                         const size_t num_frames) = 0;
+
+ protected:
+  ~ObjcExceptionDelegate() {}
+};
+
 //! \brief Installs the Objective-C exception preprocessor.
 //!
 //! When code raises an Objective-C exception, unwind the stack looking for
@@ -30,7 +57,7 @@ namespace crashpad {
 //!
 //! This should be installed at the same time the CrashpadClient installs the
 //! signal handler. It should only be installed once.
-void InstallObjcExceptionPreprocessor();
+void InstallObjcExceptionPreprocessor(ObjcExceptionDelegate* delegate);
 
 }  // namespace crashpad
 

client/ios_handler/exception_processor.mm

@@ -12,7 +12,7 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
-#include "util/ios/exception_processor.h"
+#include "client/ios_handler/exception_processor.h"
 
 #include <Availability.h>
 #import <Foundation/Foundation.h>
@@ -35,10 +35,15 @@
 #include <typeinfo>
 
 #include "base/bit_cast.h"
+#include "base/format_macros.h"
 #include "base/logging.h"
 #include "base/memory/free_deleter.h"
+#include "base/numerics/safe_conversions.h"
+#include "base/strings/stringprintf.h"
 #include "base/strings/sys_string_conversions.h"
 #include "build/build_config.h"
+#include "client/annotation.h"
+#include "client/simulate_crash_ios.h"
 
 namespace {
 
@@ -80,20 +85,101 @@ struct __cxa_exception {
   _Unwind_Exception unwindHeader;
 };
 
-objc_exception_preprocessor g_next_preprocessor;
-bool g_exception_preprocessor_installed;
+int LoggingUnwStep(unw_cursor_t* cursor) {
+  int rv = unw_step(cursor);
+  if (rv < 0) {
+    LOG(ERROR) << "unw_step: " << rv;
+  }
+  return rv;
+}
 
-void TerminatingFromUncaughtNSException(id exception, const char* sinkhole) {
-  // TODO(justincohen): This is incomplete, as the signal handler will not have
-  // access to the exception name and reason.  Pass that along somehow here.
-  NSString* exception_message_ns = [NSString
-      stringWithFormat:@"%@: %@", [exception name], [exception reason]];
-  std::string exception_message = base::SysNSStringToUTF8(exception_message_ns);
-  LOG(INFO) << "Terminating from Objective-C exception: " << exception_message
-            << " with sinkhole: " << sinkhole;
-  // TODO(justincohen): This is temporary, as crashpad can capture this
-  // exception directly instead.
-  std::terminate();
+std::string FormatStackTrace(const std::vector<uint64_t>& addresses,
+                             size_t max_length) {
+  std::string stack_string;
+  for (uint64_t address : addresses) {
+    std::string address_string = base::StringPrintf("0x%" PRIx64, address);
+    if (stack_string.size() + address_string.size() > max_length)
+      break;
+    stack_string += address_string + " ";
+  }
+
+  if (!stack_string.empty() && stack_string.back() == ' ') {
+    stack_string.resize(stack_string.size() - 1);
+  }
+
+  return stack_string;
+}
+
+std::string GetTraceString() {
+  std::vector<uint64_t> addresses;
+  unw_context_t context;
+  unw_getcontext(&context);
+  unw_cursor_t cursor;
+  unw_init_local(&cursor, &context);
+  while (LoggingUnwStep(&cursor) > 0) {
+    unw_word_t ip = 0;
+    unw_get_reg(&cursor, UNW_REG_IP, &ip);
+    addresses.push_back(ip);
+  }
+  return FormatStackTrace(addresses, 1024);
+}
+
+crashpad::ObjcExceptionDelegate* g_exception_delegate;
+objc_exception_preprocessor g_next_preprocessor;
+NSUncaughtExceptionHandler* g_next_uncaught_exception_handler;
+
+static void SetNSExceptionAnnotations(id exception,
+                                      std::string& name,
+                                      std::string& reason) {
+  name = base::SysNSStringToUTF8([exception name]);
+  reason = base::SysNSStringToUTF8([exception reason]);
+  static crashpad::StringAnnotation<256> nameKey("exceptionName");
+  nameKey.Set(name);
+  static crashpad::StringAnnotation<512> reasonKey("exceptionReason");
+  reasonKey.Set(reason);
+}
+
+static void ObjcUncaughtExceptionHandler(NSException* exception) {
+  std::string name, reason;
+  SetNSExceptionAnnotations(exception, name, reason);
+  NSArray<NSNumber*>* addressArray = [exception callStackReturnAddresses];
+  if ([addressArray count] > 0) {
+    static crashpad::StringAnnotation<256> nameKey("UncaughtNSException");
+    nameKey.Set("true");
+    std::vector<uint64_t> addresses;
+    NSArray<NSNumber*>* addressArray = [exception callStackReturnAddresses];
+    for (NSNumber* address in addressArray)
+      addresses.push_back([address unsignedLongLongValue]);
+    g_exception_delegate->HandleUncaughtNSException(&addresses[0],
+                                                    addresses.size());
+  } else {
+    LOG(WARNING) << "Uncaught Objective-C exception name: " << name
+                 << " reason: " << reason << " with no "
+                 << " -callStackReturnAddresses.";
+    crashpad::NativeCPUContext cpu_context;
+    crashpad::CaptureContext(&cpu_context);
+    g_exception_delegate->HandleUncaughtNSExceptionWithContext(&cpu_context);
+  }
+}
+
+// This function is used to make it clear to the crash processor that an
+// uncaught NSException was recorded here.
+static __attribute__((noinline)) id HANDLE_UNCAUGHT_NSEXCEPTION(
+    id exception,
+    const char* sinkhole) {
+  std::string name, reason;
+  SetNSExceptionAnnotations(exception, name, reason);
+  LOG(WARNING) << "Handling Objective-C exception name: " << name
+               << " reason: " << reason << " with sinkhole: " << sinkhole;
+  crashpad::NativeCPUContext cpu_context;
+  crashpad::CaptureContext(&cpu_context);
+  g_exception_delegate->HandleUncaughtNSExceptionWithContext(&cpu_context);
+
+  // Remove the uncaught exception handler so we don't record this twice.
+  NSSetUncaughtExceptionHandler(g_next_uncaught_exception_handler);
+  g_next_uncaught_exception_handler = nullptr;
+
+  return g_next_preprocessor ? g_next_preprocessor(exception) : exception;
 }
 
 // Returns true if |path| equals |sinkhole| on device. Simulator paths prepend
@@ -113,15 +199,25 @@ bool ModulePathMatchesSinkhole(const char* path, const char* sinkhole) {
 #endif
 }
 
-int LoggingUnwStep(unw_cursor_t* cursor) {
-  int rv = unw_step(cursor);
-  if (rv < 0) {
-    LOG(ERROR) << "unw_step: " << rv;
-  }
-  return rv;
-}
-
 id ObjcExceptionPreprocessor(id exception) {
+  static bool seen_first_exception;
+
+  static crashpad::StringAnnotation<256> firstexception("firstexception");
+  static crashpad::StringAnnotation<256> lastexception("lastexception");
+  static crashpad::StringAnnotation<1024> firstexception_bt(
+      "firstexception_bt");
+  static crashpad::StringAnnotation<1024> lastexception_bt("lastexception_bt");
+  auto* key = seen_first_exception ? &lastexception : &firstexception;
+  auto* bt_key = seen_first_exception ? &lastexception_bt : &firstexception_bt;
+  NSString* value = [NSString
+      stringWithFormat:@"%@ reason %@", [exception name], [exception reason]];
+  key->Set(base::SysNSStringToUTF8(value));
+
+  // This exception preprocessor runs prior to the one in libobjc, which sets
+  // the -[NSException callStackReturnAddresses].
+  bt_key->Set(GetTraceString());
+  seen_first_exception = true;
+
   // Unwind the stack looking for any exception handlers. If an exception
   // handler is encountered, test to see if it is a function known to catch-
   // and-rethrow as a "top-level" exception handler. Various routines in
@@ -229,10 +325,13 @@ id ObjcExceptionPreprocessor(id exception) {
         "CFRunLoopRunSpecific",
         "_CFXNotificationPost",
         "__NSFireDelayedPerform",
+        // If this exception is going to end up at EHFrame, record the uncaught
+        // exception instead.
+        "_ZN4base3mac15CallWithEHFrameEU13block_pointerFvvE",
     };
     for (const char* sinkhole : kExceptionSymbolNameSinkholes) {
       if (strcmp(sinkhole, proc_name) == 0) {
-        TerminatingFromUncaughtNSException(exception, sinkhole);
+        return HANDLE_UNCAUGHT_NSEXCEPTION(exception, sinkhole);
       }
     }
 
@@ -257,7 +356,7 @@ id ObjcExceptionPreprocessor(id exception) {
         0) {
       for (const char* sinkhole : kExceptionLibraryPathSinkholes) {
         if (ModulePathMatchesSinkhole(dl_info.dli_fname, sinkhole)) {
-          TerminatingFromUncaughtNSException(exception, sinkhole);
+          return HANDLE_UNCAUGHT_NSEXCEPTION(exception, sinkhole);
         }
       }
     }
@@ -305,7 +404,7 @@ id ObjcExceptionPreprocessor(id exception) {
                 reinterpret_cast<unw_word_t>(gesture_environment_min_imp) &&
             caller_frame_info.start_ip <=
                 reinterpret_cast<unw_word_t>(gesture_environment_max_imp)) {
-          TerminatingFromUncaughtNSException(exception,
+          return HANDLE_UNCAUGHT_NSEXCEPTION(exception,
                                              "_UIGestureEnvironmentUpdate");
         }
       }
@@ -319,37 +418,43 @@ id ObjcExceptionPreprocessor(id exception) {
   // If no handler is found, __cxa_throw would call failed_throw and terminate.
   // See:
   // https://github.com/llvm/llvm-project/blob/c5d2746fbea7/libcxxabi/src/cxa_exception.cpp
-  // __cxa_throw. Instead, terminate via TerminatingFromUncaughtNSException so
-  // the exception name and reason are properly recorded.
+  // __cxa_throw. Instead, call HANDLE_UNCAUGHT_NSEXCEPTION so the exception
+  // name and reason are properly recorded.
   if (!handler_found) {
-    TerminatingFromUncaughtNSException(exception, "__cxa_throw");
+    return HANDLE_UNCAUGHT_NSEXCEPTION(exception, "__cxa_throw");
   }
 
   // Forward to the next preprocessor.
-  if (g_next_preprocessor)
-    return g_next_preprocessor(exception);
-
-  return exception;
+  return g_next_preprocessor ? g_next_preprocessor(exception) : exception;
 }
 
 }  // namespace
 
 namespace crashpad {
 
-void InstallObjcExceptionPreprocessor() {
-  DCHECK(!g_exception_preprocessor_installed);
+void InstallObjcExceptionPreprocessor(ObjcExceptionDelegate* delegate) {
+  DCHECK(!g_next_preprocessor);
 
+  // Preprocessor.
   g_next_preprocessor =
       objc_setExceptionPreprocessor(&ObjcExceptionPreprocessor);
-  g_exception_preprocessor_installed = true;
+
+  // Uncaught processor.
+  g_exception_delegate = delegate;
+  g_next_uncaught_exception_handler = NSGetUncaughtExceptionHandler();
+  NSSetUncaughtExceptionHandler(&ObjcUncaughtExceptionHandler);
 }
 
 void UninstallObjcExceptionPreprocessor() {
-  DCHECK(g_exception_preprocessor_installed);
+  DCHECK(g_next_preprocessor);
 
   objc_setExceptionPreprocessor(g_next_preprocessor);
+  g_exception_delegate = nullptr;
+
+  NSSetUncaughtExceptionHandler(g_next_uncaught_exception_handler);
+  g_next_uncaught_exception_handler = nullptr;
+
   g_next_preprocessor = nullptr;
-  g_exception_preprocessor_installed = false;
 }
 
 }  // namespace crashpad

util/BUILD.gn

@@ -378,8 +378,6 @@ crashpad_static_library("util") {
 
   if (crashpad_is_ios) {
     sources += [
-      "ios/exception_processor.h",
-      "ios/exception_processor.mm",
       "ios/ios_intermediate_dump_data.cc",
       "ios/ios_intermediate_dump_data.h",
       "ios/ios_intermediate_dump_format.h",
@@ -807,7 +805,6 @@ source_set("util_test") {
 
   if (crashpad_is_ios) {
     sources += [
-      "ios/exception_processor_test.mm",
       "ios/ios_intermediate_dump_reader_test.cc",
       "ios/ios_intermediate_dump_writer_test.cc",
       "ios/scoped_vm_read_test.cc",