fix: Remove symbolication that causes deadlocks

Author: noahsmartin

Sources/Sentry/SentryCrashStackEntryMapper.m

@@ -25,36 +25,15 @@ - (SentryFrame *)sentryCrashStackEntryToSentryFrame:(SentryCrashStackEntry)stack
 {
     SentryFrame *frame = [[SentryFrame alloc] init];
 
-    if (stackEntry.symbolAddress != 0) {
-        frame.symbolAddress = sentry_formatHexAddressUInt64(stackEntry.symbolAddress);
-    }
-
     frame.instructionAddress = sentry_formatHexAddressUInt64(stackEntry.address);
 
-    if (stackEntry.symbolName != NULL) {
-        frame.function = [NSString stringWithCString:stackEntry.symbolName
-                                            encoding:NSUTF8StringEncoding];
-    }
-
-    // If there is no symbolication, because debug was disabled
-    // we get image from the cache.
-    if (stackEntry.imageAddress == 0 && stackEntry.imageName == NULL) {
-        SentryBinaryImageInfo *info = [SentryDependencyContainer.sharedInstance.binaryImageCache
-            imageByAddress:(uint64_t)stackEntry.address];
-
-        frame.imageAddress = sentry_formatHexAddressUInt64(info.address);
-        frame.package = info.name;
-        frame.inApp = @([self.inAppLogic isInApp:info.name]);
-    } else {
-        frame.imageAddress = sentry_formatHexAddressUInt64(stackEntry.imageAddress);
+    // Get image from the cache.
+    SentryBinaryImageInfo *info = [SentryDependencyContainer.sharedInstance.binaryImageCache
+        imageByAddress:(uint64_t)stackEntry.address];
 
-        if (stackEntry.imageName != NULL) {
-            NSString *imageName = [NSString stringWithCString:stackEntry.imageName
-                                                     encoding:NSUTF8StringEncoding];
-            frame.package = imageName;
-            frame.inApp = @([self.inAppLogic isInApp:imageName]);
-        }
-    }
+    frame.imageAddress = sentry_formatHexAddressUInt64(info.address);
+    frame.package = info.name;
+    frame.inApp = @([self.inAppLogic isInApp:info.name]);
 
     return frame;
 }

Sources/Sentry/SentryDefaultThreadInspector.m

@@ -32,7 +32,7 @@ @interface SentryDefaultThreadInspector ()
 // async-signal-safe.
 unsigned int
 getStackEntriesFromThread(SentryCrashThread thread, struct SentryCrashMachineContext *context,
-    SentryCrashStackEntry *buffer, unsigned int maxEntries, bool asyncUnsafeSymbolicate)
+    SentryCrashStackEntry *buffer, unsigned int maxEntries)
 {
     sentrycrashmc_getContextForThread(thread, context, NO);
     SentryCrashStackCursor stackCursor;
@@ -43,10 +43,8 @@ @interface SentryDefaultThreadInspector ()
     while (stackCursor.advanceCursor(&stackCursor)) {
         if (entries == maxEntries)
             break;
-        if (asyncUnsafeSymbolicate == false || stackCursor.symbolicate(&stackCursor)) {
-            buffer[entries] = stackCursor.stackEntry;
-            entries++;
-        }
+        buffer[entries] = stackCursor.stackEntry;
+        entries++;
     }
 
     return entries;
@@ -56,12 +54,10 @@ @implementation SentryDefaultThreadInspector
 
 - (id)initWithStacktraceBuilder:(SentryStacktraceBuilder *)stacktraceBuilder
        andMachineContextWrapper:(id<SentryCrashMachineContextWrapper>)machineContextWrapper
-                    symbolicate:(BOOL)symbolicate
 {
     if (self = [super init]) {
         self.stacktraceBuilder = stacktraceBuilder;
         self.machineContextWrapper = machineContextWrapper;
-        self.symbolicate = symbolicate;
     }
     return self;
 }
@@ -75,13 +71,11 @@ - (instancetype)initWithOptions:(SentryOptions *_Nullable)options
         [[SentryCrashStackEntryMapper alloc] initWithInAppLogic:inAppLogic];
     SentryStacktraceBuilder *stacktraceBuilder =
         [[SentryStacktraceBuilder alloc] initWithCrashStackEntryMapper:crashStackEntryMapper];
-    stacktraceBuilder.symbolicate = options.debug;
 
     id<SentryCrashMachineContextWrapper> machineContextWrapper =
         [[SentryCrashDefaultMachineContextWrapper alloc] init];
     return [self initWithStacktraceBuilder:stacktraceBuilder
-                  andMachineContextWrapper:machineContextWrapper
-                               symbolicate:options.debug];
+                  andMachineContextWrapper:machineContextWrapper];
 }
 
 - (SentryStacktrace *)stacktraceForCurrentThreadAsyncUnsafe
