Skip to content

Commit 0823cb6

Browse files
authored
fix(r8): Synthesize file name for foreign members and classes without sourceFile annotation (#70)
From the test cases in #69 it seems r8-retrace always synthesizes filenames from class names, so this PR attempts to do the same for mapper/cache and also updates existing test cases to match the new behavior (the tests in `r8-inline.rs` should be passing with this change)
1 parent 4b77917 commit 0823cb6

File tree

9 files changed

+282
-161
lines changed

9 files changed

+282
-161
lines changed

src/cache/mod.rs

Lines changed: 89 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -72,6 +72,7 @@
7272
mod debug;
7373
mod raw;
7474

75+
use std::borrow::Cow;
7576
use std::cmp::Ordering;
7677
use std::fmt::Write;
7778

@@ -83,6 +84,16 @@ use crate::{java, stacktrace, DeobfuscatedSignature, StackFrame, StackTrace, Thr
8384

8485
pub use raw::{ProguardCache, PRGCACHE_VERSION};
8586

87+
/// Result of looking up member mappings for a frame.
88+
/// Contains: (members, prepared_frame, rewrite_rules, had_mappings, outer_source_file)
89+
type MemberLookupResult<'data> = (
90+
&'data [raw::Member],
91+
StackFrame<'data>,
92+
Vec<RewriteRule<'data>>,
93+
bool,
94+
Option<&'data str>,
95+
);
96+
8697
/// Errors returned while loading/parsing a serialized [`ProguardCache`].
8798
///
8899
/// After a `ProguardCache` was successfully parsed via [`ProguardCache::parse`], an Error that occurs during
@@ -311,21 +322,18 @@ impl<'data> ProguardCache<'data> {
311322
}
312323

313324
/// Finds member entries for a frame and collects rewrite rules without building frames.
314-
/// Returns (member_slice, prepared_frame, rewrite_rules, had_mappings).
315325
fn find_members_and_rules(
316326
&'data self,
317327
frame: &StackFrame<'data>,
318-
) -> Option<(
319-
&'data [raw::Member],
320-
StackFrame<'data>,
321-
Vec<RewriteRule<'data>>,
322-
bool,
323-
)> {
328+
) -> Option<MemberLookupResult<'data>> {
324329
let class = self.get_class(frame.class)?;
325330
let original_class = self
326331
.read_string(class.original_name_offset)
327332
.unwrap_or(frame.class);
328333

334+
// Get the outer source file for synthesis
335+
let outer_source_file = self.read_string(class.file_name_offset).ok();
336+
329337
let mut prepared_frame = frame.clone();
330338
prepared_frame.class = original_class;
331339

@@ -376,7 +384,13 @@ impl<'data> ProguardCache<'data> {
376384
}
377385
}
378386

379-
Some((mapping_entries, prepared_frame, rewrite_rules, had_mappings))
387+
Some((
388+
mapping_entries,
389+
prepared_frame,
390+
rewrite_rules,
391+
had_mappings,
392+
outer_source_file,
393+
))
380394
}
381395

382396
/// Remaps a single stack frame through the complete processing pipeline.
@@ -413,7 +427,7 @@ impl<'data> ProguardCache<'data> {
413427

414428
let effective = self.prepare_frame_for_mapping(frame, carried_outline_pos);
415429

416-
let Some((members, prepared_frame, rewrite_rules, had_mappings)) =
430+
let Some((members, prepared_frame, rewrite_rules, had_mappings, outer_source_file)) =
417431
self.find_members_and_rules(&effective)
418432
else {
419433
return Some(RemappedFrameIter::empty());
@@ -432,6 +446,7 @@ impl<'data> ProguardCache<'data> {
432446
members.iter(),
433447
skip_count,
434448
had_mappings,
449+
outer_source_file,
435450
))
436451
}
437452

@@ -746,6 +761,8 @@ pub struct RemappedFrameIter<'r, 'data> {
746761
skip_count: usize,
747762
/// Whether there were mapping entries (for should_skip determination).
748763
had_mappings: bool,
764+
/// The source file of the outer class for synthesis.
765+
outer_source_file: Option<&'data str>,
749766
}
750767

751768
impl<'r, 'data> RemappedFrameIter<'r, 'data> {
@@ -754,6 +771,7 @@ impl<'r, 'data> RemappedFrameIter<'r, 'data> {
754771
inner: None,
755772
skip_count: 0,
756773
had_mappings: false,
774+
outer_source_file: None,
757775
}
758776
}
759777

