bugsnag / bugsnag-cocoa

BugSnag error monitoring & exception reporter for iOS, macOS, tvOS and watchOS
https://docs.bugsnag.com/platforms/ios
MIT License
234 stars 128 forks source link

[PLAT-12346] Harmonize all calls to manual error reporting, and ensure proper frame stripping in all cases #1668

Closed kstenerud closed 2 months ago

kstenerud commented 2 months ago

Goal

Code along the critical path responsible for generating a stack trace and stripping off the stack trace generation frames themselves must always always be present exactly as written so that the top of the stack is always consistent, and the code can accurately strip off the right number of frames.

Recently, Apple introduced optimizations that inline or outline objective-c code, and can't be turned off. We need to ensure that these optimizations are never run on our call stack generation and fixup code.

Design

Since we can't instruct the compiler directly to not optimize these calls (Bugsnag.notifyXYZ, BugsnagClient.notifyXYZ), we must confound the compiler so that it gives up trying to optimize them.

I've added a new utility method BSGPreventInlining(), which takes a string and returns the last string it was given (no thread safety because the string contents don't matter at all and should never be used for anything).

The key concepts here are:

Ultimately, this becomes a reverse-halting-problem kind of cat-and-mouse game, where each tries to outwit and outlast the other. I think that this current design should be enough to keep the optimizations at bay long enough (a few years at least) for someone to finally allow optnone compiler directives in objective-c code...

rr

Note: This new design also corrects a bug where too many stack entries get stripped if you call the BugsnagClient notify methods directly (rather than calling Bugsnag.notifyXYZ).

Testing

Existing e2e tests check call stack integrity already, and I also verified manually just to be sure.

github-actions[bot] commented 2 months ago

Bugsnag.framework binary size increased by 112 bytes from 718,312 to 718,424

    FILE SIZE        VM SIZE    
 --------------  -------------- 
  +0.2%    +408  +0.2%    +408    __TEXT,__text
  +0.6%    +128  +0.6%    +128    __DATA,__cfstring
  +0.0%     +80  +0.0%     +80    String Table
  +0.0%     +32  +0.0%     +32    Symbol Table
  +0.2%     +31  +0.2%     +31    __TEXT,__cstring
  [ = ]       0  +0.1%      +8    __DATA,__bss
  +0.3%      +8  +0.3%      +8    __TEXT,__unwind_info
  -0.7%     -32  -0.7%     -32    __DATA,__objc_selrefs
  -1.8%     -96  -0.9%    -104    [__DATA]
  [ = ]       0  -4.3%    -112    [__LINKEDIT]
  -2.8%    -447  -2.8%    -447    [__TEXT]
  +0.0%    +112  [ = ]       0    TOTAL

Generated by :no_entry_sign: Danger