diff --git a/CHANGELOG.md b/CHANGELOG.md index 54dff9fef7f..c89bbb55657 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,11 @@ # Changelog +## Unreleased + +### Fixes + +- Android: Identify and correctly structure Java/Kotlin frames in mixed Tombstone stack traces. ([#5116](https://github.com/getsentry/sentry-java/pull/5116)) + ## 8.35.0 ### Fixes 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 1f142b52c9a..72e2509246e 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 @@ -40,6 +40,20 @@ public class TombstoneParser implements Closeable { @Nullable private final String nativeLibraryDir; private final Map excTypeValueMap = new HashMap<>(); + private static boolean isJavaFrame(@NonNull final BacktraceFrame frame) { + final String fileName = frame.fileName; + return !fileName.endsWith(".so") + && !fileName.endsWith("app_process64") + && (fileName.endsWith(".jar") + || fileName.endsWith(".odex") + || fileName.endsWith(".vdex") + || fileName.endsWith(".oat") + || fileName.startsWith("[anon:dalvik-") + || fileName.startsWith(" frames = new ArrayList<>(); for (BacktraceFrame frame : thread.backtrace) { - if (frame.fileName.endsWith("libart.so")) { + if (frame.fileName.endsWith("libart.so") + || Objects.equals(frame.functionName, "art_jni_trampoline")) { // We ignore all ART frames for time being because they aren't actionable for app developers continue; } @@ -135,27 +150,29 @@ private SentryStackTrace createStackTrace(@NonNull final TombstoneThread thread) continue; } final SentryStackFrame stackFrame = new SentryStackFrame(); - stackFrame.setPackage(frame.fileName); - stackFrame.setFunction(frame.functionName); - stackFrame.setInstructionAddr(formatHex(frame.pc)); - - // 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, - // isInApp() returns null, making nativeLibraryDir the effective in-app check. - // epitaph returns "" for unset function names, which would incorrectly return true - // from isInApp(), so we treat empty as false to let nativeLibraryDir decide. - final String functionName = frame.functionName; - @Nullable - Boolean inApp = - functionName.isEmpty() - ? Boolean.FALSE - : SentryStackTraceFactory.isInApp(functionName, inAppIncludes, inAppExcludes); - - final boolean isInNativeLibraryDir = - nativeLibraryDir != null && frame.fileName.startsWith(nativeLibraryDir); - inApp = (inApp != null && inApp) || isInNativeLibraryDir; - - stackFrame.setInApp(inApp); + if (isJavaFrame(frame)) { + stackFrame.setPlatform("java"); + final String module = extractJavaModuleName(frame.functionName); + stackFrame.setFunction(extractJavaFunctionName(frame.functionName)); + stackFrame.setModule(module); + + // For Java frames, check in-app against the module (package name), which is what + // inAppIncludes/inAppExcludes are designed to match against. + @Nullable + Boolean inApp = + (module == null || module.isEmpty()) + ? Boolean.FALSE + : SentryStackTraceFactory.isInApp(module, inAppIncludes, inAppExcludes); + stackFrame.setInApp(inApp != null && inApp); + } else { + stackFrame.setPackage(frame.fileName); + stackFrame.setFunction(frame.functionName); + stackFrame.setInstructionAddr(formatHex(frame.pc)); + + final boolean isInNativeLibraryDir = + nativeLibraryDir != null && frame.fileName.startsWith(nativeLibraryDir); + stackFrame.setInApp(isInNativeLibraryDir); + } frames.add(0, stackFrame); } @@ -176,6 +193,48 @@ private SentryStackTrace createStackTrace(@NonNull final TombstoneThread thread) return stacktrace; } + /** + * Normalizes a PrettyMethod-formatted function name by stripping the return type prefix and + * parameter list suffix that dex2oat may include when compiling AOT frames into the symtab. + * + *

