From 6bb6c9b26afd9844b9e35fcaa39fb24893e7abb5 Mon Sep 17 00:00:00 2001
From: Adam Harrison <adamharrison@google.com>
Date: Fri, 2 Feb 2018 11:14:00 -0800
Subject: [PATCH] Fix crash when an NSException is thrown.

old_handlers is zeroish whenever an NSException is thrown. This caused PROT_WRITE to never be set and resulted in an EXC_BAD_ACCESS when trying to set the handler to NULL.

Change-Id: Ibb7da448204431c7602b1001f3a5216303c4c9d1
Reviewed-on: https://chromium-review.googlesource.com/899907
Reviewed-by: Mark Mentovai <mark@chromium.org>
---
 src/client/ios/Breakpad.mm                  |  2 +-
 src/client/ios/exception_handler_no_mach.cc | 21 ++++++++++-----------
 2 files changed, 11 insertions(+), 12 deletions(-)

diff --git a/src/client/ios/Breakpad.mm b/src/client/ios/Breakpad.mm
index 3dd44a24..c2a4202a 100644
--- a/src/client/ios/Breakpad.mm
+++ b/src/client/ios/Breakpad.mm
@@ -50,7 +50,7 @@
 #if !TARGET_OS_TV && !TARGET_OS_WATCH
 #import "client/mac/handler/exception_handler.h"
 #else
-#import "client/ios/handler/exception_handler_no_mach.h"
+#import "client/ios/exception_handler_no_mach.h"
 #endif  // !TARGET_OS_TV && !TARGET_OS_WATCH
 
 #if !defined(__EXCEPTIONS) || (__clang__ && !__has_feature(cxx_exceptions))
diff --git a/src/client/ios/exception_handler_no_mach.cc b/src/client/ios/exception_handler_no_mach.cc
index 23f246e9..aa8489d7 100644
--- a/src/client/ios/exception_handler_no_mach.cc
+++ b/src/client/ios/exception_handler_no_mach.cc
@@ -31,7 +31,7 @@
 #include <TargetConditionals.h>
 
 #include "client/mac/handler/minidump_generator.h"
-#include "client/ios/handler/exception_handler_no_mach.h"
+#include "client/ios/exception_handler_no_mach.h"
 
 #ifndef USE_PROTECTED_ALLOCATIONS
 #if TARGET_OS_TV
@@ -200,8 +200,6 @@ bool ExceptionHandler::InstallHandlers() {
   // If a handler is already installed, something is really wrong.
   if (gProtectedData.handler != NULL) {
     return false;
-  }
-  gProtectedData.handler = this;
   for (int i = 0; i < kNumHandledSignals; ++i) {
     struct sigaction sa;
     memset(&sa, 0, sizeof(sa));
@@ -213,11 +211,12 @@ bool ExceptionHandler::InstallHandlers() {
     if (sigaction(kExceptionSignals[i], &sa, old_handlers[i].get()) == -1) {
       return false;
     }
-#if USE_PROTECTED_ALLOCATIONS
-    assert(((size_t)(gProtectedData.protected_buffer) & PAGE_MASK) == 0);
-    mprotect(gProtectedData.protected_buffer, PAGE_SIZE, PROT_READ);
-#endif  // USE_PROTECTED_ALLOCATIONS
   }
+  gProtectedData.handler = this;
+#if USE_PROTECTED_ALLOCATIONS
+  assert(((size_t)(gProtectedData.protected_buffer) & PAGE_MASK) == 0);
+  mprotect(gProtectedData.protected_buffer, PAGE_SIZE, PROT_READ);
+#endif  // USE_PROTECTED_ALLOCATIONS
   installed_exception_handler_ = true;
   return true;
 }
@@ -226,13 +225,13 @@ bool ExceptionHandler::UninstallHandlers() {
   for (int i = 0; i < kNumHandledSignals; ++i) {
     if (old_handlers[i].get()) {
       sigaction(kExceptionSignals[i], old_handlers[i].get(), NULL);
-#if USE_PROTECTED_ALLOCATIONS
-      mprotect(gProtectedData.protected_buffer, PAGE_SIZE, PROT_READ | PROT_WRITE);
-#endif  // USE_PROTECTED_ALLOCATIONS
       old_handlers[i].reset();
     }
-    gProtectedData.handler = NULL;
   }
+#if USE_PROTECTED_ALLOCATIONS
+  mprotect(gProtectedData.protected_buffer, PAGE_SIZE, PROT_READ | PROT_WRITE);
+#endif  // USE_PROTECTED_ALLOCATIONS
+  gProtectedData.handler = NULL;
   installed_exception_handler_ = false;
   return true;
 }