Skip to content

Non-null filtering, because sometimes insn's are null and transformer… #76

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

Closed
wants to merge 1 commit into from

Conversation

Animowany
Copy link
Contributor

… catches error.

Copy link
Collaborator

@EpicPlayerA10 EpicPlayerA10 left a comment

Choose a reason for hiding this comment

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

If instruction is null then some transformer is badly designed. But if you really want, i've requested some small changes.

@@ -19,7 +20,8 @@ public class CompareInstructionsTransformer extends FramedInstructionsTransforme
@Override
protected Stream<AbstractInsnNode> getInstructionsStream(Stream<AbstractInsnNode> stream) {
return stream
.filter(insn -> AsmMathHelper.isMathCompare(insn.getOpcode()));
.filter(Objects::nonNull)
Copy link
Collaborator

Choose a reason for hiding this comment

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

You don't need to check here if it is null as you already check it in FramedInstructionsTransformer

@@ -19,7 +20,8 @@ public class MathOperationsTransformer extends FramedInstructionsTransformer {
@Override
protected Stream<AbstractInsnNode> getInstructionsStream(Stream<AbstractInsnNode> stream) {
return stream
.filter(insn -> AsmMathHelper.isMathOperation(insn.getOpcode()));
.filter(Objects::nonNull)
Copy link
Collaborator

Choose a reason for hiding this comment

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

same here

@@ -18,7 +19,8 @@ public class NumberCastsTransformer extends FramedInstructionsTransformer {
@Override
protected Stream<AbstractInsnNode> getInstructionsStream(Stream<AbstractInsnNode> stream) {
return stream
.filter(insn -> AsmMathHelper.isNumberCast(insn.getOpcode()));
.filter(Objects::nonNull)
Copy link
Collaborator

Choose a reason for hiding this comment

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

same here

@EpicPlayerA10
Copy link
Collaborator

And for future reference, don't commit from a master branch, but create a new fresh branch for your PR. This will make it easier for you in your future PRs to any open-source repository.

@Animowany
Copy link
Contributor Author

You wrote universalnumbertransformer, and i occured many nullpointers, so just inserted a check and now everything is fine.

@EpicPlayerA10
Copy link
Collaborator

You wrote universalnumbertransformer, and i occured many nullpointers, so just inserted a check and now everything is fine.

Hmm. That's unexpected behaviour. Maybe i do some sanity checks after each transformer if any transformer didn't accidentally break.

@Animowany
Copy link
Contributor Author

You wrote universalnumbertransformer, and i occured many nullpointers, so just inserted a check and now everything is fine.

Hmm. That's unexpected behaviour. Maybe i do some sanity checks after each transformer if any transformer didn't accidentally break.

im now deobfuscating pretty heavily obfuscated jar, so maybe it has some as null presented insn's if its even possible

@EpicPlayerA10
Copy link
Collaborator

You wrote universalnumbertransformer, and i occured many nullpointers, so just inserted a check and now everything is fine.

Hmm. That's unexpected behaviour. Maybe i do some sanity checks after each transformer if any transformer didn't accidentally break.

im now deobfuscating pretty heavily obfuscated jar, so maybe it has some as null presented insn's if its even possible

If jar is valid and it is running well, then there shouldn't be any null instructions. Some transformer might broke it.

@Janmm14
Copy link
Contributor

Janmm14 commented Aug 24, 2024

i never had to check an instruction being null with asm api except for getNext()/gtPrevious()

@narumii
Copy link
Owner

narumii commented Aug 25, 2024

You wrote universalnumbertransformer, and i occured many nullpointers, so just inserted a check and now everything is fine.

Hmm. That's unexpected behaviour. Maybe i do some sanity checks after each transformer if any transformer didn't accidentally break.

im now deobfuscating pretty heavily obfuscated jar, so maybe it has some as null presented insn's if its even possible

If jar is valid and it is running well, then there shouldn't be any null instructions. Some transformer might broke it.

Perhaps better solution would be attaching the jar in question as a test suite?
Null insn should not happen, but at this point i would not be suprised since it is asm.

@narumii narumii closed this Aug 25, 2024
@EpicPlayerA10
Copy link
Collaborator

@Animowany Your issue should be partially resolved by 521c096 . Now the deobfuscator will throw an error which transformer did something bad in instructions list, instead of acting this weird behaviour with replacing all instructions with nulls. By this change you can then find the root cause of an error very easily.

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.

4 participants