e.g. "void com.example.MyClass.myMethod(int, java.lang.String)" -> + * "com.example.MyClass.myMethod" + */ + private static String normalizeFunctionName(String fqFunctionName) { + String normalized = fqFunctionName.trim(); + + // When dex2oat compiles AOT frames, PrettyMethod with_signature format may be used: + // "void com.example.MyClass.myMethod(int, java.lang.String)" + // A space is never part of a normal fully-qualified method name, so its presence + // reliably indicates the with_signature format. + final int spaceIndex = normalized.indexOf(' '); + if (spaceIndex >= 0) { + final int parenIndex = normalized.indexOf('(', spaceIndex); + normalized = + normalized.substring(spaceIndex + 1, parenIndex >= 0 ? parenIndex : normalized.length()); + } + + return normalized; + } + + private static @Nullable String extractJavaModuleName(String fqFunctionName) { + final String normalized = normalizeFunctionName(fqFunctionName); + if (normalized.contains(".")) { + return normalized.substring(0, normalized.lastIndexOf(".")); + } else { + return null; + } + } + + private static @Nullable String extractJavaFunctionName(String fqFunctionName) { + final String normalized = normalizeFunctionName(fqFunctionName); + if (normalized.contains(".")) { + return normalized.substring(normalized.lastIndexOf(".") + 1); + } else { + return normalized; + } + } + @NonNull private List createException(@NonNull Tombstone tombstone) { final SentryException exception = new SentryException(); @@ -312,7 +371,7 @@ private DebugMeta createDebugMeta(@NonNull final Tombstone tombstone) { // 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)) { + if (currentModule != null && Objects.equals(mappingName, currentModule.mappingName)) { currentModule.extendTo(mapping.endAddress); continue; } @@ -327,7 +386,7 @@ private DebugMeta createDebugMeta(@NonNull final Tombstone tombstone) { // Start a new module currentModule = new ModuleAccumulator(mapping); - } else if (currentModule != null && mappingName.equals(currentModule.mappingName)) { + } else if (currentModule != null && Objects.equals(mappingName, currentModule.mappingName)) { // Extend the current module with this mapping (same file, continuation) currentModule.extendTo(mapping.endAddress); } 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 34e704188c4..4c26a87eeca 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 @@ -13,46 +13,46 @@ import java.util.zip.GZIPInputStream import kotlin.test.Test import kotlin.test.assertEquals import kotlin.test.assertNotNull +import kotlin.test.assertNull import org.mockito.kotlin.mock class TombstoneParserTest { val expectedRegisters = setOf( + "x0", + "x1", + "x2", + "x3", + "x4", + "x5", + "x6", + "x7", "x8", "x9", - "esr", - "lr", - "pst", "x10", - "x12", "x11", - "x14", + "x12", "x13", - "x16", + "x14", "x15", - "sp", - "x18", + "x16", "x17", + "x18", "x19", - "pc", - "x21", "x20", - "x0", - "x23", - "x1", + "x21", "x22", - "x2", - "x25", - "x3", + "x23", "x24", - "x4", - "x27", - "x5", + "x25", "x26", - "x6", - "x29", - "x7", + "x27", "x28", + "x29", + "lr", + "sp", + "pc", + "pst", ) val inAppIncludes = arrayListOf("io.sentry.samples.android") @@ -63,78 +63,13 @@ class TombstoneParserTest { val parser = TombstoneParser(inAppIncludes, inAppExcludes, nativeLibraryDir) @Test - fun `parses a snapshot tombstone into Event`() { - val tombstoneStream = - GZIPInputStream(TombstoneParserTest::class.java.getResourceAsStream("/tombstone.pb.gz")) - val streamParser = - TombstoneParser(tombstoneStream, inAppIncludes, inAppExcludes, nativeLibraryDir) - val event = streamParser.parse() - - // top-level data - assertNotNull(event.eventId) - assertEquals( - "Fatal signal SIGSEGV (11), SEGV_MAPERR (1), pid = 21891 (io.sentry.samples.android)", - event.message!!.formatted, - ) - assertEquals("native", event.platform) - assertEquals("FATAL", event.level!!.name) - - // exception - // we only track one native exception (no nesting, one crashed thread) - assertEquals(1, event.exceptions!!.size) - val exception = event.exceptions!![0] - assertEquals("SIGSEGV", exception.type) - assertEquals("Segfault", exception.value) - val crashedThreadId = exception.threadId - assertNotNull(crashedThreadId) - - val mechanism = exception.mechanism - assertEquals("Tombstone", mechanism!!.type) - assertEquals(false, mechanism.isHandled) - assertEquals(true, mechanism.synthetic) - assertEquals("SIGSEGV", mechanism.meta!!["name"]) - assertEquals(11, mechanism.meta!!["number"]) - assertEquals("SEGV_MAPERR", mechanism.meta!!["code_name"]) - assertEquals(1, mechanism.meta!!["code"]) - - // threads - assertEquals(62, event.threads!!.size) - for (thread in event.threads!!) { - assertNotNull(thread.id) - if (thread.id == crashedThreadId) { - assert(thread.isCrashed == true) - } - assert(thread.stacktrace!!.frames!!.isNotEmpty()) - - for (frame in thread.stacktrace!!.frames!!) { - assertNotNull(frame.function) - assertNotNull(frame.`package`) - assertNotNull(frame.instructionAddr) - - if (thread.id == crashedThreadId) { - if (frame.isInApp!!) { - assert( - frame.function!!.startsWith(inAppIncludes[0]) || - frame.`package`!!.startsWith(nativeLibraryDir) - ) - } - } - } - - assert(thread.stacktrace!!.registers!!.keys.containsAll(expectedRegisters)) - } + fun `parses tombstone into Event`() { + assertTombstoneParsesCorrectly("/tombstone.pb.gz") + } - // debug-meta - assertEquals(352, event.debugMeta!!.images!!.size) - for (image in event.debugMeta!!.images!!) { - assertEquals("elf", image.type) - assertNotNull(image.debugId) - assertNotNull(image.codeId) - assertNotNull(image.codeFile) - val imageAddress = image.imageAddr!!.removePrefix("0x").toLong(16) - assert(imageAddress > 0) - assert(image.imageSize!! > 0) - } + @Test + fun `parses tombstone_r8 with OAT frames into Event`() { + assertTombstoneParsesCorrectly("/tombstone_r8.pb.gz") } @Test @@ -417,6 +352,218 @@ class TombstoneParserTest { } } + @Test + fun `java frames snapshot test`() { + assertJavaFramesSnapshot("/tombstone.pb.gz", "/tombstone_java_frames.json.gz") + } + + @Test + fun `tombstone_r8 java frames snapshot test`() { + assertJavaFramesSnapshot("/tombstone_r8.pb.gz", "/tombstone_r8_java_frames.json.gz") + } + + @Test + fun `extracts java function and module from plain PrettyMethod format`() { + val event = parseTombstoneWithJavaFunctionName("com.example.MyClass.myMethod") + val frame = event.threads!![0].stacktrace!!.frames!![0] + assertEquals("java", frame.platform) + assertEquals("myMethod", frame.function) + assertEquals("com.example.MyClass", frame.module) + } + + @Test + fun `extracts java function and module from PrettyMethod with_signature format`() { + val event = + parseTombstoneWithJavaFunctionName("void com.example.MyClass.myMethod(int, java.lang.String)") + val frame = event.threads!![0].stacktrace!!.frames!![0] + assertEquals("java", frame.platform) + assertEquals("myMethod", frame.function) + assertEquals("com.example.MyClass", frame.module) + } + + @Test + fun `extracts java function and module from PrettyMethod with_signature with object return type`() { + val event = + parseTombstoneWithJavaFunctionName("java.lang.String com.example.MyClass.myMethod(int)") + val frame = event.threads!![0].stacktrace!!.frames!![0] + assertEquals("java", frame.platform) + assertEquals("myMethod", frame.function) + assertEquals("com.example.MyClass", frame.module) + } + + @Test + fun `extracts java function and module from PrettyMethod with_signature with no params`() { + val event = parseTombstoneWithJavaFunctionName("void com.example.MyClass.myMethod()") + val frame = event.threads!![0].stacktrace!!.frames!![0] + assertEquals("java", frame.platform) + assertEquals("myMethod", frame.function) + assertEquals("com.example.MyClass", frame.module) + } + + @Test + fun `handles bare function name without package`() { + val event = parseTombstoneWithJavaFunctionName("myMethod") + val frame = event.threads!![0].stacktrace!!.frames!![0] + assertEquals("java", frame.platform) + assertEquals("myMethod", frame.function) + assertNull(frame.module) + } + + @Test + fun `handles PrettyMethod with_signature bare function name`() { + val event = parseTombstoneWithJavaFunctionName("void myMethod()") + val frame = event.threads!![0].stacktrace!!.frames!![0] + assertEquals("java", frame.platform) + assertEquals("myMethod", frame.function) + assertNull(frame.module) + } + + @Test + fun `java frame with_signature format is correctly detected as inApp`() { + val event = + parseTombstoneWithJavaFunctionName("void io.sentry.samples.android.MyClass.myMethod(int)") + val frame = event.threads!![0].stacktrace!!.frames!![0] + assertEquals("java", frame.platform) + assertEquals(true, frame.isInApp) + } + + @Test + fun `java frame with_signature format is correctly detected as not inApp`() { + val event = + parseTombstoneWithJavaFunctionName( + "void android.os.Handler.handleCallback(android.os.Message)" + ) + val frame = event.threads!![0].stacktrace!!.frames!![0] + assertEquals("java", frame.platform) + assertEquals(false, frame.isInApp) + } + + private fun parseTombstoneWithJavaFunctionName(functionName: String): io.sentry.SentryEvent { + val tombstone = + Tombstone.Builder() + .pid(1234) + .tid(1234) + .signal(Signal(11, "SIGSEGV", 1, "SEGV_MAPERR", false, 0, 0, false, 0, null)) + .addThread( + TombstoneThread( + 1234, + "main", + emptyList(), + emptyList(), + emptyList(), + listOf( + BacktraceFrame(0, 0x1000, 0, functionName, 0, "/data/app/base.apk!classes.oat", 0, "") + ), + emptyList(), + 0, + 0, + ) + ) + .build() + + val parser = TombstoneParser(inAppIncludes, inAppExcludes, nativeLibraryDir) + return parser.parse(tombstone) + } + + private fun assertTombstoneParsesCorrectly(tombstoneResource: String) { + val tombstoneStream = + GZIPInputStream(TombstoneParserTest::class.java.getResourceAsStream(tombstoneResource)) + val parser = TombstoneParser(tombstoneStream, inAppIncludes, inAppExcludes, nativeLibraryDir) + val event = parser.parse() + + // top-level data + assertNotNull(event.eventId) + assertNotNull(event.message!!.formatted) + assertEquals("native", event.platform) + assertEquals("FATAL", event.level!!.name) + + // exception + assertEquals(1, event.exceptions!!.size) + val exception = event.exceptions!![0] + assertNotNull(exception.type) + val crashedThreadId = exception.threadId + assertNotNull(crashedThreadId) + + val mechanism = exception.mechanism + assertNotNull(mechanism) + assertEquals("Tombstone", mechanism.type) + assertEquals(false, mechanism.isHandled) + assertEquals(true, mechanism.synthetic) + + // threads + assert(event.threads!!.isNotEmpty()) + var hasCrashedThread = false + for (thread in event.threads!!) { + assertNotNull(thread.id) + if (thread.id == crashedThreadId) { + assert(thread.isCrashed == true) + hasCrashedThread = true + } + assert(thread.stacktrace!!.frames!!.isNotEmpty()) + + for (frame in thread.stacktrace!!.frames!!) { + assertNotNull(frame.function) + if (frame.platform == "java") { + assertNotNull(frame.module) + assert(frame.function!!.isNotEmpty()) { + "Java frame has empty function name in thread ${thread.id}" + } + assertNotNull(frame.isInApp) + } else { + assertNotNull(frame.`package`) + assertNotNull(frame.instructionAddr) + } + } + + assert(thread.stacktrace!!.registers!!.keys.containsAll(expectedRegisters)) { + "Thread ${thread.id} is missing registers: ${expectedRegisters - thread.stacktrace!!.registers!!.keys}" + } + } + assert(hasCrashedThread) { "No crashed thread found matching exception threadId" } + + // debug-meta + assertNotNull(event.debugMeta) + assert(event.debugMeta!!.images!!.isNotEmpty()) + for (image in event.debugMeta!!.images!!) { + assertEquals("elf", image.type) + assertNotNull(image.debugId) + assertNotNull(image.codeId) + assertNotNull(image.codeFile) + val imageAddress = image.imageAddr!!.removePrefix("0x").toLong(16) + assert(imageAddress > 0) + assert(image.imageSize!! > 0) + } + } + + private fun assertJavaFramesSnapshot(tombstoneResource: String, snapshotResource: String) { + val tombstoneStream = + GZIPInputStream(TombstoneParserTest::class.java.getResourceAsStream(tombstoneResource)) + val parser = TombstoneParser(tombstoneStream, inAppIncludes, inAppExcludes, nativeLibraryDir) + val event = parser.parse() + + val logger = mock() + val writer = StringWriter() + val jsonWriter = JsonObjectWriter(writer, 100) + jsonWriter.beginObject() + // Sort threads by ID for deterministic output (epitaph uses HashMap for threads) + for (thread in event.threads!!.sortedBy { it.id }) { + val javaFrames = thread.stacktrace!!.frames!!.filter { it.platform == "java" } + if (javaFrames.isEmpty()) continue + jsonWriter.name(thread.id.toString()) + jsonWriter.beginArray() + for (frame in javaFrames) { + frame.serialize(jsonWriter, logger) + } + jsonWriter.endArray() + } + jsonWriter.endObject() + + val actualJson = writer.toString() + val expectedJson = readGzippedResourceFile(snapshotResource) + + assertEquals(expectedJson, actualJson) + } + private fun serializeDebugMeta(debugMeta: DebugMeta): String { val logger = mock() val writer = StringWriter() diff --git a/sentry-android-core/src/test/resources/tombstone_java_frames.json.gz b/sentry-android-core/src/test/resources/tombstone_java_frames.json.gz new file mode 100644 index 00000000000..db643090f6c Binary files /dev/null and b/sentry-android-core/src/test/resources/tombstone_java_frames.json.gz differ diff --git a/sentry-android-core/src/test/resources/tombstone_r8.pb.gz b/sentry-android-core/src/test/resources/tombstone_r8.pb.gz new file mode 100644 index 00000000000..4f55afcb11c Binary files /dev/null and b/sentry-android-core/src/test/resources/tombstone_r8.pb.gz differ diff --git a/sentry-android-core/src/test/resources/tombstone_r8_java_frames.json.gz b/sentry-android-core/src/test/resources/tombstone_r8_java_frames.json.gz new file mode 100644 index 00000000000..6d2668e7de6 Binary files /dev/null and b/sentry-android-core/src/test/resources/tombstone_r8_java_frames.json.gz differ diff --git a/sentry-samples/sentry-samples-android/build.gradle.kts b/sentry-samples/sentry-samples-android/build.gradle.kts index bb2c3954ca6..1512f1c1267 100644 --- a/sentry-samples/sentry-samples-android/build.gradle.kts +++ b/sentry-samples/sentry-samples-android/build.gradle.kts @@ -7,6 +7,7 @@ plugins { id("com.android.application") alias(libs.plugins.kotlin.android) alias(libs.plugins.kotlin.compose) + alias(libs.plugins.sentry) } android { @@ -116,6 +117,13 @@ android { @Suppress("UnstableApiUsage") packagingOptions { jniLibs { useLegacyPackaging = true } } } +sentry { + autoUploadProguardMapping = false + autoUploadNativeSymbols = false + autoUploadSourceContext = false + autoInstallation { enabled = false } +} + dependencies { implementation( kotlin(Config.kotlinStdLib, org.jetbrains.kotlin.config.KotlinCompilerVersion.VERSION)