Remove KSCrash symbols from user-reported exceptions (#590)

* Remove KSCrash symbols from user-reported exceptions

* Fix noinline attribute usage

* Fix format

* Fix new header author

* Add top symbol check to C++ crash test

* Use correct documentation comments style

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()
     }
 

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

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
         )

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 {
@@ -66,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"))
@@ -127,15 +132,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 +167,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)

Sources/KSCrashCore/include/KSCompilerDefines.h

@@ -0,0 +1,46 @@
+//
+//  KSCompilerDefines.h
+//
+//  Created by Nikolay Volosatov 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))
+
+/** 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`.
+ */
+#define KS_THWART_TAIL_CALL_OPTIMISATION __asm__ __volatile__("");
+
+#endif  // HDR_KSCompilerDefines_h

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
 }
 
 // ============================================================================