Skip to content

Commit

Permalink
fix: Cache binary images to be used for crashes (#2939)
Browse files Browse the repository at this point in the history
During crashes we use 2 async-signal-unsafe functions _dyld_get_image_header and _dyld_get_image_name.
This may lead to a crash report not being properly saved to disk.

This proposal will cache binary images during runtime and use it when the app crashes.

Co-authored-by: Andrew McKnight <andrew.mcknight@sentry.io>
Co-authored-by: Philipp Hofmann <philipp.hofmann@sentry.io>
  • Loading branch information
3 people committed Jun 2, 2023
1 parent 79a9b4f commit 6943de0
Show file tree
Hide file tree
Showing 15 changed files with 591 additions and 33 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
### Features

- Experimental support for Swift Async stacktraces (#3051)
- Cache binary images to be used for crashes (#2939)

### Fixes

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,8 +32,8 @@
</TestAction>
<LaunchAction
buildConfiguration = "Debug"
selectedDebuggerIdentifier = "Xcode.DebuggerFoundation.Debugger.LLDB"
selectedLauncherIdentifier = "Xcode.DebuggerFoundation.Launcher.LLDB"
selectedDebuggerIdentifier = ""
selectedLauncherIdentifier = "Xcode.IDEFoundation.Launcher.PosixSpawn"
launchStyle = "0"
useCustomWorkingDirectory = "NO"
ignoresPersistentStateOnLaunch = "NO"
Expand Down
2 changes: 1 addition & 1 deletion Samples/macOS-Swift/macOS-Swift/Info.plist
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@
<key>NSMainStoryboardFile</key>
<string>Main</string>
<key>NSPrincipalClass</key>
<string>SentryCrashExceptionApplication</string>
<string>NSApplication</string>
<key>NSSupportsAutomaticTermination</key>
<true/>
<key>NSSupportsSuddenTermination</key>
Expand Down
12 changes: 12 additions & 0 deletions Sentry.xcodeproj/project.pbxproj
Original file line number Diff line number Diff line change
Expand Up @@ -757,6 +757,8 @@
D85D3BEA278DF63D001B2889 /* SentryByteCountFormatterTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = D85D3BE9278DF63D001B2889 /* SentryByteCountFormatterTests.swift */; };
D8603DD6284F8497000E1227 /* SentryBaggage.m in Sources */ = {isa = PBXBuildFile; fileRef = D8603DD4284F8497000E1227 /* SentryBaggage.m */; };
D8603DD8284F894C000E1227 /* SentryBaggage.h in Headers */ = {isa = PBXBuildFile; fileRef = D8603DD7284F894C000E1227 /* SentryBaggage.h */; };
D865892F29D6ECA7000BE151 /* SentryCrashBinaryImageCache.h in Headers */ = {isa = PBXBuildFile; fileRef = D865892D29D6ECA7000BE151 /* SentryCrashBinaryImageCache.h */; };
D865893029D6ECA7000BE151 /* SentryCrashBinaryImageCache.c in Sources */ = {isa = PBXBuildFile; fileRef = D865892E29D6ECA7000BE151 /* SentryCrashBinaryImageCache.c */; };
D867063D27C3BC2400048851 /* SentryCoreDataTrackingIntegration.h in Headers */ = {isa = PBXBuildFile; fileRef = D867063A27C3BC2400048851 /* SentryCoreDataTrackingIntegration.h */; };
D867063E27C3BC2400048851 /* SentryCoreDataSwizzling.h in Headers */ = {isa = PBXBuildFile; fileRef = D867063B27C3BC2400048851 /* SentryCoreDataSwizzling.h */; };
D867063F27C3BC2400048851 /* SentryCoreDataTracker.h in Headers */ = {isa = PBXBuildFile; fileRef = D867063C27C3BC2400048851 /* SentryCoreDataTracker.h */; };
Expand Down Expand Up @@ -797,6 +799,7 @@
D8CB741B2947286500A5F964 /* SentryEnvelopeItemHeader.m in Sources */ = {isa = PBXBuildFile; fileRef = D8CB741A2947286500A5F964 /* SentryEnvelopeItemHeader.m */; };
D8CB742B294B1DD000A5F964 /* SentryUIApplicationTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = D8CB742A294B1DD000A5F964 /* SentryUIApplicationTests.swift */; };
D8CB742E294B294B00A5F964 /* MockUIScene.m in Sources */ = {isa = PBXBuildFile; fileRef = D8CB742D294B294B00A5F964 /* MockUIScene.m */; };
D8CCFC632A1520C900DE232E /* SentryBinaryImageCacheTests.m in Sources */ = {isa = PBXBuildFile; fileRef = D8CCFC622A1520C900DE232E /* SentryBinaryImageCacheTests.m */; };
D8CE69BC277E39C700C6EC5C /* SentryFileIOTrackingIntegrationObjCTests.m in Sources */ = {isa = PBXBuildFile; fileRef = D8CE69BB277E39C700C6EC5C /* SentryFileIOTrackingIntegrationObjCTests.m */; };
D8F6A2472885512100320515 /* SentryPredicateDescriptor.m in Sources */ = {isa = PBXBuildFile; fileRef = D8F6A2452885512100320515 /* SentryPredicateDescriptor.m */; };
D8F6A24B2885515C00320515 /* SentryPredicateDescriptor.h in Headers */ = {isa = PBXBuildFile; fileRef = D8F6A24A2885515B00320515 /* SentryPredicateDescriptor.h */; };
Expand Down Expand Up @@ -1688,6 +1691,8 @@
D85D3BE9278DF63D001B2889 /* SentryByteCountFormatterTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = SentryByteCountFormatterTests.swift; sourceTree = "<group>"; };
D8603DD4284F8497000E1227 /* SentryBaggage.m */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.objc; path = SentryBaggage.m; sourceTree = "<group>"; };
D8603DD7284F894C000E1227 /* SentryBaggage.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; name = SentryBaggage.h; path = include/SentryBaggage.h; sourceTree = "<group>"; };
D865892D29D6ECA7000BE151 /* SentryCrashBinaryImageCache.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; path = SentryCrashBinaryImageCache.h; sourceTree = "<group>"; };
D865892E29D6ECA7000BE151 /* SentryCrashBinaryImageCache.c */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.c; path = SentryCrashBinaryImageCache.c; sourceTree = "<group>"; };
D867063A27C3BC2400048851 /* SentryCoreDataTrackingIntegration.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; name = SentryCoreDataTrackingIntegration.h; path = include/SentryCoreDataTrackingIntegration.h; sourceTree = "<group>"; };
D867063B27C3BC2400048851 /* SentryCoreDataSwizzling.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; name = SentryCoreDataSwizzling.h; path = include/SentryCoreDataSwizzling.h; sourceTree = "<group>"; };
D867063C27C3BC2400048851 /* SentryCoreDataTracker.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; name = SentryCoreDataTracker.h; path = include/SentryCoreDataTracker.h; sourceTree = "<group>"; };
Expand Down Expand Up @@ -1731,6 +1736,7 @@
D8CB742A294B1DD000A5F964 /* SentryUIApplicationTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = SentryUIApplicationTests.swift; sourceTree = "<group>"; };
D8CB742C294B294B00A5F964 /* MockUIScene.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; path = MockUIScene.h; sourceTree = "<group>"; };
D8CB742D294B294B00A5F964 /* MockUIScene.m */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.objc; path = MockUIScene.m; sourceTree = "<group>"; };
D8CCFC622A1520C900DE232E /* SentryBinaryImageCacheTests.m */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.objc; path = SentryBinaryImageCacheTests.m; sourceTree = "<group>"; };
D8CE69BB277E39C700C6EC5C /* SentryFileIOTrackingIntegrationObjCTests.m */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.objc; path = SentryFileIOTrackingIntegrationObjCTests.m; sourceTree = "<group>"; };
D8F01DE42A126B62008F4996 /* HybridPod.podspec */ = {isa = PBXFileReference; explicitFileType = text.script.ruby; path = HybridPod.podspec; sourceTree = "<group>"; };
D8F01DE52A126BF5008F4996 /* HybridTest.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = HybridTest.swift; sourceTree = "<group>"; };
Expand Down Expand Up @@ -2358,6 +2364,8 @@
63FE704720DA4C1000CDBAE8 /* SentryCrashDoctor.m */,
63FE704C20DA4C1000CDBAE8 /* SentryCrashReport.h */,
63FE704420DA4C1000CDBAE8 /* SentryCrashReport.c */,
D865892D29D6ECA7000BE151 /* SentryCrashBinaryImageCache.h */,
D865892E29D6ECA7000BE151 /* SentryCrashBinaryImageCache.c */,
63FE704620DA4C1000CDBAE8 /* SentryCrashReportFields.h */,
63FE704820DA4C1000CDBAE8 /* SentryCrashReportFixer.h */,
63FE6FEA20DA4C1000CDBAE8 /* SentryCrashReportFixer.c */,
Expand Down Expand Up @@ -2473,6 +2481,7 @@
isa = PBXGroup;
children = (
63FE71F120DA66EA00CDBAE8 /* Container+DeepSearch_Tests.m */,
D8CCFC622A1520C900DE232E /* SentryBinaryImageCacheTests.m */,
63FE71F620DA66EB00CDBAE8 /* FileBasedTestCase.h */,
63FE71D920DA66E700CDBAE8 /* FileBasedTestCase.m */,
7B7725D7292F5DC20015BBF9 /* SentryCrashInstallationTests.m */,
Expand Down Expand Up @@ -3550,6 +3559,7 @@
63FE710920DA4C1000CDBAE8 /* SentryCrashFileUtils.h in Headers */,
03F84D1F27DD414C008FE43F /* SentryAsyncSafeLogging.h in Headers */,
7BE3C76B2445C27A00A38442 /* SentryCurrentDateProvider.h in Headers */,
D865892F29D6ECA7000BE151 /* SentryCrashBinaryImageCache.h in Headers */,
6344DDB41EC309E000D9160D /* SentryCrashReportSink.h in Headers */,
7D427C62237B1D200046BAC8 /* SentrySDK.h in Headers */,
D867063F27C3BC2400048851 /* SentryCoreDataTracker.h in Headers */,
Expand Down Expand Up @@ -4090,6 +4100,7 @@
8EAE9806261E87120073B6B3 /* SentryUIViewControllerPerformanceTracker.m in Sources */,
D88817D826D7149100BF2251 /* SentryTraceContext.m in Sources */,
8EBF870926140D37001A6853 /* SentryPerformanceTracker.m in Sources */,
D865893029D6ECA7000BE151 /* SentryCrashBinaryImageCache.c in Sources */,
7BC9A20428F4166D001E7C4C /* SentryMeasurementValue.m in Sources */,
D859696B27BECD8F0036A46E /* SentryCoreDataTrackingIntegration.m in Sources */,
7BD86EC7264A641D005439DB /* SentrySysctl.m in Sources */,
Expand Down Expand Up @@ -4244,6 +4255,7 @@
7B984A9F28E572AF001F4BEE /* CrashReport.swift in Sources */,
7BED3576266F7BFF00EAA70D /* TestSentryCrashWrapper.m in Sources */,
7BC6EC18255C44540059822A /* SentryDebugMetaTests.swift in Sources */,
D8CCFC632A1520C900DE232E /* SentryBinaryImageCacheTests.m in Sources */,
A811D867248E2770008A41EA /* SentrySystemEventBreadcrumbsTest.swift in Sources */,
7B7725D8292F5DC20015BBF9 /* SentryCrashInstallationTests.m in Sources */,
7B82D54924E2A2D400EE670F /* SentryIdTests.swift in Sources */,
Expand Down
3 changes: 3 additions & 0 deletions Sources/Sentry/SentryCrashIntegration.m
Original file line number Diff line number Diff line change
Expand Up @@ -140,6 +140,7 @@ - (void)startCrashHandler
[SentryCrashIntegration sendAllSentryCrashReports];
}
};
[self.crashAdapter startBinaryImageCache];
[self.dispatchQueueWrapper dispatchOnce:&installationToken block:block];
}