@@ -763,11 +781,13 @@ impl<'r, 'data> RemappedFrameIter<'r, 'data> {
763781
members: std::slice::Iter<'data, raw::Member>,
764782
skip_count: usize,
765783
had_mappings: bool,
784+
outer_source_file: Option<&'data str>,
766785
) -> Self {
767786
Self {
768787
inner: Some((cache, frame, members)),
769788
skip_count,
770789
had_mappings,
790+
outer_source_file,
771791
}
772792
}
773793

@@ -782,9 +802,9 @@ impl<'r, 'data> RemappedFrameIter<'r, 'data> {
782802
fn next_inner(&mut self) -> Option<StackFrame<'data>> {
783803
let (cache, frame, members) = self.inner.as_mut()?;
784804
if frame.parameters.is_none() {
785-
iterate_with_lines(cache, frame, members)
805+
iterate_with_lines(cache, frame, members, self.outer_source_file)
786806
} else {
787-
iterate_without_lines(cache, frame, members)
807+
iterate_without_lines(cache, frame, members, self.outer_source_file)
788808
}
789809
}
790810
}
@@ -806,6 +826,7 @@ fn iterate_with_lines<'a>(
806826
cache: &ProguardCache<'a>,
807827
frame: &mut StackFrame<'a>,
808828
members: &mut std::slice::Iter<'_, raw::Member>,
829+
outer_source_file: Option<&str>,
809830
) -> Option<StackFrame<'a>> {
810831
for member in members {
811832
// skip any members which do not match our frames line
@@ -814,7 +835,7 @@ fn iterate_with_lines<'a>(
814835
{
815836
continue;
816837
}
817-
// parents of inlined frames dont have an `endline`, and
838+
// parents of inlined frames don't have an `endline`, and
818839
// the top inlined frame need to be correctly offset.
819840
let line = if member.original_endline == u32::MAX
820841
|| member.original_endline == member.original_startline
@@ -828,22 +849,19 @@ fn iterate_with_lines<'a>(
828849
.read_string(member.original_class_offset)
829850
.unwrap_or(frame.class);
830851

831-
let file = if member.original_file_offset != u32::MAX {
852+
let file: Option<Cow<'_, str>> = if member.original_file_offset != u32::MAX {
832853
let Ok(file_name) = cache.read_string(member.original_file_offset) else {
833854
continue;
834855
};
835856

836857
if file_name == "R8$$SyntheticClass" {
837-
extract_class_name(class)
858+
extract_class_name(class).map(Cow::Borrowed)
838859
} else {
839-
Some(file_name)
860+
Some(Cow::Borrowed(file_name))
840861
}
841-
} else if member.original_class_offset != u32::MAX {
842-
// when an inlined function is from a foreign class, we
843-
// don’t know the file it is defined in.
844-
None
845862
} else {
846-
frame.file
863+
// Synthesize from class name (input filename is not reliable)
864+
synthesize_source_file(class, outer_source_file).map(Cow::Owned)
847865
};
848866

849867
let Ok(method) = cache.read_string(member.original_name_offset) else {
@@ -866,6 +884,7 @@ fn iterate_without_lines<'a>(
866884
cache: &ProguardCache<'a>,
867885
frame: &mut StackFrame<'a>,
868886
members: &mut std::slice::Iter<'_, raw::Member>,
887+
outer_source_file: Option<&str>,
869888
) -> Option<StackFrame<'a>> {
870889
let member = members.next()?;
871890

@@ -875,10 +894,13 @@ fn iterate_without_lines<'a>(
875894

876895
let method = cache.read_string(member.original_name_offset).ok()?;
877896

897+
// Synthesize from class name (input filename is not reliable)
898+
let file = synthesize_source_file(class, outer_source_file).map(Cow::Owned);
899+
878900
Some(StackFrame {
879901
class,
880902
method,
881-
file: None,
903+
file,
882904
line: 0,
883905
parameters: frame.parameters,
884906
method_synthesized: member.is_synthesized(),
@@ -891,6 +913,31 @@ fn extract_class_name(full_path: &str) -> Option<&str> {
891913
after_last_period.split('$').next()
892914
}
893915

916+
/// Synthesizes a source file name from a class name.
917+
/// For Kotlin top-level classes ending in "Kt", the suffix is stripped and ".kt" is used.
918+
/// Otherwise, the extension is derived from the reference file, defaulting to ".java".
919+
/// For example: ("com.example.MainKt", Some("Other.java")) -> "Main.kt" (Kt suffix takes precedence)
920+
/// For example: ("com.example.Main", Some("Other.kt")) -> "Main.kt"
921+
/// For example: ("com.example.MainKt", None) -> "Main.kt"
922+
/// For inner classes: ("com.example.Main$Inner", None) -> "Main.java"
923+
fn synthesize_source_file(class_name: &str, reference_file: Option<&str>) -> Option<String> {
924+
let base = extract_class_name(class_name)?;
925+
926+
// For Kotlin top-level classes (ending in "Kt"), always use .kt extension and strip suffix
927+
// This takes precedence over reference_file since Kt suffix is a strong Kotlin indicator
928+
if base.ends_with("Kt") && base.len() > 2 {
929+
let kotlin_base = &base[..base.len() - 2];
930+
return Some(format!("{}.kt", kotlin_base));
931+
}
932+
933+
// If we have a reference file, derive extension from it
934+
if let Some(ext) = reference_file.and_then(|f| f.rfind('.').map(|pos| &f[pos..])) {
935+
return Some(format!("{}{}", base, ext));
936+
}
937+
938+
Some(format!("{}.java", base))
939+
}
940+
894941
/// Converts a Java class name to its JVM descriptor format.
895942
///
896943
/// For example, `java.lang.NullPointerException` becomes `Ljava/lang/NullPointerException;`.
@@ -929,6 +976,8 @@ fn compute_skip_count(rewrite_rules: &[RewriteRule<'_>], thrown_descriptor: Opti
929976

930977
#[cfg(test)]
931978
mod tests {
979+
use std::borrow::Cow;
980+
932981
use crate::{ProguardMapping, StackFrame, StackTrace, Throwable};
933982

934983
use super::raw::ProguardCache;
@@ -955,15 +1004,15 @@ com.example.MainFragment$onActivityCreated$4 -> com.example.MainFragment$g:
9551004
class: "com.example.MainFragment$g",
9561005
method: "onClick",
9571006
line: 2,
958-
file: Some("SourceFile"),
1007+
file: Some(Cow::Borrowed("SourceFile")),
9591008
parameters: None,
9601009
method_synthesized: false,
9611010
},
9621011
StackFrame {
9631012
class: "android.view.View",
9641013
method: "performClick",
9651014
line: 7393,
966-
file: Some("View.java"),
1015+
file: Some(Cow::Borrowed("View.java")),
9671016
parameters: None,
9681017
method_synthesized: false,
9691018
},
@@ -977,7 +1026,7 @@ com.example.MainFragment$onActivityCreated$4 -> com.example.MainFragment$g:
9771026
class: "com.example.MainFragment$g",
9781027
method: "onClick",
9791028
line: 1,
980-
file: Some("SourceFile"),
1029+
file: Some(Cow::Borrowed("SourceFile")),
9811030
parameters: None,
9821031
method_synthesized: false,
9831032
}],
@@ -986,13 +1035,13 @@ com.example.MainFragment$onActivityCreated$4 -> com.example.MainFragment$g:
9861035
};
9871036
let expect = "\
9881037
com.example.MainFragment$RocketException: Crash!
989-
at com.example.MainFragment$Rocket.fly(<unknown>:85)
990-
at com.example.MainFragment$onActivityCreated$4.onClick(SourceFile:65)
1038+
at com.example.MainFragment$Rocket.fly(MainFragment.java:85)
1039+
at com.example.MainFragment$onActivityCreated$4.onClick(MainFragment.java:65)
9911040
at android.view.View.performClick(View.java:7393)
9921041
Caused by: com.example.MainFragment$EngineFailureException: Engines overheating
993-
at com.example.MainFragment$Rocket.startEngines(<unknown>:90)
994-
at com.example.MainFragment$Rocket.fly(<unknown>:83)
995-
at com.example.MainFragment$onActivityCreated$4.onClick(SourceFile:65)\n";
1042+
at com.example.MainFragment$Rocket.startEngines(MainFragment.java:90)
1043+
at com.example.MainFragment$Rocket.fly(MainFragment.java:83)
1044+
at com.example.MainFragment$onActivityCreated$4.onClick(MainFragment.java:65)\n";
9961045

9971046
let mapping = ProguardMapping::new(mapping.as_bytes());
9981047
let mut cache = Vec::new();
@@ -1031,13 +1080,13 @@ Caused by: com.example.MainFragment$d: Engines overheating
10311080

10321081
let expect = "\
10331082
com.example.MainFragment$RocketException: Crash!
1034-
at com.example.MainFragment$Rocket.fly(<unknown>:85)
1035-
at com.example.MainFragment$onActivityCreated$4.onClick(SourceFile:65)
1083+
at com.example.MainFragment$Rocket.fly(MainFragment.java:85)
1084+
at com.example.MainFragment$onActivityCreated$4.onClick(MainFragment.java:65)
10361085
at android.view.View.performClick(View.java:7393)
10371086
Caused by: com.example.MainFragment$EngineFailureException: Engines overheating
1038-
at com.example.MainFragment$Rocket.startEngines(<unknown>:90)
1039-
at com.example.MainFragment$Rocket.fly(<unknown>:83)
1040-
at com.example.MainFragment$onActivityCreated$4.onClick(SourceFile:65)
1087+
at com.example.MainFragment$Rocket.startEngines(MainFragment.java:90)
1088+
at com.example.MainFragment$Rocket.fly(MainFragment.java:83)
1089+
at com.example.MainFragment$onActivityCreated$4.onClick(MainFragment.java:65)
10411090
... 13 more\n";
10421091

10431092
let mapping = ProguardMapping::new(mapping.as_bytes());
@@ -1069,7 +1118,7 @@ java.lang.NullPointerException: Boom
10691118
at a.a(SourceFile:4)";
10701119
let expect = "\
10711120
java.lang.NullPointerException: Boom
1072-
at some.Class.caller(SourceFile:7)
1121+
at some.Class.caller(Class.java:7)
10731122
";
10741123

10751124
assert_eq!(cache.remap_stacktrace(input).unwrap(), expect);
@@ -1094,7 +1143,7 @@ java.lang.NullPointerException: Boom
10941143
at a.call(SourceFile:4)";
10951144
let expected_npe = "\
10961145
java.lang.NullPointerException: Boom
1097-
at some.Class.outer(SourceFile:7)
1146+
at some.Class.outer(Class.java:7)
10981147
";
10991148
assert_eq!(cache.remap_stacktrace(input_npe).unwrap(), expected_npe);
11001149

@@ -1103,7 +1152,7 @@ java.lang.IllegalStateException: Boom
11031152
at a.call(SourceFile:4)";
11041153
let expected_ise = "\
11051154
java.lang.IllegalStateException: Boom
1106-
at some.Class.outer(SourceFile:7)
1155+
at some.Class.outer(Class.java:7)
11071156
";
11081157
assert_eq!(cache.remap_stacktrace(input_ise).unwrap(), expected_ise);
11091158
}
@@ -1135,7 +1184,7 @@ java.lang.NullPointerException: Boom
11351184
// not replaced with the original "at a.call(SourceFile:4)"
11361185
let expected = "\
11371186
java.lang.NullPointerException: Boom
1138-
at some.Other.method(SourceFile:30)
1187+
at some.Other.method(Other.java:30)
11391188
";
11401189

11411190
let actual = cache.remap_stacktrace(input).unwrap();
@@ -1169,15 +1218,15 @@ some.Other -> b:
11691218
class: "a",
11701219
method: "call",
11711220
line: 4,
1172-
file: Some("SourceFile"),
1221+
file: Some(Cow::Borrowed("SourceFile")),
11731222
parameters: None,
11741223
method_synthesized: false,
11751224
},
11761225
StackFrame {
11771226
class: "b",
11781227
method: "run",
11791228
line: 5,
1180-
file: Some("SourceFile"),
1229+
file: Some(Cow::Borrowed("SourceFile")),
11811230
parameters: None,
11821231
method_synthesized: false,
11831232
},

