From c6c54f90dc31d5a784828906ef25666675447e4a Mon Sep 17 00:00:00 2001 From: Skylot Date: Wed, 3 Jul 2019 14:27:21 +0300 Subject: [PATCH] fix: comment out instructions before super call in constructor (#685) --- .../main/java/jadx/core/codegen/InsnGen.java | 3 ++ .../java/jadx/core/dex/attributes/AFlag.java | 4 +- .../core/dex/visitors/PrepareForCodeGen.java | 37 +++++++++++++++++ .../others/TestInsnsBeforeSuper.java | 41 +++++++++++++++++++ .../smali/others/TestInsnsBeforeSuper/A.smali | 14 +++++++ .../smali/others/TestInsnsBeforeSuper/B.smali | 30 ++++++++++++++ 6 files changed, 128 insertions(+), 1 deletion(-) create mode 100644 jadx-core/src/test/java/jadx/tests/integration/others/TestInsnsBeforeSuper.java create mode 100644 jadx-core/src/test/smali/others/TestInsnsBeforeSuper/A.smali create mode 100644 jadx-core/src/test/smali/others/TestInsnsBeforeSuper/B.smali diff --git a/jadx-core/src/main/java/jadx/core/codegen/InsnGen.java b/jadx-core/src/main/java/jadx/core/codegen/InsnGen.java index 60e97876f..9f748a2fc 100644 --- a/jadx-core/src/main/java/jadx/core/codegen/InsnGen.java +++ b/jadx-core/src/main/java/jadx/core/codegen/InsnGen.java @@ -227,6 +227,9 @@ public class InsnGen { if (attachInsns) { code.attachLineAnnotation(insn); } + if (insn.contains(AFlag.COMMENT_OUT)) { + code.add("// "); + } } if (insn.getResult() != null) { SSAVar var = insn.getResult().getSVar(); diff --git a/jadx-core/src/main/java/jadx/core/dex/attributes/AFlag.java b/jadx-core/src/main/java/jadx/core/dex/attributes/AFlag.java index d218cde95..2b2a549f7 100644 --- a/jadx-core/src/main/java/jadx/core/dex/attributes/AFlag.java +++ b/jadx-core/src/main/java/jadx/core/dex/attributes/AFlag.java @@ -15,9 +15,11 @@ public enum AFlag { DONT_WRAP, DONT_INLINE, DONT_GENERATE, // process as usual, but don't output to generated code + COMMENT_OUT, // process as usual, but comment insn in generated code + REMOVE, // can be completely removed + RESTART_CODEGEN, DONT_RENAME, // do not rename during deobfuscation - REMOVE, // can be completely removed ADDED_TO_REGION, FINALLY_INSNS, diff --git a/jadx-core/src/main/java/jadx/core/dex/visitors/PrepareForCodeGen.java b/jadx-core/src/main/java/jadx/core/dex/visitors/PrepareForCodeGen.java index a57d4cd86..003a213a7 100644 --- a/jadx-core/src/main/java/jadx/core/dex/visitors/PrepareForCodeGen.java +++ b/jadx-core/src/main/java/jadx/core/dex/visitors/PrepareForCodeGen.java @@ -2,21 +2,26 @@ package jadx.core.dex.visitors; import java.util.Iterator; import java.util.List; +import java.util.Objects; import jadx.core.dex.attributes.AFlag; import jadx.core.dex.instructions.ArithNode; import jadx.core.dex.instructions.ArithOp; import jadx.core.dex.instructions.InsnType; +import jadx.core.dex.instructions.args.ArgType; import jadx.core.dex.instructions.args.InsnArg; import jadx.core.dex.instructions.args.InsnWrapArg; import jadx.core.dex.instructions.args.RegisterArg; import jadx.core.dex.instructions.mods.ConstructorInsn; import jadx.core.dex.instructions.mods.TernaryInsn; import jadx.core.dex.nodes.BlockNode; +import jadx.core.dex.nodes.IBlock; import jadx.core.dex.nodes.InsnNode; import jadx.core.dex.nodes.MethodNode; import jadx.core.dex.regions.conditions.IfCondition; import jadx.core.dex.regions.conditions.IfCondition.Mode; +import jadx.core.dex.visitors.regions.AbstractRegionVisitor; +import jadx.core.dex.visitors.regions.DepthRegionTraversal; import jadx.core.dex.visitors.regions.variables.ProcessVariables; import jadx.core.dex.visitors.shrink.CodeShrinkVisitor; import jadx.core.utils.exceptions.JadxException; @@ -33,6 +38,8 @@ import jadx.core.utils.exceptions.JadxException; ) public class PrepareForCodeGen extends AbstractVisitor { + public static final SuperCallRegionVisitor SUPER_CALL_REGION_VISITOR = new SuperCallRegionVisitor(); + @Override public void visit(MethodNode mth) throws JadxException { List blocks = mth.getBasicBlocks(); @@ -48,6 +55,7 @@ public class PrepareForCodeGen extends AbstractVisitor { removeParenthesis(block); modifyArith(block); } + commentOutInsnsBeforeSuper(mth); } private static void removeInstructions(BlockNode block) { @@ -171,4 +179,33 @@ public class PrepareForCodeGen extends AbstractVisitor { } } } + + private void commentOutInsnsBeforeSuper(MethodNode mth) { + if (mth.isConstructor() && !Objects.equals(mth.getParentClass().getSuperClass(), ArgType.OBJECT)) { + DepthRegionTraversal.traverse(mth, SUPER_CALL_REGION_VISITOR); + } + } + + private static final class SuperCallRegionVisitor extends AbstractRegionVisitor { + @Override + public void processBlock(MethodNode mth, IBlock container) { + for (InsnNode insn : container.getInstructions()) { + InsnType insnType = insn.getType(); + if ((insnType == InsnType.CONSTRUCTOR) && ((ConstructorInsn) insn).isSuper()) { + // found super call + commentOutInsns(container, insn); + // TODO: process all previous regions (in case of branching before super call) + } + } + } + + private static void commentOutInsns(IBlock container, InsnNode superInsn) { + for (InsnNode insn : container.getInstructions()) { + if (insn == superInsn) { + break; + } + insn.add(AFlag.COMMENT_OUT); + } + } + } } diff --git a/jadx-core/src/test/java/jadx/tests/integration/others/TestInsnsBeforeSuper.java b/jadx-core/src/test/java/jadx/tests/integration/others/TestInsnsBeforeSuper.java new file mode 100644 index 000000000..f31c71109 --- /dev/null +++ b/jadx-core/src/test/java/jadx/tests/integration/others/TestInsnsBeforeSuper.java @@ -0,0 +1,41 @@ +package jadx.tests.integration.others; + +import org.junit.jupiter.api.Test; + +import jadx.core.dex.nodes.ClassNode; +import jadx.tests.api.SmaliTest; + +import static jadx.tests.api.utils.JadxMatchers.containsOne; +import static org.hamcrest.MatcherAssert.assertThat; + +public class TestInsnsBeforeSuper extends SmaliTest { + // @formatter:off + /* + public class A { + public A(String s) { + } + } + + public class B extends A { + public B(String str) { + checkNull(str); + super(str); + } + + public void checkNull(Object o) { + if (o == null) { + throw new NullPointerException(); + } + } + } + */ + // @formatter:on + + @Test + public void test() { + ClassNode cls = getClassNodeFromSmaliFiles("B"); + String code = cls.getCode().toString(); + + assertThat(code, containsOne("// checkNull(str);")); + } +} diff --git a/jadx-core/src/test/smali/others/TestInsnsBeforeSuper/A.smali b/jadx-core/src/test/smali/others/TestInsnsBeforeSuper/A.smali new file mode 100644 index 000000000..fa4cbbf56 --- /dev/null +++ b/jadx-core/src/test/smali/others/TestInsnsBeforeSuper/A.smali @@ -0,0 +1,14 @@ +.class public Lothers/A; +.super Ljava/lang/Object; + + +# direct methods +.method public constructor (Ljava/lang/String;)V + .registers 3 + + .prologue + + invoke-direct {p0}, Ljava/lang/Object;->()V + + return-void +.end method diff --git a/jadx-core/src/test/smali/others/TestInsnsBeforeSuper/B.smali b/jadx-core/src/test/smali/others/TestInsnsBeforeSuper/B.smali new file mode 100644 index 000000000..c69aebbcb --- /dev/null +++ b/jadx-core/src/test/smali/others/TestInsnsBeforeSuper/B.smali @@ -0,0 +1,30 @@ +.class public Lothers/B; +.super Lothers/A; + + +# direct methods +.method public constructor (Ljava/lang/String;)V + .registers 3 + + .prologue + invoke-static {p1}, Lothers/B;->checkNull(Ljava/lang/Object;)V + + invoke-direct {p0, p1}, Lothers/A;->(Ljava/lang/String;)V + + return-void +.end method + + +.method public static checkNull(Ljava/lang/Object;)V + .registers 3 + + .prologue + if-nez p0, :cond_8 + + new-instance v0, Ljava/lang/NullPointerException; + invoke-direct {v0}, Ljava/lang/NullPointerException;->()V + throw v0 + + :cond_8 + return-void +.end method