Skip to content

Commit

Permalink
[PLAT-7335] Use Function Starts for symbolication (#1214)
Browse files Browse the repository at this point in the history
  • Loading branch information
nickdowell committed Nov 3, 2021
1 parent dc539a1 commit 098e9aa
Show file tree
Hide file tree
Showing 14 changed files with 311 additions and 304 deletions.
42 changes: 20 additions & 22 deletions Bugsnag.xcodeproj/project.pbxproj

Large diffs are not rendered by default.

52 changes: 27 additions & 25 deletions Bugsnag/KSCrash/Source/KSCrash/Recording/BSG_KSCrashReport.c
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,6 @@
#include "BSG_KSBacktrace_Private.h"
#include "BSG_KSCrashReportFields.h"
#include "BSG_KSCrashReportVersion.h"
#include "BSG_KSDynamicLinker.h"
#include "BSG_KSFileUtils.h"
#include "BSG_KSJSONCodec.h"
#include "BSG_KSMach.h"
Expand All @@ -43,7 +42,9 @@
#include "BSG_KSLogger.h"
#include "BSG_KSCrashContext.h"
#include "BSG_KSCrashSentry.h"
#include "BSG_Symbolicate.h"

#include <mach-o/loader.h>
#include <sys/time.h>

#ifdef __arm64__
Expand Down Expand Up @@ -525,27 +526,27 @@ void bsg_kscrw_i_logCrashType(
*
* @param entryNum The backtrace entry number.
*
* @param address The program counter value (instruction address).
* @param address The program counter or return address value.
*
* @param dlInfo Information about the nearest symbols to the address.
* @param info Information about the function that contains the address.
*/
void bsg_kscrw_i_logBacktraceEntry(const int entryNum, const uintptr_t address,
const Dl_info *const dlInfo) {
struct bsg_symbolicate_result *info) {
char faddrBuff[20];
char saddrBuff[20];

const char *fname = bsg_ksfulastPathEntry(dlInfo->dli_fname);
const char *fname = bsg_ksfulastPathEntry(info->image->name);
if (fname == NULL) {
sprintf(faddrBuff, BSG_POINTER_FMT, (uintptr_t)dlInfo->dli_fbase);
sprintf(faddrBuff, BSG_POINTER_FMT, (uintptr_t)info->image->header);
fname = faddrBuff;
}

uintptr_t offset = address - (uintptr_t)dlInfo->dli_saddr;
const char *sname = dlInfo->dli_sname;
uintptr_t offset = address - (uintptr_t)info->function_address;
const char *sname = info->function_name;
if (sname == NULL) {
sprintf(saddrBuff, BSG_POINTER_SHORT_FMT, (uintptr_t)dlInfo->dli_fbase);
sprintf(saddrBuff, BSG_POINTER_SHORT_FMT, (uintptr_t)info->image->header);
sname = saddrBuff;
offset = address - (uintptr_t)dlInfo->dli_fbase;
offset = address - (uintptr_t)info->image->header;
}

BSG_KSLOGBASIC_ALWAYS(BSG_TRACE_FMT, entryNum, fname, address, sname,
Expand All @@ -562,7 +563,7 @@ void bsg_kscrw_i_logBacktrace(const uintptr_t *const backtrace,
const int backtraceLength,
const int skippedEntries) {
if (backtraceLength > 0) {
Dl_info symbolicated[backtraceLength];
struct bsg_symbolicate_result symbolicated[backtraceLength];
bsg_ksbt_symbolicate(backtrace, symbolicated, backtraceLength,
skippedEntries);

Expand Down Expand Up @@ -726,23 +727,24 @@ void bsg_kscrw_i_writeAddressReferencedByString(
*/
void bsg_kscrw_i_writeBacktraceEntry(
const BSG_KSCrashReportWriter *const writer, const char *const key,
const uintptr_t address, const Dl_info *const info) {
const uintptr_t address, struct bsg_symbolicate_result *info) {
writer->beginObject(writer, key);
{
if (info->dli_saddr != NULL) {
if (info->dli_fname != NULL) {
writer->addStringElement(writer, BSG_KSCrashField_ObjectName,
bsg_ksfulastPathEntry(info->dli_fname));
}
if (info->image && info->image->header) {
writer->addUIntegerElement(writer, BSG_KSCrashField_ObjectAddr,
(uintptr_t)info->dli_fbase);
if (info->dli_sname != NULL) {
const char *sname = info->dli_sname;
writer->addStringElement(writer, BSG_KSCrashField_SymbolName,
sname);
}
(uintptr_t)info->image->header);
}
if (info->image && info->image->name) {
writer->addStringElement(writer, BSG_KSCrashField_ObjectName,
bsg_ksfulastPathEntry(info->image->name));
}
if (info->function_address) {
writer->addUIntegerElement(writer, BSG_KSCrashField_SymbolAddr,
(uintptr_t)info->dli_saddr);
info->function_address);
}
if (info->function_name) {
writer->addStringElement(writer, BSG_KSCrashField_SymbolName,
info->function_name);
}
writer->addUIntegerElement(writer, BSG_KSCrashField_InstructionAddr,
address);
Expand Down Expand Up @@ -773,7 +775,7 @@ void bsg_kscrw_i_writeBacktrace(const BSG_KSCrashReportWriter *const writer,
writer->beginArray(writer, BSG_KSCrashField_Contents);
{
if (backtraceLength > 0) {
Dl_info symbolicated[backtraceLength];
struct bsg_symbolicate_result symbolicated[backtraceLength];
bsg_ksbt_symbolicate(backtrace, symbolicated, backtraceLength,
skippedEntries);

Expand Down
3 changes: 2 additions & 1 deletion Bugsnag/KSCrash/Source/KSCrash/Recording/BSG_KSSystemInfo.m
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@

#import "BSG_KSSystemInfo.h"
#import "BSG_KSSystemInfoC.h"
#import "BSG_KSDynamicLinker.h"
#import "BSG_KSMachHeaders.h"
#import "BSG_KSJSONCodecObjC.h"
#import "BSG_KSMach.h"
Expand All @@ -41,6 +40,8 @@
#import "BSG_KSCrash.h"

#import <CommonCrypto/CommonDigest.h>
#import <mach-o/dyld.h>

#if BSG_PLATFORM_IOS || BSG_PLATFORM_TVOS
#import "BSGUIKit.h"
#endif
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,12 +36,6 @@ extern "C" {

#include <sys/_structs.h>

#ifdef __LP64__
#define BSG_STRUCT_NLIST struct nlist_64
#else
#define BSG_STRUCT_NLIST struct nlist
#endif

#ifdef __arm64__
#define BSG_STRUCT_MCONTEXT_L _STRUCT_MCONTEXT64
#else
Expand Down
31 changes: 3 additions & 28 deletions Bugsnag/KSCrash/Source/KSCrash/Recording/Tools/BSG_KSBacktrace.c
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@

#include "BSG_KSBacktrace_Private.h"

#include "BSG_KSDynamicLinker.h"
#include "BSG_KSMach.h"

/**
Expand All @@ -38,30 +37,6 @@
#define BSG_PACStrippingMaskArm64e 0x0000000fffffffff
#endif

/** Remove any pointer tagging from an instruction address
* On armv7 the least significant bit of the pointer distinguishes
* between thumb mode (2-byte instructions) and normal mode (4-byte
* instructions). On arm64 all instructions are 4-bytes wide so the two least
* significant bytes should always be 0. On x86_64 and i386, instructions are
* variable length so all bits are signficant.
*/
#if defined(__arm__)
#define DETAG_INSTRUCTION_ADDRESS(A) ((A) & ~(1UL))
#elif defined(__arm64__)
#define DETAG_INSTRUCTION_ADDRESS(A) ((A) & ~(3UL))
#else
#define DETAG_INSTRUCTION_ADDRESS(A) (A)
#endif

/** Step backwards by one instruction.
* The backtrace of an objective-C program is expected to contain return
* addresses not call instructions, as that is what can easily be read from
* the stack. This is not a problem except for a few cases where the return
* address is inside a different symbol than the call address.
*/
#define CALL_INSTRUCTION_FROM_RETURN_ADDRESS(A) \
(DETAG_INSTRUCTION_ADDRESS((A)) - 1)

/** Represents an entry in a frame list.
* This is modeled after the various i386/x64 frame walkers in the xnu source,
* and seems to work fine in ARM as well. I haven't included the args pointer
Expand Down Expand Up @@ -225,17 +200,17 @@ int bsg_ksbt_backtraceSelf(uintptr_t *const backtraceBuffer,
}

void bsg_ksbt_symbolicate(const uintptr_t *const backtraceBuffer,
Dl_info *const symbolsBuffer, const int numEntries,
struct bsg_symbolicate_result *symbolsBuffer, const int numEntries,
const int skippedEntries) {
int i = 0;

if (!skippedEntries && i < numEntries) {
bsg_ksdldladdr(backtraceBuffer[i], &symbolsBuffer[i]);
bsg_symbolicate(backtraceBuffer[i], &symbolsBuffer[i]);
i++;
}

for (; i < numEntries; i++) {
bsg_ksdldladdr(CALL_INSTRUCTION_FROM_RETURN_ADDRESS(backtraceBuffer[i]),
bsg_symbolicate(CALL_INSTRUCTION_FROM_RETURN_ADDRESS(backtraceBuffer[i]),
&symbolsBuffer[i]);
}
}
33 changes: 31 additions & 2 deletions Bugsnag/KSCrash/Source/KSCrash/Recording/Tools/BSG_KSBacktrace.h
Original file line number Diff line number Diff line change
Expand Up @@ -34,10 +34,39 @@
extern "C" {
#endif

#include <dlfcn.h>
#include "BSG_Symbolicate.h"

#include <mach/mach.h>
#include <pthread.h>

/**
* Remove any pointer tagging from an instruction address
*
* On armv7 the least significant bit of the pointer distinguishes
* between thumb mode (2-byte instructions) and normal mode (4-byte
* instructions). On arm64 all instructions are 4-bytes wide so the two least
* significant bytes should always be 0. On x86_64 and i386, instructions are
* variable length so all bits are signficant.
*/
#if defined(__arm__)
#define DETAG_INSTRUCTION_ADDRESS(A) ((A) & ~(1UL))
#elif defined(__arm64__)
#define DETAG_INSTRUCTION_ADDRESS(A) ((A) & ~(3UL))
#else
#define DETAG_INSTRUCTION_ADDRESS(A) (A)
#endif

/**
* Step backwards by one instruction.
*
* The backtrace of an objective-C program is expected to contain return
* addresses not call instructions, as that is what can easily be read from
* the stack. This is not a problem except for a few cases where the return
* address is inside a different symbol than the call address.
*/
#define CALL_INSTRUCTION_FROM_RETURN_ADDRESS(A) \
(DETAG_INSTRUCTION_ADDRESS((A)) - 1)

/** Generate a backtrace on the specified mach thread (async-safe).
*
*
Expand Down Expand Up @@ -98,7 +127,7 @@ int bsg_ksbt_backtraceSelf(uintptr_t *backtraceBuffer, int maxEntries);
* backtrace.
*/
void bsg_ksbt_symbolicate(const uintptr_t *backtraceBuffer,
Dl_info *symbolsBuffer, int numEntries,
struct bsg_symbolicate_result *symbolsBuffer, int numEntries,
int skippedEntries);

#ifdef __cplusplus
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ extern "C" {
#include "BSG_KSArchSpecific.h"
#include "BSG_KSBacktrace.h"

#include <stdbool.h>
#include <sys/ucontext.h>

/** Point at which bsg_ksbt_backtraceLength() will give up trying to count.
Expand Down
109 changes: 0 additions & 109 deletions Bugsnag/KSCrash/Source/KSCrash/Recording/Tools/BSG_KSDynamicLinker.c

This file was deleted.

Loading

0 comments on commit 098e9aa

Please sign in to comment.