diff --git a/jadx-core/src/main/java/jadx/core/dex/instructions/mods/ConstructorInsn.java b/jadx-core/src/main/java/jadx/core/dex/instructions/mods/ConstructorInsn.java index cfdfea86f9db68a6e36bdb2191be5b1161d9924a..d4dc6b6bb615ace8ddc36a5e5127c2cd2728bb38 100644 --- a/jadx-core/src/main/java/jadx/core/dex/instructions/mods/ConstructorInsn.java +++ b/jadx-core/src/main/java/jadx/core/dex/instructions/mods/ConstructorInsn.java @@ -15,7 +15,7 @@ public class ConstructorInsn extends InsnNode implements CallMthInterface { private final CallType callType; private final RegisterArg instanceArg; - private enum CallType { + public enum CallType { CONSTRUCTOR, // just new instance SUPER, // super call THIS, // call constructor from other constructor 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 7e9c41aae692bad99cde25ff32643c51a5cc4217..6cc675f0463f77ab6af1e270456e90c15f56ceca 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 @@ -1,9 +1,16 @@ package jadx.core.dex.visitors; +import java.util.Collections; +import java.util.HashSet; import java.util.Iterator; import java.util.List; +import java.util.Set; + +import org.jetbrains.annotations.Nullable; import jadx.core.dex.attributes.AFlag; +import jadx.core.dex.attributes.AType; +import jadx.core.dex.attributes.nodes.DeclareVariablesAttr; import jadx.core.dex.instructions.ArithNode; import jadx.core.dex.instructions.ArithOp; import jadx.core.dex.instructions.InsnType; @@ -13,16 +20,16 @@ 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.IRegion; +import jadx.core.dex.nodes.InsnContainer; import jadx.core.dex.nodes.InsnNode; import jadx.core.dex.nodes.MethodNode; +import jadx.core.dex.regions.Region; 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.BlockUtils; +import jadx.core.utils.InsnList; import jadx.core.utils.exceptions.JadxException; /** @@ -52,7 +59,7 @@ public class PrepareForCodeGen extends AbstractVisitor { removeParenthesis(block); modifyArith(block); } - commentOutInsnsInConstructor(mth); + moveConstructorInConstructor(mth); } private static void removeInstructions(BlockNode block) { @@ -179,15 +186,52 @@ public class PrepareForCodeGen extends AbstractVisitor { } } - private void commentOutInsnsInConstructor(MethodNode mth) { + /** + * Check that 'super' or 'this' call in constructor is a first instruction. + * Otherwise move to top and add a warning if code breaks. + */ + private void moveConstructorInConstructor(MethodNode mth) { if (mth.isConstructor()) { ConstructorInsn constrInsn = searchConstructorCall(mth); if (constrInsn != null && !constrInsn.contains(AFlag.DONT_GENERATE)) { - DepthRegionTraversal.traverse(mth, new ConstructorRegionVisitor(constrInsn)); + Region oldRootRegion = mth.getRegion(); + boolean firstInsn = BlockUtils.isFirstInsn(mth, constrInsn); + DeclareVariablesAttr declVarsAttr = oldRootRegion.get(AType.DECLARE_VARIABLES); + if (firstInsn && declVarsAttr == null) { + // move not needed + return; + } + + // move constructor instruction to new root region + String callType = constrInsn.getCallType().toString().toLowerCase(); + BlockNode blockByInsn = BlockUtils.getBlockByInsn(mth, constrInsn); + if (blockByInsn == null) { + mth.addWarn("Failed to move " + callType + " instruction to top"); + return; + } + InsnList.remove(blockByInsn, constrInsn); + + Region region = new Region(null); + region.add(new InsnContainer(Collections.singletonList(constrInsn))); + region.add(oldRootRegion); + mth.setRegion(region); + + if (!firstInsn) { + Set regArgs = new HashSet<>(); + constrInsn.getRegisterArgs(regArgs); + regArgs.remove(mth.getThisArg()); + regArgs.removeAll(mth.getArguments(false)); + if (!regArgs.isEmpty()) { + mth.addWarn("Illegal instructions before constructor call"); + } else { + mth.addComment("JADX INFO: " + callType + " call moved to the top of the method (can break code semantics)"); + } + } } } } + @Nullable private ConstructorInsn searchConstructorCall(MethodNode mth) { for (BlockNode block : mth.getBasicBlocks()) { for (InsnNode insn : block.getInstructions()) { @@ -202,72 +246,4 @@ public class PrepareForCodeGen extends AbstractVisitor { } return null; } - - private static final class ConstructorRegionVisitor extends AbstractRegionVisitor { - private final ConstructorInsn constrInsn; - private int regionDepth; - private boolean found; - private boolean brokenCode; - private int commentedCount; - - public ConstructorRegionVisitor(ConstructorInsn constrInsn) { - this.constrInsn = constrInsn; - } - - @Override - public boolean enterRegion(MethodNode mth, IRegion region) { - if (found) { - return false; - } - regionDepth++; - return true; - } - - @Override - public void leaveRegion(MethodNode mth, IRegion region) { - if (!found) { - regionDepth--; - region.add(AFlag.COMMENT_OUT); - commentedCount++; - } - } - - @Override - public void processBlock(MethodNode mth, IBlock container) { - if (found) { - return; - } - for (InsnNode insn : container.getInstructions()) { - if (insn == constrInsn) { - found = true; - addMethodMsg(mth); - break; - } - insn.add(AFlag.COMMENT_OUT); - commentedCount++; - if (!brokenCode) { - RegisterArg resArg = insn.getResult(); - if (resArg != null) { - for (RegisterArg arg : resArg.getSVar().getUseList()) { - if (arg.getParentInsn() == constrInsn) { - brokenCode = true; - break; - } - } - } - } - } - } - - private void addMethodMsg(MethodNode mth) { - if (commentedCount > 0) { - String msg = "Illegal instructions before constructor call commented (this can break semantics)"; - if (brokenCode || regionDepth > 1) { - mth.addWarn(msg); - } else { - mth.addComment("JADX WARN: " + msg); - } - } - } - } } diff --git a/jadx-core/src/main/java/jadx/core/utils/BlockUtils.java b/jadx-core/src/main/java/jadx/core/utils/BlockUtils.java index 48d58bd7b9335f46cdc397c0f7b70a72244b00eb..c4b2bd124fe7e9cd9386a321a5066b7772672004 100644 --- a/jadx-core/src/main/java/jadx/core/utils/BlockUtils.java +++ b/jadx-core/src/main/java/jadx/core/utils/BlockUtils.java @@ -574,6 +574,14 @@ public class BlockUtils { return insns; } + public static boolean isFirstInsn(MethodNode mth, InsnNode insn) { + BlockNode enterBlock = mth.getEnterBlock(); + if (enterBlock == null || enterBlock.getInstructions().isEmpty()) { + return false; + } + return enterBlock.getInstructions().get(0) == insn; + } + /** * Replace insn by index i in block, * for proper copy attributes, assume attributes are not overlap 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 index c0185d659926edcd4795b6452748bb272ddf58bc..f8fd01e1e7b38b28d8637418b4a0d4e353db553d 100644 --- a/jadx-core/src/test/java/jadx/tests/integration/others/TestInsnsBeforeSuper.java +++ b/jadx-core/src/test/java/jadx/tests/integration/others/TestInsnsBeforeSuper.java @@ -37,6 +37,6 @@ public class TestInsnsBeforeSuper extends SmaliTest { ClassNode cls = getClassNodeFromSmaliFiles("B"); String code = cls.getCode().toString(); - assertThat(code, containsOne("// checkNull(str);")); + assertThat(code, containsOne("checkNull(str);")); } } diff --git a/jadx-core/src/test/java/jadx/tests/integration/others/TestInsnsBeforeThis.java b/jadx-core/src/test/java/jadx/tests/integration/others/TestInsnsBeforeThis.java index 0fe08b2e2b0b4c2fddb8aa292c54919cdc64e4b4..c425766e3a95565b4f638cbdb83a7192b32ec9bf 100644 --- a/jadx-core/src/test/java/jadx/tests/integration/others/TestInsnsBeforeThis.java +++ b/jadx-core/src/test/java/jadx/tests/integration/others/TestInsnsBeforeThis.java @@ -35,6 +35,6 @@ public class TestInsnsBeforeThis extends SmaliTest { ClassNode cls = getClassNodeFromSmali(); String code = cls.getCode().toString(); - assertThat(code, containsOne("// checkNull(str);")); + assertThat(code, containsOne("checkNull(str);")); } }