fix binary image extraction logic in tombstone parsing (#583)

Author: snowp

platform/jvm/capture/src/main/kotlin/io/bitdrift/capture/reports/FatalIssueReporter.kt

@@ -152,7 +152,7 @@ internal class FatalIssueReporter(
         val lastReasonResult = latestAppExitInfoProvider.get(activityManager)
         if (lastReasonResult is LatestAppExitReasonResult.Valid) {
             val lastReason = lastReasonResult.applicationExitInfo
-            lastReason.traceInputStream?.let {
+            lastReason.traceInputStream?.use {
                 mapToFatalIssueType(lastReason.reason)?.let { fatalIssueType ->
                     fatalIssueReporterProcessor.persistAppExitReport(
                         fatalIssueType = fatalIssueType,

platform/jvm/capture/src/main/kotlin/io/bitdrift/capture/reports/processor/NativeCrashProcessor.kt

@@ -17,7 +17,6 @@ import io.bitdrift.capture.reports.binformat.v1.Report
 import io.bitdrift.capture.reports.binformat.v1.ReportType
 import io.bitdrift.capture.reports.binformat.v1.Thread
 import io.bitdrift.capture.reports.binformat.v1.ThreadDetails
-import io.bitdrift.capture.reports.processor.ReportFrameBuilder.toOffset
 import java.io.InputStream
 
 /**
@@ -48,32 +47,26 @@ internal object NativeCrashProcessor {
         val threadOffsets = mutableListOf<Int>()
         val binaryImageOffsets = mutableListOf<Int>()
 
+        val referencedBuildIds = mutableSetOf<String>()
+
         tombstone.threadsMap.forEach { (tid, thread) ->
             val frameOffsets =
                 thread.currentBacktraceList
                     .map { frame ->
                         val frameAddress = frame.pc.toULong()
-                        val functionOffset = frame.functionOffset.toULong()
                         val isNativeFrame = frame.pc != 0L
                         val imageId: String? = if (isNativeFrame) frame.buildId else null
 
-                        if (isNativeFrame) {
-                            binaryImageOffsets.add(
-                                BinaryImage.createBinaryImage(
-                                    builder,
-                                    builder.toOffset(imageId),
-                                    builder.toOffset(frame.fileName),
-                                    frameAddress,
-                                ),
-                            )
+                        if (!frame.buildId.isNullOrEmpty()) {
+                            referencedBuildIds.add(frame.buildId)
                         }
+
                         val frameData =
                             FrameData(
                                 symbolName = frame.functionName,
                                 fileName = if (!isNativeFrame) frame.fileName else null,
                                 imageId = imageId,
                                 frameAddress = frameAddress,
-                                symbolAddress = frameAddress - functionOffset,
                             )
                         ReportFrameBuilder.build(
                             FrameType.AndroidNative,
@@ -107,6 +100,21 @@ internal object NativeCrashProcessor {
                 ThreadDetails.createThreadsVector(builder, threadOffsets.toIntArray()),
             )
 
+        referencedBuildIds.forEach { buildId ->
+            tombstone.memoryMappingsList
+                .filter { it.buildId == buildId }
+                .minByOrNull { it.beginAddress }
+                ?.let {
+                    binaryImageOffsets.add(
+                        BinaryImage.createBinaryImage(
+                            builder,
+                            builder.createString(buildId),
+                            builder.createString(it.mappingName),
+                            it.beginAddress.toULong(),
+                        ),
+                    )
+                }
+        }
         return Report.createReport(
             builder,
             sdk,
@@ -130,7 +138,9 @@ internal object NativeCrashProcessor {
             builder.createString(signalName.ifEmpty { description })
         val causeText =
             tombstone.causesList.firstOrNull()?.humanReadable
-                ?: tombstone.abortMessage.ifEmpty { signalDescriptions[signalName] ?: "Native crash" }
+                ?: tombstone.abortMessage.ifEmpty {
+                    signalDescriptions[signalName] ?: "Native crash"
+                }
         val cause = builder.createString(causeText)
         return Error.createError(
             builder,

platform/jvm/capture/src/test/kotlin/io/bitdrift/capture/reports/processor/NativeCrashProcessorTest.kt

@@ -0,0 +1,76 @@
+// capture-sdk - bitdrift's client SDK
+// Copyright Bitdrift, Inc. All rights reserved.
+//
+// Use of this source code is governed by a source available license that can be found in the
+// LICENSE file or at:
+// https://polyformproject.org/wp-content/uploads/2020/06/PolyForm-Shield-1.0.0.txt
+
+package io.bitdrift.capture.reports.processor
+
+import com.google.flatbuffers.FlatBufferBuilder
+import io.bitdrift.capture.TombstoneProtos
+import io.bitdrift.capture.reports.binformat.v1.Report
+import org.assertj.core.api.Assertions.assertThat
+import org.junit.Test
+import java.io.ByteArrayInputStream
+import java.io.ByteArrayOutputStream
+
+class NativeCrashProcessorTest {
+    @Test
+    fun `dedupes by build-id and picks the lowest address memory map for report`() {
+        val tombstone =
+            TombstoneProtos.Tombstone
+                .newBuilder()
+                .setTid(1)
+                .also {
+                    it.putThreads(
+                        1,
+                        TombstoneProtos.Thread
+                            .newBuilder()
+                            .setId(1)
+                            .addCurrentBacktrace(
+                                TombstoneProtos.BacktraceFrame
+                                    .newBuilder()
+                                    .setBuildId("build-id"),
+                            ).build(),
+                    )
+
+                    it.addMemoryMappings(
+                        TombstoneProtos.MemoryMapping
+                            .newBuilder()
+                            .setBuildId("build-id")
+                            .setBeginAddress(200),
+                    )
+                    it.addMemoryMappings(
+                        TombstoneProtos.MemoryMapping
+                            .newBuilder()
+                            .setBuildId("build-id")
+                            .setBeginAddress(150),
+                    )
+                    it.addMemoryMappings(
+                        TombstoneProtos.MemoryMapping
+                            .newBuilder()
+                            .setBuildId("build-id")
+                            .setBeginAddress(100),
+                    )
+                }.build()
+
+        val report = makeReport(tombstone)
+
+        assertThat(report.binaryImagesLength).isEqualTo(1)
+        assertThat(report.binaryImages(0)?.loadAddress).isEqualTo(100.toULong())
+    }
+
+    fun makeReport(tombstone: TombstoneProtos.Tombstone): Report {
+        val out = ByteArrayOutputStream()
+        tombstone.writeTo(out)
+        val tombstone = ByteArrayInputStream(out.toByteArray())
+
+        val flatBufferBuilder = FlatBufferBuilder()
+        val reportOffset = NativeCrashProcessor.process(flatBufferBuilder, 0, 0, 0, "description", tombstone)
+
+        flatBufferBuilder.finish(reportOffset)
+
+        return Report.getRootAsReport(flatBufferBuilder.dataBuffer())
+    }
+}