Expand All @@ -158,6 +159,8 @@ - (void)uninstall
installationToken = 0;
}

[self.crashAdapter stopBinaryImageCache];

[NSNotificationCenter.defaultCenter removeObserver:self
name:NSCurrentLocaleDidChangeNotification
object:nil];
Expand Down
11 changes: 11 additions & 0 deletions Sources/Sentry/SentryCrashWrapper.m
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
#import "SentryCrashWrapper.h"
#import "SentryCrash.h"
#import "SentryCrashBinaryImageCache.h"
#import "SentryCrashMonitor_AppState.h"
#import "SentryCrashMonitor_System.h"
#import <Foundation/Foundation.h>
Expand Down Expand Up @@ -80,6 +81,16 @@ - (bytes)appMemorySize
return 0;
}

- (void)startBinaryImageCache
{
sentrycrashbic_startCache();
}

- (void)stopBinaryImageCache
{
sentrycrashbic_stopCache();
}

@end

NS_ASSUME_NONNULL_END
4 changes: 4 additions & 0 deletions Sources/Sentry/include/SentryCrashWrapper.h
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,10 @@ SENTRY_NO_INIT

+ (instancetype)sharedInstance;

- (void)startBinaryImageCache;

- (void)stopBinaryImageCache;

- (BOOL)crashedLastLaunch;