@@ -144,8 +138,6 @@ - (SentryStacktrace *)stacktraceForCurrentThreadAsyncUnsafe
         thread_act_array_t suspendedThreads = NULL;
         mach_msg_type_number_t numSuspendedThreads = 0;
 
-        bool symbolicate = self.symbolicate;
-
         // SentryThreadInspector is crashing when there is too many threads.
         // We add a limit of 70 threads because in test with up to 100 threads it seems fine.
         // We are giving it an extra safety margin.
@@ -165,7 +157,7 @@ - (SentryStacktrace *)stacktraceForCurrentThreadAsyncUnsafe
         for (int i = 0; i < numSuspendedThreads; i++) {
             if (suspendedThreads[i] != currentThread) {
                 int numberOfEntries = getStackEntriesFromThread(suspendedThreads[i], context,
-                    threadsInfos[i].stackEntries, MAX_STACKTRACE_LENGTH, symbolicate);
+                    threadsInfos[i].stackEntries, MAX_STACKTRACE_LENGTH);
                 threadsInfos[i].stackLength = numberOfEntries;
             } else {
                 // We can't use 'getStackEntriesFromThread' to retrieve stack frames from the

Sources/Sentry/SentryStacktraceBuilder.m

@@ -24,7 +24,6 @@ - (id)initWithCrashStackEntryMapper:(SentryCrashStackEntryMapper *)crashStackEnt
 {
     if (self = [super init]) {
         self.crashStackEntryMapper = crashStackEntryMapper;
-        self.symbolicate = NO;
     }
     return self;
 }
@@ -41,10 +40,8 @@ - (SentryStacktrace *)retrieveStacktraceFromCursor:(SentryCrashStackCursor)stack
             // skip the marker frame
             continue;
         }
-        if (self.symbolicate == NO || stackCursor.symbolicate(&stackCursor)) {
-            frame = [self.crashStackEntryMapper mapStackEntryWithCursor:stackCursor];
-            [frames addObject:frame];
-        }
+        frame = [self.crashStackEntryMapper mapStackEntryWithCursor:stackCursor];
+        [frames addObject:frame];
     }
 
     return [SentryStacktraceBuilder buildStacktraceFromFrames:frames];
@@ -96,7 +93,6 @@ - (nullable SentryStacktrace *)buildStacktraceForCurrentThreadAsyncUnsafe
     SENTRY_LOG_DEBUG(@"Building async-unsafe stack trace...");
     SentryCrashStackCursor stackCursor;
     sentrycrashsc_initSelfThread(&stackCursor, 0);
-    stackCursor.symbolicate = sentrycrashsymbolicator_symbolicate_async_unsafe;
     return [self retrieveStacktraceFromCursor:stackCursor];
 }
 

Sources/Sentry/include/SentryDefaultThreadInspector.h

@@ -15,8 +15,7 @@ NS_ASSUME_NONNULL_BEGIN
 SENTRY_NO_INIT
 
 - (id)initWithStacktraceBuilder:(SentryStacktraceBuilder *)stacktraceBuilder
-       andMachineContextWrapper:(id<SentryCrashMachineContextWrapper>)machineContextWrapper
-                    symbolicate:(BOOL)symbolicate;
+       andMachineContextWrapper:(id<SentryCrashMachineContextWrapper>)machineContextWrapper;
 
 - (instancetype)initWithOptions:(SentryOptions *_Nullable)options;
 

Sources/Sentry/include/SentryStacktraceBuilder.h

@@ -15,12 +15,6 @@ NS_ASSUME_NONNULL_BEGIN
 @interface SentryStacktraceBuilder : NSObject
 SENTRY_NO_INIT
 
