-
-
Notifications
You must be signed in to change notification settings - Fork 8
feat(r8-tests): Add R8 ambiguous tests #71
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,46 @@ | ||
| # r8-ambiguous.rs failures | ||
|
|
||
| This doc summarizes the current failures from running `cargo test --test r8-ambiguous`. | ||
|
|
||
| ## `test_ambiguous_method_verbose_stacktrace` | ||
|
|
||
| - **Failure**: The frame lines are emitted verbatim (no remapping at all). | ||
| - **Why**: | ||
| - The input frames have **no line numbers** (e.g. `(Foo.java)`), which means `frame.line == 0`. | ||
| - The current remapper does not produce any remapped candidates for these frames (so `format_frames` falls back to printing the original line). This indicates a gap in the “no-line” / best-effort ambiguous member mapping behavior for `line == 0` frames. | ||
|
|
||
| ## `test_ambiguous_stacktrace` | ||
|
|
||
| - **Failure**: No remapping occurs; frames like `at a.a.a(Unknown Source)` are preserved. | ||
| - **Why**: | ||
| - These frames have `Unknown Source` with **no line number**, so `frame.line == 0`. | ||
| - For `line == 0`, the current implementation ends up with **no remapped candidates** and prints the original frame line unchanged (instead of emitting ambiguous alternatives `foo`/`bar`). | ||
|
|
||
| ## `test_ambiguous_missing_line_stacktrace` | ||
|
|
||
| - **Failure**: Output uses `R8.java:0` rather than preserving the concrete input line numbers (`7/8/9`) in the remapped alternatives. | ||
| - **Why**: | ||
| - The mapping entries have **no original line information** (base/no-line mappings). | ||
| - The current mapping logic uses the mapping’s “original start line” (which defaults to `0`) rather than propagating the **caller-provided minified line** when available. | ||
|
|
||
| ## `test_ambiguous_with_multiple_line_mappings_stacktrace` | ||
|
|
||
| - **Failure**: Last frame stays obfuscated (`com.android.tools.r8.Internal.zza(Unknown)`), expected a deobfuscated `Internal.foo(Internal.java:0)`-style frame. | ||
| - **Why**: | ||
| - `(...(Unknown))` parses as a frame with `file = "Unknown"` and `line = 0`. | ||
| - All available member mappings are **line-ranged** (e.g. `10:10`, `11:11`, `12:12`), so with `line == 0` they do not match and the remapper produces **no candidates**, falling back to the original frame line. | ||
|
|
||
| ## `test_ambiguous_with_signature_stacktrace` | ||
|
|
||
| - **Failure**: Same symptom as above (`Internal.zza(Unknown)` remains), expected deobfuscated member. | ||
| - **Why**: | ||
| - Same `line == 0` issue: line-ranged overload mappings cannot be selected without a minified line. | ||
| - The remapper currently has no fallback that returns “best effort” candidates for `line == 0` frames (e.g., returning all overloads, or preferring a base mapping if present). | ||
|
|
||
| ## `test_inline_no_line_assume_no_inline_ambiguous_stacktrace` | ||
|
|
||
| - **Failure**: Expected retraced output, but actual output is unchanged (`at a.foo(Unknown Source)`). | ||
| - **Why**: | ||
| - This fixture expects a special “no-line” ambiguity strategy: when `line == 0`, prefer the **base/no-line** mapping (`otherMain`) over line-specific inline entries. | ||
| - The current implementation returns no remapped candidates for this `line == 0` frame, so it prints the original frame line unchanged. | ||
|
|
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,344 @@ | ||
| //! Tests for R8 ambiguous method retracing functionality. | ||
| //! | ||
| //! These tests are based on the R8 retrace test suite from: | ||
| //! src/test/java/com/android/tools/r8/retrace/stacktraces/ | ||
|
|
||
| use proguard::{ProguardCache, ProguardMapper, ProguardMapping}; | ||
|
|
||
| // ============================================================================= | ||
| // AmbiguousStackTrace | ||
| // ============================================================================= | ||
|
|
||
| const AMBIGUOUS_STACKTRACE_MAPPING: &str = "\ | ||
| com.android.tools.r8.R8 -> a.a: | ||
| void foo(int) -> a | ||
| void bar(int, int) -> a | ||
| "; | ||
|
|
||
| #[test] | ||
| fn test_ambiguous_stacktrace() { | ||
| let input = "\ | ||
| com.android.tools.r8.CompilationException: foo[parens](Source:3) | ||
| at a.a.a(Unknown Source) | ||
| at a.a.a(Unknown Source) | ||
| at com.android.tools.r8.R8.main(Unknown Source) | ||
| Caused by: com.android.tools.r8.CompilationException: foo[parens](Source:3) | ||
| at a.a.a(Unknown Source) | ||
| ... 42 more | ||
| "; | ||
|
|
||
| let expected = "\ | ||
| com.android.tools.r8.CompilationException: foo[parens](Source:3) | ||
| at com.android.tools.r8.R8.foo(R8.java:0) | ||
| at com.android.tools.r8.R8.bar(R8.java:0) | ||
| at com.android.tools.r8.R8.foo(R8.java:0) | ||
| at com.android.tools.r8.R8.bar(R8.java:0) | ||
| at com.android.tools.r8.R8.main(Unknown Source) | ||
| Caused by: com.android.tools.r8.CompilationException: foo[parens](Source:3) | ||
| at com.android.tools.r8.R8.foo(R8.java:0) | ||
| at com.android.tools.r8.R8.bar(R8.java:0) | ||
| ... 42 more | ||
| "; | ||
|
|
||
| let mapper = ProguardMapper::from(AMBIGUOUS_STACKTRACE_MAPPING); | ||
| let actual = mapper.remap_stacktrace(input).unwrap(); | ||
| assert_eq!(actual.trim(), expected.trim()); | ||
|
|
||
| let mapping = ProguardMapping::new(AMBIGUOUS_STACKTRACE_MAPPING.as_bytes()); | ||
| let mut buf = Vec::new(); | ||
| ProguardCache::write(&mapping, &mut buf).unwrap(); | ||
| let cache = ProguardCache::parse(&buf).unwrap(); | ||
| cache.test(); | ||
|
|
||
| let actual = cache.remap_stacktrace(input).unwrap(); | ||
| assert_eq!(actual.trim(), expected.trim()); | ||
| } | ||
|
|
||
| // ============================================================================= | ||
| // AmbiguousMissingLineStackTrace | ||
| // ============================================================================= | ||
|
|
||
| const AMBIGUOUS_MISSING_LINE_MAPPING: &str = "\ | ||
| com.android.tools.r8.R8 -> a.a: | ||
| void foo(int) -> a | ||
| void bar(int, int) -> a | ||
| "; | ||
|
|
||
| #[test] | ||
| fn test_ambiguous_missing_line_stacktrace() { | ||
| let input = "\ | ||
| com.android.tools.r8.CompilationException: foo[parens](Source:3) | ||
| at a.a.a(Unknown Source:7) | ||
| at a.a.a(Unknown Source:8) | ||
| at com.android.tools.r8.R8.main(Unknown Source) | ||
| Caused by: com.android.tools.r8.CompilationException: foo[parens](Source:3) | ||
| at a.a.a(Unknown Source:9) | ||
| ... 42 more | ||
| "; | ||
|
|
||
| let expected = "\ | ||
| com.android.tools.r8.CompilationException: foo[parens](Source:3) | ||
| at com.android.tools.r8.R8.foo(R8.java:7) | ||
| at com.android.tools.r8.R8.bar(R8.java:7) | ||
| at com.android.tools.r8.R8.foo(R8.java:8) | ||
| at com.android.tools.r8.R8.bar(R8.java:8) | ||
| at com.android.tools.r8.R8.main(Unknown Source) | ||
| Caused by: com.android.tools.r8.CompilationException: foo[parens](Source:3) | ||
| at com.android.tools.r8.R8.foo(R8.java:9) | ||
| at com.android.tools.r8.R8.bar(R8.java:9) | ||
| ... 42 more | ||
| "; | ||
|
|
||
| let mapper = ProguardMapper::from(AMBIGUOUS_MISSING_LINE_MAPPING); | ||
| let actual = mapper.remap_stacktrace(input).unwrap(); | ||
| assert_eq!(actual.trim(), expected.trim()); | ||
|
|
||
| let mapping = ProguardMapping::new(AMBIGUOUS_MISSING_LINE_MAPPING.as_bytes()); | ||
| let mut buf = Vec::new(); | ||
| ProguardCache::write(&mapping, &mut buf).unwrap(); | ||
| let cache = ProguardCache::parse(&buf).unwrap(); | ||
| cache.test(); | ||
|
|
||
| let actual = cache.remap_stacktrace(input).unwrap(); | ||
| assert_eq!(actual.trim(), expected.trim()); | ||
| } | ||
|
|
||
| // ============================================================================= | ||
| // AmbiguousInlineFramesStackTrace | ||
| // ============================================================================= | ||
|
|
||
| const AMBIGUOUS_INLINE_FRAMES_MAPPING: &str = "\ | ||
| com.android.tools.r8.R8 -> a.a: | ||
| 1:1:void foo(int):42:44 -> a | ||
| 1:1:void bar(int, int):32 -> a | ||
| 1:1:void baz(int, int):10 -> a | ||
| "; | ||
|
|
||
| #[test] | ||
| fn test_ambiguous_inline_frames_stacktrace() { | ||
| let input = "\ | ||
| com.android.tools.r8.CompilationException: | ||
| at a.a.a(Unknown Source:1) | ||
| "; | ||
|
|
||
| let expected = "\ | ||
| com.android.tools.r8.CompilationException: | ||
| at com.android.tools.r8.R8.foo(R8.java:42) | ||
|
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Similar here -- retrace actually expands this to mention all of the potential inline frames: |
||
| at com.android.tools.r8.R8.bar(R8.java:32) | ||
| at com.android.tools.r8.R8.baz(R8.java:10) | ||
| "; | ||
|
|
||
| let mapper = ProguardMapper::from(AMBIGUOUS_INLINE_FRAMES_MAPPING); | ||
| let actual = mapper.remap_stacktrace(input).unwrap(); | ||
| assert_eq!(actual.trim(), expected.trim()); | ||
|
|
||
| let mapping = ProguardMapping::new(AMBIGUOUS_INLINE_FRAMES_MAPPING.as_bytes()); | ||
| let mut buf = Vec::new(); | ||
| ProguardCache::write(&mapping, &mut buf).unwrap(); | ||
| let cache = ProguardCache::parse(&buf).unwrap(); | ||
| cache.test(); | ||
|
|
||
| let actual = cache.remap_stacktrace(input).unwrap(); | ||
| assert_eq!(actual.trim(), expected.trim()); | ||
| } | ||
|
|
||
| // ============================================================================= | ||
| // AmbiguousMultipleInlineStackTrace | ||
| // ============================================================================= | ||
|
|
||
| const AMBIGUOUS_MULTIPLE_INLINE_MAPPING: &str = "\ | ||
| com.android.tools.r8.Internal -> com.android.tools.r8.Internal: | ||
| 10:10:void some.inlinee1(int):10:10 -> zza | ||
| 10:10:void foo(int):10 -> zza | ||
| 11:12:void foo(int):11:12 -> zza | ||
| 10:10:void some.inlinee2(int, int):20:20 -> zza | ||
| 10:10:void foo(int, int):42 -> zza | ||
| "; | ||
|
|
||
| #[test] | ||
| fn test_ambiguous_multiple_inline_stacktrace() { | ||
| let input = "\ | ||
| java.lang.IndexOutOfBoundsException | ||
| at com.android.tools.r8.Internal.zza(SourceFile:10) | ||
| "; | ||
|
|
||
| let expected = "\ | ||
| java.lang.IndexOutOfBoundsException | ||
|
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Here the order in retrace is different: seems like retrace has two alternative expansion paths for a single obfuscated frame: When it prints with depth 0: inlinee1 I guess this also depends on the decision if we're going to implement alternative stacktraces or not |
||
| at some.inlinee1(some.java:10) | ||
| at com.android.tools.r8.Internal.foo(Internal.java:10) | ||
| at some.inlinee2(some.java:20) | ||
| at com.android.tools.r8.Internal.foo(Internal.java:42) | ||
| "; | ||
|
|
||
| let mapper = ProguardMapper::from(AMBIGUOUS_MULTIPLE_INLINE_MAPPING); | ||
| let actual = mapper.remap_stacktrace(input).unwrap(); | ||
| assert_eq!(actual.trim(), expected.trim()); | ||
|
|
||
| let mapping = ProguardMapping::new(AMBIGUOUS_MULTIPLE_INLINE_MAPPING.as_bytes()); | ||
| let mut buf = Vec::new(); | ||
| ProguardCache::write(&mapping, &mut buf).unwrap(); | ||
| let cache = ProguardCache::parse(&buf).unwrap(); | ||
| cache.test(); | ||
|
|
||
| let actual = cache.remap_stacktrace(input).unwrap(); | ||
| assert_eq!(actual.trim(), expected.trim()); | ||
| } | ||
|
|
||
| // ============================================================================= | ||
| // AmbiguousMethodVerboseStackTrace (non-verbose retrace output) | ||
| // ============================================================================= | ||
|
|
||
| const AMBIGUOUS_METHOD_VERBOSE_MAPPING: &str = "\ | ||
| com.android.tools.r8.naming.retrace.Main -> a.a: | ||
| com.android.Foo main(java.lang.String[],com.android.Bar) -> a | ||
| com.android.Foo main(java.lang.String[]) -> b | ||
| void main(com.android.Bar) -> b | ||
| "; | ||
|
|
||
| #[test] | ||
| fn test_ambiguous_method_verbose_stacktrace() { | ||
| let input = "\ | ||
| Exception in thread \"main\" java.lang.NullPointerException | ||
| at a.a.c(Foo.java) | ||
| at a.a.b(Bar.java) | ||
| at a.a.a(Baz.java) | ||
| "; | ||
|
|
||
| let expected = "\ | ||
| Exception in thread \"main\" java.lang.NullPointerException | ||
| at com.android.tools.r8.naming.retrace.Main.c(Main.java:0) | ||
| at com.android.tools.r8.naming.retrace.Main.main(Main.java:0) | ||
| at com.android.tools.r8.naming.retrace.Main.main(Main.java:0) | ||
| "; | ||
|
|
||
| let mapper = ProguardMapper::from(AMBIGUOUS_METHOD_VERBOSE_MAPPING); | ||
| let actual = mapper.remap_stacktrace(input).unwrap(); | ||
| assert_eq!(actual.trim(), expected.trim()); | ||
|
|
||
| let mapping = ProguardMapping::new(AMBIGUOUS_METHOD_VERBOSE_MAPPING.as_bytes()); | ||
| let mut buf = Vec::new(); | ||
| ProguardCache::write(&mapping, &mut buf).unwrap(); | ||
| let cache = ProguardCache::parse(&buf).unwrap(); | ||
| cache.test(); | ||
|
|
||
| let actual = cache.remap_stacktrace(input).unwrap(); | ||
| assert_eq!(actual.trim(), expected.trim()); | ||
| } | ||
|
|
||
| // ============================================================================= | ||
| // AmbiguousWithMultipleLineMappingsStackTrace | ||
| // ============================================================================= | ||
|
|
||
| const AMBIGUOUS_WITH_MULTIPLE_LINE_MAPPINGS_MAPPING: &str = "\ | ||
| com.android.tools.r8.Internal -> com.android.tools.r8.Internal: | ||
| 10:10:void foo(int):10:10 -> zza | ||
| 11:11:void foo(int):11:11 -> zza | ||
| 12:12:void foo(int):12:12 -> zza | ||
| "; | ||
|
|
||
| #[test] | ||
| fn test_ambiguous_with_multiple_line_mappings_stacktrace() { | ||
| let input = "\ | ||
| java.lang.IndexOutOfBoundsException | ||
| at java.util.ArrayList.get(ArrayList.java:411) | ||
| at com.android.tools.r8.Internal.zza(Unknown) | ||
| "; | ||
|
|
||
| let expected = "\ | ||
| java.lang.IndexOutOfBoundsException | ||
| at java.util.ArrayList.get(ArrayList.java:411) | ||
| at com.android.tools.r8.Internal.foo(Internal.java:0) | ||
| "; | ||
|
|
||
| let mapper = ProguardMapper::from(AMBIGUOUS_WITH_MULTIPLE_LINE_MAPPINGS_MAPPING); | ||
| let actual = mapper.remap_stacktrace(input).unwrap(); | ||
| assert_eq!(actual.trim(), expected.trim()); | ||
|
|
||
| let mapping = ProguardMapping::new(AMBIGUOUS_WITH_MULTIPLE_LINE_MAPPINGS_MAPPING.as_bytes()); | ||
| let mut buf = Vec::new(); | ||
| ProguardCache::write(&mapping, &mut buf).unwrap(); | ||
| let cache = ProguardCache::parse(&buf).unwrap(); | ||
| cache.test(); | ||
|
|
||
| let actual = cache.remap_stacktrace(input).unwrap(); | ||
| assert_eq!(actual.trim(), expected.trim()); | ||
| } | ||
|
|
||
| // ============================================================================= | ||
| // AmbiguousWithSignatureStackTrace (non-verbose retrace output) | ||
| // ============================================================================= | ||
|
|
||
| const AMBIGUOUS_WITH_SIGNATURE_MAPPING: &str = "\ | ||
| com.android.tools.r8.Internal -> com.android.tools.r8.Internal: | ||
| 10:10:void foo(int):10:10 -> zza | ||
| 11:11:void foo(int, int):11:11 -> zza | ||
| 12:12:void foo(int, boolean):12:12 -> zza | ||
| 13:13:boolean foo(int, int):13:13 -> zza | ||
| "; | ||
|
|
||
| #[test] | ||
| fn test_ambiguous_with_signature_stacktrace() { | ||
| let input = "\ | ||
| java.lang.IndexOutOfBoundsException | ||
| at java.util.ArrayList.get(ArrayList.java:411) | ||
| at com.android.tools.r8.Internal.zza(Unknown) | ||
| "; | ||
|
|
||
| let expected = "\ | ||
| java.lang.IndexOutOfBoundsException | ||
| at java.util.ArrayList.get(ArrayList.java:411) | ||
| at com.android.tools.r8.Internal.foo(Internal.java:0) | ||
| "; | ||
|
|
||
| let mapper = ProguardMapper::from(AMBIGUOUS_WITH_SIGNATURE_MAPPING); | ||
| let actual = mapper.remap_stacktrace(input).unwrap(); | ||
| assert_eq!(actual.trim(), expected.trim()); | ||
|
|
||
| let mapping = ProguardMapping::new(AMBIGUOUS_WITH_SIGNATURE_MAPPING.as_bytes()); | ||
| let mut buf = Vec::new(); | ||
| ProguardCache::write(&mapping, &mut buf).unwrap(); | ||
| let cache = ProguardCache::parse(&buf).unwrap(); | ||
| cache.test(); | ||
|
|
||
| let actual = cache.remap_stacktrace(input).unwrap(); | ||
| assert_eq!(actual.trim(), expected.trim()); | ||
| } | ||
|
|
||
| // ============================================================================= | ||
| // InlineNoLineAssumeNoInlineAmbiguousStackTrace | ||
| // ============================================================================= | ||
|
|
||
| const INLINE_NO_LINE_ASSUME_NO_INLINE_AMBIGUOUS_MAPPING: &str = "\ | ||
| retrace.Main -> a: | ||
| void otherMain(java.lang.String[]) -> foo | ||
| 2:2:void method1(java.lang.String):0:0 -> foo | ||
| 2:2:void main(java.lang.String[]):0 -> foo | ||
| "; | ||
|
|
||
| #[test] | ||
| fn test_inline_no_line_assume_no_inline_ambiguous_stacktrace() { | ||
| let input = "\ | ||
| Exception in thread \"main\" java.lang.NullPointerException | ||
| at a.foo(Unknown Source) | ||
| "; | ||
|
|
||
| // When no line info is available, prefer base (no-line) mappings if present. | ||
| let expected = "\ | ||
| Exception in thread \"main\" java.lang.NullPointerException | ||
| at retrace.Main.otherMain(Main.java:0) | ||
| "; | ||
|
|
||
| let mapper = ProguardMapper::from(INLINE_NO_LINE_ASSUME_NO_INLINE_AMBIGUOUS_MAPPING); | ||
| let actual = mapper.remap_stacktrace(input).unwrap(); | ||
| assert_eq!(actual.trim(), expected.trim()); | ||
|
|
||
| let mapping = | ||
| ProguardMapping::new(INLINE_NO_LINE_ASSUME_NO_INLINE_AMBIGUOUS_MAPPING.as_bytes()); | ||
| let mut buf = Vec::new(); | ||
| ProguardCache::write(&mapping, &mut buf).unwrap(); | ||
| let cache = ProguardCache::parse(&buf).unwrap(); | ||
| cache.test(); | ||
|
|
||
| let actual = cache.remap_stacktrace(input).unwrap(); | ||
| assert_eq!(actual.trim(), expected.trim()); | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
R8's expected output looks like this:
but our UI does not really support the
<OR>case, so I'm unsure if we should tackle this now? Ideally though, we should, because the current remapped stacktrace can be quite misleading.