Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,7 @@ minidump-processor = "0.26.1"
minidump-unwind = "0.26.1"
moka = { version = "0.12.8", features = ["future", "sync"] }
prettytable-rs = "0.10.0"
proguard = "5.9.0"
proguard = "5.10.1"
rand = "0.9.0"
rayon = "1.10.0"
regex = "1.5.5"
Expand Down
86 changes: 49 additions & 37 deletions crates/symbolicator-proguard/src/symbolication.rs
Original file line number Diff line number Diff line change
Expand Up @@ -250,7 +250,12 @@ impl ProguardService {
// We create the proguard frame according to these priorities:
// * Use the frame's line number if it exists
// * Use the frame's parameters if they exist
// * Use line number 0
// * Use line number 0 as a fallback
//
// The line-0 fallback is intentional and matches retrace behavior: it triggers
// the no-line-info path in ProguardCache, which picks the first range group and
// resolves inline chains (e.g. the `line_0_1` test produces `onCreate` +
// `barInternalInject`). Retrace does the same via `allRangesForLine(0, true)`.
let proguard_frame = frame
.lineno
.map(|lineno| {
Expand All @@ -265,17 +270,6 @@ impl ProguardService {
)
})
})
// This is for parity with the Python implementation. It's unclear why remapping a frame with line 0
// would produce useful information, and I have no conclusive evidence that it does.
// See the `line_0_1` and `line_0_2` unit tests in this file for examples of the results this produces.
//
// TODO(@loewenheim): Find out if this is useful and remove it otherwise.
// The PR that introduced this was https://github.com/getsentry/symbolicator/pull/1434.
//
// UPDATE(@loewenheim): The retrace implementation at https://dl.google.com/android/repository/commandlinetools-mac-11076708_latest.zip
// returns the same value whether you give it line 0 or no line at all, and it is the same result that our implementation
// gives with line 0. This indicates that the _behavior_ is correct, but we should be able to get there without
// backfilling the line number with 0.
.unwrap_or_else(|| proguard::StackFrame::new(&frame.module, &frame.function, 0));

