Skip to content
Closed
Show file tree
Hide file tree
Changes from all 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
3 changes: 2 additions & 1 deletion src/builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -292,7 +292,8 @@ impl<'s> ParsedProguardMapping<'s> {
None
};
// If the mapping has no line records, use a sentinel to distinguish from
// an explicit 0:0 minified range.
// an explicit 0:0 minified range (mirrors R8's "minifiedRange == null").
// https://r8.googlesource.com/r8/+/main/src/main/java/com/android/tools/r8/naming/ClassNamingForNameMapper.java
let (startline, endline) = line_mapping
.as_ref()
.map_or((usize::MAX, usize::MAX), |line_mapping| {
Expand Down
61 changes: 61 additions & 0 deletions src/cache/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -898,6 +898,8 @@ pub struct RemappedFrameIter<'r, 'data> {
has_line_info: bool,
/// The source file of the outer class for synthesis.
outer_source_file: Option<&'data str>,
/// Tracks if any line-mapped result was emitted (prevents late fallback).
had_line_match: bool,
}

impl<'r, 'data> RemappedFrameIter<'r, 'data> {
Expand All @@ -914,6 +916,7 @@ impl<'r, 'data> RemappedFrameIter<'r, 'data> {
had_mappings: false,
has_line_info: false,
outer_source_file: None,
had_line_match: false,
}
}

Expand All @@ -933,6 +936,7 @@ impl<'r, 'data> RemappedFrameIter<'r, 'data> {
had_mappings,
has_line_info,
outer_source_file,
had_line_match: false,
}
}

Expand All @@ -944,6 +948,7 @@ impl<'r, 'data> RemappedFrameIter<'r, 'data> {
had_mappings: false,
has_line_info: false,
outer_source_file: None,
had_line_match: false,
}
}

Expand All @@ -965,6 +970,8 @@ impl<'r, 'data> RemappedFrameIter<'r, 'data> {
let out = if frame.parameters.is_none() {
// If we have no line number, treat it as unknown. If there are base (no-line) mappings
// present, prefer those over line-mapped entries.
// R8: RetraceMethodResultImpl#narrowByPosition + filterOnNoPosition
// https://r8.googlesource.com/r8/+/main/src/main/java/com/android/tools/r8/retrace/internal/RetraceMethodResultImpl.java
if frame.line == 0 {
let frames = collect_no_line_frames(
cache,
Expand All @@ -986,13 +993,30 @@ impl<'r, 'data> RemappedFrameIter<'r, 'data> {
}

// With a concrete line number, skip base entries if there are line mappings.
let members_slice = members.as_slice();
let mapped = iterate_with_lines(
cache,
&mut frame,
&mut members,
self.outer_source_file,
self.has_line_info,
);
if mapped.is_some() && self.has_line_info {
// Remember that we already produced a line-specific result.
self.had_line_match = true;
}
if mapped.is_none() && !self.had_line_match {
// Align with R8 retrace fallback ordering: try line matches first, then fall back
// to mappings without a minified range.
// https://r8-review.googlesource.com/c/r8/+/72203/5
let frames = collect_no_minified_range_frames_with_line(
cache,
&frame,
members_slice,
self.outer_source_file,
)?;
return self.enqueue_frames(frames);
}
self.inner = Some((cache, frame, members));
mapped
} else {
Expand Down Expand Up @@ -1059,6 +1083,43 @@ fn collect_no_line_frames<'a>(
}
}

// Align with R8 retrace fallback ordering: try line matches first, then fall back
// to mappings without a minified range.
// https://r8-review.googlesource.com/c/r8/+/72203/5
fn collect_no_minified_range_frames_with_line<'a>(
cache: &ProguardCache<'a>,
frame: &StackFrame<'a>,
members: &[raw::Member],
outer_source_file: Option<&str>,
) -> Option<Vec<StackFrame<'a>>> {
let mut frames = Vec::new();
for member in members.iter().filter(|m| is_implicit_no_range(m)) {
if member.original_endline != u32::MAX
&& member.original_endline > member.original_startline
&& member.original_startline > 0
{
for line in member.original_startline..=member.original_endline {
let mut mapped = map_member_without_lines(cache, frame, member, outer_source_file)?;
mapped.line = line as usize;
frames.push(mapped);
}
continue;
}

let mut mapped = map_member_without_lines(cache, frame, member, outer_source_file)?;
if member.original_startline > 0 {
mapped.line = member.original_startline as usize;
}
frames.push(mapped);
}

if frames.is_empty() {
None
} else {
Some(frames)
}
}

enum NoLineSelection<'a> {
ExplicitBase(Vec<&'a raw::Member>, bool),
ImplicitNoRange(Vec<&'a raw::Member>, bool),
Expand Down
39 changes: 39 additions & 0 deletions src/mapper.rs
Original file line number Diff line number Diff line change
Expand Up @@ -280,6 +280,8 @@ fn collect_no_line_frames<'a>(
mapping_entries: &'a [MemberMapping<'a>],
collected: &mut CollectedFrames<'a>,
) {
// Align with R8 "no position" semantics: prefer base/no-range candidates rather than
// including all line-ranged mappings.
let selection = select_no_line_selection(mapping_entries);
let (candidates, suppress_line) = match selection {
NoLineSelection::ExplicitBase(candidates, suppress_line) => (candidates, suppress_line),
Expand All @@ -305,6 +307,36 @@ fn collect_no_line_frames<'a>(
}
}

