Skip to content

Commit 4d2873c

Browse files
loewenheimloewenheim
committed
Ref and fix test
1 parent 1e75759 commit 4d2873c

1 file changed

Lines changed: 39 additions & 85 deletions

File tree

‎crates/symbolicator-proguard/src/symbolication.rs‎

Lines changed: 39 additions & 85 deletions
Original file line numberDiff line numberDiff line change
@@ -145,19 +145,19 @@ impl ProguardService {
145145
let mut remapped_stacktraces: Vec<_> = stacktraces
146146
.into_iter()
147147
.map(|raw_stacktrace| {
148-
let mut st = Self::map_stacktrace(
148+
let mut frames = Self::map_stacktrace(
149149
&mappers,
150-
raw_stacktrace,
150+
&raw_stacktrace.frames,
151151
release_package.as_deref(),
152152
&mut stats,
153153
);
154154

155155
if frame_order == FrameOrder::CallerFirst {
156156
// The symbolicated frames are expected in "caller first" order.
157-
st.frames.reverse();
157+
frames.reverse();
158158
}
159159

160-
st
160+
JvmStacktrace { frames }
161161
})
162162
.collect();
163163

@@ -200,14 +200,14 @@ impl ProguardService {
200200

201201
fn map_stacktrace(
202202
mappers: &[&proguard::ProguardCache],
203-
stacktrace: JvmStacktrace,
203+
stacktrace: &[JvmFrame],
204204
release_package: Option<&str>,
205205
stats: &mut SymbolicationStats,
206-
) -> JvmStacktrace {
206+
) -> Vec<JvmFrame> {
207207
let mut carried_outline_pos = vec![None; mappers.len()];
208208
let mut remapped_frames = Vec::new();
209209

210-
for frame in stacktrace.frames.into_iter() {
210+
'frames: for frame in stacktrace {
211211
let deobfuscated_signature = frame.signature.as_ref().and_then(|signature| {
212212
mappers
213213
.iter()
@@ -249,7 +249,6 @@ impl ProguardService {
249249
// backfilling the line number with 0.
250250
.unwrap_or_else(|| proguard::StackFrame::new(&frame.module, &frame.function, 0));
251251

252-
let mut skip_frame = false;
253252
let mut mapped_result = None;
254253

255254
let mut remap_buffer = Vec::new();
@@ -261,8 +260,7 @@ impl ProguardService {
261260
proguard_frame.parameters(),
262261
) {
263262
carried_outline_pos[mapper_idx] = Some(proguard_frame.line());
264-
skip_frame = true;
265-
break;
263+
continue 'frames;
266264
}
267265

268266
let effective = mapper.prepare_frame_for_mapping(
@@ -272,32 +270,28 @@ impl ProguardService {
272270

273271
// First, try to remap the whole frame.
274272
if let Some(frames_out) =
275-
Self::map_full_frame(mapper, &frame, &effective, &mut remap_buffer)
273+
Self::map_full_frame(mapper, frame, &effective, &mut remap_buffer)
276274
{
277275
mapped_result = Some(frames_out);
278276
break;
279277
}
280278

281279
// Second, try to remap the frame's method.
282-
if let Some(frames_out) = Self::map_class_method(mapper, &frame) {
280+
if let Some(frames_out) = Self::map_class_method(mapper, frame) {
283281
mapped_result = Some(frames_out);
284282
break;
285283
}
286284

287285
// Third, try to remap just the frame's class.
288-
if let Some(frames_out) = Self::map_class(mapper, &frame) {
286+
if let Some(frames_out) = Self::map_class(mapper, frame) {
289287
mapped_result = Some(frames_out);
290288
break;
291289
}
292290
}
293291

294-
if skip_frame {
295-
continue;
296-
}
297-
298292
// Fix up the frames' in-app fields only if they were actually mapped
299293
if let Some(frames) = mapped_result.as_mut() {
300-
for mapped_frame in frames.iter_mut() {
294+
for mapped_frame in frames {
301295
// mark the frame as in_app after deobfuscation based on the release package name
302296
// only if it's not present
303297
if let Some(package) = release_package
@@ -335,9 +329,7 @@ impl ProguardService {
335329
remapped_frames.extend(frames);
336330
}
337331

338-
JvmStacktrace {
339-
frames: remapped_frames,
340-
}
332+
remapped_frames
341333
}
342334

343335
/// Remaps an exception using the provided mappers.
@@ -522,7 +514,7 @@ mod tests {
522514

523515
fn map_frames_via_stacktrace(
524516
proguard_source: &[u8],
525-
mut frames: Vec<JvmFrame>,
517+
frames: &mut [JvmFrame],
526518
release_package: Option<&str>,
527519
frame_order: FrameOrder,
528520
) -> Vec<JvmFrame> {
@@ -538,11 +530,10 @@ mod tests {
538530

539531
let mut remapped_frames = ProguardService::map_stacktrace(
540532
&[&cache],
541-
JvmStacktrace { frames },
533+
frames,
542534
release_package,
543535
&mut SymbolicationStats::default(),
544-
)
545-
.frames;
536+
);
546537

547538
if frame_order == FrameOrder::CallerFirst {
548539
remapped_frames.reverse();
@@ -605,7 +596,7 @@ io.sentry.sample.MainActivity -> io.sentry.sample.MainActivity:
605596
1:1:void foo():44 -> t
606597
1:1:void onClickHandler(android.view.View):40 -> t";
607598

608-
let frames = [
599+
let mut frames = [
609600
JvmFrame {
610601
function: "onClick".to_owned(),
611602
module: "e.a.c.a".to_owned(),
@@ -651,12 +642,8 @@ io.sentry.sample.MainActivity -> io.sentry.sample.MainActivity:
651642
},
652643
];
653644

654-
let mapped_frames = map_frames_via_stacktrace(
655-
proguard_source,
656-
frames.to_vec(),
657-
None,
658-
FrameOrder::CallerFirst,
659-
);
645+
let mapped_frames =
646+
map_frames_via_stacktrace(proguard_source, &mut frames, None, FrameOrder::CallerFirst);
660647

661648
insta::assert_yaml_snapshot!(mapped_frames, @r###"
662649
- function: onClick
@@ -707,7 +694,7 @@ org.slf4j.helpers.Util$ClassContext -> org.a.b.g$b:
707694
65:65:void <init>() -> <init>
708695
";
709696

710-
let frames = [
697+
let mut frames = [
711698
JvmFrame {
712699
function: "a".to_owned(),
713700
module: "org.a.b.g$a".to_owned(),
@@ -744,7 +731,7 @@ org.slf4j.helpers.Util$ClassContext -> org.a.b.g$b:
744731

745732
let mapped_frames = map_frames_via_stacktrace(
746733
proguard_source,
747-
frames.to_vec(),
734+
&mut frames,
748735
Some("org.slf4j"),
749736
FrameOrder::CallerFirst,
750737
);
@@ -769,12 +756,8 @@ org.slf4j.helpers.Util$ClassContext -> org.a.b.g$b:
769756
..Default::default()
770757
};
771758

772-
let remapped = map_frames_via_stacktrace(
773-
b"",
774-
vec![frame.clone()],
775-
Some("android"),
776-
FrameOrder::CallerFirst,
777-
);
759+
let remapped =
760+
map_frames_via_stacktrace(b"", &mut [frame], Some("android"), FrameOrder::CalleeFirst);
778761

779762
assert_eq!(remapped.len(), 1);
780763
// The frame didn't get mapped, so we shouldn't set `in_app` even though
@@ -815,12 +798,8 @@ org.slf4j.helpers.Util$ClassContext -> org.a.b.g$b:
815798
..Default::default()
816799
};
817800

818-
let mapped_frames = map_frames_via_stacktrace(
819-
proguard_source,
820-
vec![frame.clone()],
821-
None,
822-
FrameOrder::CallerFirst,
823-
);
801+
let mapped_frames =
802+
map_frames_via_stacktrace(proguard_source, &mut [frame], None, FrameOrder::CalleeFirst);
824803

825804
// Without the "line 0" change, the second frame doesn't exist.
826805
// The `retrace` implementation at
@@ -871,12 +850,8 @@ y.b -> y.b:
871850
..Default::default()
872851
};
873852

874-
let mapped_frames = map_frames_via_stacktrace(
875-
proguard_source,
876-
vec![frame.clone()],
877-
None,
878-
FrameOrder::CallerFirst,
879-
);
853+
let mapped_frames =
854+
map_frames_via_stacktrace(proguard_source, &mut [frame], None, FrameOrder::CalleeFirst);
880855

881856
// Without the "line 0" change, the module is "com.google.firebase.concurrent.CustomThreadFactory$$ExternalSyntheticLambda0".
882857
// The `retrace` implementation at
@@ -946,7 +921,7 @@ io.wzieba.r8fullmoderenamessources.R -> a.d:
946921
void <init>() -> <init>
947922
# {"id":"com.android.tools.r8.synthesized"}"#;
948923

949-
let frames: Vec<JvmFrame> = serde_json::from_str(
924+
let mut frames: Vec<JvmFrame> = serde_json::from_str(
950925
r#"[{
951926
"function": "a",
952927
"abs_path": "SourceFile",
@@ -1000,12 +975,8 @@ io.wzieba.r8fullmoderenamessources.R -> a.d:
1000975
)
1001976
.unwrap();
1002977

1003-
let mapped_frames = map_frames_via_stacktrace(
1004-
proguard_source,
1005-
frames.clone(),
1006-
None,
1007-
FrameOrder::CallerFirst,
1008-
);
978+
let mapped_frames =
979+
map_frames_via_stacktrace(proguard_source, &mut frames, None, FrameOrder::CallerFirst);
1009980

1010981
insta::assert_yaml_snapshot!(mapped_frames, @r###"
1011982
- function: foo
@@ -1090,7 +1061,7 @@ com.mycompany.android.Delegate -> b80.h:
10901061
com.mycompany.android.IMapAnnotations mapAnnotations -> a
10911062
# {"id":"com.android.tools.r8.residualsignature","signature":"Lbv0/b;"}"#;
10921063

1093-
let frames: Vec<JvmFrame> = serde_json::from_str(
1064+
let mut frames: Vec<JvmFrame> = serde_json::from_str(
10941065
r#"[
10951066
{
10961067
"function": "a",
@@ -1119,12 +1090,8 @@ com.mycompany.android.Delegate -> b80.h:
11191090
)
11201091
.unwrap();
11211092

1122-
let mapped_frames = map_frames_via_stacktrace(
1123-
proguard_source,
1124-
frames.clone(),
1125-
None,
1126-
FrameOrder::CallerFirst,
1127-
);
1093+
let mapped_frames =
1094+
map_frames_via_stacktrace(proguard_source, &mut frames, None, FrameOrder::CallerFirst);
11281095

11291096
insta::assert_yaml_snapshot!(mapped_frames, @r###"
11301097
- function: render
@@ -1147,23 +1114,10 @@ com.mycompany.android.Delegate -> b80.h:
11471114
index: 0
11481115
- function: createProjectionMarker
11491116
filename: SourceFile
1150-
module: uu0.MapAnnotations
1151-
abs_path: SourceFile
1152-
lineno: 0
1153-
index: 1
1154-
- function: createProjectionMarker
1155-
filename: MapAnnotations.kt
11561117
module: com.mycompany.android.MapAnnotations
1157-
abs_path: MapAnnotations.kt
1158-
lineno: 0
1159-
index: 1
1160-
- function: m
1161-
filename: SourceFile
1162-
module: ev.StuffKt$$ExternalSyntheticOutline0
11631118
abs_path: SourceFile
1164-
lineno: 1
1165-
index: 2
1166-
method_synthesized: true
1119+
lineno: 43
1120+
index: 1
11671121
"###);
11681122
}
11691123

@@ -1182,7 +1136,7 @@ some.Class -> b:
11821136
# {"id":"com.android.tools.r8.outlineCallsite","positions":{"1":4,"2":5},"outline":"La;a()I"}
11831137
"#;
11841138

1185-
let frames = vec![
1139+
let mut frames = [
11861140
JvmFrame {
11871141
function: "a".to_owned(),
11881142
module: "a".to_owned(),
@@ -1200,7 +1154,7 @@ some.Class -> b:
12001154
];
12011155

12021156
let remapped =
1203-
map_frames_via_stacktrace(proguard_source, frames, None, FrameOrder::CalleeFirst);
1157+
map_frames_via_stacktrace(proguard_source, &mut frames, None, FrameOrder::CalleeFirst);
12041158

12051159
assert_eq!(remapped.len(), 1);
12061160
assert_eq!(remapped[0].module, "some.Class");
@@ -1211,7 +1165,7 @@ some.Class -> b:
12111165

12121166
#[test]
12131167
fn remap_outline_complex() {
1214-
let frames: Vec<JvmFrame> = serde_json::from_str(
1168+
let mut frames: Vec<JvmFrame> = serde_json::from_str(
12151169
r#"[
12161170
{
12171171
"function": "b",
@@ -1329,7 +1283,7 @@ some.Class -> b:
13291283

13301284
let remapped_frames = map_frames_via_stacktrace(
13311285
MAPPING_OUTLINE_COMPLEX,
1332-
frames,
1286+
&mut frames,
13331287
None,
13341288
FrameOrder::CalleeFirst,
13351289
);

0 commit comments

Comments
 (0)