- (NSTimeInterval)durationFromCrashStateInitToLastCrash;
Expand Down
180 changes: 180 additions & 0 deletions Sources/SentryCrash/Recording/SentryCrashBinaryImageCache.c
Original file line number Diff line number Diff line change
@@ -0,0 +1,180 @@
#include "SentryCrashBinaryImageCache.h"
#include "SentryCrashDynamicLinker.h"
#include <mach-o/dyld.h>
#include <pthread.h>
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <unistd.h>

#if TEST || TESTCI

typedef void (*SentryRegisterImageCallback)(const struct mach_header *mh, intptr_t vmaddr_slide);
typedef void (*SentryRegisterFunction)(SentryRegisterImageCallback function);

static SentryRegisterFunction _sentry_register_func_for_add_image
= &_dyld_register_func_for_add_image;
static SentryRegisterFunction _sentry_register_func_for_remove_image
= &_dyld_register_func_for_remove_image;

static void (*SentryWillAddImageCallback)(void) = NULL;

void
sentry_setRegisterFuncForAddImage(SentryRegisterFunction addFunction)
{
_sentry_register_func_for_add_image = addFunction;
}

void
sentry_setRegisterFuncForRemoveImage(SentryRegisterFunction removeFunction)
{
_sentry_register_func_for_remove_image = removeFunction;
}