// Align with R8 retrace fallback ordering: try line matches first, then fall back
// to mappings without a minified range.
// https://r8-review.googlesource.com/c/r8/+/72203/5
fn collect_no_minified_range_frames_with_line<'a>(
frame: &StackFrame<'a>,
mapping_entries: &'a [MemberMapping<'a>],
collected: &mut CollectedFrames<'a>,
) {
for member in mapping_entries.iter().filter(|m| is_implicit_no_range(m)) {
if let Some(original_endline) = member.original_endline {
if original_endline > member.original_startline && member.original_startline > 0 {
for line in member.original_startline..=original_endline {
let mut mapped = map_member_without_lines(frame, member);
mapped.line = line;
collected.frames.push(mapped);
collected.rewrite_rules.extend(member.rewrite_rules.iter());
}
continue;
}
}

let mut mapped = map_member_without_lines(frame, member);
if member.original_startline > 0 {
mapped.line = member.original_startline;
}
collected.frames.push(mapped);
collected.rewrite_rules.extend(member.rewrite_rules.iter());
}
}

fn collect_base_frames_with_line<'a>(
frame: &StackFrame<'a>,
mapping_entries: &'a [MemberMapping<'a>],
Expand Down Expand Up @@ -679,6 +711,9 @@ impl<'s> ProguardMapper<'s> {
// If the stacktrace has no line number, treat it as unknown and remap without
// applying line filters. If there are explicit 0:0 mappings, prefer those.
if frame.line == 0 {
// No-position lookup: do not include line-ranged mappings.
// R8: RetraceMethodResultImpl#narrowByPosition + filterOnNoPosition
// https://r8.googlesource.com/r8/+/main/src/main/java/com/android/tools/r8/retrace/internal/RetraceMethodResultImpl.java
collect_no_line_frames(&frame, mapping_entries, &mut collected);
return collected;
}
Expand All @@ -702,6 +737,10 @@ impl<'s> ProguardMapper<'s> {
}
}

if has_line_info && collected.frames.is_empty() {
collect_no_minified_range_frames_with_line(&frame, mapping_entries, &mut collected);
}

if collected.frames.is_empty() {
collected
.frames
Expand Down
6 changes: 4 additions & 2 deletions src/mapping.rs
Original file line number Diff line number Diff line change
Expand Up @@ -643,8 +643,10 @@ fn parse_proguard_field_or_method(
original_endline,
}),
// Preserve original line info even when no minified range is present.
// Use a sentinel to distinguish from an explicit 0:0 minified range, but keep
// an explicit 0:0 when the original line is also 0.
// Use a sentinel to distinguish from an explicit 0:0 minified range (R8 models
// this as a null minified range), but keep an explicit 0:0 when the original line
// is also 0.
// https://r8.googlesource.com/r8/+/main/src/main/java/com/android/tools/r8/naming/ClassNamingForNameMapper.java
(None, None, Some(original_startline)) => Some(LineMapping {
startline: if original_startline == 0 {
0
Expand Down
4 changes: 4 additions & 0 deletions src/utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,10 @@ pub(crate) fn resolve_no_line_output_line(
startline: usize,
endline: usize,
) -> usize {
// Explicit 0:0 minified ranges are preserved as real positions; sentinels are handled by
// callers by keeping frame_line when present.
// R8 keeps minified range presence explicit (null vs 0:0).
// https://r8.googlesource.com/r8/+/main/src/main/java/com/android/tools/r8/naming/ClassNamingForNameMapper.java
if startline == 0 && endline == 0 {
original_startline
.filter(|value| *value > 0 && *value != usize::MAX)
Expand Down
26 changes: 20 additions & 6 deletions tests/r8-method-overloading.rs
Original file line number Diff line number Diff line change
Expand Up @@ -84,16 +84,22 @@ const RETRACE_MAPPING_WITH_OVERLOADS_MAPPING: &str = r#"some.Class -> A:
"#;

#[test]
fn test_retrace_mapping_with_overloads_api_has_3_candidates() {
fn test_retrace_mapping_with_overloads_api_has_2_candidates_no_position() {
let mapper = ProguardMapper::from(RETRACE_MAPPING_WITH_OVERLOADS_MAPPING);

// Equivalent to upstream's `retracer.retraceClass(A).lookupMethod("a")` size == 3:
// - select(java.util.List)
// - sync() (line-mapped range)
// - cancel(java.lang.String[])
// For stacktrace remapping with no position (line == 0), align with R8's
// "no position" semantics: do not include line-ranged mappings.
let frame = StackFrame::new("A", "a", 0);
let remapped: Vec<_> = mapper.remap_frame(&frame).collect();
assert_eq!(remapped.len(), 3);
assert_eq!(remapped.len(), 2);

let mapping = ProguardMapping::new(RETRACE_MAPPING_WITH_OVERLOADS_MAPPING.as_bytes());
let mut buf = Vec::new();
ProguardCache::write(&mapping, &mut buf).unwrap();
let cache = ProguardCache::parse(&buf).unwrap();
cache.test();
let remapped: Vec<_> = cache.remap_frame(&frame).collect();
assert_eq!(remapped.len(), 2);
}

#[test]
Expand All @@ -104,4 +110,12 @@ fn test_retrace_mapping_with_overloads_api_includes_sync_with_line() {
let frame = StackFrame::new("A", "a", 3);
let remapped: Vec<_> = mapper.remap_frame(&frame).collect();
assert!(remapped.iter().any(|f| f.method() == "sync"));

let mapping = ProguardMapping::new(RETRACE_MAPPING_WITH_OVERLOADS_MAPPING.as_bytes());
let mut buf = Vec::new();
ProguardCache::write(&mapping, &mut buf).unwrap();
let cache = ProguardCache::parse(&buf).unwrap();
cache.test();
let remapped: Vec<_> = cache.remap_frame(&frame).collect();
assert!(remapped.iter().any(|f| f.method() == "sync"));
}