Pass NSException to the NSException handler with no extra processing

[kstenerud]

Goal

NSException is actually implemented using the C++ exception ABI. The Bugsnag C++ handler was doing some preprocessing of the NSException before passing it to the default C++ handler (which then calls the NSException handler). After this, the NSException handler would do the same processing over again.

Since both the C++ and NSException handlers call bsg_kscrashsentry_beginHandlingCrash, and this function checks for re-entry to detect a crash in the crash handler, it gets confused into thinking that a crash has happened in the crash handler when there's actually nothing amiss. The resulting report on the dashboard is still correct, but the messaging on the device is misleading, and the crash handler is doing unnecessary extra work.

Design

Don't do any processing in the C++ handler if it has caught an NSException. Simply pass it along and let the NSException handler do everything (basically behaving the same way it would if C++ exception capture were disabled).

Testing

Tested manually on real iphone, watch, macos, and simulator tvos.

[github-actions]

Bugsnag.framework binary size did not change - 820,608 bytes

    FILE SIZE        VM SIZE    
 --------------  -------------- 
  +0.5%     +60  +0.5%     +60    [__TEXT]
  -0.0%     -60  -0.0%     -60    __TEXT,__text
  [ = ]       0  [ = ]       0    TOTAL

Generated by 🚫 Danger

[nickdowell]

How do NSExceptions now get captured?

Does bsg_g_originalTerminateHandler end up calling bsg_ksnsexc_i_handleException via NSUncaughtExceptionHandler?

[kstenerud]

The ObjC runtime sets its own custom C++ exception handler that checks for NSException and does processing if it is (calling whatever NSUncaughtExceptionHandler is registered, or calling abort() if none is).

We tap into both the C++ exception handler and the NSException handler, but since we don't want to process twice, we check in C++ land for NSException and just pass the exception along for the ObjC runtime to find. The runtime then completes the loop back to our NSException handler.

[nickdowell]

What would happen if bsg_g_originalTerminateHandler is (for example) Crashlytics's NSUncaughtExceptionHandler?

Will this PR mean that crashes we previously reported in apps that use both would no longer be detected?

[kstenerud]

Hmm actually, if someone else injects themselves into the callback chain AND forgets to pass it along, we'd get an abort signal instead (if you hook the c++ exception callback and don't pass it along to the next handler, the ObjC runtime will never see it).

So, also capturing information at the C++ level would give an extra defence against poorly implemented third party software (but only if they hook the handler BEFORE we do; if they hook after us and forget to pass it on, all we'll see is https://app.bugsnag.com/bugsnag-test-projects/karl-watchos/errors/628c812aa8150d0009d7c56f?filters[event.since]=30d&filters[error.status]=open&event_id=628c812a00946907a7480000 ).

[kstenerud]

Nope, my initial testing was flawed, and further testing showed that this C++ code is not capturing NSException data after all. So we're better off going with the approach in this PR to ensure that bsg_kscrashsentry_beginHandlingCrash never gets called more than once for a single crash.

[kstenerud]

Closed in favour of #1389

[kstenerud]

Reopening because the other approach leaves open a concurrency risk.