diff --git a/CHANGELOG.md b/CHANGELOG.md index 02e1a8a906e..e7f0f98a92b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,7 @@ - Prevent duplicated breadcrumbs on tombstone-merged native crash events ([#5888](https://github.com/getsentry/sentry-java/pull/5888)) - Prevent a class of Session Replay deadlocks by confining lifecycle state changes to Android's main thread ([#5965](https://github.com/getsentry/sentry-java/pull/5965)) +- Symbolicate tombstone native frames for libraries loaded directly from APKs ([#5992](https://github.com/getsentry/sentry-java/pull/5992)) ### Performance diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/internal/tombstone/TombstoneParser.java b/sentry-android-core/src/main/java/io/sentry/android/core/internal/tombstone/TombstoneParser.java index c3966615899..b6ac22b12ef 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/internal/tombstone/TombstoneParser.java +++ b/sentry-android-core/src/main/java/io/sentry/android/core/internal/tombstone/TombstoneParser.java @@ -147,6 +147,12 @@ private SentryStackTrace createStackTrace(@NonNull final TombstoneThread thread) stackFrame.setPackage(frame.fileName); stackFrame.setFunction(frame.functionName); stackFrame.setInstructionAddr(formatHex(frame.pc)); + if (!frame.buildId.isEmpty() && frame.pc >= frame.relPc) { + // libunwindstack has already resolved rel_pc against the embedded or standalone ELF. + // The containing file offset (for example, the offset inside an APK) is irrelevant to the + // ELF's runtime image address. + stackFrame.setImageAddr(formatHex(frame.pc - frame.relPc)); + } // inAppIncludes/inAppExcludes filter by Java/Kotlin package names, which don't overlap // with native C/C++ function names (e.g., "crash", "__libc_init"). For native frames, @@ -256,20 +262,58 @@ private Message constructMessage(@NonNull final Tombstone tombstone) { * runtime instruction addresses in the files uploaded for symbolication. */ private static class ModuleAccumulator { + private static final long PAGE_SIZE_4KIB = 4096; + private static final long PAGE_SIZE_16KIB = 16384; + String mappingName; String buildId; long beginAddress; long endAddress; + long previousBeginAddress; + long previousEndAddress; + long previousOffset; - ModuleAccumulator(MemoryMapping mapping) { + ModuleAccumulator(final @NotNull MemoryMapping mapping) { this.mappingName = mapping.mappingName; this.buildId = mapping.buildId; this.beginAddress = mapping.beginAddress; this.endAddress = mapping.endAddress; + this.previousBeginAddress = mapping.beginAddress; + this.previousEndAddress = mapping.endAddress; + this.previousOffset = mapping.offset; } - void extendTo(long newEndAddress) { - this.endAddress = newEndAddress; + boolean isSameModule(final @NotNull MemoryMapping mapping) { + return mappingName.equals(mapping.mappingName) && buildId.equals(mapping.buildId); + } + + boolean canExtendTo(final @NotNull MemoryMapping mapping, final long pageSize) { + + if (!mappingName.equals(mapping.mappingName) + || mapping.beginAddress < previousEndAddress + || mapping.offset < previousOffset) { + return false; + } + + // PT_LOAD virtual-address and file-offset gaps can differ by one segment-alignment unit. + // Compare adjacent mappings so this difference does not accumulate across the module. + final long previousSize = previousEndAddress - previousBeginAddress; + final long addressGap = mapping.beginAddress - previousEndAddress; + final long fileOffsetGap = mapping.offset - (previousOffset + previousSize); + final long delta = addressGap - fileOffsetGap; + + // Android ELFs built for 16 KiB pages can also run on 4 KiB devices, so the ELF alignment + // can be larger than the tombstone's runtime page size. + final long runtimePageSize = pageSize > 0 ? pageSize : PAGE_SIZE_4KIB; + final long alignmentTolerance = Math.max(runtimePageSize, PAGE_SIZE_16KIB); + return delta >= -alignmentTolerance && delta <= alignmentTolerance; + } + + void extendTo(final @NotNull MemoryMapping mapping) { + this.endAddress = Math.max(endAddress, mapping.endAddress); + this.previousBeginAddress = mapping.beginAddress; + this.previousEndAddress = mapping.endAddress; + this.previousOffset = mapping.offset; } DebugImage toDebugImage() { @@ -280,7 +324,7 @@ DebugImage toDebugImage() { image.setCodeId(buildId); image.setCodeFile(mappingName); - final String debugId = NativeEventUtils.buildIdToDebugId(buildId); + final @Nullable String debugId = NativeEventUtils.buildIdToDebugId(buildId); image.setDebugId(debugId != null ? debugId : buildId); image.setImageAddr(formatHex(beginAddress)); @@ -295,14 +339,12 @@ private DebugMeta createDebugMeta(@NonNull final Tombstone tombstone) { final List images = new ArrayList<>(); // Coalesce memory mappings into modules similar to how sentry-native does it. - // A module consists of all readable mappings for the same file, starting from - // the first mapping that has a valid ELF header (indicated by offset 0 with build_id). - // In sentry-native, is_valid_elf_header() reads the ELF magic bytes from memory, - // which is only present at the start of the file (offset 0). We use offset == 0 - // combined with non-empty build_id as a proxy for this check. - ModuleAccumulator currentModule = null; - - for (MemoryMapping mapping : tombstone.memoryMappings) { + // Android's libunwindstack has already parsed each ELF and records its build ID in the + // tombstone. An ELF stored uncompressed inside an APK starts at a non-zero container offset, + // so the mapping offset cannot be used to validate whether the mapping starts an ELF. + @Nullable ModuleAccumulator currentModule = null; + + for (final @NotNull MemoryMapping mapping : tombstone.memoryMappings) { // Skip mappings that are not readable if (!mapping.read) { continue; @@ -315,18 +357,16 @@ private DebugMeta createDebugMeta(@NonNull final Tombstone tombstone) { } final boolean hasBuildId = !mapping.buildId.isEmpty(); - final boolean isFileStart = mapping.offset == 0; - - if (hasBuildId && isFileStart) { - // Check for duplicated mappings: On Android, the same ELF can have multiple - // mappings at offset 0 with different permissions (r--p, r-xp, r--p). - // If it's the same file as the current module, just extend it. - if (currentModule != null && mappingName.equals(currentModule.mappingName)) { - currentModule.extendTo(mapping.endAddress); + + if (hasBuildId) { + // The same ELF can have multiple mappings with its build ID. APK-embedded ELFs all share + // the APK mapping name, so the build ID is also required to distinguish their modules. + if (currentModule != null && currentModule.isSameModule(mapping)) { + currentModule.extendTo(mapping); continue; } - // Flush the previous module (different file) + // Flush the previous module (different ELF) if (currentModule != null) { final DebugImage image = currentModule.toDebugImage(); if (image != null) { @@ -336,9 +376,9 @@ private DebugMeta createDebugMeta(@NonNull final Tombstone tombstone) { // Start a new module currentModule = new ModuleAccumulator(mapping); - } else if (currentModule != null && mappingName.equals(currentModule.mappingName)) { - // Extend the current module with this mapping (same file, continuation) - currentModule.extendTo(mapping.endAddress); + } else if (currentModule != null && currentModule.canExtendTo(mapping, tombstone.pageSize)) { + // Extend the current module with this mapping (same ELF, continuation). + currentModule.extendTo(mapping); } } diff --git a/sentry-android-core/src/test/java/io/sentry/android/core/internal/tombstone/TombstoneParserTest.kt b/sentry-android-core/src/test/java/io/sentry/android/core/internal/tombstone/TombstoneParserTest.kt index 70fc48fd9be..03e73b1ec93 100644 --- a/sentry-android-core/src/test/java/io/sentry/android/core/internal/tombstone/TombstoneParserTest.kt +++ b/sentry-android-core/src/test/java/io/sentry/android/core/internal/tombstone/TombstoneParserTest.kt @@ -5,6 +5,7 @@ import com.abovevacant.epitaph.core.MemoryMapping import com.abovevacant.epitaph.core.Signal import com.abovevacant.epitaph.core.Tombstone import com.abovevacant.epitaph.core.TombstoneThread +import com.google.common.truth.Truth.assertThat import io.sentry.ILogger import io.sentry.JsonObjectWriter import io.sentry.protocol.DebugMeta @@ -318,6 +319,247 @@ class TombstoneParserTest { assertEquals(0x7000003000 - 0x7000000000, image.imageSize) } + @Test + fun `creates images for multiple ELF files embedded in same APK`() { + val apkPath = "/data/app/io.sentry.sample/base.apk" + val firstBuildId = "f1c3bcc0279865fe3058404b2831d9e64135386c" + val secondBuildId = "a1647a1813da20ea7e0dad6cbc11486dfaeaeb8e" + + val tombstone = + Tombstone.Builder() + .pid(1234) + .tid(1234) + .signal(Signal(11, "SIGSEGV", 1, "SEGV_MAPERR", false, 0, 0, false, 0, null)) + .addMemoryMapping( + MemoryMapping( + 0x7000000000, + 0x7000001000, + 0x156c000, + true, + false, + true, + apkPath, + firstBuildId, + 0, + ) + ) + .addMemoryMapping( + MemoryMapping( + 0x7000001000, + 0x7000002000, + 0x156d000, + true, + false, + false, + apkPath, + firstBuildId, + 0, + ) + ) + .addMemoryMapping( + MemoryMapping( + 0x7100000000, + 0x7100001000, + 0x1578000, + true, + false, + true, + apkPath, + secondBuildId, + 0, + ) + ) + .addMemoryMapping( + MemoryMapping( + 0x7100001000, + 0x7100003000, + 0x1579000, + true, + false, + false, + apkPath, + secondBuildId, + 0, + ) + ) + .build() + + val images = parser.parse(tombstone).debugMeta!!.images!! + + assertThat(images).hasSize(2) + assertThat(images.map { it.codeFile }).containsExactly(apkPath, apkPath) + assertThat(images.map { it.codeId }).containsExactly(firstBuildId, secondBuildId).inOrder() + assertThat(images.map { it.imageAddr }) + .containsExactly("0x7000000000", "0x7100000000") + .inOrder() + assertThat(images.map { it.imageSize }).containsExactly(0x2000L, 0x3000L).inOrder() + } + + @Test + fun `coalesces multiple ELF continuations aligned for 16 KiB pages`() { + val apkPath = "/data/app/io.sentry.sample/base.apk" + val buildId = "f1c3bcc0279865fe3058404b2831d9e64135386c" + + val tombstone = + Tombstone.Builder() + .pid(1234) + .tid(1234) + .signal(Signal(11, "SIGSEGV", 1, "SEGV_MAPERR", false, 0, 0, false, 0, null)) + .pageSize(0x4000) + .addMemoryMapping( + MemoryMapping( + 0x7000000000, + 0x7000001000, + 0x156c000, + true, + false, + true, + apkPath, + buildId, + 0, + ) + ) + // The virtual-address progression can differ from the file-offset progression by one page. + .addMemoryMapping( + MemoryMapping( + 0x7000005000, + 0x7000007000, + 0x156d000, + true, + false, + false, + apkPath, + "", + 0, + ) + ) + // Each PT_LOAD adds one page of alignment drift. Validation must compare adjacent + // mappings so this drift does not accumulate from the start of the module. + .addMemoryMapping( + MemoryMapping( + 0x700000b000, + 0x700000d000, + 0x156f000, + true, + true, + false, + apkPath, + "", + 0, + ) + ) + .build() + + val image = parser.parse(tombstone).debugMeta!!.images!!.single() + + assertThat(image.imageAddr).isEqualTo("0x7000000000") + assertThat(image.imageSize).isEqualTo(0xd000) + } + + @Test + fun `does not include a different embedded ELF without build ID in previous image`() { + val apkPath = "/data/app/io.sentry.sample/base.apk" + val buildId = "f1c3bcc0279865fe3058404b2831d9e64135386c" + + val tombstone = + Tombstone.Builder() + .pid(1234) + .tid(1234) + .signal(Signal(11, "SIGSEGV", 1, "SEGV_MAPERR", false, 0, 0, false, 0, null)) + .pageSize(0x4000) + .addMemoryMapping( + MemoryMapping( + 0x7000000000, + 0x7000001000, + 0x156c000, + true, + false, + true, + apkPath, + buildId, + 0, + ) + ) + // A different ELF in the APK that does not have a GNU build ID. + .addMemoryMapping( + MemoryMapping( + 0x7100000000, + 0x7100010000, + 0x163c000, + true, + false, + true, + apkPath, + "", + 0x4000, + ) + ) + .build() + + val image = parser.parse(tombstone).debugMeta!!.images!!.single() + + assertThat(image.codeId).isEqualTo(buildId) + assertThat(image.imageAddr).isEqualTo("0x7000000000") + assertThat(image.imageSize).isEqualTo(0x1000) + } + + @Test + fun `sets image address on frame for ELF embedded in APK`() { + val apkPath = "/data/app/io.sentry.sample/base.apk" + val buildId = "a1647a1813da20ea7e0dad6cbc11486dfaeaeb8e" + val imageAddress = 0x7000000000 + val relativePc = 0xac4L + + val tombstone = + Tombstone.Builder() + .pid(1234) + .tid(1234) + .signal(Signal(11, "SIGSEGV", 1, "SEGV_MAPERR", false, 0, 0, false, 0, null)) + .addMemoryMapping( + MemoryMapping( + imageAddress, + imageAddress + 0x1000, + 0x156c000, + true, + false, + true, + apkPath, + buildId, + 0, + ) + ) + .addThread( + TombstoneThread( + 1234, + "main", + emptyList(), + emptyList(), + emptyList(), + listOf( + BacktraceFrame( + relativePc, + imageAddress + relativePc, + 0, + "crash", + 0, + "$apkPath!libnative-sample.so", + 0x156c000, + buildId, + ) + ), + emptyList(), + 0, + 0, + ) + ) + .build() + + val frame = parser.parse(tombstone).threads!!.single().stacktrace!!.frames!!.single() + + assertThat(frame.instructionAddr).isEqualTo("0x7000000ac4") + assertThat(frame.imageAddr).isEqualTo("0x7000000000") + } + @Test fun `debugId falls back to codeId when OleGuidFormatter conversion fails`() { // Create a tombstone with a memory mapping that has an invalid buildId @@ -387,6 +629,42 @@ class TombstoneParserTest { assertEquals("c0bcc3f1-9827-fe65-3058-404b2831d9e6", validImage.debugId) } + @Test + fun `parses APK embedded ELF from full tombstone fixture`() { + val tombstoneStream = + GZIPInputStream( + TombstoneParserTest::class.java.getResourceAsStream("/tombstone_apk_embedded.pb.gz") + ) + val event = + TombstoneParser(tombstoneStream, inAppIncludes, inAppExcludes, nativeLibraryDir).parse() + val buildId = "a1647a1813da20ea7e0dad6cbc11486dfaeaeb8e" + + val images = event.debugMeta!!.images!! + val image = images.single { it.codeId == buildId } + + assertThat(image.type).isEqualTo("elf") + assertThat(image.codeFile).endsWith("/base.apk") + assertThat(image.debugId).isEqualTo("187a64a1-da13-ea20-7e0d-ad6cbc11486d") + assertThat(image.imageAddr).isEqualTo("0x7646619000") + assertThat(image.imageSize).isEqualTo(0x5000) + + val frames = + event.threads!! + .flatMap { it.stacktrace!!.frames!! } + .filter { it.`package`!!.endsWith("base.apk!libnative-sample.so") } + + assertThat(frames).hasSize(2) + assertThat(frames.map { it.imageAddr }).containsExactly("0x7646619000", "0x7646619000") + assertThat(frames.map { it.instructionAddr }).containsExactly("0x7646619ac4", "0x7646619ae4") + + // This tombstone was captured on a 4 KiB device, but these ELFs use 16 KiB PT_LOAD + // alignment. Their mappings still need to be fully coalesced. + assertThat(images.single { it.codeId == "f86d542eccd3f652ab08ff210b5d009ef6c14cfe" }.imageSize) + .isEqualTo(0xcc000) + assertThat(images.single { it.codeId == "d76a948eaadeea8d0d1b17cdb026d8b4e4c39384" }.imageSize) + .isEqualTo(0x9000) + } + @Test fun `debug meta images snapshot test`() { // test against a full snapshot so that we can track regressions in the VMA -> module reduction diff --git a/sentry-android-core/src/test/resources/tombstone_apk_embedded.pb.gz b/sentry-android-core/src/test/resources/tombstone_apk_embedded.pb.gz new file mode 100644 index 00000000000..59481c31fd5 Binary files /dev/null and b/sentry-android-core/src/test/resources/tombstone_apk_embedded.pb.gz differ