diff --git a/src/builder.rs b/src/builder.rs index 37a8448..9e2d68b 100644 --- a/src/builder.rs +++ b/src/builder.rs @@ -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| { diff --git a/src/cache/mod.rs b/src/cache/mod.rs index 830de97..fdf72f5 100644 --- a/src/cache/mod.rs +++ b/src/cache/mod.rs @@ -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> { @@ -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, } } @@ -933,6 +936,7 @@ impl<'r, 'data> RemappedFrameIter<'r, 'data> { had_mappings, has_line_info, outer_source_file, + had_line_match: false, } } @@ -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, } } @@ -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, @@ -986,6 +993,7 @@ 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, @@ -993,6 +1001,22 @@ impl<'r, 'data> RemappedFrameIter<'r, 'data> { 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 { @@ -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>> { + 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), diff --git a/src/mapper.rs b/src/mapper.rs index ee84d5a..23c27d8 100644 --- a/src/mapper.rs +++ b/src/mapper.rs @@ -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), @@ -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>], @@ -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; } @@ -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 diff --git a/src/mapping.rs b/src/mapping.rs index 3fdc28e..af4493f 100644 --- a/src/mapping.rs +++ b/src/mapping.rs @@ -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 diff --git a/src/utils.rs b/src/utils.rs index c0cc8ea..d7b6e5d 100644 --- a/src/utils.rs +++ b/src/utils.rs @@ -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) diff --git a/tests/r8-method-overloading.rs b/tests/r8-method-overloading.rs index 14e0123..547610f 100644 --- a/tests/r8-method-overloading.rs +++ b/tests/r8-method-overloading.rs @@ -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] @@ -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")); }