Skip to content

Commit 74ba936

Browse files
romtsnclaude
andauthored
fix: Resolve inline groups in no-base-entries fallback (#88)
## Summary - When a frame has no line number and there are no base entries (endline==0), detect if the first range group forms an inline chain (multiple entries sharing the same startline/endline) and resolve each entry with its proper original line instead of emitting all entries with line 0 - This matches retrace's `allRangesForLine(0, true)` behavior, which picks the first range containing line 0 and returns all entries in that group - Applied to both `ProguardCache` and `ProguardMapper` Fixes CI in getsentry/symbolicator#1896 where the regression has been discovered ## Test plan - [ ] Existing tests pass - [ ] Verify with mapping files that contain inline groups without base entries 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
1 parent df1c763 commit 74ba936

File tree

4 files changed

+148
-22
lines changed

4 files changed

+148
-22
lines changed

src/cache/mod.rs

Lines changed: 42 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1163,26 +1163,56 @@ fn resolve_no_line_frames<'a>(
11631163
return resolve_base_entries(cache, frame, &base_entries, outer_source_file);
11641164
}
11651165

1166-
// No base entries — fall back to all entries with output line 0.
1166+
// No base entries — check if the first range group forms an inline group
1167+
// (multiple entries sharing the same startline/endline). If so, resolve
1168+
// that group with proper output lines. Otherwise, fall back to emitting
1169+
// a single frame with line 0 (ambiguous non-inline case).
1170+
//
1171+
// This matches retrace's `allRangesForLine(0, true)` which picks the first
1172+
// range containing line 0 and returns all entries in that range group.
1173+
// Whether this is intentional retrace behavior or accidental is debatable,
1174+
// but we match it because users compare our output against retrace-based tools.
11671175
let mut frames = Vec::new();
11681176
if let Some(first) = members.first() {
1169-
let all_same = members.iter().all(|m| {
1170-
m.original_class_offset == first.original_class_offset
1171-
&& m.original_name_offset == first.original_name_offset
1172-
});
1173-
if all_same {
1174-
if let Some(f) =
1175-
map_member_without_lines(cache, frame, first, outer_source_file, Some(0))
1176-
{
1177-
frames.push(f);
1177+
let first_start = first.startline();
1178+
let first_end = first.endline();
1179+
let first_group: Vec<_> = members
1180+
.iter()
1181+
.take_while(|m| m.startline() == first_start && m.endline() == first_end)
1182+
.collect();
1183+
1184+
if first_group.len() > 1 {
1185+
// Inline group: multiple entries share the same range.
1186+
// Resolve each with its proper original line.
1187+
for member in &first_group {
1188+
let line = compute_member_output_line(member).or(Some(0));
1189+
if let Some(f) =
1190+
map_member_without_lines(cache, frame, member, outer_source_file, line)
1191+
{
1192+
frames.push(f);
1193+
}
11781194
}
11791195
} else {
1180-
for member in members {
1196+
// Ambiguous: each entry has a different range. Collapse to one
1197+
// frame with line 0, matching retrace behavior.
1198+
let all_same = members.iter().all(|m| {
1199+
m.original_class_offset == first.original_class_offset
1200+
&& m.original_name_offset == first.original_name_offset
1201+
});
1202+
if all_same {
11811203
if let Some(f) =
1182-
map_member_without_lines(cache, frame, member, outer_source_file, Some(0))
1204+
map_member_without_lines(cache, frame, first, outer_source_file, Some(0))
11831205
{
11841206
frames.push(f);
11851207
}
1208+
} else {
1209+
for member in members {
1210+
if let Some(f) =
1211+
map_member_without_lines(cache, frame, member, outer_source_file, Some(0))
1212+
{
1213+
frames.push(f);
1214+
}
1215+
}
11861216
}
11871217
}
11881218
}

src/mapper.rs

Lines changed: 40 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -318,23 +318,53 @@ fn resolve_no_line_frames<'s>(
318318
return;
319319
}
320320

321-
// No base entries — fall back to all entries with output line 0.
321+
// No base entries — check if the first range group forms an inline group
322+
// (multiple entries sharing the same startline/endline). If so, resolve
323+
// that group with proper output lines. Otherwise, fall back to emitting
324+
// a single frame with line 0 (ambiguous non-inline case).
325+
//
326+
// This matches retrace's `allRangesForLine(0, true)` which picks the first
327+
// range containing line 0 and returns all entries in that range group.
328+
// Whether this is intentional retrace behavior or accidental is debatable,
329+
// but we match it because users compare our output against retrace-based tools.
322330
let Some(first) = mapping_entries.first() else {
323331
return;
324332
};
325-
let unambiguous = mapping_entries.iter().all(|m| m.original == first.original);
326-
if unambiguous {
327-
collected
328-
.frames
329-
.push(map_member_without_lines(frame, first, Some(0)));
330-
collected.rewrite_rules.extend(first.rewrite_rules.iter());
331-
} else {
332-
for member in mapping_entries {
333+
334+
let first_start = first.startline;
335+
let first_end = first.endline;
336+
let first_group: Vec<_> = mapping_entries
337+
.iter()
338+
.take_while(|m| m.startline == first_start && m.endline == first_end)
339+
.collect();
340+
341+
if first_group.len() > 1 {
342+
// Inline group: multiple entries share the same range.
343+
// Resolve each with its proper original line.
344+
for member in &first_group {
345+
let line = member.original_startline.filter(|&v| v > 0).or(Some(0));
333346
collected
334347
.frames
335-
.push(map_member_without_lines(frame, member, Some(0)));
348+
.push(map_member_without_lines(frame, member, line));
336349
collected.rewrite_rules.extend(member.rewrite_rules.iter());
337350
}
351+
} else {
352+
// Ambiguous: each entry has a different range. Collapse to one
353+
// frame with line 0, matching retrace behavior.
354+
let unambiguous = mapping_entries.iter().all(|m| m.original == first.original);
355+
if unambiguous {
356+
collected
357+
.frames
358+
.push(map_member_without_lines(frame, first, Some(0)));
359+
collected.rewrite_rules.extend(first.rewrite_rules.iter());
360+
} else {
361+
for member in mapping_entries {
362+
collected
363+
.frames
364+
.push(map_member_without_lines(frame, member, Some(0)));
365+
collected.rewrite_rules.extend(member.rewrite_rules.iter());
366+
}
367+
}
338368
}
339369
}
340370

tests/r8.rs

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ static MAPPING_OUTLINE: &[u8] = include_bytes!("res/mapping-outline.txt");
1212
static MAPPING_OUTLINE_COMPLEX: &[u8] = include_bytes!("res/mapping-outline-complex.txt");
1313
static MAPPING_REWRITE_COMPLEX: &str = include_str!("res/mapping-rewrite-complex.txt");
1414
static MAPPING_ZERO_LINE_INFO: &[u8] = include_bytes!("res/mapping-zero-line-info.txt");
15+
static MAPPING_INLINE_NO_BASE: &str = include_str!("res/mapping-inline-no-base.txt");
1516

1617
static MAPPING_WIN_R8: LazyLock<Vec<u8>> = LazyLock::new(|| {
1718
MAPPING_R8
@@ -577,3 +578,57 @@ fn test_method_with_zero_zero_and_line_specific_mappings_cache() {
577578
assert_eq!(remapped_frame.line(), Some(70));
578579
assert_eq!(mapped.next(), None);
579580
}
581+
582+
#[test]
583+
fn test_inline_group_no_base_entries() {
584+
// Regression test: when a frame has no line number and all mapping entries
585+
// have non-zero endline (no base entries), the first range group should be
586+
// detected as an inline chain and each entry should get its proper original
587+
// line number instead of all being collapsed to line 0.
588+
let mapper = ProguardMapper::new(ProguardMapping::new(MAPPING_INLINE_NO_BASE.as_bytes()));
589+
590+
let test = mapper.remap_stacktrace(
591+
r#"
592+
java.lang.RuntimeException: Crash
593+
at a.b.onClick(SourceFile:0)"#,
594+
);
595+
596+
assert_eq!(
597+
test.unwrap().trim(),
598+
r#"java.lang.RuntimeException: Crash
599+
at com.example.app.MainActivity.innerCall(MainActivity.kt:54)
600+
at com.example.app.MainActivity.middleCall(MainActivity.kt:44)
601+
at com.example.app.MainActivity.onClick(MainActivity.kt:30)"#
602+
.trim()
603+
);
604+
}
605+
606+
#[test]
607+
fn test_inline_group_no_base_entries_cache() {
608+
// Same regression test as above, but using ProguardCache.
609+
let mapping = ProguardMapping::new(MAPPING_INLINE_NO_BASE.as_bytes());
610+
let mut buf = Vec::new();
611+
ProguardCache::write(&mapping, &mut buf).unwrap();
612+
let cache = ProguardCache::parse(&buf).unwrap();
613+
cache.test();
614+
615+
let frame = StackFrame::new("a.b", "onClick", 0);
616+
let mut mapped = cache.remap_frame(&frame);
617+
618+
let frame1 = mapped.next().unwrap();
619+
assert_eq!(frame1.class(), "com.example.app.MainActivity");
620+
assert_eq!(frame1.method(), "innerCall");
621+
assert_eq!(frame1.line(), Some(54));
622+
623+
let frame2 = mapped.next().unwrap();
624+
assert_eq!(frame2.class(), "com.example.app.MainActivity");
625+
assert_eq!(frame2.method(), "middleCall");
626+
assert_eq!(frame2.line(), Some(44));
627+
628+
let frame3 = mapped.next().unwrap();
629+
assert_eq!(frame3.class(), "com.example.app.MainActivity");
630+
assert_eq!(frame3.method(), "onClick");
631+
assert_eq!(frame3.line(), Some(30));
632+
633+
assert_eq!(mapped.next(), None);
634+
}
Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
# compiler: R8
2+
# compiler_version: 8.6.17
3+
# min_api: 21
4+
# pg_map_id: abc123
5+
# common_typos_disable
6+
com.example.app.MainActivity -> a.b:
7+
# {"id":"sourceFile","fileName":"MainActivity.kt"}
8+
1:1:void innerCall():54:54 -> onClick
9+
1:1:void middleCall():44 -> onClick
10+
1:1:void onClick(android.view.View):30 -> onClick
11+
2:5:void otherMethod():100:100 -> otherMethod

0 commit comments

Comments
 (0)