Merge 35e1010 into 382d6c1

supervacuus · web-flow · commit 238420bc59f0 · 2026-02-25T13:43:26.000+01:00
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -5,6 +5,7 @@
 ### Fixes
 
 - Fix crash when unregistering `SystemEventsBroadcastReceiver` with try-catch block. ([#5106](https://github.com/getsentry/sentry-java/pull/5106))
+- Identify and correctly structure Java/Kotlin frames in mixed Tombstone stack traces. ([#5116](https://github.com/getsentry/sentry-java/pull/5116))
 
 ## 8.33.0
 
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
@@ -33,6 +33,20 @@ public class TombstoneParser implements Closeable {
   @Nullable private final String nativeLibraryDir;
   private final Map<String, String> excTypeValueMap = new HashMap<>();
 
+  private static boolean isJavaFrame(@NonNull final TombstoneProtos.BacktraceFrame frame) {
+    final String fileName = frame.getFileName();
+    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("<anonymous:")
+            || fileName.startsWith("[anon_shmem:dalvik-")
+            || fileName.startsWith("/memfd:jit-cache"));
+  }
+
   private static String formatHex(long value) {
     return String.format("0x%x", value);
   }
@@ -108,7 +122,8 @@ private SentryStackTrace createStackTrace(@NonNull final TombstoneProtos.Thread
     final List<SentryStackFrame> frames = new ArrayList<>();
 
     for (TombstoneProtos.BacktraceFrame frame : thread.getCurrentBacktraceList()) {
-      if (frame.getFileName().endsWith("libart.so")) {
+      if (frame.getFileName().endsWith("libart.so")
+          || Objects.equals(frame.getFunctionName(), "art_jni_trampoline")) {
         // We ignore all ART frames for time being because they aren't actionable for app developers
         continue;
       }
@@ -118,27 +133,29 @@ private SentryStackTrace createStackTrace(@NonNull final TombstoneProtos.Thread
         continue;
       }
       final SentryStackFrame stackFrame = new SentryStackFrame();
-      stackFrame.setPackage(frame.getFileName());
-      stackFrame.setFunction(frame.getFunctionName());
-      stackFrame.setInstructionAddr(formatHex(frame.getPc()));
-
-      // 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.
-      // Protobuf 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.getFunctionName();
-      @Nullable
-      Boolean inApp =
-          functionName.isEmpty()
-              ? Boolean.FALSE
-              : SentryStackTraceFactory.isInApp(functionName, inAppIncludes, inAppExcludes);
-
-      final boolean isInNativeLibraryDir =
-          nativeLibraryDir != null && frame.getFileName().startsWith(nativeLibraryDir);
-      inApp = (inApp != null && inApp) || isInNativeLibraryDir;
-
-      stackFrame.setInApp(inApp);
+      if (isJavaFrame(frame)) {
+        stackFrame.setPlatform("java");
+        final String module = extractJavaModuleName(frame.getFunctionName());
+        stackFrame.setFunction(extractJavaFunctionName(frame.getFunctionName()));
+        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.getFileName());
+        stackFrame.setFunction(frame.getFunctionName());
+        stackFrame.setInstructionAddr(formatHex(frame.getPc()));
+
+        final boolean isInNativeLibraryDir =
+            nativeLibraryDir != null && frame.getFileName().startsWith(nativeLibraryDir);
+        stackFrame.setInApp(isInNativeLibraryDir);
+      }
       frames.add(0, stackFrame);
     }
 
@@ -159,6 +176,53 @@ private SentryStackTrace createStackTrace(@NonNull final TombstoneProtos.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.
+   *
+   * <p>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) {
+      // Strip return type prefix
+      normalized = normalized.substring(spaceIndex + 1).trim();
+
+      // Strip parameter list suffix
+      final int parenIndex = normalized.indexOf('(');
+      if (parenIndex >= 0) {
+        normalized = normalized.substring(0, parenIndex);
+      }
+    }
+
+    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 "";
+    }
+  }
+
+  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<SentryException> createException(@NonNull TombstoneProtos.Tombstone tombstone) {
     final SentryException exception = new SentryException();
@@ -296,7 +360,7 @@ private DebugMeta createDebugMeta(@NonNull final TombstoneProtos.Tombstone tombs
         // 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.getEndAddress());
           continue;
         }
@@ -311,7 +375,7 @@ private DebugMeta createDebugMeta(@NonNull final TombstoneProtos.Tombstone tombs
 
         // 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.getEndAddress());
       }
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
@@ -101,14 +101,20 @@ class TombstoneParserTest {
 
       for (frame in thread.stacktrace!!.frames!!) {
         assertNotNull(frame.function)
-        assertNotNull(frame.`package`)
-        assertNotNull(frame.instructionAddr)
+        if (frame.platform == "java") {
+          // Java frames have module instead of package/instructionAddr
+          assertNotNull(frame.module)
+        } else {
+          assertNotNull(frame.`package`)
+          assertNotNull(frame.instructionAddr)
+        }
 
         if (thread.id == crashedThreadId) {
           if (frame.isInApp!!) {
             assert(
-              frame.function!!.startsWith(inAppIncludes[0]) ||
-                frame.`package`!!.startsWith(nativeLibraryDir)
+              frame.module?.startsWith(inAppIncludes[0]) == true ||
+                frame.function!!.startsWith(inAppIncludes[0]) ||
+                frame.`package`?.startsWith(nativeLibraryDir) == true
             )
           }
         }
@@ -429,6 +435,148 @@ class TombstoneParserTest {
     }
   }
 
+  @Test
+  fun `java frames snapshot test for all threads`() {
+    val tombstoneStream =
+      GZIPInputStream(TombstoneParserTest::class.java.getResourceAsStream("/tombstone.pb.gz"))
+    val parser = TombstoneParser(tombstoneStream, inAppIncludes, inAppExcludes, nativeLibraryDir)
+    val event = parser.parse()
+
+    val logger = mock<ILogger>()
+    val writer = StringWriter()
+    val jsonWriter = JsonObjectWriter(writer, 100)
+    jsonWriter.beginObject()
+    for (thread in event.threads!!) {
+      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("/tombstone_java_frames.json.gz")
+
+    assertEquals(expectedJson, actualJson)
+  }
+
+  @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)
+    assertEquals("", 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)
+    assertEquals("", 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 =
+      TombstoneProtos.Tombstone.newBuilder()
+        .setPid(1234)
+        .setTid(1234)
+        .setSignalInfo(
+          TombstoneProtos.Signal.newBuilder()
+            .setNumber(11)
+            .setName("SIGSEGV")
+            .setCode(1)
+            .setCodeName("SEGV_MAPERR")
+        )
+        .putThreads(
+          1234,
+          TombstoneProtos.Thread.newBuilder()
+            .setId(1234)
+            .setName("main")
+            .addCurrentBacktrace(
+              TombstoneProtos.BacktraceFrame.newBuilder()
+                .setPc(0x1000)
+                .setFunctionName(functionName)
+                .setFileName("/data/app/base.apk!classes.oat")
+            )
+            .build(),
+        )
+        .build()
+
+    val parser =
+      TombstoneParser(
+        ByteArrayInputStream(tombstone.toByteArray()),
+        inAppIncludes,
+        inAppExcludes,
+        nativeLibraryDir,
+      )
+    return parser.parse()
+  }
+
   private fun serializeDebugMeta(debugMeta: DebugMeta): String {
     val logger = mock<ILogger>()
     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
