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

feat: add generic method information to .jcst #564

Merged
merged 3 commits into from
Apr 9, 2019
Merged

feat: add generic method information to .jcst #564

merged 3 commits into from
Apr 9, 2019

Conversation

asashour
Copy link
Contributor

@asashour asashour commented Apr 8, 2019

This change writes the following method information to .jcst, which is the first part of #552.

  • Method if any of its arguments is GenericObject or GenericType, and only those arguments are saved.
  • Method if it has varargs.

Other options:

  • Currently method shortId is saved, however we can save its hashCode instead (we can also verify if two methods have same hashCodes before writing).
  • Writing method throws, will make many more methods are saved, not sure if this is needed.

@skylot
Copy link
Owner

skylot commented Apr 8, 2019

Great work!
One issue: in new android-5.1.jar you removed some files used in resource decoding (android folder inside jar). I don't remember if they really needed because resource decoding is a little messy right now and don't have any tests. And btw which android version you use to build new jcst file? I think we need the latest version, but in this case, we need to rename jar and update these resource files.

About using hashCode as method id: it is possible, but we can't use java built-in string hashCode, because it can change. And we need to support collision case because we can't "rename" methods. Also in new Android can be added a new method which can collide with old one and jadx will find old instead of missing a new one, but this is very unlikely :).
So I think string shortId is easier to implement and use.

throws info needed to fix cases of not needed try/catch wrapping like in test TestTryAfterDeclaration (PR #497). In this test generated two try blocks and this is exactly what written in bytecode. One of the blocks wraps System.out.println which not throws any exception and this lead to compilation error. So if jadx knows throws it can eliminate such incorrect try wrapping.

@asashour
Copy link
Contributor Author

asashour commented Apr 8, 2019

One issue: in new android-5.1.jar you removed some files used in resource decoding (android folder inside jar).

Actually, it wasn't deliberate. I just used the tool ConvertToClsSet to point to clsp-data\android-5.1.jar, it seems to delete it, I will change this behavior, not to remove other files.

And which android version you use to build new jcst file? I think we need the latest version, but in this case, we need to rename jar and update these resource files.

It is the latest, SDK tools says the below, but I don't know which is the actual android version.

tools   | 26.1.1  | Android SDK Tools 26.1.1 | tools\

I also don't know where the source for these resource files is.

For throws, I know the test method, but I would leave this for further PR/discussion, since I think there is a way to tackle it, without looking into the throw possibilities.

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