Merge pull request #675 from bugsnag/master-fix-locking

Fix DYLD lock mechanism preventing compilation on iOS <10

Bugsnag.podspec.json

@@ -1,6 +1,6 @@
 {
   "name": "Bugsnag",
-  "version": "5.23.2",
+  "version": "5.23.3",
   "summary": "Cocoa notifier for SDK for bugsnag.com",
   "homepage": "https://bugsnag.com",
   "license": "MIT",
@@ -9,7 +9,7 @@
   },
   "source": {
     "git": "https://github.com/bugsnag/bugsnag-cocoa.git",
-    "tag": "v5.23.2"
+    "tag": "v5.23.3"
   },
   "frameworks": ["Foundation", "SystemConfiguration"],
   "libraries": [

CHANGELOG.md

@@ -1,6 +1,13 @@
 Changelog
 =========
 
+## 5.23.3 (2020-06-05)
+
+## Bug Fixes
+
+* Fix DYLD lock mechanism preventing compilation on iOS <10.
+  [#675](https://github.com/bugsnag/bugsnag-cocoa/pull/675)
+
 ## 5.23.2 (2020-05-13)
 
 ## Bug Fixes

Source/BugsnagNotifier.m

@@ -47,7 +47,7 @@
 #import <AppKit/AppKit.h>
 #endif
 
-NSString *const NOTIFIER_VERSION = @"5.23.2";
+NSString *const NOTIFIER_VERSION = @"5.23.3";
 NSString *const NOTIFIER_URL = @"https://github.com/bugsnag/bugsnag-cocoa";
 NSString *const BSTabCrash = @"crash";
 NSString *const BSAttributeDepth = @"depth";

Source/KSCrash/Source/KSCrash/Recording/BSG_KSCrash.m

@@ -269,10 +269,10 @@ - (BOOL)install {
  * behaviour.
  */
 - (void)listenForLoadedBinaries {
+    bsg_check_unfair_lock_support();
     bsg_initialise_mach_binary_headers(BSG_INITIAL_MACH_BINARY_IMAGE_ARRAY_SIZE);
 
-    // Note: Internally, access to DYLD's binary image store is guarded by an OSSpinLock.  We therefore don't need to
-    // add additional guards around our access.
+    // Note: Access to DYLD's binary image store is guarded by locks.
     _dyld_register_func_for_remove_image(&bsg_mach_binary_image_removed);
     _dyld_register_func_for_add_image(&bsg_mach_binary_image_added);
 }

Source/KSCrash/Source/KSCrash/Recording/Tools/BSG_KSMachHeaders.h

@@ -10,6 +10,8 @@
 #define BSG_KSMachHeaders_h
 
 #import <mach/machine.h>
+#import <os/lock.h>
+#import <libkern/OSAtomic.h>
 
 /**
  * An encapsulation of the Mach header - either 64 or 32 bit, along with some additional information required for
@@ -36,54 +38,28 @@ typedef struct {
 
 static BSG_Mach_Binary_Images bsg_mach_binary_images;
 
+// MARK: - Locking
+
 /**
- * An OS-version-specific lock, used to synchronise access to the array of binary image info.
+ * An OS-version-specific lock, used to synchronise access to the array of binary image info.  A combination of
+ * compile-time determination of the OS and and run-time determination of the OS version is used to ensure that
+ * the correct lock mechanism is used.
  *
  * os_unfair_lock is available from specific OS versions onwards:
  *     https://developer.apple.com/documentation/os/os_unfair_lock
  *
  * It deprecates OSSpinLock:
  *     https://developer.apple.com/library/archive/documentation/System/Conceptual/ManPages_iPhoneOS/man3/spinlock.3.html
  *
- *  The #defined BSG_DYLD_CACHE_LOCK/UNLOCK avoid spurious warnings on later OSs.
+ * The imported headers have specific version info: <os/lock.h> and <libkern/OSAtomic.h>
  */
 
-#if defined(__IPHONE_10_0) || defined(__MAC_10_12) || defined(__TVOS_10_0) || defined(__WATCHOS_3_0)
-    #pragma clang diagnostic push
-    #pragma clang diagnostic ignored "-Wunguarded-availability"
-
-    #import <os/lock.h>
-    static os_unfair_lock bsg_mach_binary_images_access_lock = OS_UNFAIR_LOCK_INIT;
-
-    #ifndef BSG_DYLD_CACHE_LOCK
-    #define BSG_DYLD_CACHE_LOCK \
-        _Pragma("clang diagnostic push") \
-        _Pragma("clang diagnostic ignored \"-Wunguarded-availability\"") \
-        os_unfair_lock_lock(&bsg_mach_binary_images_access_lock); \
-        _Pragma("clang diagnostic pop")
-    #endif
-
-    #ifndef BSG_DYLD_CACHE_UNLOCK
-    #define BSG_DYLD_CACHE_UNLOCK \
-        _Pragma("clang diagnostic push") \
-        _Pragma("clang diagnostic ignored \"-Wunguarded-availability\"") \
-        os_unfair_lock_unlock(&bsg_mach_binary_images_access_lock); \
-        _Pragma("clang diagnostic pop")
-    #endif
-
-    #pragma clang diagnostic pop
-#else
-    #import <libkern/OSAtomic.h>
-    static OSSpinLock bsg_mach_binary_images_access_lock = OS_SPINLOCK_INIT;
-
-    #ifndef BSG_DYLD_CACHE_LOCK
-    #define BSG_DYLD_CACHE_LOCK OSSpinLockLock(&bsg_mach_binary_images_access_lock);
-    #endif
-
-    #ifndef BSG_DYLD_CACHE_UNLOCK
-    #define BSG_DYLD_CACHE_UNLOCK OSSpinLockUnlock(&bsg_mach_binary_images_access_lock);
-    #endif
-#endif
+void bsg_spin_lock(void);
+void bsg_spin_unlock(void);
+void bsg_unfair_lock(void);
+void bsg_unfair_unlock(void);
+void bsg_dyld_cache_lock(void);
+void bsg_dyld_cache_unlock(void);
 
 // MARK: - Replicate the DYLD API
 
@@ -126,4 +102,9 @@ void bsg_mach_binary_image_removed(const struct mach_header *mh, intptr_t slide)
  */
 BSG_Mach_Binary_Images *bsg_initialise_mach_binary_headers(uint32_t initialSize);
 
+/**
+ * Determines whether the OS supports unfair locks or not.
+ */
+void bsg_check_unfair_lock_support(void);
+
 #endif /* BSG_KSMachHeaders_h */

Source/KSCrash/Source/KSCrash/Recording/Tools/BSG_KSMachHeaders.m

@@ -12,6 +12,88 @@
 #import "BSG_KSDynamicLinker.h"
 #import "BSG_KSMachHeaders.h"
 
+// MARK: - Locking
+
+// Pragma's hide unavoidable (and expected) deprecation/unavailable warnings
+_Pragma("clang diagnostic push")
+_Pragma("clang diagnostic ignored \"-Wunguarded-availability\"")
+static os_unfair_lock bsg_mach_binary_images_access_lock_unfair = OS_UNFAIR_LOCK_INIT;
+_Pragma("clang diagnostic pop")
+
+_Pragma("clang diagnostic push")
+_Pragma("clang diagnostic ignored \"-Wdeprecated-declarations\"")
+static OSSpinLock bsg_mach_binary_images_access_lock_spin = OS_SPINLOCK_INIT;
+_Pragma("clang diagnostic pop")
+
+static BOOL bsg_unfair_lock_supported;
+
+// Lock helpers.  These use bulky Pragmas to hide warnings so are in their own functions for clarity.
+
+void bsg_spin_lock() {
+    _Pragma("clang diagnostic push")
+    _Pragma("clang diagnostic ignored \"-Wdeprecated-declarations\"")
+    OSSpinLockLock(&bsg_mach_binary_images_access_lock_spin);
+    _Pragma("clang diagnostic pop")
+}
+
+void bsg_spin_unlock() {
+    _Pragma("clang diagnostic push")
+    _Pragma("clang diagnostic ignored \"-Wdeprecated-declarations\"")
+    OSSpinLockUnlock(&bsg_mach_binary_images_access_lock_spin);
+    _Pragma("clang diagnostic pop")
+}
+
+void bsg_unfair_lock() {
+    _Pragma("clang diagnostic push")
+    _Pragma("clang diagnostic ignored \"-Wunguarded-availability\"")
+    os_unfair_lock_lock(&bsg_mach_binary_images_access_lock_unfair);
+    _Pragma("clang diagnostic pop")
+}
+
+void bsg_unfair_unlock() {
+    _Pragma("clang diagnostic push")
+    _Pragma("clang diagnostic ignored \"-Wunguarded-availability\"")
+    os_unfair_lock_unlock(&bsg_mach_binary_images_access_lock_unfair);
+    _Pragma("clang diagnostic pop")
+}
+
+// Lock and unlock sections of code
+
+void bsg_dyld_cache_lock() {
+    if (bsg_unfair_lock_supported) {
+        bsg_unfair_lock();
+    } else {
+        bsg_spin_lock();
+    }
+}
+
+void bsg_dyld_cache_unlock() {
+    if (bsg_unfair_lock_supported) {
+        bsg_unfair_unlock();
+    } else {
+        bsg_spin_unlock();
+    }
+}
+
+BOOL BSGIsUnfairLockSupported(NSProcessInfo *processInfo) {
+    NSOperatingSystemVersion minSdk = {0,0,0};
+#if TARGET_OS_IOS
+    minSdk.majorVersion = 10;
+#elif TARGET_OS_OSX
+    minSdk.majorVersion = 10;
+    minSdk.minorVersion = 12;
+#elif TARGET_OS_TV
+    minSdk.majorVersion = 10;
+#elif TARGET_OS_WATCH
+    minSdk.majorVersion = 3;
+#endif
+    return [processInfo isOperatingSystemAtLeastVersion:minSdk];
+}
+
+void bsg_check_unfair_lock_support() {
+    bsg_unfair_lock_supported = BSGIsUnfairLockSupported([NSProcessInfo processInfo]);
+}
+
 // MARK: - Replicate the DYLD API
 
 uint32_t bsg_dyld_image_count(void) {
@@ -53,7 +135,7 @@ intptr_t bsg_dyld_get_image_vmaddr_slide(uint32_t imageIndex) {
  */
 void bsg_add_mach_binary_image(BSG_Mach_Binary_Image_Info element) {
 
-    BSG_DYLD_CACHE_LOCK
+    bsg_dyld_cache_lock();
 
     // Expand array if necessary.  We're slightly paranoid here.  An OOM is likely to be indicative of bigger problems
     // but we should still do *our* best not to crash the app.
@@ -69,15 +151,15 @@ void bsg_add_mach_binary_image(BSG_Mach_Binary_Image_Info element) {
         }
         else {
             // Exit early, don't expand the array, don't store the header info and unlock
-            BSG_DYLD_CACHE_UNLOCK
+            bsg_dyld_cache_unlock();
             return;
         }
     }
 
     // Store the value, increment the number of used elements
     bsg_mach_binary_images.contents[bsg_mach_binary_images.used++] = element;
 
-    BSG_DYLD_CACHE_UNLOCK
+    bsg_dyld_cache_unlock();
 }
 
 /**
@@ -87,7 +169,7 @@ void bsg_add_mach_binary_image(BSG_Mach_Binary_Image_Info element) {
  */
 void bsg_remove_mach_binary_image(uint64_t imageVmAddr) {
 
-    BSG_DYLD_CACHE_LOCK
+    bsg_dyld_cache_lock();
 
     for (uint32_t i=0; i<bsg_mach_binary_images.used; i++) {
         BSG_Mach_Binary_Image_Info item = bsg_mach_binary_images.contents[i];
@@ -104,9 +186,16 @@ void bsg_remove_mach_binary_image(uint64_t imageVmAddr) {
         }
     }
 
-    BSG_DYLD_CACHE_UNLOCK
+    bsg_dyld_cache_unlock();
 }
 
+/**
+ * Create an empty array with initial capacity to hold Mach header info.
+ *
+ * @param initialSize The initial array capacity
+ *
+ * @returns A struct for holding Mach binary image info
+*/
 BSG_Mach_Binary_Images *bsg_initialise_mach_binary_headers(uint32_t initialSize) {
     bsg_mach_binary_images.contents = (BSG_Mach_Binary_Image_Info *)malloc(initialSize * sizeof(BSG_Mach_Binary_Image_Info));
     bsg_mach_binary_images.used = 0;

VERSION

@@ -1 +1 @@
-5.23.2
+5.23.3