diff --git a/Sources/Sentry/include/SentryCrashBinaryImageCache.h b/Sources/Sentry/include/SentryCrashBinaryImageCache.h index 01fbe576b54..e5e2cc6da50 100644 --- a/Sources/Sentry/include/SentryCrashBinaryImageCache.h +++ b/Sources/Sentry/include/SentryCrashBinaryImageCache.h @@ -20,15 +20,15 @@ void sentrycrashbic_iterateOverImages(sentrycrashbic_imageIteratorCallback index void sentrycrashbic_startCache(void); /** - * Stops the cache from monitoring binary image being loaded or removed. - * This will also clean the cache. + * This is a no-op TODO: Remove this */ void sentrycrashbic_stopCache(void); /** * Register a callback to be called every time a new binary image is added to the cache. * After register, this callback will be called for every image already in the cache, - * this is a thread safe operation. + * this is a thread safe operation. The callback can be called multiple times for the same image + * If the image was being registered on a different thread at the same time the callback is registered. */ void sentrycrashbic_registerAddedCallback(sentrycrashbic_cacheChangeCallback callback); diff --git a/Sources/SentryCrash/Recording/SentryCrashBinaryImageCache.c b/Sources/SentryCrash/Recording/SentryCrashBinaryImageCache.c index 7fe4c4dd046..64018125395 100644 --- a/Sources/SentryCrash/Recording/SentryCrashBinaryImageCache.c +++ b/Sources/SentryCrash/Recording/SentryCrashBinaryImageCache.c @@ -1,8 +1,10 @@ #include "SentryCrashBinaryImageCache.h" #include "SentryCrashDynamicLinker.h" +#include #include #include #include +#include #include #include #include @@ -60,94 +62,110 @@ sentry_resetFuncForAddRemoveImage(void) # define _will_add_image() #endif // defined(SENTRY_TEST) || defined(SENTRY_TEST_CI) || defined(DEBUG) -typedef struct SentryCrashBinaryImageNode { +#define MAX_DYLD_IMAGES 4096 + +// Entry lifecycle states +#define IMAGE_EMPTY 0 // Slot reserved but data not written, or write failed +#define IMAGE_READY 1 // Published, visible to readers +#define IMAGE_REMOVED 3 // Image was unloaded + +typedef struct { + _Atomic(uint32_t) state; SentryCrashBinaryImage image; - bool available; - struct SentryCrashBinaryImageNode *next; -} SentryCrashBinaryImageNode; +} PublishedBinaryImage; -static SentryCrashBinaryImageNode rootNode = { 0 }; -static SentryCrashBinaryImageNode *tailNode = NULL; -static pthread_mutex_t binaryImagesMutex = PTHREAD_MUTEX_INITIALIZER; +// g_next_index monotonically increases and is never reset (except in test-only reset). +// Each slot is used at most once, so there are no stale state flags from prior runs. +static PublishedBinaryImage g_images[MAX_DYLD_IMAGES]; +static _Atomic(uint32_t) g_next_index = 0; -static sentrycrashbic_cacheChangeCallback imageAddedCallback = NULL; -static sentrycrashbic_cacheChangeCallback imageRemovedCallback = NULL; +static _Atomic(sentrycrashbic_cacheChangeCallback) g_addedCallback = NULL; +static _Atomic(sentrycrashbic_cacheChangeCallback) g_removedCallback = NULL; static void -binaryImageAdded(const struct mach_header *header, intptr_t slide) +add_dyld_image(const struct mach_header *mh) { - pthread_mutex_lock(&binaryImagesMutex); - if (tailNode == NULL) { - pthread_mutex_unlock(&binaryImagesMutex); - return; - } - pthread_mutex_unlock(&binaryImagesMutex); + // Check dladdr first, before reserving a slot in the array. Dl_info info; - if (!dladdr(header, &info) || info.dli_fname == NULL) { + if (!dladdr(mh, &info) || info.dli_fname == NULL) { return; } - SentryCrashBinaryImage binaryImage = { 0 }; - if (!sentrycrashdl_getBinaryImageForHeader( - (const void *)header, info.dli_fname, &binaryImage, false)) { + // Test hook: called just before adding the image + _will_add_image(); + + uint32_t idx = atomic_fetch_add_explicit(&g_next_index, 1, memory_order_relaxed); + + if (idx >= MAX_DYLD_IMAGES) { return; } - SentryCrashBinaryImageNode *newNode = malloc(sizeof(SentryCrashBinaryImageNode)); - newNode->available = true; - newNode->image = binaryImage; - newNode->next = NULL; - _will_add_image(); - pthread_mutex_lock(&binaryImagesMutex); - // Recheck tailNode as it could be null when - // stopped from another thread. - if (tailNode != NULL) { - tailNode->next = newNode; - tailNode = tailNode->next; - } else { - free(newNode); - newNode = NULL; + PublishedBinaryImage *entry = &g_images[idx]; + + if (!sentrycrashdl_getBinaryImageForHeader(mh, info.dli_fname, &entry->image, false)) { + // Leave state as IMAGE_EMPTY so the entry is never published. + return; } - pthread_mutex_unlock(&binaryImagesMutex); - if (newNode && imageAddedCallback) { - imageAddedCallback(&newNode->image); + + sentrycrashbic_cacheChangeCallback callback + = atomic_load_explicit(&g_addedCallback, memory_order_acquire); + + atomic_store_explicit(&entry->state, IMAGE_READY, memory_order_release); + + if (callback != NULL) { + callback(&entry->image); } } static void -binaryImageRemoved(const struct mach_header *header, intptr_t slide) +dyld_add_image_cb(const struct mach_header *mh, intptr_t slide) { - SentryCrashBinaryImageNode *nextNode = &rootNode; + add_dyld_image(mh); +} - while (nextNode != NULL) { - if (nextNode->image.address == (uint64_t)header) { - nextNode->available = false; - if (imageRemovedCallback) { - imageRemovedCallback(&nextNode->image); +static void +dyld_remove_image_cb(const struct mach_header *mh, intptr_t slide) +{ + sentrycrashbic_cacheChangeCallback callback + = atomic_load_explicit(&g_removedCallback, memory_order_acquire); + + uint32_t count = atomic_load_explicit(&g_next_index, memory_order_acquire); + if (count > MAX_DYLD_IMAGES) + count = MAX_DYLD_IMAGES; + + for (uint32_t i = 0; i < count; i++) { + PublishedBinaryImage *src = &g_images[i]; + if (src->image.address == (uintptr_t)mh) { + atomic_store_explicit(&src->state, IMAGE_REMOVED, memory_order_release); + if (callback) { + callback(&src->image); } - break; + return; } - nextNode = nextNode->next; } } +static void +dyld_tracker_start(void) +{ + sentry_dyld_register_func_for_add_image(dyld_add_image_cb); + sentry_dyld_register_func_for_remove_image(dyld_remove_image_cb); +} + void sentrycrashbic_iterateOverImages(sentrycrashbic_imageIteratorCallback callback, void *context) { - /** - We can't use locks here because this is meant to be used during crashes, - where we can't use async unsafe functions. In order to avoid potential problems, - we choose an approach that doesn't remove nodes from the list. - */ - SentryCrashBinaryImageNode *nextNode = &rootNode; - - // If tailNode is null it means the cache was stopped, therefore we end the iteration. - // This will minimize any race condition effect without the need for locks. - while (nextNode != NULL && tailNode != NULL) { - if (nextNode->available) { - callback(&nextNode->image, context); + uint32_t count = atomic_load_explicit(&g_next_index, memory_order_acquire); + + if (count > MAX_DYLD_IMAGES) + count = MAX_DYLD_IMAGES; + + for (uint32_t i = 0; i < count; i++) { + PublishedBinaryImage *src = &g_images[i]; + + if (atomic_load_explicit(&src->state, memory_order_acquire) == IMAGE_READY) { + callback(&src->image, context); } - nextNode = nextNode->next; } } @@ -159,7 +177,7 @@ sentrycrashbic_iterateOverImages(sentrycrashbic_imageIteratorCallback callback, * @return true if dyld is not found in the loaded images and should be added to the cache, * false if dyld is already present in the loaded images. */ -bool +static bool sentrycrashbic_shouldAddDyld(void) { // dyld is different from libdyld.dylib; the latter contains the public API @@ -170,93 +188,104 @@ sentrycrashbic_shouldAddDyld(void) // Since Apple no longer includes dyld in the images listed `_dyld_image_count` and related // functions We manually include it to our cache. -SentryCrashBinaryImageNode * -sentrycrashbic_getDyldNode(void) +// Note: This bypasses add_dyld_image() because dladdr() returns NULL for dyld, so we need +// to use sentrycrashdl_getBinaryImageForHeader() directly with a hardcoded filename. +static void +sentrycrashbic_addDyldNode(void) { const struct mach_header *header = sentryDyldHeader; - SentryCrashBinaryImage binaryImage = { 0 }; - if (!sentrycrashdl_getBinaryImageForHeader((const void *)header, "dyld", &binaryImage, false)) { - return NULL; + uint32_t idx = atomic_fetch_add_explicit(&g_next_index, 1, memory_order_relaxed); + if (idx >= MAX_DYLD_IMAGES) { + return; } - SentryCrashBinaryImageNode *newNode = malloc(sizeof(SentryCrashBinaryImageNode)); - newNode->available = true; - newNode->image = binaryImage; - newNode->next = NULL; + PublishedBinaryImage *entry = &g_images[idx]; + if (!sentrycrashdl_getBinaryImageForHeader( + (const void *)header, "dyld", &entry->image, false)) { + return; + } - return newNode; + atomic_store_explicit(&entry->state, IMAGE_READY, memory_order_release); } -void -sentrycrashbic_startCache(void) +static void +sentrycrashbic_startCacheImpl(void) { - pthread_mutex_lock(&binaryImagesMutex); - if (tailNode != NULL) { - // Already initialized - pthread_mutex_unlock(&binaryImagesMutex); - return; - } - if (sentrycrashbic_shouldAddDyld()) { sentrycrashdl_initialize(); - SentryCrashBinaryImageNode *dyldNode = sentrycrashbic_getDyldNode(); - tailNode = dyldNode; - rootNode.next = dyldNode; - } else { - tailNode = &rootNode; - rootNode.next = NULL; + sentrycrashbic_addDyldNode(); } - pthread_mutex_unlock(&binaryImagesMutex); + // During this call the callback is invoked synchronously for every existing image. + dyld_tracker_start(); +} +void +sentrycrashbic_startCache(void) +{ // During a call to _dyld_register_func_for_add_image() the callback func is called for every // existing image - sentry_dyld_register_func_for_add_image(&binaryImageAdded); - sentry_dyld_register_func_for_remove_image(&binaryImageRemoved); + // This must be done on a background thread to not block app launch due to the extensive use of + // locks in the image added callback. The main culprit is the calls to `dladdr`. The downside of + // doing this async is if there is a crash very shortly after app launch we might not have + // recorded all the load addresses of images yet. We think this is an acceptible tradeoff to not + // block app launch, since it's always possible to crash early in app launch before Sentry can + // capture the crash. +#if defined(SENTRY_TEST) || defined(SENTRY_TEST_CI) + sentrycrashbic_startCacheImpl(); +#else + static dispatch_once_t once_token = 0; + dispatch_once(&once_token, ^{ + dispatch_async( + dispatch_get_global_queue(QOS_CLASS_DEFAULT, 0), ^{ sentrycrashbic_startCacheImpl(); }); + }); +#endif } void sentrycrashbic_stopCache(void) -{ - pthread_mutex_lock(&binaryImagesMutex); - if (tailNode == NULL) { - pthread_mutex_unlock(&binaryImagesMutex); - return; - } - - SentryCrashBinaryImageNode *node = rootNode.next; - rootNode.next = NULL; - tailNode = NULL; +{ } - while (node != NULL) { - SentryCrashBinaryImageNode *nextNode = node->next; - free(node); - node = nextNode; - } - - pthread_mutex_unlock(&binaryImagesMutex); -} - -static void -initialReportToCallback(SentryCrashBinaryImage *image, void *context) +// Resetting can create race conditions so should only be done in controlled test environments. +#if defined(SENTRY_TEST) || defined(SENTRY_TEST_CI) +void +sentry_resetBinaryImageCache(void) { - sentrycrashbic_cacheChangeCallback callback = (sentrycrashbic_cacheChangeCallback)context; - callback(image); + uint32_t count = atomic_load_explicit(&g_next_index, memory_order_relaxed); + if (count > MAX_DYLD_IMAGES) + count = MAX_DYLD_IMAGES; + for (uint32_t i = 0; i < count; i++) { + atomic_store_explicit(&g_images[i].state, IMAGE_EMPTY, memory_order_relaxed); + } + atomic_store_explicit(&g_next_index, 0, memory_order_relaxed); + atomic_store_explicit(&g_addedCallback, NULL, memory_order_relaxed); + atomic_store_explicit(&g_removedCallback, NULL, memory_order_relaxed); } +#endif void sentrycrashbic_registerAddedCallback(sentrycrashbic_cacheChangeCallback callback) { - imageAddedCallback = callback; - if (callback) { - pthread_mutex_lock(&binaryImagesMutex); - sentrycrashbic_iterateOverImages(&initialReportToCallback, callback); - pthread_mutex_unlock(&binaryImagesMutex); + atomic_store_explicit(&g_addedCallback, callback, memory_order_release); + + if (callback != NULL) { + // Call for all existing images already in the cache + uint32_t count = atomic_load_explicit(&g_next_index, memory_order_acquire); + if (count > MAX_DYLD_IMAGES) + count = MAX_DYLD_IMAGES; + + for (uint32_t i = 0; i < count; i++) { + PublishedBinaryImage *src = &g_images[i]; + if (atomic_load_explicit(&src->state, memory_order_acquire) != IMAGE_READY) { + continue; + } + callback(&src->image); + } } } void sentrycrashbic_registerRemovedCallback(sentrycrashbic_cacheChangeCallback callback) { - imageRemovedCallback = callback; + atomic_store_explicit(&g_removedCallback, callback, memory_order_release); } diff --git a/Sources/Swift/Core/Helper/SentryBinaryImageCache.swift b/Sources/Swift/Core/Helper/SentryBinaryImageCache.swift index bc1009097c1..4e4515cd9ae 100644 --- a/Sources/Swift/Core/Helper/SentryBinaryImageCache.swift +++ b/Sources/Swift/Core/Helper/SentryBinaryImageCache.swift @@ -42,8 +42,7 @@ import Foundation return } let image = imagePtr.pointee - SentryDependencyContainer.sharedInstance().binaryImageCache.binaryImageAdded( - imageName: image.name, vmAddress: image.vmAddress, address: image.address, size: image.size, uuid: image.uuid) + SentryDependencyContainer.sharedInstance().binaryImageCache.binaryImageAdded(image: image) } sentrycrashbic_registerRemovedCallback { imagePtr in guard let imagePtr else { @@ -64,14 +63,8 @@ import Foundation } } - // We have to expand `SentryCrashBinaryImage` since the model is defined in SentryPrivate - @objc(binaryImageAdded:vmAddress:address:size:uuid:) - public func binaryImageAdded(imageName: UnsafePointer?, - vmAddress: UInt64, - address: UInt64, - size: UInt64, - uuid: UnsafePointer?) { - guard let imageName else { + func binaryImageAdded(image: SentryCrashBinaryImage) { + guard let imageName = image.name else { SentrySDKLog.warning("The image name was NULL. Can't add image to cache.") return } @@ -82,10 +75,10 @@ import Foundation let newImage = SentryBinaryImageInfo( name: nameString, - uuid: Self.convertUUID(uuid), - vmAddress: vmAddress, - address: address, - size: size + uuid: Self.convertUUID(image.uuid), + vmAddress: image.vmAddress, + address: image.address, + size: image.size ) lock.synchronized { @@ -118,18 +111,16 @@ import Foundation dispatchQueueWrapper: Dependencies.dispatchQueueWrapper) } } - - @objc - public static func convertUUID(_ value: UnsafePointer?) -> String? { + + private static func convertUUID(_ value: UnsafePointer?) -> String? { guard let value = value else { return nil } var uuidBuffer = [CChar](repeating: 0, count: 37) sentrycrashdl_convertBinaryImageUUID(value, &uuidBuffer) return String(cString: uuidBuffer, encoding: .ascii) } - - @objc - public func binaryImageRemoved(_ imageAddress: UInt64) { + + func binaryImageRemoved(_ imageAddress: UInt64) { lock.synchronized { guard let index = indexOfImage(address: imageAddress) else { return } self.cache?.remove(at: index) @@ -166,24 +157,7 @@ import Foundation return nil } - @objc(imagePathsForInAppInclude:) - public func imagePathsFor(inAppInclude: String) -> Set { - lock.synchronized { - var imagePaths = Set() - - guard let cache = self.cache else { return imagePaths } - - for info in cache { - if SentryInAppLogic.isImageNameInApp(info.name, inAppInclude: inAppInclude) { - imagePaths.insert(info.name) - } - } - return imagePaths - } - } - - @objc - public func getAllBinaryImages() -> [SentryBinaryImageInfo] { + func getAllBinaryImages() -> [SentryBinaryImageInfo] { lock.synchronized { return cache ?? [] } diff --git a/Sources/Swift/Core/Integrations/Performance/SentryUIViewControllerSwizzling.swift b/Sources/Swift/Core/Integrations/Performance/SentryUIViewControllerSwizzling.swift index bc608806928..86c162b6cda 100644 --- a/Sources/Swift/Core/Integrations/Performance/SentryUIViewControllerSwizzling.swift +++ b/Sources/Swift/Core/Integrations/Performance/SentryUIViewControllerSwizzling.swift @@ -1,4 +1,5 @@ @_implementationOnly import _SentryPrivate +import MachO #if (os(iOS) || os(tvOS) || os(visionOS)) && !SENTRY_NO_UI_FRAMEWORK import UIKit @@ -19,7 +20,6 @@ class SentryUIViewControllerSwizzling { private let subClassFinder: SentrySubClassFinder private let imagesActedOnSubclassesOfUIViewControllers: NSMutableSet private let processInfoWrapper: SentryProcessInfoSource - private let binaryImageCache: SentryBinaryImageCache private let performanceTracker: SentryUIViewControllerPerformanceTracker init( @@ -28,7 +28,6 @@ class SentryUIViewControllerSwizzling { objcRuntimeWrapper: SentryObjCRuntimeWrapper, subClassFinder: SentrySubClassFinder, processInfoWrapper: SentryProcessInfoSource, - binaryImageCache: SentryBinaryImageCache, performanceTracker: SentryUIViewControllerPerformanceTracker ) { self.options = options @@ -38,19 +37,23 @@ class SentryUIViewControllerSwizzling { self.subClassFinder = subClassFinder self.imagesActedOnSubclassesOfUIViewControllers = NSMutableSet() self.processInfoWrapper = processInfoWrapper - self.binaryImageCache = binaryImageCache self.performanceTracker = performanceTracker } func start() { - for inAppInclude in inAppLogic.inAppIncludes { - let imagePathsToInAppInclude = binaryImageCache.imagePathsFor(inAppInclude: inAppInclude) + let imageNames = (0..<_dyld_image_count()).compactMap { i in + _dyld_get_image_name(i).map { String(cString: $0) } + } - if !imagePathsToInAppInclude.isEmpty { - for imagePath in imagePathsToInAppInclude { - swizzleUIViewControllers(ofImage: imagePath) + for inAppInclude in inAppLogic.inAppIncludes { + var found = false + for imageName in imageNames { + if SentryInAppLogic.isImageNameInApp(imageName, inAppInclude: inAppInclude) { + found = true + swizzleUIViewControllers(ofImage: imageName) } - } else { + } + if !found { SentrySDKLog.warning( "Failed to find the binary image(s) for inAppInclude <\(inAppInclude)> and, therefore can't instrument UIViewControllers in these binaries." ) diff --git a/Sources/Swift/SentryCrash/SentryDebugImageProvider.swift b/Sources/Swift/SentryCrash/SentryDebugImageProvider.swift index 1fb5c94d2cc..24a11b6c8e5 100644 --- a/Sources/Swift/SentryCrash/SentryDebugImageProvider.swift +++ b/Sources/Swift/SentryCrash/SentryDebugImageProvider.swift @@ -1,6 +1,8 @@ // swiftlint:disable missing_docs @_implementationOnly import _SentryPrivate +// Wraps `SentryBinaryImageCache` which is not reentrant. It's a faster way to get an image name from an address +// but cannot be used from async signal safe code. @_spi(Private) @objc public class SentryDebugImageProvider: NSObject { private static let debugImageType = "macho" diff --git a/Sources/Swift/SentryDependencyContainer.swift b/Sources/Swift/SentryDependencyContainer.swift index fe8919d2d76..2fe4c97b0d0 100644 --- a/Sources/Swift/SentryDependencyContainer.swift +++ b/Sources/Swift/SentryDependencyContainer.swift @@ -234,7 +234,6 @@ extension SentryFileManager: SentryFileManagerProtocol { } objcRuntimeWrapper: objcRuntimeWrapper, subClassFinder: subClassFinder, processInfoWrapper: processInfoWrapper, - binaryImageCache: binaryImageCache, performanceTracker: uiViewControllerPerformanceTracker ) diff --git a/Tests/SentryTests/Integrations/Performance/UIViewController/SentryUIViewControllerSwizzlingTests.swift b/Tests/SentryTests/Integrations/Performance/UIViewController/SentryUIViewControllerSwizzlingTests.swift index 1c0844d2e1e..a94387c1f7b 100644 --- a/Tests/SentryTests/Integrations/Performance/UIViewController/SentryUIViewControllerSwizzlingTests.swift +++ b/Tests/SentryTests/Integrations/Performance/UIViewController/SentryUIViewControllerSwizzlingTests.swift @@ -12,13 +12,11 @@ class SentryUIViewControllerSwizzlingTests: XCTestCase { let objcRuntimeWrapper = SentryTestObjCRuntimeWrapper() let subClassFinder: TestSubClassFinder let processInfoWrapper = MockSentryProcessInfo() - let binaryImageCache: SentryBinaryImageCache let performanceTracker = SentryUIViewControllerPerformanceTracker() var options: Options init() { subClassFinder = TestSubClassFinder(dispatchQueue: dispatchQueue, objcRuntimeWrapper: objcRuntimeWrapper, swizzleClassNameExcludes: []) - binaryImageCache = SentryDependencyContainer.sharedInstance().binaryImageCache options = Options.noIntegrations() @@ -29,15 +27,15 @@ class SentryUIViewControllerSwizzlingTests: XCTestCase { } var sut: SentryUIViewControllerSwizzling { - return SentryUIViewControllerSwizzling(options: options, dispatchQueue: dispatchQueue, objcRuntimeWrapper: objcRuntimeWrapper, subClassFinder: subClassFinder, processInfoWrapper: processInfoWrapper, binaryImageCache: binaryImageCache, performanceTracker: performanceTracker) + return SentryUIViewControllerSwizzling(options: options, dispatchQueue: dispatchQueue, objcRuntimeWrapper: objcRuntimeWrapper, subClassFinder: subClassFinder, processInfoWrapper: processInfoWrapper, performanceTracker: performanceTracker) } var sutWithDefaultObjCRuntimeWrapper: SentryUIViewControllerSwizzling { - return SentryUIViewControllerSwizzling(options: options, dispatchQueue: dispatchQueue, objcRuntimeWrapper: SentryDependencyContainer.sharedInstance().objcRuntimeWrapper, subClassFinder: subClassFinder, processInfoWrapper: processInfoWrapper, binaryImageCache: binaryImageCache, performanceTracker: performanceTracker) + return SentryUIViewControllerSwizzling(options: options, dispatchQueue: dispatchQueue, objcRuntimeWrapper: SentryDependencyContainer.sharedInstance().objcRuntimeWrapper, subClassFinder: subClassFinder, processInfoWrapper: processInfoWrapper, performanceTracker: performanceTracker) } var testableSut: TestSentryUIViewControllerSwizzling { - return TestSentryUIViewControllerSwizzling(options: options, dispatchQueue: dispatchQueue, objcRuntimeWrapper: objcRuntimeWrapper, subClassFinder: subClassFinder, processInfoWrapper: processInfoWrapper, binaryImageCache: binaryImageCache, performanceTracker: performanceTracker) + return TestSentryUIViewControllerSwizzling(options: options, dispatchQueue: dispatchQueue, objcRuntimeWrapper: objcRuntimeWrapper, subClassFinder: subClassFinder, processInfoWrapper: processInfoWrapper, performanceTracker: performanceTracker) } var delegate: MockApplication.MockApplicationDelegate { @@ -187,34 +185,6 @@ class SentryUIViewControllerSwizzlingTests: XCTestCase { XCTAssertNotNil(SentrySDK.span) } - /// Xcode 16 introduces a new flag ENABLE_DEBUG_DYLIB (https://developer.apple.com/documentation/xcode/build-settings-reference#Enable-Debug-Dylib-Support) - /// If this flag is enabled, debug builds of app and app extension targets on supported platforms and SDKs - /// will be built with the main binary code in a separate “NAME.debug.dylib”. - /// This test adds this debug.dylib and checks if it gets swizzled. - func testSwizzle_DebugDylib_GetsSwizzled() { - let imageName = String( - cString: class_getImageName(SentryUIViewControllerSwizzlingTests.self)!, - encoding: .utf8)! as NSString - - let debugDylib = "\(imageName).debug.dylib" - - let image = createCrashBinaryImage(0, name: debugDylib) - SentryDependencyContainer.sharedInstance().binaryImageCache.start(false) - SentryDependencyContainer.sharedInstance().binaryImageCache.binaryImageAdded(imageName: image.name, - vmAddress: image.vmAddress, - address: image.address, - size: image.size, - uuid: image.uuid) - - let sut = fixture.sut - sut.start() - - let subClassFinderInvocations = fixture.subClassFinder.invocations - let result = subClassFinderInvocations.invocations.filter { $0.imageName == debugDylib } - - XCTAssertEqual(1, result.count) - } - func testSwizzle_fromScene_invalidNotification_NoObject() { let swizzler = fixture.testableSut diff --git a/Tests/SentryTests/SentryBinaryImageCacheTests.swift b/Tests/SentryTests/SentryBinaryImageCacheTests.swift index 277a63d5c47..f4bb91e0b25 100644 --- a/Tests/SentryTests/SentryBinaryImageCacheTests.swift +++ b/Tests/SentryTests/SentryBinaryImageCacheTests.swift @@ -18,11 +18,7 @@ class SentryBinaryImageCacheTests: XCTestCase { } private func addBinaryImageToSut(_ binaryImage: SentryCrashBinaryImage) { - sut.binaryImageAdded(imageName: binaryImage.name, - vmAddress: binaryImage.vmAddress, - address: binaryImage.address, - size: binaryImage.size, - uuid: binaryImage.uuid) + sut.binaryImageAdded(image: binaryImage) } func testBinaryImageAdded() { @@ -56,11 +52,7 @@ class SentryBinaryImageCacheTests: XCTestCase { } func testBinaryImageAdded_WithNilName() { - sut.binaryImageAdded(imageName: nil, - vmAddress: 100, - address: 1_000, - size: 100, - uuid: nil) + sut.binaryImageAdded(image: .init(address: 1_000, vmAddress: 100, size: 100, name: nil, uuid: nil, cpuType: 0, cpuSubType: 0, crashInfoMessage: nil, crashInfoMessage2: nil)) XCTAssertEqual(self.sut.cache?.count, 0) } @@ -129,25 +121,6 @@ class SentryBinaryImageCacheTests: XCTestCase { XCTAssertNil(sut.imageByAddress(399)) } - func testImagePathByName() { - let binaryImage = createCrashBinaryImage(0) - let binaryImage2 = createCrashBinaryImage(1) - addBinaryImageToSut(binaryImage) - addBinaryImageToSut(binaryImage2) - - let paths = sut.imagePathsFor(inAppInclude: "Expected Name at 0") - XCTAssertEqual(paths.first, "Expected Name at 0") - - let paths2 = sut.imagePathsFor(inAppInclude: "Expected Name at 1") - XCTAssertEqual(paths2.first, "Expected Name at 1") - - let bothPaths = sut.imagePathsFor(inAppInclude: "Expected") - XCTAssertEqual(bothPaths, ["Expected Name at 0", "Expected Name at 1"]) - - let didNotFind = sut.imagePathsFor(inAppInclude: "Name at 0") - XCTAssertTrue(didNotFind.isEmpty) - } - func testBinaryImageWithNULLName_DoesNotAddImage() { let address = UInt64(100) diff --git a/Tests/SentryTests/SentryCrash/SentryCrashBinaryImageCacheTests.m b/Tests/SentryTests/SentryCrash/SentryCrashBinaryImageCacheTests.m index de10fee61c8..de7aebc23b6 100644 --- a/Tests/SentryTests/SentryCrash/SentryCrashBinaryImageCacheTests.m +++ b/Tests/SentryTests/SentryCrash/SentryCrashBinaryImageCacheTests.m @@ -6,11 +6,12 @@ #include #include -// Exposing test only functions from `SentryCrashBinaryImageCache.m` +// Exposing test only functions from `SentryCrashBinaryImageCache.c` void sentry_setRegisterFuncForAddImage(void *addFunction); void sentry_setRegisterFuncForRemoveImage(void *removeFunction); void sentry_resetFuncForAddRemoveImage(void); void sentry_setFuncForBeforeAdd(void (*callback)(void)); +void sentry_resetBinaryImageCache(void); static void (*addBinaryImage)(const struct mach_header *mh, intptr_t vmaddr_slide); static void (*removeBinaryImage)(const struct mach_header *mh, intptr_t vmaddr_slide); @@ -59,6 +60,17 @@ [array addObject:[NSValue valueWithPointer:image]]; } +static void +addBinaryImageNameToArray(SentryCrashBinaryImage *image, void *context) +{ + NSMutableArray *array = (__bridge NSMutableArray *)context; + if (image->name) { + [array addObject:[NSString stringWithUTF8String:image->name]]; + } else { + [array addObject:@""]; + } +} + dispatch_semaphore_t delaySemaphore = NULL; dispatch_semaphore_t delayCalled = NULL; static void @@ -103,8 +115,7 @@ - (void)setUp - (void)tearDown { sentrycrashdl_clearDyld(); - sentry_resetFuncForAddRemoveImage(); - sentrycrashbic_stopCache(); + sentry_resetBinaryImageCache(); sentry_setFuncForBeforeAdd(NULL); [SentryDependencyContainer reset]; } @@ -288,7 +299,11 @@ - (void)testSentryBinaryImageCacheIntegration [imageCache start:false]; // by calling start, SentryBinaryImageCache will register a callback with // `SentryCrashBinaryImageCache` that should be called for every image already cached. - XCTAssertEqual(5, imageCache.cache.count); + NSMutableArray *paths = [NSMutableArray new]; + [imageCache.cache enumerateObjectsUsingBlock:^(SentryBinaryImageInfo *_Nonnull obj, + NSUInteger __unused idx, BOOL *_Nonnull __unused stop) { [paths addObject:obj.name]; }]; + XCTAssertEqual( + 5, imageCache.cache.count, @"Cache should start with 5 images but contained %@", paths); addBinaryImage([mach_headers_test_cache[5] pointerValue], 0); XCTAssertEqual(6, imageCache.cache.count); @@ -306,7 +321,10 @@ - (void)assertBinaryImageCacheLength:(int)expected { int counter = 0; sentrycrashbic_iterateOverImages(countNumberOfImagesInCache, &counter); - XCTAssertEqual(counter, expected); + NSMutableArray *names = [NSMutableArray new]; + sentrycrashbic_iterateOverImages(addBinaryImageNameToArray, (__bridge void *)(names)); + XCTAssertEqual( + counter, expected, @"Cache should have %d images but contained %@", expected, names); } - (void)assertCachedBinaryImages diff --git a/Tests/SentryTests/SentryCrash/SentryCrashStackEntryMapperTests.swift b/Tests/SentryTests/SentryCrash/SentryCrashStackEntryMapperTests.swift index b9472326be3..a103f45eb94 100644 --- a/Tests/SentryTests/SentryCrash/SentryCrashStackEntryMapperTests.swift +++ b/Tests/SentryTests/SentryCrash/SentryCrashStackEntryMapperTests.swift @@ -31,11 +31,7 @@ class SentryCrashStackEntryMapperTests: XCTestCase { func testImageFromCache() { let image = createCrashBinaryImage(2_488_998_912) SentryDependencyContainer.sharedInstance().binaryImageCache.start(false) - SentryDependencyContainer.sharedInstance().binaryImageCache.binaryImageAdded(imageName: image.name, - vmAddress: image.vmAddress, - address: image.address, - size: image.size, - uuid: image.uuid) + SentryDependencyContainer.sharedInstance().binaryImageCache.binaryImageAdded(image: image) var cursor = SentryCrashStackCursor() cursor.stackEntry.address = 2_488_998_950 diff --git a/Tests/SentryTests/SentryCrash/SentryDebugImageProviderTests.swift b/Tests/SentryTests/SentryCrash/SentryDebugImageProviderTests.swift index d3311b4cfe3..12c7432577e 100644 --- a/Tests/SentryTests/SentryCrash/SentryDebugImageProviderTests.swift +++ b/Tests/SentryTests/SentryCrash/SentryDebugImageProviderTests.swift @@ -19,11 +19,7 @@ class SentryDebugImageProviderTests: XCTestCase { func getSut(images: [SentryCrashBinaryImage] = []) -> SentryDebugImageProvider { cache.start(false) for image in images { - cache.binaryImageAdded(imageName: image.name, - vmAddress: image.vmAddress, - address: image.address, - size: image.size, - uuid: image.uuid) + cache.binaryImageAdded(image: image) } let provider = SentryDebugImageProvider()