Merge pull request #794 from DataDog/ncreated/RUMM-2025-filter-out-st…

…ack-frames-with-addr-0x00 RUMM-2025 Filter out unrecognized trailing stack frame in `error.stack`
DataDog · Mar 29, 2022 · 770206d · 770206d
2 parents 1c3e2b0 + 5e7e5ed
commit 770206d
Show file tree

Hide file tree

Showing 3 changed files with 62 additions and 2 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -4,6 +4,7 @@
 * [FEATURE] Add tvOS Support. See [#793][]
 * [BUGFIX] Strip query parameters from span resource. See [#728][]
 * [BUGFIX] Stop reporting pre-warmed application launch time. See [#789][]
+* [IMPROVEMENT] Crash Reporting: Filter out unrecognized trailing `???` stack frame in `error.stack`. See [#794][]
 
 # 1.9.0 / 01-26-2022
 
@@ -328,6 +329,7 @@
 [#729]: https://github.com/DataDog/dd-sdk-ios/issues/729
 [#789]: https://github.com/DataDog/dd-sdk-ios/issues/789
 [#793]: https://github.com/DataDog/dd-sdk-ios/issues/793
+[#794]: https://github.com/DataDog/dd-sdk-ios/issues/794
 [@00FA9A]: https://github.com/00FA9A
 [@Britton-Earnin]: https://github.com/Britton-Earnin
 [@Hengyu]: https://github.com/Hengyu

diff --git a/Sources/DatadogCrashReporting/PLCrashReporterIntegration/DDCrashReportExporter.swift b/Sources/DatadogCrashReporting/PLCrashReporterIntegration/DDCrashReportExporter.swift
@@ -122,7 +122,22 @@ internal struct DDCrashReportExporter {
             return unavailable // should never be reached
         }
 
-        return string(from: stackFrames)
+        return string(from: sanitized(stackFrames: stackFrames))
+    }
+
+    // MARK: - Sanitizing
+
+    private func sanitized(stackFrames: [StackFrame]) -> [StackFrame] {
+        guard let lastFrame = stackFrames.last else {
+            return stackFrames
+        }
+
+        // RUMM-2025: Often the last frame has no library name nor its base address. This results with
+        // producing malformed frame, e.g. `XX  ???                   0x00000001045f0250  0x000000000 + 4368302672`
+        // which can't be symbolicated. To make it cleaner in UI and to avoid BE symbolication errors, we filter
+        // out such trailing frame. Ref.: https://github.com/microsoft/plcrashreporter/issues/193
+        let sanitizedFrames = lastFrame.libraryBaseAddress == nil ? stackFrames.dropLast() : stackFrames
+        return sanitizedFrames
     }
 
     // MARK: - Exporting threads and binary images
@@ -131,7 +146,7 @@ internal struct DDCrashReportExporter {
         return crashReport.threads.map { thread in
             return DDCrashReport.Thread(
                 name: "Thread \(thread.threadNumber)",
-                stack: string(from: thread.stackFrames),
+                stack: string(from: thread.stackFrames), // we don't sanitize frames in `error.threads[]`
                 crashed: thread.crashed,
                 state: nil // TODO: RUMM-1462 Send registers state for crashed thread
             )

diff --git a/Tests/DatadogCrashReportingTests/PLCrashReporterIntegration/DDCrashReportExporterTests.swift b/Tests/DatadogCrashReportingTests/PLCrashReporterIntegration/DDCrashReportExporterTests.swift
@@ -204,6 +204,26 @@ class DDCrashReportExporterTests: XCTestCase {
         XCTAssertEqual(actualStack, expectedStack)
     }
 
+    func testWhenLastFrameInTheStackHasNoLibraryBaseAddress_itIsFilteredOut() {
+        let stackFrames: [StackFrame] = [
+            .init(number: 0, libraryName: "Foo", libraryBaseAddress: 100, instructionPointer: 102),
+            .init(number: 1, libraryName: "Foo", libraryBaseAddress: 100, instructionPointer: 112),
+            .init(number: 2, libraryName: "Bizz", libraryBaseAddress: 400, instructionPointer: 432),
+            .init(number: 3, libraryName: "Bizz", libraryBaseAddress: nil, instructionPointer: 432),
+        ]
+
+        crashReport.exceptionInfo = .init(name: .mockAny(), reason: .mockAny(), stackFrames: stackFrames)
+
+        let actualStack = exporter.export(crashReport).stack
+        let expectedStack = """
+        0   Foo                                 0x0000000000000066 0x64 + 2
+        1   Foo                                 0x0000000000000070 0x64 + 12
+        2   Bizz                                0x00000000000001b0 0x190 + 32
+        """
+
+        XCTAssertEqual(actualStack, expectedStack)
+    }
+
     // MARK: - Formatting threads
 
     func testExportingThreads() {
@@ -251,6 +271,29 @@ class DDCrashReportExporterTests: XCTestCase {
         XCTAssertEqual(exportedThreads[2].stack, expectedOtherThreadStack)
     }
 
+    func testWhenLastFrameInThreadStackHasNoLibraryBaseAddress_itIsNotFilteredOut() {
+        let crashedThreadStackFrames: [StackFrame] = [
+            .init(number: 0, libraryName: "Foo", libraryBaseAddress: 100, instructionPointer: 102),
+            .init(number: 1, libraryName: "Foo", libraryBaseAddress: 100, instructionPointer: 112),
+            .init(number: 2, libraryName: "Bizz", libraryBaseAddress: 400, instructionPointer: 432),
+            .init(number: 3, libraryName: nil, libraryBaseAddress: nil, instructionPointer: 432),
+        ]
+
+        crashReport.threads = [
+            .init(threadNumber: 0, crashed: true, stackFrames: crashedThreadStackFrames),
+        ]
+
+        let actualStack = exporter.export(crashReport).threads[0].stack
+        let expectedStack = """
+        0   Foo                                 0x0000000000000066 0x64 + 2
+        1   Foo                                 0x0000000000000070 0x64 + 12
+        2   Bizz                                0x00000000000001b0 0x190 + 32
+        3   ???                                 0x00000000000001b0 0x0 + 0
+        """
+
+        XCTAssertEqual(actualStack, expectedStack)
+    }
+
     // MARK: - Formatting binary images
 
     func testExportingBinaryImages() {