Merge pull request #724 from DataDog/maxep/RUMM-1883/report-invalid-i…

…mages RUMM-1883 Report binary image with no UUID
DataDog · Jan 19, 2022 · 85f9785 · 85f9785
2 parents 744191c + d635cdf
commit 85f9785
Show file tree

Hide file tree

Showing 4 changed files with 27 additions and 19 deletions.
diff --git a/Sources/DatadogCrashReporting/PLCrashReporterIntegration/CrashReport.swift b/Sources/DatadogCrashReporting/PLCrashReporterIntegration/CrashReport.swift
@@ -91,7 +91,7 @@ internal struct BinaryImageInfo {
     }
 
     /// The UUID of this image.
-    var uuid: String
+    var uuid: String?
     /// The name of this image (referenced by "library name" in the stack frame).
     var imageName: String
     /// If its a system library image.
@@ -232,14 +232,12 @@ extension ThreadInfo {
 
 extension BinaryImageInfo {
     init?(from imageInfo: PLCrashReportBinaryImageInfo) {
-        guard let imagePath = imageInfo.imageName,
-              let imageUUID = imageInfo.hasImageUUID ? imageInfo.imageUUID : nil else {
-            // We can drop this image as it won't be useful for symbolication - both
-            // "image name" and "uuid" are necessary.
+        guard let imagePath = imageInfo.imageName else {
+            // We can drop this image as it won't be useful for symbolication
             return nil
         }
 
-        self.uuid = imageUUID
+        self.uuid = imageInfo.imageUUID
         self.imageName = URL(fileURLWithPath: imagePath).lastPathComponent
 
         #if targetEnvironment(simulator)
@@ -303,18 +301,17 @@ extension BinaryImageInfo.CodeType {
 
 extension StackFrame {
     init(from stackFrame: PLCrashReportStackFrameInfo, number: Int, in crashReport: PLCrashReport) {
-        if let image = crashReport.image(forAddress: stackFrame.instructionPointer),
-           let imageInfo = BinaryImageInfo(from: image) {
-            self.libraryName = imageInfo.imageName
-            self.libraryBaseAddress = imageInfo.imageBaseAddress
-        } else {
-            // Without "library name" and its "base address" symbolication will not be possible,
-            // but the presence of this frame in the stack will be still relevant.
-            self.libraryName = nil
-            self.libraryBaseAddress = nil
-        }
-
         self.number = number
         self.instructionPointer = stackFrame.instructionPointer
+
+        // Without "library name" and its "base address" symbolication will not be possible,
+        // but the presence of this frame in the stack will be still relevant.
+        let image = crashReport.image(forAddress: stackFrame.instructionPointer)
+
+        self.libraryBaseAddress = image?.imageBaseAddress
+
+        if let imagePath = image?.imageName {
+            self.libraryName = URL(fileURLWithPath: imagePath).lastPathComponent
+        }
     }
 }
diff --git a/Sources/DatadogCrashReporting/PLCrashReporterIntegration/DDCrashReportExporter.swift b/Sources/DatadogCrashReporting/PLCrashReporterIntegration/DDCrashReportExporter.swift
@@ -150,7 +150,7 @@ internal struct DDCrashReportExporter {
 
             return DDCrashReport.BinaryImage(
                 libraryName: image.imageName,
-                uuid: image.uuid,
+                uuid: image.uuid ?? unavailable,
                 architecture: image.codeType?.architectureName ?? unavailable,
                 isSystemLibrary: image.isSystemImage,
                 loadAddress: loadAddressHex,

diff --git a/Tests/DatadogCrashReportingTests/Mocks.swift b/Tests/DatadogCrashReportingTests/Mocks.swift
@@ -252,7 +252,7 @@ extension ThreadInfo {
 
 extension BinaryImageInfo {
     static func mockWith(
-        uuid: String = .mockAny(),
+        uuid: String? = .mockAny(),
         imageName: String = .mockAny(),
         isSystemImage: Bool = .random(),
         architectureName: String? = .mockAny(),

diff --git a/Tests/DatadogCrashReportingTests/PLCrashReporterIntegration/DDCrashReportExporterTests.swift b/Tests/DatadogCrashReportingTests/PLCrashReporterIntegration/DDCrashReportExporterTests.swift
@@ -275,6 +275,17 @@ class DDCrashReportExporterTests: XCTestCase {
         }
     }
 
+    func testExportingBinaryImageWhenUUIDIsUnavailable() {
+        // Given
+        crashReport.binaryImages = [.mockWith(uuid: nil)]
+
+        // When
+        let exportedImages = exporter.export(crashReport).binaryImages
+
+        // Then
+        XCTAssertEqual(exportedImages.first?.uuid, "???")
+    }
+
     func testExportingBinaryImageAddressRange() throws {
         let randomImageLoadAddress: UInt64 = .mockRandom()
         let randomImageSize: UInt64 = .mockRandom()