From 24a5ed07cdc0908644862daf7aa3fb4c91c80330 Mon Sep 17 00:00:00 2001 From: Roman Zavarnitsyn Date: Fri, 30 Jan 2026 15:13:34 +0100 Subject: [PATCH 1/2] fix(r8-tests): Fix overload lookup to include line-ranged candidates without file --- src/cache/mod.rs | 22 +++++++++++++++++ src/mapper.rs | 64 ++++++++++++++++++++++++++++++++++++++++++++---- 2 files changed, 81 insertions(+), 5 deletions(-) diff --git a/src/cache/mod.rs b/src/cache/mod.rs index 830de97..586f669 100644 --- a/src/cache/mod.rs +++ b/src/cache/mod.rs @@ -74,6 +74,7 @@ mod raw; use std::borrow::Cow; use std::cmp::Ordering; +use std::collections::HashSet; use std::fmt::Write; use thiserror::Error; @@ -971,6 +972,7 @@ impl<'r, 'data> RemappedFrameIter<'r, 'data> { &frame, members.as_slice(), self.outer_source_file, + frame.file().is_none(), )?; return self.enqueue_frames(frames); } @@ -1026,7 +1028,27 @@ fn collect_no_line_frames<'a>( frame: &StackFrame<'a>, members: &[raw::Member], outer_source_file: Option<&str>, + include_line_ranged: bool, ) -> Option>> { + if include_line_ranged { + let mut seen = HashSet::new(); + let mut frames = Vec::new(); + for member in members { + if !seen.insert((member.original_class_offset, member.original_name_offset)) { + continue; + } + let mut mapped = map_member_without_lines(cache, frame, member, outer_source_file)?; + mapped.line = 0; + frames.push(mapped); + } + + return if frames.is_empty() { + None + } else { + Some(frames) + }; + } + let selection = select_no_line_selection(members); let (candidates, suppress_line) = match selection { NoLineSelection::ExplicitBase(candidates, suppress_line) => (candidates, suppress_line), diff --git a/src/mapper.rs b/src/mapper.rs index ee84d5a..60549de 100644 --- a/src/mapper.rs +++ b/src/mapper.rs @@ -1,5 +1,5 @@ use std::borrow::Cow; -use std::collections::HashMap; +use std::collections::{HashMap, HashSet}; use std::fmt; use std::fmt::{Error as FmtError, Write}; use std::iter::FusedIterator; @@ -121,6 +121,8 @@ fn has_line_range(member: &MemberMapping<'_>) -> bool { pub struct RemappedFrameIter<'m> { inner: Option<(StackFrame<'m>, MemberIter<'m>)>, has_line_info: bool, + include_line_ranged_no_file: bool, + seen_no_line: HashSet<(&'m str, &'m str)>, } impl<'m> RemappedFrameIter<'m> { @@ -128,12 +130,21 @@ impl<'m> RemappedFrameIter<'m> { Self { inner: None, has_line_info: false, + include_line_ranged_no_file: false, + seen_no_line: HashSet::new(), } } - fn members(frame: StackFrame<'m>, members: MemberIter<'m>, has_line_info: bool) -> Self { + fn members( + frame: StackFrame<'m>, + members: MemberIter<'m>, + has_line_info: bool, + include_line_ranged_no_file: bool, + ) -> Self { Self { inner: Some((frame, members)), has_line_info, + include_line_ranged_no_file, + seen_no_line: HashSet::new(), } } } @@ -143,13 +154,34 @@ impl<'m> Iterator for RemappedFrameIter<'m> { fn next(&mut self) -> Option { let (frame, ref mut members) = self.inner.as_mut()?; if frame.parameters.is_none() { - iterate_with_lines(frame, members, self.has_line_info) + if self.include_line_ranged_no_file && frame.line == 0 { + iterate_with_lines_no_file(frame, members, &mut self.seen_no_line) + } else { + iterate_with_lines(frame, members, self.has_line_info) + } } else { iterate_without_lines(frame, members) } } } +fn iterate_with_lines_no_file<'a>( + frame: &StackFrame<'a>, + members: &mut MemberIter<'a>, + seen: &mut HashSet<(&'a str, &'a str)>, +) -> Option> { + for member in members { + let class = member.original_class.unwrap_or(frame.class); + if !seen.insert((class, member.original)) { + continue; + } + let mut mapped = map_member_without_lines(frame, member); + mapped.line = 0; + return Some(mapped); + } + None +} + fn map_member_with_lines<'a>( frame: &StackFrame<'a>, member: &MemberMapping<'a>, @@ -279,7 +311,23 @@ fn collect_no_line_frames<'a>( frame: &StackFrame<'a>, mapping_entries: &'a [MemberMapping<'a>], collected: &mut CollectedFrames<'a>, + include_line_ranged: bool, ) { + if include_line_ranged { + let mut seen = HashSet::new(); + for member in mapping_entries { + let class = member.original_class.unwrap_or(frame.class); + if !seen.insert((class, member.original)) { + continue; + } + let mut mapped = map_member_without_lines(frame, member); + mapped.line = 0; + collected.frames.push(mapped); + collected.rewrite_rules.extend(member.rewrite_rules.iter()); + } + return; + } + let selection = select_no_line_selection(mapping_entries); let (candidates, suppress_line) = match selection { NoLineSelection::ExplicitBase(candidates, suppress_line) => (candidates, suppress_line), @@ -679,7 +727,12 @@ 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 { - collect_no_line_frames(&frame, mapping_entries, &mut collected); + collect_no_line_frames( + &frame, + mapping_entries, + &mut collected, + frame.file().is_none(), + ); return collected; } @@ -758,6 +811,7 @@ impl<'s> ProguardMapper<'s> { return RemappedFrameIter::empty(); }; + let include_line_ranged_no_file = frame.line == 0 && frame.file().is_none(); let mut frame = frame.clone(); frame.class = class.original; @@ -772,7 +826,7 @@ impl<'s> ProguardMapper<'s> { }; let has_line_info = members.all_mappings.iter().any(|m| has_line_range(m)); - RemappedFrameIter::members(frame, mappings, has_line_info) + RemappedFrameIter::members(frame, mappings, has_line_info, include_line_ranged_no_file) } /// Remaps a throwable which is the first line of a full stacktrace. From 000de2248c151ab973e4d6ca62b226818e289ed6 Mon Sep 17 00:00:00 2001 From: Roman Zavarnitsyn Date: Tue, 3 Feb 2026 17:52:59 +0100 Subject: [PATCH 2/2] =?UTF-8?q?Align=20retrace=20no=E2=80=91position=20han?= =?UTF-8?q?dling=20with=20R8=20semantics?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- src/builder.rs | 3 +- src/cache/mod.rs | 83 +++++++++++++++++++------- src/mapper.rs | 103 ++++++++++++++------------------- src/mapping.rs | 6 +- src/utils.rs | 4 ++ tests/r8-method-overloading.rs | 26 +++++++-- 6 files changed, 135 insertions(+), 90 deletions(-) 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 586f669..fdf72f5 100644 --- a/src/cache/mod.rs +++ b/src/cache/mod.rs @@ -74,7 +74,6 @@ mod raw; use std::borrow::Cow; use std::cmp::Ordering; -use std::collections::HashSet; use std::fmt::Write; use thiserror::Error; @@ -899,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> { @@ -915,6 +916,7 @@ impl<'r, 'data> RemappedFrameIter<'r, 'data> { had_mappings: false, has_line_info: false, outer_source_file: None, + had_line_match: false, } } @@ -934,6 +936,7 @@ impl<'r, 'data> RemappedFrameIter<'r, 'data> { had_mappings, has_line_info, outer_source_file, + had_line_match: false, } } @@ -945,6 +948,7 @@ impl<'r, 'data> RemappedFrameIter<'r, 'data> { had_mappings: false, has_line_info: false, outer_source_file: None, + had_line_match: false, } } @@ -966,13 +970,14 @@ 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, &frame, members.as_slice(), self.outer_source_file, - frame.file().is_none(), )?; return self.enqueue_frames(frames); } @@ -988,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, @@ -995,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 { @@ -1028,27 +1050,7 @@ fn collect_no_line_frames<'a>( frame: &StackFrame<'a>, members: &[raw::Member], outer_source_file: Option<&str>, - include_line_ranged: bool, ) -> Option>> { - if include_line_ranged { - let mut seen = HashSet::new(); - let mut frames = Vec::new(); - for member in members { - if !seen.insert((member.original_class_offset, member.original_name_offset)) { - continue; - } - let mut mapped = map_member_without_lines(cache, frame, member, outer_source_file)?; - mapped.line = 0; - frames.push(mapped); - } - - return if frames.is_empty() { - None - } else { - Some(frames) - }; - } - let selection = select_no_line_selection(members); let (candidates, suppress_line) = match selection { NoLineSelection::ExplicitBase(candidates, suppress_line) => (candidates, suppress_line), @@ -1081,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 60549de..23c27d8 100644 --- a/src/mapper.rs +++ b/src/mapper.rs @@ -1,5 +1,5 @@ use std::borrow::Cow; -use std::collections::{HashMap, HashSet}; +use std::collections::HashMap; use std::fmt; use std::fmt::{Error as FmtError, Write}; use std::iter::FusedIterator; @@ -121,8 +121,6 @@ fn has_line_range(member: &MemberMapping<'_>) -> bool { pub struct RemappedFrameIter<'m> { inner: Option<(StackFrame<'m>, MemberIter<'m>)>, has_line_info: bool, - include_line_ranged_no_file: bool, - seen_no_line: HashSet<(&'m str, &'m str)>, } impl<'m> RemappedFrameIter<'m> { @@ -130,21 +128,12 @@ impl<'m> RemappedFrameIter<'m> { Self { inner: None, has_line_info: false, - include_line_ranged_no_file: false, - seen_no_line: HashSet::new(), } } - fn members( - frame: StackFrame<'m>, - members: MemberIter<'m>, - has_line_info: bool, - include_line_ranged_no_file: bool, - ) -> Self { + fn members(frame: StackFrame<'m>, members: MemberIter<'m>, has_line_info: bool) -> Self { Self { inner: Some((frame, members)), has_line_info, - include_line_ranged_no_file, - seen_no_line: HashSet::new(), } } } @@ -154,34 +143,13 @@ impl<'m> Iterator for RemappedFrameIter<'m> { fn next(&mut self) -> Option { let (frame, ref mut members) = self.inner.as_mut()?; if frame.parameters.is_none() { - if self.include_line_ranged_no_file && frame.line == 0 { - iterate_with_lines_no_file(frame, members, &mut self.seen_no_line) - } else { - iterate_with_lines(frame, members, self.has_line_info) - } + iterate_with_lines(frame, members, self.has_line_info) } else { iterate_without_lines(frame, members) } } } -fn iterate_with_lines_no_file<'a>( - frame: &StackFrame<'a>, - members: &mut MemberIter<'a>, - seen: &mut HashSet<(&'a str, &'a str)>, -) -> Option> { - for member in members { - let class = member.original_class.unwrap_or(frame.class); - if !seen.insert((class, member.original)) { - continue; - } - let mut mapped = map_member_without_lines(frame, member); - mapped.line = 0; - return Some(mapped); - } - None -} - fn map_member_with_lines<'a>( frame: &StackFrame<'a>, member: &MemberMapping<'a>, @@ -311,23 +279,9 @@ fn collect_no_line_frames<'a>( frame: &StackFrame<'a>, mapping_entries: &'a [MemberMapping<'a>], collected: &mut CollectedFrames<'a>, - include_line_ranged: bool, ) { - if include_line_ranged { - let mut seen = HashSet::new(); - for member in mapping_entries { - let class = member.original_class.unwrap_or(frame.class); - if !seen.insert((class, member.original)) { - continue; - } - let mut mapped = map_member_without_lines(frame, member); - mapped.line = 0; - collected.frames.push(mapped); - collected.rewrite_rules.extend(member.rewrite_rules.iter()); - } - return; - } - + // 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), @@ -353,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>], @@ -727,12 +711,10 @@ 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 { - collect_no_line_frames( - &frame, - mapping_entries, - &mut collected, - frame.file().is_none(), - ); + // 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; } @@ -755,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 @@ -811,7 +797,6 @@ impl<'s> ProguardMapper<'s> { return RemappedFrameIter::empty(); }; - let include_line_ranged_no_file = frame.line == 0 && frame.file().is_none(); let mut frame = frame.clone(); frame.class = class.original; @@ -826,7 +811,7 @@ impl<'s> ProguardMapper<'s> { }; let has_line_info = members.all_mappings.iter().any(|m| has_line_range(m)); - RemappedFrameIter::members(frame, mappings, has_line_info, include_line_ranged_no_file) + RemappedFrameIter::members(frame, mappings, has_line_info) } /// Remaps a throwable which is the first line of a full stacktrace. 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")); }