diff --git a/Cargo.lock b/Cargo.lock index 610edfaac..9d9ae8426 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3768,9 +3768,9 @@ dependencies = [ [[package]] name = "proguard" -version = "5.9.0" +version = "5.10.1" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "485ce6a0eaff8ca5566dde882ee2ef65dc25ba9f3ef1dba1129a8dd78a181952" +checksum = "c8229689975f1a875da21896b99d7cdd0dc6acecd54e9a37c7faf59e27d2add2" dependencies = [ "serde", "serde_json", diff --git a/Cargo.toml b/Cargo.toml index 349d174e7..346b4bd40 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -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" diff --git a/crates/symbolicator-proguard/src/symbolication.rs b/crates/symbolicator-proguard/src/symbolication.rs index e3dd77efb..e078e8883 100644 --- a/crates/symbolicator-proguard/src/symbolication.rs +++ b/crates/symbolicator-proguard/src/symbolication.rs @@ -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| { @@ -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; @@ -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 @@ -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) @@ -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 @@ -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 @@ -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 @@ -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 @@ -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 "###); } @@ -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 "###); diff --git a/crates/symbolicator-proguard/tests/integration/snapshots/integration__proguard__resolving_inline.snap b/crates/symbolicator-proguard/tests/integration/snapshots/integration__proguard__resolving_inline.snap index ebd84f46c..31d38b082 100644 --- a/crates/symbolicator-proguard/tests/integration/snapshots/integration__proguard__resolving_inline.snap +++ b/crates/symbolicator-proguard/tests/integration/snapshots/integration__proguard__resolving_inline.snap @@ -1,5 +1,6 @@ --- source: crates/symbolicator-proguard/tests/integration/proguard.rs +assertion_line: 181 expression: response --- exceptions: @@ -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: {} diff --git a/crates/symbolicator-proguard/tests/integration/snapshots/integration__proguard__rewrite_frames.snap b/crates/symbolicator-proguard/tests/integration/snapshots/integration__proguard__rewrite_frames.snap index e136676e7..47674400a 100644 --- a/crates/symbolicator-proguard/tests/integration/snapshots/integration__proguard__rewrite_frames.snap +++ b/crates/symbolicator-proguard/tests/integration/snapshots/integration__proguard__rewrite_frames.snap @@ -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: {} diff --git a/crates/symbolicator-proguard/tests/integration/snapshots/integration__proguard__source_lookup_with_proguard.snap b/crates/symbolicator-proguard/tests/integration/snapshots/integration__proguard__source_lookup_with_proguard.snap index f1e908701..f37b776fb 100644 --- a/crates/symbolicator-proguard/tests/integration/snapshots/integration__proguard__source_lookup_with_proguard.snap +++ b/crates/symbolicator-proguard/tests/integration/snapshots/integration__proguard__source_lookup_with_proguard.snap @@ -1,5 +1,6 @@ --- source: crates/symbolicator-proguard/tests/integration/proguard.rs +assertion_line: 482 expression: response --- exceptions: @@ -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 @@ -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 @@ -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()" @@ -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