-/**
- * Whether the stack trace frames should be fully symbolicated
- * or only contain instruction address and binary image.
- */
-@property (nonatomic) BOOL symbolicate;
-
 - (id)initWithCrashStackEntryMapper:(SentryCrashStackEntryMapper *)crashStackEntryMapper;
 
 /**

Sources/SentryCrash/Recording/SentryCrashReport.c

@@ -799,20 +799,6 @@ writeBacktrace(const SentryCrashReportWriter *const writer, const char *const ke
             while (stackCursor->advanceCursor(stackCursor)) {
                 writer->beginObject(writer, NULL);
                 {
-                    if (stackCursor->symbolicate(stackCursor)) {
-                        if (stackCursor->stackEntry.imageName != NULL) {
-                            writer->addStringElement(writer, SentryCrashField_ObjectName,
-                                sentrycrashfu_lastPathEntry(stackCursor->stackEntry.imageName));
-                        }
-                        writer->addUIntegerElement(writer, SentryCrashField_ObjectAddr,
-                            stackCursor->stackEntry.imageAddress);
-                        if (stackCursor->stackEntry.symbolName != NULL) {
-                            writer->addStringElement(writer, SentryCrashField_SymbolName,
-                                stackCursor->stackEntry.symbolName);
-                        }
-                        writer->addUIntegerElement(writer, SentryCrashField_SymbolAddr,
-                            stackCursor->stackEntry.symbolAddress);
-                    }
                     writer->addUIntegerElement(
                         writer, SentryCrashField_InstructionAddr, stackCursor->stackEntry.address);
                 }

Sources/SentryCrash/Recording/Tools/SentryCrashStackCursor.c

@@ -46,17 +46,12 @@ sentrycrashsc_resetCursor(SentryCrashStackCursor *cursor)
     cursor->state.currentDepth = 0;
     cursor->state.hasGivenUp = false;
     cursor->stackEntry.address = 0;
-    cursor->stackEntry.imageAddress = 0;
-    cursor->stackEntry.imageName = NULL;
-    cursor->stackEntry.symbolAddress = 0;
-    cursor->stackEntry.symbolName = NULL;
 }
 
 void
 sentrycrashsc_initCursor(SentryCrashStackCursor *cursor,
     void (*resetCursor)(SentryCrashStackCursor *), bool (*advanceCursor)(SentryCrashStackCursor *))
 {
-    cursor->symbolicate = sentrycrashsymbolicator_symbolicate_async_unsafe_sentryDlAddr;
     cursor->advanceCursor = advanceCursor != NULL ? advanceCursor : g_advanceCursor;
     cursor->resetCursor = resetCursor != NULL ? resetCursor : sentrycrashsc_resetCursor;
     cursor->resetCursor(cursor);

Sources/SentryCrash/Recording/Tools/SentryCrashStackCursor.h

@@ -46,20 +46,6 @@ extern "C" {
 typedef struct {
     /** Current address in the stack trace. */
     uintptr_t address;
-
-    /** The name (if any) of the binary image the current address falls
-     * inside. */
-    const char *imageName;
-
-    /** The starting address of the binary image the current address falls
-     * inside. */
-    uintptr_t imageAddress;
-
-    /** The name (if any) of the closest symbol to the current address. */
-    const char *symbolName;
-
-    /** The address of the closest symbol to the current address. */
-    uintptr_t symbolAddress;
 } SentryCrashStackEntry;
 
 typedef struct SentryCrashStackCursor {
@@ -79,10 +65,6 @@ typedef struct SentryCrashStackCursor {
     /** Advance the cursor to the next stack entry. */
     bool (*advanceCursor)(struct SentryCrashStackCursor *);
 
-    /** Attempt to symbolicate the current address, filling in the fields in
-     * stackEntry. */
-    bool (*symbolicate)(struct SentryCrashStackCursor *);
-
     /** Internal context-specific information. */
     void *context[SentryCrashSC_CONTEXT_SIZE];
 } SentryCrashStackCursor;

Sources/SentryCrash/Recording/Tools/SentryCrashSymbolicator.c

@@ -54,10 +54,7 @@ static bool
 symbolicate_internal(SentryCrashStackCursor *cursor, bool useDlAddr)
 {
     if (cursor->stackEntry.address == SentryCrashSC_ASYNC_MARKER) {
-        cursor->stackEntry.imageAddress = 0;
-        cursor->stackEntry.imageName = 0;
-        cursor->stackEntry.symbolAddress = 0;
-        cursor->stackEntry.symbolName = "__sentrycrash__async_marker__";
+
         return true;
     }
 
@@ -75,18 +72,7 @@ symbolicate_internal(SentryCrashStackCursor *cursor, bool useDlAddr)
             CALL_INSTRUCTION_FROM_RETURN_ADDRESS(cursor->stackEntry.address), &symbolsBuffer);
     }
 
-    if (symbols_succeed) {
-        cursor->stackEntry.imageAddress = (uintptr_t)symbolsBuffer.dli_fbase;
-        cursor->stackEntry.imageName = symbolsBuffer.dli_fname;
-        cursor->stackEntry.symbolAddress = (uintptr_t)symbolsBuffer.dli_saddr;
-        cursor->stackEntry.symbolName = symbolsBuffer.dli_sname;
-        return true;
-    }
 
-    cursor->stackEntry.imageAddress = 0;
-    cursor->stackEntry.imageName = 0;
-    cursor->stackEntry.symbolAddress = 0;
-    cursor->stackEntry.symbolName = 0;
     return false;
 }