Skip to content

Enable symbolication of native stack frames in ANR events #4061

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 13 commits into from
Mar 17, 2025
Merged
Show file tree
Hide file tree
Changes from 9 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,12 @@
# Changelog

## Unreleased

### Features

- Add native stack frame address information and debug image metadata to ANR events ([#4061](https://github.com/getsentry/sentry-java/pull/4061))
- This enables symbolication for stripped native code in ANRs

## 8.1.0

### Features
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,8 @@
import io.sentry.hints.AbnormalExit;
import io.sentry.hints.Backfillable;
import io.sentry.hints.BlockingFlushHint;
import io.sentry.protocol.DebugImage;
import io.sentry.protocol.DebugMeta;
import io.sentry.protocol.Message;
import io.sentry.protocol.SentryId;
import io.sentry.protocol.SentryThread;
Expand Down Expand Up @@ -267,6 +269,11 @@ private void reportAsSentryEvent(
event.setMessage(sentryMessage);
} else if (result.type == ParseResult.Type.DUMP) {
event.setThreads(result.threads);
if (result.debugImages != null) {
final DebugMeta debugMeta = new DebugMeta();
debugMeta.setImages(result.debugImages);
event.setDebugMeta(debugMeta);
}
}
event.setLevel(SentryLevel.FATAL);
event.setTimestamp(DateUtils.getDateTime(anrTimestamp));
Expand Down Expand Up @@ -311,15 +318,19 @@ private void reportAsSentryEvent(
final Lines lines = Lines.readLines(reader);

final ThreadDumpParser threadDumpParser = new ThreadDumpParser(options, isBackground);
final List<SentryThread> threads = threadDumpParser.parse(lines);
threadDumpParser.parse(lines);

final @NotNull List<SentryThread> threads = threadDumpParser.getThreads();
final @NotNull List<DebugImage> debugImages = threadDumpParser.getDebugImages();

if (threads.isEmpty()) {
// if the list is empty this means the system failed to capture a proper thread dump of
// the android threads, and only contains kernel-level threads and statuses, those ANRs
// are not actionable and neither they are reported by Google Play Console, so we just
// fall back to not reporting them
return new ParseResult(ParseResult.Type.NO_DUMP);
}
return new ParseResult(ParseResult.Type.DUMP, dump, threads);
return new ParseResult(ParseResult.Type.DUMP, dump, threads, debugImages);
} catch (Throwable e) {
options.getLogger().log(SentryLevel.WARNING, "Failed to parse ANR thread dump", e);
return new ParseResult(ParseResult.Type.ERROR, dump);
Expand Down Expand Up @@ -403,24 +414,31 @@ enum Type {
final Type type;
final byte[] dump;
final @Nullable List<SentryThread> threads;
final @Nullable List<DebugImage> debugImages;

ParseResult(final @NotNull Type type) {
this.type = type;
this.dump = null;
this.threads = null;
this.debugImages = null;
}

ParseResult(final @NotNull Type type, final byte[] dump) {
this.type = type;
this.dump = dump;
this.threads = null;
this.debugImages = null;
}

ParseResult(
final @NotNull Type type, final byte[] dump, final @Nullable List<SentryThread> threads) {
final @NotNull Type type,
final byte[] dump,
final @Nullable List<SentryThread> threads,
final @Nullable List<DebugImage> debugImages) {
this.type = type;
this.dump = dump;
this.threads = threads;
this.debugImages = debugImages;
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -22,9 +22,14 @@
import io.sentry.SentryLockReason;
import io.sentry.SentryOptions;
import io.sentry.SentryStackTraceFactory;
import io.sentry.protocol.DebugImage;
import io.sentry.protocol.SentryStackFrame;
import io.sentry.protocol.SentryStackTrace;
import io.sentry.protocol.SentryThread;
import java.math.BigInteger;
import java.nio.BufferUnderflowException;
import java.nio.ByteBuffer;
import java.nio.ByteOrder;
import java.util.ArrayList;
import java.util.Collections;
import java.util.HashMap;
Expand All @@ -42,12 +47,40 @@ public class ThreadDumpParser {
private static final Pattern BEGIN_UNMANAGED_NATIVE_THREAD_RE =
Pattern.compile("\"(.*)\" (.*) ?sysTid=(\\d+)");

// For reference, see native_stack_dump.cc and tombstone_proto_to_text.cpp in Android sources
// Groups
// 0:entire regex
// 1:index
// 2:pc
// 3:mapinfo
// 4:filename
// 5:mapoffset
// 6:function
// 7:fnoffset
// 7:buildid
private static final Pattern NATIVE_RE =
Pattern.compile(
" *(?:native: )?#\\d+ \\S+ [0-9a-fA-F]+\\s+(.*?)\\s+\\((.*)\\+(\\d+)\\)(?: \\(.*\\))?");
private static final Pattern NATIVE_NO_LOC_RE =
Pattern.compile(
" *(?:native: )?#\\d+ \\S+ [0-9a-fA-F]+\\s+(.*)\\s*\\(?(.*)\\)?(?: \\(.*\\))?");
// " native: #12 pc 0xabcd1234"
" *(?:native: )?#(\\d+) \\S+ ([0-9a-fA-F]+)"
// The map info includes a filename and an optional offset into the file
+ ("\\s+("
// "/path/to/file.ext",
+ "(.*?)"
// optional " (deleted)" suffix (deleted files) needed here to bias regex
// correctly
+ "(?:\\s+\\(deleted\\))?"
// " (offset 0xabcd1234)", if the mapping is not into the beginning of the file
+ "(?:\\s+\\(offset (.*?)\\))?"
+ ")")
// Optional function
+ ("(?:\\s+\\((?:"
+ "\\?\\?\\?" // " (???) marks a missing function, so don't capture it in a group
+ "|(.*?)(?:\\+(\\d+))?" // " (func+1234)", offset is
// optional
+ ")\\))?")
// Optional " (BuildId: abcd1234abcd1234abcd1234abcd1234abcd1234)"
+ "(?:\\s+\\(BuildId: (.*?)\\))?");

private static final Pattern JAVA_RE =
Pattern.compile(" *at (?:(.+)\\.)?([^.]+)\\.([^.]+)\\((.*):([\\d-]+)\\)");
private static final Pattern JNI_RE =
Expand Down Expand Up @@ -75,15 +108,48 @@ public class ThreadDumpParser {

private final @NotNull SentryStackTraceFactory stackTraceFactory;

private final @NotNull Map<String, DebugImage> debugImages;

private final @NotNull List<SentryThread> threads;

public ThreadDumpParser(final @NotNull SentryOptions options, final boolean isBackground) {
this.options = options;
this.isBackground = isBackground;
this.stackTraceFactory = new SentryStackTraceFactory(options);
this.debugImages = new HashMap<>();
this.threads = new ArrayList<>();
}

@NotNull
public List<DebugImage> getDebugImages() {
return new ArrayList<>(debugImages.values());
}

@NotNull
public List<SentryThread> parse(final @NotNull Lines lines) {
final List<SentryThread> sentryThreads = new ArrayList<>();
public List<SentryThread> getThreads() {
return threads;
}

@Nullable
private static String buildIdToDebugId(String buildId) {
try {
// Abuse BigInteger as a hex string parser. Extra byte needed to handle leading zeros.
final ByteBuffer buf = ByteBuffer.wrap(new BigInteger("10" + buildId, 16).toByteArray());
buf.get();
return String.format(
"%08x-%04x-%04x-%04x-%04x%08x",
buf.order(ByteOrder.LITTLE_ENDIAN).getInt(),
buf.getShort(),
buf.getShort(),
buf.order(ByteOrder.BIG_ENDIAN).getShort(),
buf.getShort(),
buf.getInt());
} catch (NumberFormatException | BufferUnderflowException e) {
return null;
}
}

public void parse(final @NotNull Lines lines) {

final Matcher beginManagedThreadRe = BEGIN_MANAGED_THREAD_RE.matcher("");
final Matcher beginUnmanagedNativeThreadRe = BEGIN_UNMANAGED_NATIVE_THREAD_RE.matcher("");
Expand All @@ -92,7 +158,7 @@ public List<SentryThread> parse(final @NotNull Lines lines) {
final Line line = lines.next();
if (line == null) {
options.getLogger().log(SentryLevel.WARNING, "Internal error while parsing thread dump.");
return sentryThreads;
return;
}
final String text = line.text;
// we only handle managed threads, as unmanaged/not attached do not have the thread id and
Expand All @@ -102,11 +168,10 @@ public List<SentryThread> parse(final @NotNull Lines lines) {

final SentryThread thread = parseThread(lines);
if (thread != null) {
sentryThreads.add(thread);
threads.add(thread);
}
}
}
return sentryThreads;
}

private SentryThread parseThread(final @NotNull Lines lines) {
Expand Down Expand Up @@ -176,7 +241,6 @@ private SentryStackTrace parseStacktrace(
SentryStackFrame lastJavaFrame = null;

final Matcher nativeRe = NATIVE_RE.matcher("");
final Matcher nativeNoLocRe = NATIVE_NO_LOC_RE.matcher("");
final Matcher javaRe = JAVA_RE.matcher("");
final Matcher jniRe = JNI_RE.matcher("");
final Matcher lockedRe = LOCKED_RE.matcher("");
Expand All @@ -194,20 +258,7 @@ private SentryStackTrace parseStacktrace(
break;
}
final String text = line.text;
if (matches(nativeRe, text)) {
final SentryStackFrame frame = new SentryStackFrame();
frame.setPackage(nativeRe.group(1));
frame.setFunction(nativeRe.group(2));
frame.setLineno(getInteger(nativeRe, 3, null));
frames.add(frame);
lastJavaFrame = null;
} else if (matches(nativeNoLocRe, text)) {
final SentryStackFrame frame = new SentryStackFrame();
frame.setPackage(nativeNoLocRe.group(1));
frame.setFunction(nativeNoLocRe.group(2));
frames.add(frame);
lastJavaFrame = null;
} else if (matches(javaRe, text)) {
if (matches(javaRe, text)) {
final SentryStackFrame frame = new SentryStackFrame();
final String packageName = javaRe.group(1);
final String className = javaRe.group(2);
Expand All @@ -219,6 +270,31 @@ private SentryStackTrace parseStacktrace(
frame.setInApp(stackTraceFactory.isInApp(module));
frames.add(frame);
lastJavaFrame = frame;
} else if (matches(nativeRe, text)) {
final SentryStackFrame frame = new SentryStackFrame();
frame.setPackage(nativeRe.group(3));
frame.setFunction(nativeRe.group(6));
frame.setLineno(getInteger(nativeRe, 7, null));
frame.setInstructionAddr("0x" + nativeRe.group(2));
frame.setPlatform("native");

final String buildId = nativeRe.group(8);
final String debugId = buildId == null ? null : buildIdToDebugId(buildId);
if (debugId != null) {
if (!debugImages.containsKey(debugId)) {
final DebugImage debugImage = new DebugImage();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@markushi I'm reading our dev docs which say that image_addr and image_size are required, but apparently we're not setting them here and symbolicator is still able to symbolicate the frames. Shall we amend the docs? Or is there a possibility of wrong symbolication given we don't have all the sufficient information here?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point, I think that's fine because
1.) the native dumps doesn't seem to even provide this information (which makes sense, as the state is not in memory anymore)
2.) we set the address mode to relative for every frame here as well:
frame.setAddrMode("rel:" + debugId); (link)

Maybe @loewenheim could chime in here as an expert? If all frame addresses are relative, is it fine to provide no image_addr/ image_size in debug meta?

Copy link

@loewenheim loewenheim Mar 17, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Image size is always optional (we just conservatively assume it extends to the beginning of the next image if it's not set), and the base address is only required for the absolute address mode. As you say, it's fine to leave it out with the relative address mode.

I'm a bit confused about this, though:

we set the address mode to relative for every frame here as well:
frame.setAddrMode("rel:" + debugId); (link)

The argument of rel in Symbolicator is not a debug ID, it's the index of the module in the debug_meta/images list in the event.
https://github.com/getsentry/symbolicator/blob/2e229b8470fabc5051677ef07d1e77217f921bd2/crates/symbolicator-native/src/interface.rs#L548-L556

Is this fixed up at some later point where the debug ID is replaced by an index? Otherwise this won't work.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

alright @loewenheim thanks for clearing this out! Do you think we should update the dev docs to specify those are not required in the case of relative mode?

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I would say so.

debugImage.setDebugId(debugId);
debugImage.setType("elf");
debugImage.setCodeFile(nativeRe.group(4));
debugImage.setCodeId(buildId);
debugImages.put(debugId, debugImage);
}
// The addresses in the thread dump are relative to the image
frame.setAddrMode("rel:" + debugId);
}

frames.add(frame);
lastJavaFrame = null;
} else if (matches(jniRe, text)) {
final SentryStackFrame frame = new SentryStackFrame();
final String packageName = jniRe.group(1);
Expand All @@ -227,6 +303,7 @@ private SentryStackTrace parseStacktrace(
frame.setModule(module);
frame.setFunction(jniRe.group(3));
frame.setInApp(stackTraceFactory.isInApp(module));
frame.setNative(true);
frames.add(frame);
lastJavaFrame = frame;
} else if (matches(lockedRe, text)) {
Expand Down Expand Up @@ -334,8 +411,8 @@ private Long getLong(

@Nullable
private Integer getInteger(
final @NotNull Matcher matcher, final int group, final @Nullable Integer defaultValue) {
final String str = matcher.group(group);
final @NotNull Matcher matcher, final int groupIndex, final @Nullable Integer defaultValue) {
final String str = matcher.group(groupIndex);
if (str == null || str.length() == 0) {
return defaultValue;
} else {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -305,6 +305,15 @@ class AnrV2IntegrationTest {
)
assertEquals("__start_thread", firstFrame.function)
assertEquals(64, firstFrame.lineno)
assertEquals("0x00000000000530b8", firstFrame.instructionAddr)
assertEquals("native", firstFrame.platform)
assertEquals("rel:741f3301-bbb0-b92c-58bd-c15282b8ec7b", firstFrame.addrMode)

val image = it.debugMeta?.images?.find {
it.debugId == "741f3301-bbb0-b92c-58bd-c15282b8ec7b"
}
assertNotNull(image)
assertEquals("/apex/com.android.runtime/lib64/bionic/libc.so", image.codeFile)
},
argThat<Hint> {
val hint = HintUtils.getSentrySdkHint(this)
Expand Down
Loading