Skip to content
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

fix(proguard): Remap method if there's no line number #1433

Merged
merged 4 commits into from
Apr 10, 2024

Conversation

loewenheim
Copy link
Contributor

@loewenheim loewenheim commented Apr 9, 2024

Right now, the logic for remapping a frame is:

  • Try to remap the frame with line number if available.
  • Otherwise try to remap the frame with params.
  • Otherwise don't remap the frame.

But even putting aside the matter of params, this is not what the Python implementation does. It uses 0 as the line number if it's missing.

Therefore, we now do:

  • Try to remap the frame with line number if available.
  • Otherwise try to remap the frame with params.
  • Otherwise try to remap the frame with line number 0.

I'm honestly surprised that remapping a frame with line 0 ever works, but I've seen it happen.

@loewenheim loewenheim requested a review from a team April 9, 2024 15:24
@loewenheim loewenheim self-assigned this Apr 9, 2024
Copy link
Member

@Swatinem Swatinem left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe I even added an explicit fn to the proguard mapper at one point to try remapping unambiguous method names which does not require a line number.

The 0 fallback is indeed surprising TBH.

@loewenheim loewenheim changed the title fix(proguard): Use line 0 if there's nothing else fix(proguard): Remap method if there's no line number Apr 10, 2024
@loewenheim
Copy link
Contributor Author

I updated this to use ProguardMapper::remap_method instead of line 0. I also refactored map_frame into a more functional version with less repetition (in a separate commit), but I may have gone a bit overboard with the galaxy brain style.

@loewenheim loewenheim requested a review from anonrig April 10, 2024 11:28
Copy link
Member

@Swatinem Swatinem left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lets do it

@loewenheim loewenheim merged commit 1cbb3c1 into master Apr 10, 2024
10 checks passed
@loewenheim loewenheim deleted the fix/proguard-line-0 branch April 10, 2024 11:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants