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 04e37da47..efc2a9588 100644 --- a/jadx-core/src/main/java/jadx/core/codegen/InsnGen.java +++ b/jadx-core/src/main/java/jadx/core/codegen/InsnGen.java @@ -104,8 +104,7 @@ public class InsnGen { } else if (arg.isLiteral()) { code.add(lit((LiteralArg) arg)); } else if (arg.isInsnWrap()) { - Flags flag = wrap ? Flags.BODY_ONLY : Flags.BODY_ONLY_NOWRAP; - makeInsn(((InsnWrapArg) arg).getWrapInsn(), code, flag); + addWrappedArg(code, (InsnWrapArg) arg, wrap); } else if (arg.isNamed()) { code.add(((Named) arg).getName()); } else { @@ -113,6 +112,18 @@ public class InsnGen { } } + private void addWrappedArg(CodeWriter code, InsnWrapArg arg, boolean wrap) throws CodegenException { + InsnNode wrapInsn = arg.getWrapInsn(); + if (wrapInsn.contains(AFlag.FORCE_ASSIGN_INLINE)) { + code.add('('); + makeInsn(wrapInsn, code, Flags.INLINE); + code.add(')'); + } else { + Flags flags = wrap ? Flags.BODY_ONLY : Flags.BODY_ONLY_NOWRAP; + makeInsn(wrapInsn, code, flags); + } + } + public void assignVar(CodeWriter code, InsnNode insn) throws CodegenException { RegisterArg arg = insn.getResult(); if (insn.contains(AFlag.DECLARE_VAR)) { @@ -922,10 +933,7 @@ public class InsnGen { if (parentInsn.contains(AFlag.WRAPPED)) { return false; } - if (callMthNode.getReturnType().equals(ArgType.VOID)) { - return false; - } - return true; + return !callMthNode.getReturnType().equals(ArgType.VOID); } private void makeTernary(TernaryInsn insn, CodeWriter code, Set state) throws CodegenException { 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 ea1b65daf..69348444e 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 @@ -44,6 +44,11 @@ public enum AFlag { */ IMMUTABLE_TYPE, + /** + * Force inline instruction with inline assign + */ + FORCE_ASSIGN_INLINE, + CUSTOM_DECLARE, // variable for this register don't need declaration DECLARE_VAR, diff --git a/jadx-core/src/main/java/jadx/core/dex/regions/conditions/IfInfo.java b/jadx-core/src/main/java/jadx/core/dex/regions/conditions/IfInfo.java index b7df8e494..245207266 100644 --- a/jadx-core/src/main/java/jadx/core/dex/regions/conditions/IfInfo.java +++ b/jadx-core/src/main/java/jadx/core/dex/regions/conditions/IfInfo.java @@ -1,9 +1,12 @@ package jadx.core.dex.regions.conditions; +import java.util.ArrayList; import java.util.HashSet; +import java.util.List; import java.util.Set; import jadx.core.dex.nodes.BlockNode; +import jadx.core.dex.nodes.InsnNode; public final class IfInfo { private final IfCondition condition; @@ -11,32 +14,35 @@ public final class IfInfo { private final BlockNode thenBlock; private final BlockNode elseBlock; private final Set skipBlocks; + private final List forceInlineInsns; private BlockNode outBlock; @Deprecated private BlockNode ifBlock; public IfInfo(IfCondition condition, BlockNode thenBlock, BlockNode elseBlock) { - this(condition, thenBlock, elseBlock, new HashSet<>(), new HashSet<>()); + this(condition, thenBlock, elseBlock, new HashSet<>(), new HashSet<>(), new ArrayList<>()); } public IfInfo(IfInfo info, BlockNode thenBlock, BlockNode elseBlock) { - this(info.getCondition(), thenBlock, elseBlock, info.getMergedBlocks(), info.getSkipBlocks()); + this(info.getCondition(), thenBlock, elseBlock, + info.getMergedBlocks(), info.getSkipBlocks(), info.getForceInlineInsns()); } private IfInfo(IfCondition condition, BlockNode thenBlock, BlockNode elseBlock, - Set mergedBlocks, Set skipBlocks) { + Set mergedBlocks, Set skipBlocks, List forceInlineInsns) { this.condition = condition; this.thenBlock = thenBlock; this.elseBlock = elseBlock; this.mergedBlocks = mergedBlocks; this.skipBlocks = skipBlocks; + this.forceInlineInsns = forceInlineInsns; } public static IfInfo invert(IfInfo info) { IfCondition invertedCondition = IfCondition.invert(info.getCondition()); IfInfo tmpIf = new IfInfo(invertedCondition, info.getElseBlock(), info.getThenBlock(), - info.getMergedBlocks(), info.getSkipBlocks()); + info.getMergedBlocks(), info.getSkipBlocks(), info.getForceInlineInsns()); tmpIf.setIfBlock(info.getIfBlock()); return tmpIf; } @@ -45,6 +51,7 @@ public final class IfInfo { for (IfInfo info : arr) { mergedBlocks.addAll(info.getMergedBlocks()); skipBlocks.addAll(info.getSkipBlocks()); + addInsnsForForcedInline(info.getForceInlineInsns()); } } @@ -84,6 +91,18 @@ public final class IfInfo { this.ifBlock = ifBlock; } + public List getForceInlineInsns() { + return forceInlineInsns; + } + + public void resetForceInlineInsns() { + forceInlineInsns.clear(); + } + + public void addInsnsForForcedInline(List insns) { + forceInlineInsns.addAll(insns); + } + @Override public String toString() { return "IfInfo: then: " + thenBlock + ", else: " + elseBlock; diff --git a/jadx-core/src/main/java/jadx/core/dex/visitors/regions/IfMakerHelper.java b/jadx-core/src/main/java/jadx/core/dex/visitors/regions/IfMakerHelper.java index 948861926..8525b48f5 100644 --- a/jadx-core/src/main/java/jadx/core/dex/visitors/regions/IfMakerHelper.java +++ b/jadx-core/src/main/java/jadx/core/dex/visitors/regions/IfMakerHelper.java @@ -1,10 +1,12 @@ package jadx.core.dex.visitors.regions; +import java.util.ArrayList; import java.util.Collection; import java.util.List; import java.util.Objects; import java.util.Set; +import org.jetbrains.annotations.Nullable; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -35,11 +37,13 @@ public class IfMakerHelper { private IfMakerHelper() { } + @Nullable static IfInfo makeIfInfo(BlockNode ifBlock) { - IfNode ifNode = (IfNode) BlockUtils.getLastInsn(ifBlock); - if (ifNode == null) { - throw new JadxRuntimeException("Empty IF block: " + ifBlock); + InsnNode lastInsn = BlockUtils.getLastInsn(ifBlock); + if (lastInsn == null || lastInsn.getType() != InsnType.IF) { + return null; } + IfNode ifNode = (IfNode) lastInsn; IfCondition condition = IfCondition.fromIfNode(ifNode); IfInfo info = new IfInfo(condition, ifNode.getThenBlock(), ifNode.getElseBlock()); info.setIfBlock(ifBlock); @@ -48,8 +52,11 @@ public class IfMakerHelper { } static IfInfo searchNestedIf(IfInfo info) { - IfInfo tmp = mergeNestedIfNodes(info); - return tmp != null ? tmp : info; + IfInfo next = mergeNestedIfNodes(info); + if (next != null) { + return next; + } + return info; } static IfInfo restructureIf(MethodNode mth, BlockNode block, IfInfo info) { @@ -160,12 +167,24 @@ public class IfMakerHelper { return null; } } + + boolean assignInlineNeeded = !nextIf.getForceInlineInsns().isEmpty(); + if (assignInlineNeeded) { + for (BlockNode mergedBlock : currentIf.getMergedBlocks()) { + if (mergedBlock.contains(AFlag.LOOP_START)) { + // don't inline assigns into loop condition + return currentIf; + } + } + } + if (isInversionNeeded(currentIf, nextIf)) { // invert current node for match pattern nextIf = IfInfo.invert(nextIf); } - if (!isEqualPaths(curThen, nextIf.getThenBlock()) - && !isEqualPaths(curElse, nextIf.getElseBlock())) { + boolean thenPathSame = isEqualPaths(curThen, nextIf.getThenBlock()); + boolean elsePathSame = isEqualPaths(curElse, nextIf.getElseBlock()); + if (!thenPathSame && !elsePathSame) { // complex condition, run additional checks if (checkConditionBranches(curThen, curElse) || checkConditionBranches(curElse, curThen)) { @@ -191,6 +210,15 @@ public class IfMakerHelper { } else { return currentIf; } + } else { + if (assignInlineNeeded) { + boolean sameOuts = (thenPathSame && !followThenBranch) || (elsePathSame && followThenBranch); + if (!sameOuts) { + // don't inline assigns inside simple condition + currentIf.resetForceInlineInsns(); + return currentIf; + } + } } IfInfo result = mergeIfInfo(currentIf, nextIf, followThenBranch); @@ -315,36 +343,32 @@ public class IfMakerHelper { } info.getSkipBlocks().clear(); } + for (InsnNode forceInlineInsn : info.getForceInlineInsns()) { + forceInlineInsn.add(AFlag.FORCE_ASSIGN_INLINE); + } } private static IfInfo getNextIf(IfInfo info, BlockNode block) { if (!canSelectNext(info, block)) { return null; } - BlockNode nestedIfBlock = getNextIfNode(block); - if (nestedIfBlock != null) { - return makeIfInfo(nestedIfBlock); - } - return null; + return getNextIfNodeInfo(info, block); } private static boolean canSelectNext(IfInfo info, BlockNode block) { if (block.getPredecessors().size() == 1) { return true; } - if (info.getMergedBlocks().containsAll(block.getPredecessors())) { - return true; - } - return false; + return info.getMergedBlocks().containsAll(block.getPredecessors()); } - private static BlockNode getNextIfNode(BlockNode block) { + private static IfInfo getNextIfNodeInfo(IfInfo info, BlockNode block) { if (block == null || block.contains(AType.LOOP) || block.contains(AFlag.ADDED_TO_REGION)) { return null; } InsnNode lastInsn = BlockUtils.getLastInsn(block); if (lastInsn != null && lastInsn.getType() == InsnType.IF) { - return block; + return makeIfInfo(block); } // skip this block and search in successors chain List successors = block.getSuccessors(); @@ -358,6 +382,7 @@ public class IfMakerHelper { } List insns = block.getInstructions(); boolean pass = true; + List forceInlineInsns = new ArrayList<>(); if (!insns.isEmpty()) { // check that all instructions can be inlined for (InsnNode insn : insns) { @@ -367,7 +392,9 @@ public class IfMakerHelper { break; } List useList = res.getSVar().getUseList(); - if (useList.size() != 1) { + int useCount = useList.size(); + if (useCount == 0) { + // TODO? pass = false; break; } @@ -378,12 +405,20 @@ public class IfMakerHelper { pass = false; break; } + if (useCount > 1) { + forceInlineInsns.add(insn); + } } } - if (pass) { - return getNextIfNode(next); + if (!pass) { + return null; } - return null; + IfInfo nextInfo = makeIfInfo(next); + if (nextInfo == null) { + return getNextIfNodeInfo(info, next); + } + nextInfo.addInsnsForForcedInline(forceInlineInsns); + return nextInfo; } private static void skipSimplePath(BlockNode block, Set skipped) { diff --git a/jadx-core/src/main/java/jadx/core/dex/visitors/regions/RegionMaker.java b/jadx-core/src/main/java/jadx/core/dex/visitors/regions/RegionMaker.java index 22e331b92..eba7e43e7 100644 --- a/jadx-core/src/main/java/jadx/core/dex/visitors/regions/RegionMaker.java +++ b/jadx-core/src/main/java/jadx/core/dex/visitors/regions/RegionMaker.java @@ -655,6 +655,9 @@ public class RegionMaker { } IfInfo currentIf = makeIfInfo(block); + if (currentIf == null) { + return null; + } IfInfo mergedIf = mergeNestedIfNodes(currentIf); if (mergedIf != null) { currentIf = mergedIf; diff --git a/jadx-core/src/main/java/jadx/core/dex/visitors/shrink/CodeShrinkVisitor.java b/jadx-core/src/main/java/jadx/core/dex/visitors/shrink/CodeShrinkVisitor.java index eedb71c6c..0ab722a76 100644 --- a/jadx-core/src/main/java/jadx/core/dex/visitors/shrink/CodeShrinkVisitor.java +++ b/jadx-core/src/main/java/jadx/core/dex/visitors/shrink/CodeShrinkVisitor.java @@ -20,6 +20,7 @@ import jadx.core.dex.visitors.JadxVisitor; import jadx.core.dex.visitors.ModVisitor; import jadx.core.utils.BlockUtils; import jadx.core.utils.InsnList; +import jadx.core.utils.InsnRemover; import jadx.core.utils.exceptions.JadxRuntimeException; @JadxVisitor( @@ -78,12 +79,15 @@ public class CodeShrinkVisitor extends AbstractVisitor { if (sVar == null || sVar.getAssign().contains(AFlag.DONT_INLINE)) { return; } - // allow inline only one use arg - if (sVar.getVariableUseCount() != 1) { + InsnNode assignInsn = sVar.getAssign().getParentInsn(); + if (assignInsn == null + || assignInsn.contains(AFlag.DONT_INLINE) + || assignInsn.contains(AFlag.WRAPPED)) { return; } - InsnNode assignInsn = sVar.getAssign().getParentInsn(); - if (assignInsn == null || assignInsn.contains(AFlag.DONT_INLINE)) { + // allow inline only one use arg + boolean assignInline = assignInsn.contains(AFlag.FORCE_ASSIGN_INLINE); + if (!assignInline && sVar.getVariableUseCount() != 1) { return; } List useList = sVar.getUseList(); @@ -96,6 +100,10 @@ public class CodeShrinkVisitor extends AbstractVisitor { int assignPos = insnList.getIndex(assignInsn); if (assignPos != -1) { + if (assignInline) { + // TODO? + return; + } WrapInfo wrapInfo = argsInfo.checkInline(assignPos, arg); if (wrapInfo != null) { wrapList.add(wrapInfo); @@ -106,11 +114,30 @@ public class CodeShrinkVisitor extends AbstractVisitor { if (assignBlock != null && assignInsn != arg.getParentInsn() && canMoveBetweenBlocks(assignInsn, assignBlock, block, argsInfo.getInsn())) { - inline(mth, arg, assignInsn, assignBlock); + if (assignInline) { + assignInline(mth, arg, assignInsn, assignBlock); + } else { + inline(mth, arg, assignInsn, assignBlock); + } } } } + private static void assignInline(MethodNode mth, RegisterArg arg, InsnNode assignInsn, BlockNode assignBlock) { + RegisterArg useArg = arg.getSVar().getUseList().get(0); + InsnNode useInsn = useArg.getParentInsn(); + if (useInsn == null || useInsn.contains(AFlag.DONT_GENERATE)) { + return; + } + + InsnArg replaceArg = InsnArg.wrapArg(assignInsn.copy()); + useInsn.replaceArg(useArg, replaceArg); + + assignInsn.add(AFlag.REMOVE); + assignInsn.add(AFlag.DONT_GENERATE); + InsnRemover.remove(mth, assignBlock, assignInsn); + } + private static boolean inline(MethodNode mth, RegisterArg arg, InsnNode insn, BlockNode block) { InsnNode parentInsn = arg.getParentInsn(); if (parentInsn != null && parentInsn.getType() == InsnType.RETURN) { diff --git a/jadx-core/src/test/java/jadx/tests/integration/conditions/TestConditions19.java b/jadx-core/src/test/java/jadx/tests/integration/conditions/TestConditions19.java index bb816455a..f14fc5026 100644 --- a/jadx-core/src/test/java/jadx/tests/integration/conditions/TestConditions19.java +++ b/jadx-core/src/test/java/jadx/tests/integration/conditions/TestConditions19.java @@ -2,7 +2,6 @@ package jadx.tests.integration.conditions; import org.junit.jupiter.api.Test; -import jadx.NotYetImplemented; import jadx.core.dex.nodes.ClassNode; import jadx.tests.api.IntegrationTest; @@ -41,13 +40,13 @@ public class TestConditions19 extends IntegrationTest { } @Test - @NotYetImplemented("Inner assignment or labeled block with break") public void test() { noDebugInfo(); ClassNode cls = getClassNode(TestCls.class); String code = cls.getCode().toString(); + assertThat(code, containsOne("str.length()")); assertThat(code, containsOne("System.out.println(\"done\");")); } } diff --git a/jadx-core/src/test/java/jadx/tests/integration/loops/TestLoopCondition.java b/jadx-core/src/test/java/jadx/tests/integration/loops/TestLoopCondition.java index e99f26abc..f77b153d8 100644 --- a/jadx-core/src/test/java/jadx/tests/integration/loops/TestLoopCondition.java +++ b/jadx-core/src/test/java/jadx/tests/integration/loops/TestLoopCondition.java @@ -11,27 +11,7 @@ import static org.hamcrest.MatcherAssert.assertThat; public class TestLoopCondition extends IntegrationTest { public static class TestCls { - public String f; - - private void setEnabled(boolean r1z) { - } - - public void testIfInLoop() { - int j = 0; - for (int i = 0; i < f.length(); i++) { - char ch = f.charAt(i); - if (ch == '/') { - j++; - if (j == 2) { - setEnabled(true); - return; - } - } - } - setEnabled(false); - } - - public void testMoreComplexIfInLoop(java.util.ArrayList list) throws Exception { + public void test(java.util.ArrayList list) { for (int i = 0; i != 16 && i < 255; i++) { list.set(i, "ABC"); if (i == 128) { @@ -47,12 +27,7 @@ public class TestLoopCondition extends IntegrationTest { ClassNode cls = getClassNode(TestCls.class); String code = cls.getCode().toString(); - assertThat(code, containsOne("i < this.f.length()")); assertThat(code, containsOne("list.set(i, \"ABC\")")); assertThat(code, containsOne("list.set(i, \"DEF\")")); - - assertThat(code, containsOne("if (j == 2) {")); - assertThat(code, containsOne("setEnabled(true);")); - assertThat(code, containsOne("setEnabled(false);")); } } diff --git a/jadx-core/src/test/java/jadx/tests/integration/variables/TestVariables5.java b/jadx-core/src/test/java/jadx/tests/integration/variables/TestVariables5.java index fb51bee86..06213eb27 100644 --- a/jadx-core/src/test/java/jadx/tests/integration/variables/TestVariables5.java +++ b/jadx-core/src/test/java/jadx/tests/integration/variables/TestVariables5.java @@ -18,12 +18,12 @@ public class TestVariables5 extends IntegrationTest { private boolean enabled; private void testIfInLoop() { - int j = 0; - for (int i = 0; i < f.length(); i++) { - char ch = f.charAt(i); + int i = 0; + for (int i2 = 0; i2 < f.length(); i2++) { + char ch = f.charAt(i2); if (ch == '/') { - j++; - if (j == 2) { + i++; + if (i == 2) { setEnabled(true); return; } @@ -51,6 +51,8 @@ public class TestVariables5 extends IntegrationTest { assertThat(code, not(containsString("int i2++;"))); assertThat(code, containsOne("int i = 0;")); - assertThat(code, containsOne("i++;")); + assertThat(code, containsOne("&& (i = i + 1) == 2")); + // assertThat(code, containsOne("i++;")); + // assertThat(code, containsOne("if (i == 2) {")); } }