void
sentry_setFuncForBeforeAdd(void (*callback)(void))
{
SentryWillAddImageCallback = callback;
}

void
sentry_resetFuncForAddRemoveImage(void)
{
_sentry_register_func_for_add_image = &_dyld_register_func_for_add_image;
_sentry_register_func_for_remove_image = &_dyld_register_func_for_remove_image;
}

# define sentry_dyld_register_func_for_add_image(CALLBACK) \
_sentry_register_func_for_add_image(CALLBACK);
# define sentry_dyld_register_func_for_remove_image(CALLBACK) \
_sentry_register_func_for_remove_image(CALLBACK);
# define _will_add_image() \
if (SentryWillAddImageCallback) \
SentryWillAddImageCallback();
#else
# define sentry_dyld_register_func_for_add_image(CALLBACK) \
_dyld_register_func_for_add_image(CALLBACK)
# define sentry_dyld_register_func_for_remove_image(CALLBACK) \
_dyld_register_func_for_remove_image(CALLBACK)
# define _will_add_image()
#endif

typedef struct SentryCrashBinaryImageNode {
SentryCrashBinaryImage image;
bool available;
struct SentryCrashBinaryImageNode *next;
} SentryCrashBinaryImageNode;

static SentryCrashBinaryImageNode rootNode = { 0 };
static SentryCrashBinaryImageNode *tailNode = NULL;
static pthread_mutex_t binaryImagesMutex = PTHREAD_MUTEX_INITIALIZER;

static void
binaryImageAdded(const struct mach_header *header, intptr_t slide)
{
if (tailNode == NULL) {
return;
}

Dl_info info;
if (!dladdr(header, &info) || info.dli_fname == NULL) {
return;
}

SentryCrashBinaryImage binaryImage = { 0 };
if (!sentrycrashdl_getBinaryImageForHeader(
(const void *)header, info.dli_fname, &binaryImage, false)) {
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);
}
pthread_mutex_unlock(&binaryImagesMutex);
}

static void
binaryImageRemoved(const struct mach_header *header, intptr_t slide)
{
SentryCrashBinaryImageNode *nextNode = &rootNode;

while (nextNode != NULL) {
if (nextNode->image.address == (uint64_t)header) {
nextNode->available = false;
break;
}
nextNode = nextNode->next;
}
}

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);
}
nextNode = nextNode->next;
}
}

void
sentrycrashbic_startCache(void)
{
pthread_mutex_lock(&binaryImagesMutex);
if (tailNode != NULL) {
// Already initialized
pthread_mutex_unlock(&binaryImagesMutex);
return;
}
tailNode = &rootNode;
rootNode.next = NULL;
pthread_mutex_unlock(&binaryImagesMutex);

// 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);
}

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);
}
22 changes: 22 additions & 0 deletions Sources/SentryCrash/Recording/SentryCrashBinaryImageCache.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
#ifndef SentryCrashBinaryImageCache_h
#define SentryCrashBinaryImageCache_h

#include "SentryCrashDynamicLinker.h"
#include <stdio.h>

typedef void (*sentrycrashbic_imageIteratorCallback)(SentryCrashBinaryImage *, void *context);

void sentrycrashbic_iterateOverImages(sentrycrashbic_imageIteratorCallback index, void *context);

/**
* Starts the cache that will monitor binary image being loaded or removed.
*/
void sentrycrashbic_startCache(void);

/**
* Stops the cache from monitoring binary image being loaded or removed.
* This will also clean the cache.
*/
void sentrycrashbic_stopCache(void);

#endif /* SentryCrashBinaryImageCache_h */

0 comments on commit 6943de0

Please sign in to comment.