fix: erroneous SignatureProcessor resolution of standard non-generic …#2460
Conversation
| "java.lang.Float", | ||
| }; | ||
| for (String genericType : stdNonGenericTypes) { | ||
| if (genericType.equals(ty)) { |
There was a problem hiding this comment.
Are you sure this equals check works? genericType is a java.lang.String and ty is an ArgType, thus this if expression can never get true.
Also why does the array contains primitive types? At the beginning of the method there is also the isPrimitive() check thus if it is a primitive this code should never be executed.
There was a problem hiding this comment.
.equals does work, it casts it to String automatically, but I will explicitly call .toString just so it is clear.
Well, the type can still be a primitive, but in case of primitives, even with annotations, they don't get erroneously resolved, so I just have a check at the beginning and I return false
There was a problem hiding this comment.
I have also modified the test so that our smali file also contains a field with a primitive type to showcase primitives are not an issue and should be skipped.
There was a problem hiding this comment.
I have re-read your comment and want to clarify why we have the isPrimitive() check at the start and we also check for "primitive" types.
If in our smali file the fields have primitive types but that are expressed using the fully qualified class name (using the L prefix), they are not resolved to be Primitive ArgType's, so we still have to check for them.
Example:
.field public static L:Lint;There was a problem hiding this comment.
You were more than right. I am sorry, but I suck at java. I have managed to solve the issues, re-run the tests locally, and now it should be fine.
b540c58 to
3111084
Compare
…types with Annotation Signatures
3111084 to
f6a92fa
Compare
|
@nerodesu017 please check my view on the original issue in my comment: #2459 (comment) As a result, I think jadx now correctly process types, but confusing class names clash with primitive type names should be fixed in |
I am sorry, but I am not at all familiar with the codebase, as I've said above, nor am I that knowledgeable about Java. I wanted to thank both of you for helping me out, and I have read your comment which made it a bit more clear for me. Using |
Any identifier is valid in bytecode, because VM don't check names at runtime. And jadx only rename such identifiers and only if options are not forbid this. Anyway, further discussion is better to continue in original issue and I will close current PR as it not needed anymore. |
…types with Annotation Signatures
Description
This PR aims to fix the issue where SignatureProcessor would erroneously resolve the types of fields whose main type is not a standard generic but which also have a Signature Annotation.
Related to #2459