From 0bf2b08f252a0bca34e5075557216034d7c2d0e6 Mon Sep 17 00:00:00 2001 From: Nikolay Volosatov Date: Sun, 3 Nov 2024 12:56:21 +0000 Subject: [PATCH 1/6] Remove KSCrash symbols from user-reported exceptions --- .../IntegrationTestRunner.swift | 12 +- .../UserReportConfig.swift | 108 +++++++++++++----- Samples/Tests/Core/IntegrationTestBase.swift | 7 +- Samples/Tests/IntegrationTests.swift | 75 ++++++++++-- .../KSCrashCore/include/KSCompilerDefines.h | 39 +++++++ Sources/KSCrashRecording/KSCrash.m | 7 +- Sources/KSCrashRecording/KSCrashC.c | 5 +- .../Monitors/KSCrashMonitor_CPPException.cpp | 10 +- .../Monitors/KSCrashMonitor_NSException.m | 32 +++++- .../Monitors/KSCrashMonitor_User.c | 7 +- .../KSStackCursor_SelfThread.c | 5 +- 11 files changed, 239 insertions(+), 68 deletions(-) create mode 100644 Sources/KSCrashCore/include/KSCompilerDefines.h diff --git a/Samples/Common/Sources/IntegrationTestsHelper/IntegrationTestRunner.swift b/Samples/Common/Sources/IntegrationTestsHelper/IntegrationTestRunner.swift index eb8d02c2..535ca784 100644 --- a/Samples/Common/Sources/IntegrationTestsHelper/IntegrationTestRunner.swift +++ b/Samples/Common/Sources/IntegrationTestsHelper/IntegrationTestRunner.swift @@ -40,7 +40,7 @@ public final class IntegrationTestRunner { private struct Script: Codable { var install: InstallConfig? - var userReports: [UserReportConfig]? + var userReport: UserReportConfig? var crashTrigger: CrashTriggerConfig? var report: ReportConfig? @@ -72,10 +72,8 @@ public final class IntegrationTestRunner { if let crashTrigger = script.crashTrigger { crashTrigger.crash() } - if let userReports = script.userReports { - for report in userReports { - report.report() - } + if let userReport = script.userReport { + userReport.report() } if let report = script.report { report.report() @@ -94,8 +92,8 @@ public extension IntegrationTestRunner { return data.base64EncodedString() } - static func script(userReports: [UserReportConfig], install: InstallConfig? = nil, config: RunConfig? = nil) throws -> String { - let data = try JSONEncoder().encode(Script(install: install, userReports: userReports, config: config)) + static func script(userReport: UserReportConfig, install: InstallConfig? = nil, config: RunConfig? = nil) throws -> String { + let data = try JSONEncoder().encode(Script(install: install, userReport: userReport, config: config)) return data.base64EncodedString() } diff --git a/Samples/Common/Sources/IntegrationTestsHelper/UserReportConfig.swift b/Samples/Common/Sources/IntegrationTestsHelper/UserReportConfig.swift index 32d2b296..2af6d640 100644 --- a/Samples/Common/Sources/IntegrationTestsHelper/UserReportConfig.swift +++ b/Samples/Common/Sources/IntegrationTestsHelper/UserReportConfig.swift @@ -29,46 +29,92 @@ import CrashTriggers import KSCrashRecording public struct UserReportConfig: Codable { - public enum ReportType: String, Codable { - case userException - case nsException + public struct UserException: Codable { + public var name: String + public var reason: String? + public var language: String? + public var lineOfCode: String? + public var stacktrace: [String]? + public var logAllThreads: Bool + public var terminateProgram: Bool + + public init(name: String, + reason: String? = nil, + language: String? = nil, + lineOfCode: String? = nil, + stacktrace: [String]? = nil, + logAllThreads: Bool = false, + terminateProgram: Bool = false) { + self.name = name + self.reason = reason + self.language = language + self.lineOfCode = lineOfCode + self.stacktrace = stacktrace + self.logAllThreads = logAllThreads + self.terminateProgram = terminateProgram + } } - public var reportType: ReportType + public struct NSExceptionReport: Codable { + public var name: String + public var reason: String? + public var userInfo: [String : String]? + public var logAllThreads: Bool + public var addStacktrace: Bool - public init(reportType: ReportType) { - self.reportType = reportType + public init(name: String, + reason: String? = nil, + userInfo: [String : String]? = nil, + logAllThreads: Bool = false, + addStacktrace: Bool = false) { + self.name = name + self.reason = reason + self.userInfo = userInfo + self.logAllThreads = logAllThreads + self.addStacktrace = addStacktrace + } + } + + public var userException: UserException? + public var nsException: NSExceptionReport? + + public init(userException: UserException? = nil, nsException: NSExceptionReport? = nil) { + self.userException = userException + self.nsException = nsException } } extension UserReportConfig { - public static let crashName = "Crash Name" - public static let crashReason = "Crash Reason" - public static let crashLanguage = "Crash Language" - public static let crashLineOfCode = "108" - public static let crashCustomStacktrace = ["func01", "func02", "func03"] + func report() { + userException?.report() + nsException?.report() + } +} + +extension UserReportConfig.UserException { + func report() { + KSCrash.shared.reportUserException( + name, + reason: reason, + language: language, + lineOfCode: lineOfCode, + stackTrace: stacktrace, + logAllThreads: logAllThreads, + terminateProgram: terminateProgram + ) + } +} +extension UserReportConfig.NSExceptionReport { func report() { - switch reportType { - case .userException: - KSCrash.shared.reportUserException( - Self.crashName, - reason: Self.crashReason, - language: Self.crashLanguage, - lineOfCode: Self.crashLineOfCode, - stackTrace: Self.crashCustomStacktrace, - logAllThreads: true, - terminateProgram: false - ) - case .nsException: - KSCrash.shared.report( - CrashTriggersHelper.exceptionWithStacktrace(for: NSException( - name: .init(rawValue:Self.crashName), - reason: Self.crashReason, - userInfo: ["a":"b"] - )), - logAllThreads: true - ) + var exception = NSException( + name: .init(rawValue:name), + reason: reason, + userInfo: userInfo + ) + if addStacktrace { + exception = CrashTriggersHelper.exceptionWithStacktrace(for: exception) } + KSCrash.shared.report(exception, logAllThreads: logAllThreads) } } diff --git a/Samples/Tests/Core/IntegrationTestBase.swift b/Samples/Tests/Core/IntegrationTestBase.swift index 79e189c8..1c62858e 100644 --- a/Samples/Tests/Core/IntegrationTestBase.swift +++ b/Samples/Tests/Core/IntegrationTestBase.swift @@ -211,11 +211,14 @@ class IntegrationTestBase: XCTestCase { waitForCrash() } - func launchAndMakeUserReports(_ reportTypes: [UserReportConfig.ReportType], installOverride: ((inout InstallConfig) throws -> Void)? = nil) throws { + func launchAndMakeUserReport( + userException: UserReportConfig.UserException? = nil, + nsException: UserReportConfig.NSExceptionReport? = nil, + installOverride: ((inout InstallConfig) throws -> Void)? = nil) throws { var installConfig = InstallConfig(installPath: installUrl.path) try installOverride?(&installConfig) app.launchEnvironment[IntegrationTestRunner.envKey] = try IntegrationTestRunner.script( - userReports: reportTypes.map(UserReportConfig.init(reportType:)), + userReport: .init(userException: userException, nsException: nsException), install: installConfig, config: runConfig ) diff --git a/Samples/Tests/IntegrationTests.swift b/Samples/Tests/IntegrationTests.swift index da4dd3cc..2b2483c7 100644 --- a/Samples/Tests/IntegrationTests.swift +++ b/Samples/Tests/IntegrationTests.swift @@ -28,6 +28,7 @@ import XCTest import SampleUI import CrashTriggers import IntegrationTestsHelper +import KSCrashDemangleFilter final class NSExceptionTests: IntegrationTestBase { func testGenericException() throws { @@ -127,15 +128,30 @@ final class OtherTests: IntegrationTestBase { let appleReport = try launchAndReportCrash() XCTAssertTrue(appleReport.contains(KSCrashStacktraceCheckFuncName)) } +} + +final class UserReportedTests: IntegrationTestBase { + + static let crashName = "Crash Name" + static let crashReason = "Crash Reason" + static let crashLanguage = "Crash Language" + static let crashLineOfCode = "108" + static let crashCustomStacktrace = ["func01", "func02", "func03"] func testUserReportedNSException() throws { - try launchAndMakeUserReports([.nsException]) + try launchAndMakeUserReport(nsException: .init( + name: Self.crashName, + reason: Self.crashReason, + userInfo: ["a": "b"], + logAllThreads: true, + addStacktrace: true + )) let rawReport = try readPartialCrashReport() try rawReport.validate() XCTAssertEqual(rawReport.crash?.error?.type, "nsexception") - XCTAssertEqual(rawReport.crash?.error?.reason, UserReportConfig.crashReason) - XCTAssertEqual(rawReport.crash?.error?.nsexception?.name, UserReportConfig.crashName) + XCTAssertEqual(rawReport.crash?.error?.reason, Self.crashReason) + XCTAssertEqual(rawReport.crash?.error?.nsexception?.name, Self.crashName) XCTAssertTrue(rawReport.crash?.error?.nsexception?.userInfo?.contains("a = b") ?? false) XCTAssertGreaterThanOrEqual(rawReport.crash?.threads?.count ?? 0, 2, "Expected to have at least 2 threads") let backtraceFrame = rawReport.crashedThread?.backtrace.contents.first(where: { @@ -147,31 +163,68 @@ final class OtherTests: IntegrationTestBase { app.terminate() let appleReport = try launchAndReportCrash() - XCTAssertTrue(appleReport.contains(UserReportConfig.crashName)) - XCTAssertTrue(appleReport.contains(UserReportConfig.crashReason)) + XCTAssertTrue(appleReport.contains(Self.crashName)) + XCTAssertTrue(appleReport.contains(Self.crashReason)) XCTAssertTrue(appleReport.contains(KSCrashNSExceptionStacktraceFuncName)) let state = try readState() XCTAssertFalse(state.crashedLastLaunch) } + func testUserReportedNSException_WithoutStacktrace() throws { + try launchAndMakeUserReport(nsException: .init( + name: Self.crashName, + reason: Self.crashReason, + userInfo: nil, + logAllThreads: true, + addStacktrace: false // <- Key difference + )) + + let rawReport = try readPartialCrashReport() + try rawReport.validate() + XCTAssertEqual(rawReport.crash?.error?.type, "nsexception") + XCTAssertEqual(rawReport.crash?.error?.reason, Self.crashReason) + XCTAssertEqual(rawReport.crash?.error?.nsexception?.name, Self.crashName) + XCTAssertGreaterThanOrEqual(rawReport.crash?.threads?.count ?? 0, 2, "Expected to have at least 2 threads") + let topSymbol = rawReport.crashedThread?.backtrace.contents + .compactMap(\.symbol_name).first + .flatMap(CrashReportFilterDemangle.demangledSwiftSymbol) + XCTAssertEqual(topSymbol, "UserReportConfig.NSExceptionReport.report()", + "Stacktrace should exclude all KSCrash symbols and have reporting function on top") + + XCTAssertEqual(app.state, .runningForeground, "Should not terminate app") + } + func testUserReport() throws { - try launchAndMakeUserReports([.userException]) + try launchAndMakeUserReport(userException: .init( + name: Self.crashName, + reason: Self.crashReason, + language: Self.crashLanguage, + lineOfCode: Self.crashLineOfCode, + stacktrace: Self.crashCustomStacktrace, + logAllThreads: true, + terminateProgram: false + )) let rawReport = try readPartialCrashReport() try rawReport.validate() XCTAssertEqual(rawReport.crash?.error?.type, "user") - XCTAssertEqual(rawReport.crash?.error?.reason, UserReportConfig.crashReason) - XCTAssertEqual(rawReport.crash?.error?.user_reported?.name, UserReportConfig.crashName) - XCTAssertEqual(rawReport.crash?.error?.user_reported?.backtrace, UserReportConfig.crashCustomStacktrace) + XCTAssertEqual(rawReport.crash?.error?.reason, Self.crashReason) + XCTAssertEqual(rawReport.crash?.error?.user_reported?.name, Self.crashName) + XCTAssertEqual(rawReport.crash?.error?.user_reported?.backtrace, Self.crashCustomStacktrace) XCTAssertGreaterThanOrEqual(rawReport.crash?.threads?.count ?? 0, 2, "Expected to have at least 2 threads") + let topSymbol = rawReport.crashedThread?.backtrace.contents + .compactMap(\.symbol_name).first + .flatMap(CrashReportFilterDemangle.demangledSwiftSymbol) + XCTAssertEqual(topSymbol, "UserReportConfig.UserException.report()", + "Stacktrace should exclude all KSCrash symbols and have reporting function on top") XCTAssertEqual(app.state, .runningForeground, "Should not terminate app") app.terminate() let appleReport = try launchAndReportCrash() - XCTAssertTrue(appleReport.contains(UserReportConfig.crashName)) - XCTAssertTrue(appleReport.contains(UserReportConfig.crashReason)) + XCTAssertTrue(appleReport.contains(Self.crashName)) + XCTAssertTrue(appleReport.contains(Self.crashReason)) let state = try readState() XCTAssertFalse(state.crashedLastLaunch) diff --git a/Sources/KSCrashCore/include/KSCompilerDefines.h b/Sources/KSCrashCore/include/KSCompilerDefines.h new file mode 100644 index 00000000..3dad35bc --- /dev/null +++ b/Sources/KSCrashCore/include/KSCompilerDefines.h @@ -0,0 +1,39 @@ +// +// KSCompilerDefines.h +// +// Created by Karl Stenerud on 2024-11-03. +// +// Copyright (c) 2012 Karl Stenerud. All rights reserved. +// +// Permission is hereby granted, free of charge, to any person obtaining a copy +// of this software and associated documentation files (the "Software"), to deal +// in the Software without restriction, including without limitation the rights +// to use, copy, modify, merge, publish, distribute, sublicense, and/or sell +// copies of the Software, and to permit persons to whom the Software is +// furnished to do so, subject to the following conditions: +// +// The above copyright notice and this permission notice shall remain in place +// in this source code. +// +// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +// IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +// FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE +// AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +// LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, +// OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN +// THE SOFTWARE. +// + +#ifndef HDR_KSCompilerDefines_h +#define HDR_KSCompilerDefines_h + +/// Disables optimisations to ensure a function remains in stacktrace. +/// Usually used in pair with `KS_THWART_TAIL_CALL_OPTIMISATION`. +#define KS_KEEP_FUNCTION_IN_STACKTRACE __attribute__((disable_tail_calls)) __attribute__((noinline)) + +/// Extra safety measure to ensure a method is not tail-call optimised. +/// This define should be placed at the end of a function. +/// Usually used in pair with `KS_KEEP_FUNCTION_IN_STACKTRACE`. +#define KS_THWART_TAIL_CALL_OPTIMISATION __asm__ __volatile__(""); + +#endif // HDR_KSCompilerDefines_h diff --git a/Sources/KSCrashRecording/KSCrash.m b/Sources/KSCrashRecording/KSCrash.m index b54f69b3..244539f3 100644 --- a/Sources/KSCrashRecording/KSCrash.m +++ b/Sources/KSCrashRecording/KSCrash.m @@ -27,6 +27,7 @@ #import "KSCrash.h" #import "KSCrash+Private.h" +#import "KSCompilerDefines.h" #import "KSCrashC.h" #import "KSCrashConfiguration+Private.h" #import "KSCrashMonitorContext.h" @@ -266,7 +267,7 @@ - (void)reportUserException:(NSString *)name lineOfCode:(NSString *)lineOfCode stackTrace:(NSArray *)stackTrace logAllThreads:(BOOL)logAllThreads - terminateProgram:(BOOL)terminateProgram + terminateProgram:(BOOL)terminateProgram KS_KEEP_FUNCTION_IN_STACKTRACE { const char *cName = [name cStringUsingEncoding:NSUTF8StringEncoding]; const char *cReason = [reason cStringUsingEncoding:NSUTF8StringEncoding]; @@ -286,15 +287,17 @@ - (void)reportUserException:(NSString *)name } kscrash_reportUserException(cName, cReason, cLanguage, cLineOfCode, cStackTrace, logAllThreads, terminateProgram); + KS_THWART_TAIL_CALL_OPTIMISATION } -- (void)reportNSException:(NSException *)exception logAllThreads:(BOOL)logAllThreads +- (void)reportNSException:(NSException *)exception logAllThreads:(BOOL)logAllThreads KS_KEEP_FUNCTION_IN_STACKTRACE { if (_customNSExceptionReporter == NULL) { KSLOG_ERROR(@"NSExcepttion monitor needs to be installed before reporting custom exceptions"); return; } _customNSExceptionReporter(exception, logAllThreads); + KS_THWART_TAIL_CALL_OPTIMISATION } // ============================================================================ diff --git a/Sources/KSCrashRecording/KSCrashC.c b/Sources/KSCrashRecording/KSCrashC.c index 8386e842..f305c4a1 100644 --- a/Sources/KSCrashRecording/KSCrashC.c +++ b/Sources/KSCrashRecording/KSCrashC.c @@ -26,6 +26,7 @@ #include "KSCrashC.h" +#include "KSCompilerDefines.h" #include "KSCrashCachedData.h" #include "KSCrashMonitorContext.h" #include "KSCrashMonitorType.h" @@ -293,12 +294,14 @@ void kscrash_setUserInfoJSON(const char *const userInfoJSON) { kscrashreport_set const char *kscrash_getUserInfoJSON(void) { return kscrashreport_getUserInfoJSON(); } void kscrash_reportUserException(const char *name, const char *reason, const char *language, const char *lineOfCode, - const char *stackTrace, bool logAllThreads, bool terminateProgram) + const char *stackTrace, bool logAllThreads, + bool terminateProgram) KS_KEEP_FUNCTION_IN_STACKTRACE { kscm_reportUserException(name, reason, language, lineOfCode, stackTrace, logAllThreads, terminateProgram); if (g_shouldAddConsoleLogToReport) { kslog_clearLogFile(); } + KS_THWART_TAIL_CALL_OPTIMISATION } void kscrash_notifyObjCLoad(void) { kscrashstate_notifyObjCLoad(); } diff --git a/Sources/KSCrashRecording/Monitors/KSCrashMonitor_CPPException.cpp b/Sources/KSCrashRecording/Monitors/KSCrashMonitor_CPPException.cpp index 57bbb882..0e78c1eb 100644 --- a/Sources/KSCrashRecording/Monitors/KSCrashMonitor_CPPException.cpp +++ b/Sources/KSCrashRecording/Monitors/KSCrashMonitor_CPPException.cpp @@ -24,6 +24,7 @@ #include "KSCrashMonitor_CPPException.h" +#include "KSCompilerDefines.h" #include "KSCrashMonitorContext.h" #include "KSCrashMonitorContextHelper.h" #include "KSID.h" @@ -77,7 +78,7 @@ static KSStackCursor g_stackCursor; #pragma mark - Callbacks - // ============================================================================ -static void captureStackTrace(void *, std::type_info *tinfo, void (*)(void *)) __attribute__((disable_tail_calls)) +static void captureStackTrace(void *, std::type_info *tinfo, void (*)(void *)) KS_KEEP_FUNCTION_IN_STACKTRACE { if (tinfo != nullptr && strcmp(tinfo->name(), "NSException") == 0) { return; @@ -85,7 +86,7 @@ static void captureStackTrace(void *, std::type_info *tinfo, void (*)(void *)) _ if (g_captureNextStackTrace) { kssc_initSelfThread(&g_stackCursor, 2); } - __asm__ __volatile__(""); // thwart tail-call optimization + KS_THWART_TAIL_CALL_OPTIMISATION } typedef void (*cxa_throw_type)(void *, std::type_info *, void (*)(void *)); @@ -93,8 +94,7 @@ typedef void (*cxa_throw_type)(void *, std::type_info *, void (*)(void *)); extern "C" { void __cxa_throw(void *thrown_exception, std::type_info *tinfo, void (*dest)(void *)) __attribute__((weak)); -void __cxa_throw(void *thrown_exception, std::type_info *tinfo, void (*dest)(void *)) - __attribute__((disable_tail_calls)) +void __cxa_throw(void *thrown_exception, std::type_info *tinfo, void (*dest)(void *)) KS_KEEP_FUNCTION_IN_STACKTRACE { static cxa_throw_type orig_cxa_throw = NULL; if (g_cxaSwapEnabled == false) { @@ -102,7 +102,7 @@ void __cxa_throw(void *thrown_exception, std::type_info *tinfo, void (*dest)(voi } unlikely_if(orig_cxa_throw == NULL) { orig_cxa_throw = (cxa_throw_type)dlsym(RTLD_NEXT, "__cxa_throw"); } orig_cxa_throw(thrown_exception, tinfo, dest); - __asm__ __volatile__(""); // thwart tail-call optimization + KS_THWART_TAIL_CALL_OPTIMISATION __builtin_unreachable(); } } diff --git a/Sources/KSCrashRecording/Monitors/KSCrashMonitor_NSException.m b/Sources/KSCrashRecording/Monitors/KSCrashMonitor_NSException.m index 6d29663f..9e86921f 100644 --- a/Sources/KSCrashRecording/Monitors/KSCrashMonitor_NSException.m +++ b/Sources/KSCrashRecording/Monitors/KSCrashMonitor_NSException.m @@ -27,6 +27,7 @@ #import "KSCrashMonitor_NSException.h" #import +#import "KSCompilerDefines.h" #import "KSCrash+Private.h" #import "KSCrash.h" #include "KSCrashMonitorContext.h" @@ -54,7 +55,8 @@ #pragma mark - Callbacks - // ============================================================================ -static void initStackCursor(KSStackCursor *cursor, NSException *exception, uintptr_t *callstack, BOOL isUserReported) +static void initStackCursor(KSStackCursor *cursor, NSException *exception, uintptr_t *callstack, + BOOL isUserReported) KS_KEEP_FUNCTION_IN_STACKTRACE { // Use stacktrace from NSException if present, // otherwise use current thread (can happen for user-reported exceptions). @@ -67,8 +69,21 @@ static void initStackCursor(KSStackCursor *cursor, NSException *exception, uintp } kssc_initWithBacktrace(cursor, callstack, (int)numFrames, 0); } else { - kssc_initSelfThread(cursor, 0); + /* Skip frames for user-reported: + * 1. `initStackCursor` + * 2. `handleException` + * 3. `customNSExceptionReporter` + * 4. `+[KSCrash reportNSException:logAllThreads:]` + * + * Skip frames for caught exceptions (unlikely scenario): + * 1. `initStackCursor` + * 2. `handleException` + * 3. `handleUncaughtException` + */ + int const skipFrames = isUserReported ? 4 : 3; + kssc_initSelfThread(cursor, skipFrames); } + KS_THWART_TAIL_CALL_OPTIMISATION } /** Our custom excepetion handler. @@ -76,7 +91,8 @@ static void initStackCursor(KSStackCursor *cursor, NSException *exception, uintp * * @param exception The exception that was raised. */ -static void handleException(NSException *exception, BOOL isUserReported, BOOL logAllThreads) +static void handleException(NSException *exception, BOOL isUserReported, + BOOL logAllThreads) KS_KEEP_FUNCTION_IN_STACKTRACE { KSLOG_DEBUG(@"Trapped exception %@", exception); if (g_isEnabled) { @@ -127,14 +143,20 @@ static void handleException(NSException *exception, BOOL isUserReported, BOOL lo g_previousUncaughtExceptionHandler(exception); } } + KS_THWART_TAIL_CALL_OPTIMISATION } -static void customNSExceptionReporter(NSException *exception, BOOL logAllThreads) +static void customNSExceptionReporter(NSException *exception, BOOL logAllThreads) KS_KEEP_FUNCTION_IN_STACKTRACE { handleException(exception, YES, logAllThreads); + KS_THWART_TAIL_CALL_OPTIMISATION } -static void handleUncaughtException(NSException *exception) { handleException(exception, NO, YES); } +static void handleUncaughtException(NSException *exception) KS_KEEP_FUNCTION_IN_STACKTRACE +{ + handleException(exception, NO, YES); + KS_THWART_TAIL_CALL_OPTIMISATION +} // ============================================================================ #pragma mark - API - diff --git a/Sources/KSCrashRecording/Monitors/KSCrashMonitor_User.c b/Sources/KSCrashRecording/Monitors/KSCrashMonitor_User.c index 2f5ddaf1..f3d4fc3b 100644 --- a/Sources/KSCrashRecording/Monitors/KSCrashMonitor_User.c +++ b/Sources/KSCrashRecording/Monitors/KSCrashMonitor_User.c @@ -24,6 +24,7 @@ #include "KSCrashMonitor_User.h" +#include "KSCompilerDefines.h" #include "KSCrashMonitorContext.h" #include "KSCrashMonitorContextHelper.h" #include "KSID.h" @@ -41,7 +42,8 @@ static volatile bool g_isEnabled = false; void kscm_reportUserException(const char *name, const char *reason, const char *language, const char *lineOfCode, - const char *stackTrace, bool logAllThreads, bool terminateProgram) + const char *stackTrace, bool logAllThreads, + bool terminateProgram) KS_KEEP_FUNCTION_IN_STACKTRACE { if (!g_isEnabled) { KSLOG_WARN("User-reported exception monitor is not installed. Exception has not been recorded."); @@ -60,7 +62,7 @@ void kscm_reportUserException(const char *name, const char *reason, const char * KSMC_NEW_CONTEXT(machineContext); ksmc_getContextForThread(ksthread_self(), machineContext, true); KSStackCursor stackCursor; - kssc_initSelfThread(&stackCursor, 0); + kssc_initSelfThread(&stackCursor, 3); KSLOG_DEBUG("Filling out context."); KSCrash_MonitorContext context; @@ -86,6 +88,7 @@ void kscm_reportUserException(const char *name, const char *reason, const char * abort(); } } + KS_THWART_TAIL_CALL_OPTIMISATION } static const char *monitorId(void) { return "UserReported"; } diff --git a/Sources/KSCrashRecordingCore/KSStackCursor_SelfThread.c b/Sources/KSCrashRecordingCore/KSStackCursor_SelfThread.c index f6d92a8d..e3c7a969 100644 --- a/Sources/KSCrashRecordingCore/KSStackCursor_SelfThread.c +++ b/Sources/KSCrashRecordingCore/KSStackCursor_SelfThread.c @@ -26,6 +26,7 @@ #include +#include "KSCompilerDefines.h" #include "KSStackCursor_Backtrace.h" // #define KSLogger_LocalLevel TRACE @@ -38,10 +39,10 @@ typedef struct { uintptr_t backtrace[0]; } SelfThreadContext; -void kssc_initSelfThread(KSStackCursor *cursor, int skipEntries) __attribute__((disable_tail_calls)) +void kssc_initSelfThread(KSStackCursor *cursor, int skipEntries) KS_KEEP_FUNCTION_IN_STACKTRACE { SelfThreadContext *context = (SelfThreadContext *)cursor->context; int backtraceLength = backtrace((void **)context->backtrace, MAX_BACKTRACE_LENGTH); kssc_initWithBacktrace(cursor, context->backtrace, backtraceLength, skipEntries + 1); - __asm__ __volatile__(""); // thwart tail-call optimization + KS_THWART_TAIL_CALL_OPTIMISATION } From ca8fc396048ff39377828cd39a3698158f6f3971 Mon Sep 17 00:00:00 2001 From: Nikolay Volosatov Date: Sun, 3 Nov 2024 13:22:37 +0000 Subject: [PATCH 2/6] Fix noinline attribute usage --- Sources/KSCrashCore/include/KSCompilerDefines.h | 6 +++++- .../Monitors/KSCrashMonitor_CPPException.cpp | 2 +- .../KSCrashRecording/Monitors/KSCrashMonitor_NSException.m | 4 ++-- 3 files changed, 8 insertions(+), 4 deletions(-) diff --git a/Sources/KSCrashCore/include/KSCompilerDefines.h b/Sources/KSCrashCore/include/KSCompilerDefines.h index 3dad35bc..6d4f7469 100644 --- a/Sources/KSCrashCore/include/KSCompilerDefines.h +++ b/Sources/KSCrashCore/include/KSCompilerDefines.h @@ -29,7 +29,11 @@ /// Disables optimisations to ensure a function remains in stacktrace. /// Usually used in pair with `KS_THWART_TAIL_CALL_OPTIMISATION`. -#define KS_KEEP_FUNCTION_IN_STACKTRACE __attribute__((disable_tail_calls)) __attribute__((noinline)) +#define KS_KEEP_FUNCTION_IN_STACKTRACE __attribute__((disable_tail_calls)) + +/// Disables inline optimisation. +/// Usually used in pair with `KS_KEEP_FUNCTION_IN_STACKTRACE`. +#define KS_NOINLINE __attribute__((noinline)) /// Extra safety measure to ensure a method is not tail-call optimised. /// This define should be placed at the end of a function. diff --git a/Sources/KSCrashRecording/Monitors/KSCrashMonitor_CPPException.cpp b/Sources/KSCrashRecording/Monitors/KSCrashMonitor_CPPException.cpp index 0e78c1eb..cf6841fd 100644 --- a/Sources/KSCrashRecording/Monitors/KSCrashMonitor_CPPException.cpp +++ b/Sources/KSCrashRecording/Monitors/KSCrashMonitor_CPPException.cpp @@ -78,7 +78,7 @@ static KSStackCursor g_stackCursor; #pragma mark - Callbacks - // ============================================================================ -static void captureStackTrace(void *, std::type_info *tinfo, void (*)(void *)) KS_KEEP_FUNCTION_IN_STACKTRACE +static KS_NOINLINE void captureStackTrace(void *, std::type_info *tinfo, void (*)(void *)) KS_KEEP_FUNCTION_IN_STACKTRACE { if (tinfo != nullptr && strcmp(tinfo->name(), "NSException") == 0) { return; diff --git a/Sources/KSCrashRecording/Monitors/KSCrashMonitor_NSException.m b/Sources/KSCrashRecording/Monitors/KSCrashMonitor_NSException.m index 9e86921f..6186f10f 100644 --- a/Sources/KSCrashRecording/Monitors/KSCrashMonitor_NSException.m +++ b/Sources/KSCrashRecording/Monitors/KSCrashMonitor_NSException.m @@ -55,7 +55,7 @@ #pragma mark - Callbacks - // ============================================================================ -static void initStackCursor(KSStackCursor *cursor, NSException *exception, uintptr_t *callstack, +static KS_NOINLINE void initStackCursor(KSStackCursor *cursor, NSException *exception, uintptr_t *callstack, BOOL isUserReported) KS_KEEP_FUNCTION_IN_STACKTRACE { // Use stacktrace from NSException if present, @@ -91,7 +91,7 @@ static void initStackCursor(KSStackCursor *cursor, NSException *exception, uintp * * @param exception The exception that was raised. */ -static void handleException(NSException *exception, BOOL isUserReported, +static KS_NOINLINE void handleException(NSException *exception, BOOL isUserReported, BOOL logAllThreads) KS_KEEP_FUNCTION_IN_STACKTRACE { KSLOG_DEBUG(@"Trapped exception %@", exception); From 46e7bff474db1ea312160383da1099c36c1aac09 Mon Sep 17 00:00:00 2001 From: Nikolay Volosatov Date: Sun, 3 Nov 2024 13:26:09 +0000 Subject: [PATCH 3/6] Fix format --- .../KSCrashRecording/Monitors/KSCrashMonitor_CPPException.cpp | 3 ++- .../KSCrashRecording/Monitors/KSCrashMonitor_NSException.m | 4 ++-- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/Sources/KSCrashRecording/Monitors/KSCrashMonitor_CPPException.cpp b/Sources/KSCrashRecording/Monitors/KSCrashMonitor_CPPException.cpp index cf6841fd..e4183bbb 100644 --- a/Sources/KSCrashRecording/Monitors/KSCrashMonitor_CPPException.cpp +++ b/Sources/KSCrashRecording/Monitors/KSCrashMonitor_CPPException.cpp @@ -78,7 +78,8 @@ static KSStackCursor g_stackCursor; #pragma mark - Callbacks - // ============================================================================ -static KS_NOINLINE void captureStackTrace(void *, std::type_info *tinfo, void (*)(void *)) KS_KEEP_FUNCTION_IN_STACKTRACE +static KS_NOINLINE void captureStackTrace(void *, std::type_info *tinfo, + void (*)(void *)) KS_KEEP_FUNCTION_IN_STACKTRACE { if (tinfo != nullptr && strcmp(tinfo->name(), "NSException") == 0) { return; diff --git a/Sources/KSCrashRecording/Monitors/KSCrashMonitor_NSException.m b/Sources/KSCrashRecording/Monitors/KSCrashMonitor_NSException.m index 6186f10f..2d7bdd78 100644 --- a/Sources/KSCrashRecording/Monitors/KSCrashMonitor_NSException.m +++ b/Sources/KSCrashRecording/Monitors/KSCrashMonitor_NSException.m @@ -56,7 +56,7 @@ // ============================================================================ static KS_NOINLINE void initStackCursor(KSStackCursor *cursor, NSException *exception, uintptr_t *callstack, - BOOL isUserReported) KS_KEEP_FUNCTION_IN_STACKTRACE + BOOL isUserReported) KS_KEEP_FUNCTION_IN_STACKTRACE { // Use stacktrace from NSException if present, // otherwise use current thread (can happen for user-reported exceptions). @@ -92,7 +92,7 @@ static KS_NOINLINE void initStackCursor(KSStackCursor *cursor, NSException *exce * @param exception The exception that was raised. */ static KS_NOINLINE void handleException(NSException *exception, BOOL isUserReported, - BOOL logAllThreads) KS_KEEP_FUNCTION_IN_STACKTRACE + BOOL logAllThreads) KS_KEEP_FUNCTION_IN_STACKTRACE { KSLOG_DEBUG(@"Trapped exception %@", exception); if (g_isEnabled) { From adf393d5a64b8082fdebe6c6b712b05210ee5a6a Mon Sep 17 00:00:00 2001 From: Nikolay Volosatov Date: Sun, 3 Nov 2024 13:29:04 +0000 Subject: [PATCH 4/6] Fix new header author --- Sources/KSCrashCore/include/KSCompilerDefines.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Sources/KSCrashCore/include/KSCompilerDefines.h b/Sources/KSCrashCore/include/KSCompilerDefines.h index 6d4f7469..e0168753 100644 --- a/Sources/KSCrashCore/include/KSCompilerDefines.h +++ b/Sources/KSCrashCore/include/KSCompilerDefines.h @@ -1,7 +1,7 @@ // // KSCompilerDefines.h // -// Created by Karl Stenerud on 2024-11-03. +// Created by Nikolay Volosatov on 2024-11-03. // // Copyright (c) 2012 Karl Stenerud. All rights reserved. // From 96f29dd0c2af748de2dc4796945b2791038c5f9e Mon Sep 17 00:00:00 2001 From: Nikolay Volosatov Date: Sun, 3 Nov 2024 13:37:47 +0000 Subject: [PATCH 5/6] Add top symbol check to C++ crash test --- Samples/Tests/IntegrationTests.swift | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/Samples/Tests/IntegrationTests.swift b/Samples/Tests/IntegrationTests.swift index 2b2483c7..407d138c 100644 --- a/Samples/Tests/IntegrationTests.swift +++ b/Samples/Tests/IntegrationTests.swift @@ -67,6 +67,10 @@ final class CppTests: IntegrationTestBase { let rawReport = try readPartialCrashReport() try rawReport.validate() XCTAssertEqual(rawReport.crash?.error?.type, "cpp_exception") + let topSymbol = rawReport.crashedThread?.backtrace.contents + .compactMap(\.symbol_name).first + .flatMap(CrashReportFilterDemangle.demangledCppSymbol) + XCTAssertEqual(topSymbol, "sample_namespace::Report::crash()") let appleReport = try launchAndReportCrash() XCTAssertTrue(appleReport.contains("C++ exception")) From 0236bf079ae2fd1358265c31e2832606304f0a21 Mon Sep 17 00:00:00 2001 From: Nikolay Volosatov Date: Tue, 5 Nov 2024 23:27:52 +0000 Subject: [PATCH 6/6] Use correct documentation comments style --- Sources/KSCrashCore/include/KSCompilerDefines.h | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/Sources/KSCrashCore/include/KSCompilerDefines.h b/Sources/KSCrashCore/include/KSCompilerDefines.h index e0168753..24dbddcb 100644 --- a/Sources/KSCrashCore/include/KSCompilerDefines.h +++ b/Sources/KSCrashCore/include/KSCompilerDefines.h @@ -27,17 +27,20 @@ #ifndef HDR_KSCompilerDefines_h #define HDR_KSCompilerDefines_h -/// Disables optimisations to ensure a function remains in stacktrace. -/// Usually used in pair with `KS_THWART_TAIL_CALL_OPTIMISATION`. +/** Disables optimisations to ensure a function remains in stacktrace. + * Usually used in pair with `KS_THWART_TAIL_CALL_OPTIMISATION`. + */ #define KS_KEEP_FUNCTION_IN_STACKTRACE __attribute__((disable_tail_calls)) -/// Disables inline optimisation. -/// Usually used in pair with `KS_KEEP_FUNCTION_IN_STACKTRACE`. +/** Disables inline optimisation. + * Usually used in pair with `KS_KEEP_FUNCTION_IN_STACKTRACE`. + */ #define KS_NOINLINE __attribute__((noinline)) -/// Extra safety measure to ensure a method is not tail-call optimised. -/// This define should be placed at the end of a function. -/// Usually used in pair with `KS_KEEP_FUNCTION_IN_STACKTRACE`. +/** Extra safety measure to ensure a method is not tail-call optimised. + * This define should be placed at the end of a function. + * Usually used in pair with `KS_KEEP_FUNCTION_IN_STACKTRACE`. + */ #define KS_THWART_TAIL_CALL_OPTIMISATION __asm__ __volatile__(""); #endif // HDR_KSCompilerDefines_h