let mut mapped_result = None;
Expand Down Expand Up @@ -328,6 +322,19 @@ impl ProguardService {

// Fix up the frames' in-app fields only if they were actually mapped
if let Some(frames) = mapped_result.as_mut() {
// Drop frames whose methods were synthesized by the compiler (e.g. lambda bridges).
// Only filter when the original frame had a line number. This matches a retrace
// quirk: without a line number, retrace resolves via MemberNaming (which lacks
// range-level synthesized annotations) and keeps the frame. This is arguably a
// retrace bug, but we match it because users compare our output against tools
// built on retrace. See `line_0_2` test for an example.
if frame.lineno.is_some() {
frames.retain(|f| !f.method_synthesized);
if frames.is_empty() {
continue 'frames;
}
}

for mapped_frame in frames {
// mark the frame as in_app after deobfuscation based on the release package name
// only if it's not present
Expand Down Expand Up @@ -456,7 +463,7 @@ impl ProguardService {
.map(|new_frame| JvmFrame {
module: new_frame.class().to_owned(),
function: new_frame.method().to_owned(),
lineno: Some(new_frame.line() as u32),
lineno: new_frame.line().map(|l| l as u32),
abs_path: new_frame
.file()
.map(String::from)
Expand Down Expand Up @@ -713,31 +720,41 @@ io.sentry.sample.MainActivity -> io.sentry.sample.MainActivity:

insta::assert_yaml_snapshot!(mapped_frames, @r###"
- function: onClick
filename: "-.java"
module: io.sentry.sample.-$$Lambda$r3Avcbztes2hicEObh02jjhQqd4
abs_path: "-.java"
lineno: 2
index: 0
- function: onClickHandler
filename: MainActivity.java
module: io.sentry.sample.MainActivity
abs_path: MainActivity.java
lineno: 40
index: 1
- function: foo
filename: MainActivity.java
module: io.sentry.sample.MainActivity
abs_path: MainActivity.java
lineno: 44
index: 1
- function: bar
filename: MainActivity.java
module: io.sentry.sample.MainActivity
abs_path: MainActivity.java
lineno: 54
index: 1
- function: onClickHandler
filename: MainActivity.java
module: io.sentry.sample.MainActivity
lineno: 0
abs_path: MainActivity.java
lineno: 40
index: 2
signature: (android.view.View)
- function: onClickHandler
filename: MainActivity.java
module: io.sentry.sample.MainActivity
abs_path: MainActivity.java
lineno: 0
index: 3
signature: (android.view.View)
- function: onClickHandler
Expand Down Expand Up @@ -911,15 +928,14 @@ y.b -> y.b:

let mapped_frames = remap_stacktrace_caller_first(proguard_source, None, &mut [frame]);

// Without the "line 0" change, the module is "com.google.firebase.concurrent.CustomThreadFactory$$ExternalSyntheticLambda0".
// The `retrace` implementation at
// https://dl.google.com/android/repository/commandlinetools-mac-11076708_latest.zip
// also returns this, no matter whether you give it line 0 or no line at all.
// The mapped frame `run$bridge` has `method_synthesized: true`, but the
// original frame had no line number, so synthesized filtering is skipped
// (matching retrace behavior).
insta::assert_yaml_snapshot!(mapped_frames, @r###"
- function: run$bridge
filename: CustomThreadFactory
filename: CustomThreadFactory.java
module: com.google.firebase.concurrent.CustomThreadFactory$$InternalSyntheticLambda$1$53203795c28a6fcdb3bac755806c9ee73cb3e8dcd4c9bbf8ca5d25d4d9c378dd$0
abs_path: CustomThreadFactory
abs_path: CustomThreadFactory.java
lineno: 0
index: 0
method_synthesized: true
Expand Down Expand Up @@ -1048,20 +1064,6 @@ io.wzieba.r8fullmoderenamessources.R -> a.d:
abs_path: MainActivity.kt
lineno: 14
index: 1
- function: $r8$lambda$pOQDVg57r6gG0-DzwbGf17BfNbs
filename: MainActivity.kt
module: io.wzieba.r8fullmoderenamessources.MainActivity
abs_path: MainActivity.kt
lineno: 0
index: 2
method_synthesized: true
- function: onClick
filename: MainActivity
module: io.wzieba.r8fullmoderenamessources.MainActivity$$ExternalSyntheticLambda0
abs_path: MainActivity
lineno: 0
index: 3
method_synthesized: true
- function: performClick
filename: View.java
module: android.view.View
Expand Down Expand Up @@ -1108,6 +1110,8 @@ com.mycompany.android.MapAnnotations -> uu0.k:
43:46:com.mycompany.android.IProjectionMarker createProjectionMarker(com.mycompany.android.IProjectionMarkerOptions):0:0 -> l
43:46:lv0.IProjectionMarker uu0.MapAnnotations.createProjectionMarker(lv0.IProjectionMarkerOptions):0 -> l
# {"id":"com.android.tools.r8.outlineCallsite","positions":{"1":50,"3":52,"6":55},"outline":"Lev/h;b(Ljava/lang/String;Lme/company/android/logging/L;)V"}
52:52:com.mycompany.android.ViewProjectionMarker com.mycompany.android.Delegate.createProjectionMarker():79:79 -> l
52:52:com.mycompany.android.IProjectionMarker createProjectionMarker(com.mycompany.android.IProjectionMarkerOptions):63 -> l
com.mycompany.android.Renderer -> b80.f:
# {"id":"sourceFile","fileName":"Renderer.kt"}
33:40:com.mycompany.android.ViewProjectionMarker com.mycompany.android.Delegate.createProjectionMarker():101:101 -> a
Expand Down Expand Up @@ -1169,10 +1173,16 @@ com.mycompany.android.Delegate -> b80.h:
lineno: 101
index: 0
- function: createProjectionMarker
filename: SourceFile
filename: MapAnnotations.kt
module: com.mycompany.android.MapAnnotations
abs_path: SourceFile
lineno: 43
abs_path: MapAnnotations.kt
lineno: 63
index: 1
- function: createProjectionMarker
filename: Delegate.kt
module: com.mycompany.android.Delegate
abs_path: Delegate.kt
lineno: 79
index: 1
"###);
}
Expand Down Expand Up @@ -1213,7 +1223,9 @@ some.Class -> b:

insta::assert_yaml_snapshot!(remapped, @r###"
- function: outlineCaller
filename: Class.java
module: some.Class
abs_path: Class.java
lineno: 98
index: 1
"###);
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
---
source: crates/symbolicator-proguard/tests/integration/proguard.rs
assertion_line: 181
expression: response
---
exceptions:
Expand All @@ -11,22 +12,27 @@ stacktraces:
module: org.a.b
frames:
- function: onClick
filename: "-.java"
module: io.sentry.sample.-$$Lambda$r3Avcbztes2hicEObh02jjhQqd4
abs_path: "-.java"
lineno: 2
index: 0
- function: onClickHandler
filename: MainActivity.java
module: io.sentry.sample.MainActivity
abs_path: MainActivity.java
lineno: 40
index: 1
- function: foo
filename: MainActivity.java
module: io.sentry.sample.MainActivity
abs_path: MainActivity.java
lineno: 44
index: 1
- function: bar
filename: MainActivity.java
module: io.sentry.sample.MainActivity
abs_path: MainActivity.java
lineno: 54
index: 1
classes: {}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,19 +12,27 @@ stacktraces:
module: java.lang
frames:
- function: render
filename: UiBridge.java
module: com.example.flow.UiBridge
abs_path: UiBridge.java
lineno: 200
index: 0
- function: dispatch
filename: StreamRouter.java
module: com.example.flow.StreamRouter
abs_path: StreamRouter.java
lineno: 12
index: 1
- function: internalDispatch
filename: StreamRouter.java
module: com.example.flow.StreamRouter$Inline
abs_path: StreamRouter.java
lineno: 30
index: 1
- function: start
filename: Initializer.java
module: com.example.flow.Initializer
abs_path: Initializer.java
lineno: 42
index: 2
classes: {}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
---
source: crates/symbolicator-proguard/tests/integration/proguard.rs
assertion_line: 482
expression: response
---
exceptions:
Expand Down Expand Up @@ -99,13 +100,6 @@ stacktraces:
module: androidx.appcompat.widget.Toolbar$1
lineno: 7
index: 17
- function: onMenuItemClick
filename: R8$$SyntheticClass
module: io.sentry.samples.instrumentation.ui.EditActivity$$InternalSyntheticLambda$1$ebaa538726b99bb77e0f5e7c86443911af17d6e5be2b8771952ae0caa4ff2ac7$0
lineno: 0
in_app: true
index: 18
method_synthesized: true
- function: onCreate$lambda-1
filename: EditActivity.kt
module: io.sentry.samples.instrumentation.ui.EditActivity
Expand All @@ -127,8 +121,9 @@ stacktraces:
in_app: true
index: 18
- function: helloThere
filename: R8$$SyntheticClass
filename: SomeService.java
module: io.sentry.samples.instrumentation.ui.SomeService
abs_path: SomeService.java
lineno: 5
pre_context:
- package io.sentry.samples.instrumentation.ui
Expand All @@ -145,8 +140,9 @@ stacktraces:
in_app: true
index: 18
- function: helloInner
filename: R8$$SyntheticClass
filename: SomeService.java
module: io.sentry.samples.instrumentation.ui.SomeService$InnerClassOfSomeService
abs_path: SomeService.java
lineno: 10
pre_context:
- " InnerClassOfSomeService().helloInner()"
Expand All @@ -164,20 +160,23 @@ stacktraces:
in_app: true
index: 18
- function: helloOther
filename: R8$$SyntheticClass
filename: AnotherClassInSameFile.java
module: io.sentry.samples.instrumentation.ui.AnotherClassInSameFile
abs_path: AnotherClassInSameFile.java
lineno: 17
in_app: true
index: 18
- function: otherFun
filename: R8$$SyntheticClass
filename: AnotherClassInSameFile.java
module: io.sentry.samples.instrumentation.ui.AnotherClassInSameFile
abs_path: AnotherClassInSameFile.java
lineno: 21
in_app: true
index: 18
- function: helloOtherInner
filename: R8$$SyntheticClass
filename: AnotherClassInSameFile.java
module: io.sentry.samples.instrumentation.ui.AnotherClassInSameFile$AnotherInnerClass
abs_path: AnotherClassInSameFile.java
lineno: 26
in_app: true
index: 18
Expand Down
Loading