src/cache/raw.rs

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -313,15 +313,16 @@ impl<'data> ProguardCache<'data> {
313313
.map(|(obfuscated, original)| {
314314
let obfuscated_name_offset = string_table.insert(obfuscated.as_str()) as u32;
315315
let original_name_offset = string_table.insert(original.as_str()) as u32;
316-
let is_synthesized = parsed
317-
.class_infos
318-
.get(original)
319-
.map(|ci| ci.is_synthesized)
320-
.unwrap_or_default();
316+
let class_info = parsed.class_infos.get(original);
317+
let is_synthesized = class_info.map(|ci| ci.is_synthesized).unwrap_or_default();
318+
let file_name_offset = class_info
319+
.and_then(|ci| ci.source_file)
320+
.map_or(u32::MAX, |s| string_table.insert(s) as u32);
321321
let class = ClassInProgress {
322322
class: Class {
323323
original_name_offset,
324324
obfuscated_name_offset,
325+
file_name_offset,
325326
is_synthesized: is_synthesized as u8,
326327
..Default::default()
327328
},

src/lib.rs

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -25,10 +25,11 @@
2525
//! mapper
2626
//! .remap_frame(&proguard::StackFrame::new("a.a.a.b.c", "a", 13))
2727
//! .collect::<Vec<_>>(),
28-
//! vec![proguard::StackFrame::new(
28+
//! vec![proguard::StackFrame::with_file(
2929
//! "android.arch.core.internal.SafeIterableMap",
3030
//! "eldest",
31-
//! 168
31+
//! 168,
32+
//! "SafeIterableMap.java"
3233
//! )],
3334
//! );
3435
//! ```

0 commit comments

Comments
 (0)