Skip to content

Commit 54fafc7

Browse files
authored
Merge branch 'master' into rz/feat/r8-tests-inline
2 parents 0823cb6 + 1398ff4 commit 54fafc7

File tree

3 files changed

+240
-9
lines changed

3 files changed

+240
-9
lines changed

CHANGELOG.md

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,18 @@
11
# Changelog
22

3+
## 5.9.0
4+
5+
### New Features ✨
6+
7+
#### R8
8+
9+
- feat(r8): Support `rewriteFrames` annotation in ProguardCache by @romtsn in [#67](https://github.com/getsentry/rust-proguard/pull/67)
10+
- feat(r8): Support `rewriteFrames` annotation in ProguardMapper by @romtsn in [#66](https://github.com/getsentry/rust-proguard/pull/66)
11+
12+
### Bug Fixes 🐛
13+
14+
- fix: Correctly remap entries with 0 line info by @romtsn in [#68](https://github.com/getsentry/rust-proguard/pull/68)
15+
316
## 5.8.1
417

518
- build: Update dependencies and set MSRV 1.83.0 by @loewenheim in [#65](https://github.com/getsentry/rust-proguard/pull/65)

Cargo.toml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
[package]
22
name = "proguard"
3-
version = "5.8.1"
3+
version = "5.9.0"
44
authors = ["Sentry <oss@sentry.io>"]
55
keywords = ["proguard", "retrace", "android", "r8"]
66
description = "Basic proguard mapping file handling for Rust"

src/cache/mod.rs

Lines changed: 226 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -321,6 +321,148 @@ impl<'data> ProguardCache<'data> {
321321
rules
322322
}
323323

324+
/// Finds member entries for a frame and collects rewrite rules without building frames.
325+
/// Returns (member_slice, prepared_frame, rewrite_rules, had_mappings).
326+
fn find_members_and_rules(
327+
&'data self,
328+
frame: &StackFrame<'data>,
329+
) -> Option<(
330+
&'data [raw::Member],
331+
StackFrame<'data>,
332+
Vec<RewriteRule<'data>>,
333+
bool,
334+
)> {
335+
let class = self.get_class(frame.class)?;
336+
let original_class = self
337+
.read_string(class.original_name_offset)
338+
.unwrap_or(frame.class);
339+
340+
let mut prepared_frame = frame.clone();
341+
prepared_frame.class = original_class;
342+
343+
let method_name = prepared_frame.method;
344+
let mapping_entries: &[raw::Member] = if let Some(parameters) = prepared_frame.parameters {
345+
let members = self.get_class_members_by_params(class)?;
346+
Self::find_range_by_binary_search(members, |m| {
347+
let Ok(obfuscated_name) = self.read_string(m.obfuscated_name_offset) else {
348+
return Ordering::Greater;
349+
};
350+
let params = if m.params_offset != u32::MAX {
351+
self.read_string(m.params_offset).unwrap_or_default()
352+
} else {
353+
""
354+
};
355+
(obfuscated_name, params).cmp(&(method_name, parameters))
356+
})?
357+
} else {
358+
let members = self.get_class_members(class)?;
359+
Self::find_range_by_binary_search(members, |m| {
360+
let Ok(obfuscated_name) = self.read_string(m.obfuscated_name_offset) else {
361+
return Ordering::Greater;
362+
};
363+
obfuscated_name.cmp(method_name)
364+
})?
365+
};
366+
367+
// Collect rewrite rules and check had_mappings by iterating members
368+
let mut rewrite_rules = Vec::new();
369+
let mut had_mappings = false;
370+
371+
if prepared_frame.parameters.is_none() {
372+
for member in mapping_entries {
373+
// Check if this member would produce a frame (line matching)
374+
if member.endline == 0
375+
|| (prepared_frame.line >= member.startline as usize
376+
&& prepared_frame.line <= member.endline as usize)
377+
{
378+
had_mappings = true;
379+
rewrite_rules.extend(self.decode_rewrite_rules(member));
380+
}
381+
}
382+
} else {
383+
// With parameters, all members match
384+
had_mappings = !mapping_entries.is_empty();
385+
for member in mapping_entries {
386+
rewrite_rules.extend(self.decode_rewrite_rules(member));
387+
}
388+
}
389+
390+
Some((mapping_entries, prepared_frame, rewrite_rules, had_mappings))
391+
}
392+
393+
/// Remaps a single Stackframe.
394+
///
395+
/// Returns zero or more [`StackFrame`]s, based on the information in
396+
/// the proguard mapping. This can return more than one frame in the case
397+
/// of inlined functions. In that case, frames are sorted top to bottom.
398+
pub fn remap_frame<'r: 'data>(
399+
&'r self,
400+
frame: &StackFrame<'data>,
401+
) -> RemappedFrameIter<'r, 'data> {
402+
let Some(class) = self.get_class(frame.class) else {
403+
return RemappedFrameIter::empty();
404+
};
405+
406+
for entry in entries {
407+
let mut conditions = Vec::new();
408+
if let Some(condition_components) = self.rewrite_rule_components.get(
409+
entry.conditions_offset as usize
410+
..entry.conditions_offset.saturating_add(entry.conditions_len) as usize,
411+
) {
412+
for component in condition_components {
413+
match component.kind {
414+
raw::REWRITE_CONDITION_THROWS => {
415+
if let Ok(descriptor) = self.read_string(component.value) {
416+
conditions.push(RewriteCondition::Throws(descriptor));
417+
}
418+
}
419+
raw::REWRITE_CONDITION_UNKNOWN => {
420+
if let Ok(value) = self.read_string(component.value) {
421+
conditions.push(RewriteCondition::Unknown(value));
422+
}
423+
}
424+
_ => {}
425+
}
426+
}
427+
}
428+
429+
let mut actions = Vec::new();
430+
if let Some(action_components) = self.rewrite_rule_components.get(
431+
entry.actions_offset as usize
432+
..entry.actions_offset.saturating_add(entry.actions_len) as usize,
433+
) {
434+
for component in action_components {
435+
match component.kind {
436+
raw::REWRITE_ACTION_REMOVE_INNER_FRAMES => {
437+
actions
438+
.push(RewriteAction::RemoveInnerFrames(component.value as usize));
439+
}
440+
raw::REWRITE_ACTION_UNKNOWN => {
441+
if let Ok(value) = self.read_string(component.value) {
442+
actions.push(RewriteAction::Unknown(value));
443+
}
444+
}
445+
_ => {}
446+
}
447+
}
448+
}
449+
450+
// Only add rules with at least one condition and one action,
451+
// matching the validation in parse_rewrite_rule.
452+
// This guards against corrupt cache data where read_string fails,
453+
// which would otherwise result in empty conditions that match
454+
// any exception due to vacuous truth in iter().all().
455+
if !conditions.is_empty() && !actions.is_empty() {
456+
rules.push(RewriteRule {
457+
conditions,
458+
actions,
459+
});
460+
}
461+
}
462+
463+
rules
464+
}
465+
324466
/// Finds member entries for a frame and collects rewrite rules without building frames.
325467
fn find_members_and_rules(
326468
&'data self,
@@ -450,6 +592,62 @@ impl<'data> ProguardCache<'data> {
450592
))
451593
}
452594

595+
/// Remaps a single stack frame through the complete processing pipeline.
596+
///
597+
/// This method combines:
598+
/// - Outline frame detection via [`is_outline_frame`](Self::is_outline_frame)
599+
/// - Frame preparation via [`prepare_frame_for_mapping`](Self::prepare_frame_for_mapping)
600+
/// - Lazy frame remapping with rewrite rules applied via skip_count
601+
///
602+
/// # Arguments
603+
/// * `frame` - The frame to remap
604+
/// * `exception_descriptor` - Optional exception descriptor for rewrite rules (e.g., `Ljava/lang/NullPointerException;`)
605+
/// * `apply_rewrite` - Whether to apply rewrite rules (typically true only for the first frame after an exception)
606+
/// * `carried_outline_pos` - Mutable reference to track outline position across frames
607+
///
608+
/// # Returns
609+
/// - `None` if this is an outline frame (caller should skip, `carried_outline_pos` is updated internally)
610+
/// - `Some(iterator)` with remapped frames. Use [`RemappedFrameIter::had_mappings`] after collecting
611+
/// to detect if rewrite rules cleared all frames (skip if `had_mappings() && collected.is_empty()`)
612+
pub fn remap_frame_with_context<'r>(
613+
&'r self,
614+
frame: &StackFrame<'data>,
615+
exception_descriptor: Option<&str>,
616+
apply_rewrite: bool,
617+
carried_outline_pos: &mut Option<usize>,
618+
) -> Option<RemappedFrameIter<'r, 'data>>
619+
where
620+
'r: 'data,
621+
{
622+
if self.is_outline_frame(frame.class, frame.method) {
623+
*carried_outline_pos = Some(frame.line);
624+
return None;
625+
}
626+
627+
let effective = self.prepare_frame_for_mapping(frame, carried_outline_pos);
628+
629+
let Some((members, prepared_frame, rewrite_rules, had_mappings)) =
630+
self.find_members_and_rules(&effective)
631+
else {
632+
return Some(RemappedFrameIter::empty());
633+
};
634+
635+
// Compute skip_count from rewrite rules
636+
let skip_count = if apply_rewrite {
637+
compute_skip_count(&rewrite_rules, exception_descriptor)
638+
} else {
639+
0
640+
};
641+
642+
Some(RemappedFrameIter::new(
643+
self,
644+
prepared_frame,
645+
members.iter(),
646+
skip_count,
647+
had_mappings,
648+
))
649+
}
650+
453651
/// Finds the range of elements of `members` for which `f(m) == Ordering::Equal`.
454652
///
455653
/// This works by first binary searching for any element fitting the criteria
@@ -634,15 +832,13 @@ impl<'data> ProguardCache<'data> {
634832
}
635833

636834
if let Some(frame) = stacktrace::parse_frame(line) {
637-
let Some(iter) = self.remap_frame(
835+
let Some(iter) = self.remap_frame_with_context(
638836
&frame,
639837
current_exception_descriptor.as_deref(),
640838
next_frame_can_rewrite,
641839
&mut carried_outline_pos,
642840
) else {
643-
// Outline frame, skip
644-
next_frame_can_rewrite = false;
645-
current_exception_descriptor = None;
841+
// Outline frame, skip (preserve next_frame_can_rewrite for the next real frame)
646842
continue;
647843
};
648844

@@ -701,14 +897,13 @@ impl<'data> ProguardCache<'data> {
701897
let mut frames = Vec::with_capacity(trace.frames.len());
702898
let mut next_frame_can_rewrite = exception_descriptor.is_some();
703899
for f in trace.frames.iter() {
704-
let Some(iter) = self.remap_frame(
900+
let Some(iter) = self.remap_frame_with_context(
705901
f,
706902
exception_descriptor.as_deref(),
707903
next_frame_can_rewrite,
708904
&mut carried_outline_pos,
709905
) else {
710-
// Outline frame, skip
711-
next_frame_can_rewrite = false;
906+
// Outline frame, skip (preserve next_frame_can_rewrite for the next real frame)
712907
continue;
713908
};
714909
next_frame_can_rewrite = false;
@@ -775,7 +970,7 @@ impl<'r, 'data> RemappedFrameIter<'r, 'data> {
775970
}
776971
}
777972

778-
fn new(
973+
fn members(
779974
cache: &'r ProguardCache<'data>,
780975
frame: StackFrame<'data>,
781976
members: std::slice::Iter<'data, raw::Member>,
@@ -807,6 +1002,29 @@ impl<'r, 'data> RemappedFrameIter<'r, 'data> {
8071002
iterate_without_lines(cache, frame, members, self.outer_source_file)
8081003
}
8091004
}
1005+
1006+
fn new(
1007+
cache: &'r ProguardCache<'data>,
1008+
frame: StackFrame<'data>,
1009+
members: std::slice::Iter<'data, raw::Member>,
1010+
skip_count: usize,
1011+
had_mappings: bool,
1012+
) -> Self {
1013+
Self {
1014+
inner: Some((cache, frame, members)),
1015+
skip_count,
1016+
had_mappings,
1017+
}
1018+
}
1019+
1020+
fn next(&mut self) -> Option<Self::Item> {
1021+
// Lazily skip rewrite-removed frames
1022+
while self.skip_count > 0 {
1023+
self.skip_count -= 1;
1024+
self.next_inner()?;
1025+
}
1026+
self.next_inner()
1027+
}
8101028
}
8111029

8121030
impl<'data> Iterator for RemappedFrameIter<'_, 'data> {

0 commit comments

Comments
 (0)