diff --git a/jadx-core/src/main/java/jadx/core/dex/visitors/regions/TernaryMod.java b/jadx-core/src/main/java/jadx/core/dex/visitors/regions/TernaryMod.java index 48b2c5791..5ef8d9774 100644 --- a/jadx-core/src/main/java/jadx/core/dex/visitors/regions/TernaryMod.java +++ b/jadx-core/src/main/java/jadx/core/dex/visitors/regions/TernaryMod.java @@ -335,6 +335,7 @@ public class TernaryMod extends AbstractRegionVisitor implements IRegionIterativ ternInsn.simplifyCondition(); InsnRemover.unbindAllArgs(mth, phiInsn); + InsnRemover.delistPhi(mth, phiInsn); InsnRemover.unbindResult(mth, insn); InsnList.remove(block, insn); header.getInstructions().clear(); diff --git a/jadx-core/src/main/java/jadx/core/dex/visitors/ssa/SSATransform.java b/jadx-core/src/main/java/jadx/core/dex/visitors/ssa/SSATransform.java index 38e2f5700..3936bafac 100644 --- a/jadx-core/src/main/java/jadx/core/dex/visitors/ssa/SSATransform.java +++ b/jadx-core/src/main/java/jadx/core/dex/visitors/ssa/SSATransform.java @@ -57,8 +57,8 @@ public class SSATransform extends AbstractVisitor { renameVariables(mth); fixLastAssignInTry(mth); removeBlockerInsns(mth); - markThisArgs(mth.getThisArg()); tryToFixUselessPhi(mth); + markThisArgs(mth.getThisArg()); hidePhiInsns(mth); removeUnusedInvokeResults(mth); } @@ -312,6 +312,20 @@ public class SSATransform extends AbstractVisitor { if (allSame) { return replacePhiWithMove(mth, block, phi, phi.getArg(0)); } + SSAVar sameVar = isSameMove(phi); + if (sameVar != null) { + RegisterArg sameArg = sameVar.getAssign().duplicate(); + if (inlinePhiInsn(mth, block, phi, sameArg)) { + for (InsnArg arg : phi.getArguments()) { + InsnNode moveInsn = ((RegisterArg) arg).getAssignInsn(); + if (moveInsn != null) { + moveInsn.add(AFlag.REMOVE); + InsnRemover.remove(mth, moveInsn); + } + } + return true; + } + } return false; } @@ -330,6 +344,32 @@ public class SSATransform extends AbstractVisitor { return allSame; } + private static SSAVar isSameMove(PhiInsn phi) { + SSAVar var = null; + int argsCount = phi.getArgsCount(); + for (int i = 0; i < argsCount; i++) { + RegisterArg arg = phi.getArg(i); + if (arg.getSVar().getUseCount() != 1) { + return null; + } + InsnNode assignInsn = arg.getAssignInsn(); + if (assignInsn == null || assignInsn.getType() != InsnType.MOVE) { + return null; + } + InsnArg moveArg = assignInsn.getArg(0); + if (!moveArg.isRegister()) { + return null; + } + SSAVar moveVar = ((RegisterArg) moveArg).getSVar(); + if (var == null) { + var = moveVar; + } else if (var != moveVar) { + return null; + } + } + return var; + } + private static boolean removePhiList(MethodNode mth, List insnToRemove) { for (BlockNode block : mth.getBasicBlocks()) { PhiListAttr phiList = block.get(AType.PHI_LIST); @@ -372,7 +412,7 @@ public class SSATransform extends AbstractVisitor { argVar.removeUsedInPhi(phi); } // try inline - if (inlinePhiInsn(mth, block, phi)) { + if (inlinePhiInsn(mth, block, phi, phi.getArg(0))) { insns.remove(phiIndex); } else { assign.removeUsedInPhi(phi); @@ -387,29 +427,34 @@ public class SSATransform extends AbstractVisitor { return true; } - private static boolean inlinePhiInsn(MethodNode mth, BlockNode block, PhiInsn phi) { + private static boolean inlinePhiInsn(MethodNode mth, BlockNode block, PhiInsn phi, RegisterArg inlineArg) { SSAVar resVar = phi.getResult().getSVar(); if (resVar == null) { return false; } - RegisterArg arg = phi.getArg(0); - if (arg.getSVar() == null) { + if (inlineArg.getSVar() == null) { return false; } List useList = resVar.getUseList(); for (RegisterArg useArg : new ArrayList<>(useList)) { InsnNode useInsn = useArg.getParentInsn(); - if (useInsn == null || useInsn == phi || useArg.getRegNum() != arg.getRegNum()) { + if (useInsn == null || useInsn == phi) { return false; } - // replace SSAVar in 'useArg' to SSAVar from 'arg' - // no need to replace whole RegisterArg - useArg.getSVar().removeUse(useArg); - arg.getSVar().use(useArg); + if (useArg.getRegNum() == inlineArg.getRegNum()) { + // replace SSAVar in 'useArg' to SSAVar from 'arg' + // no need to replace whole RegisterArg + useArg.getSVar().removeUse(useArg); + inlineArg.getSVar().use(useArg); + } else { + if (!useInsn.replaceArg(useArg, inlineArg)) { + return false; + } + } } if (block.contains(AType.EXC_HANDLER)) { // don't inline into exception handler - InsnNode assignInsn = arg.getAssignInsn(); + InsnNode assignInsn = inlineArg.getAssignInsn(); if (assignInsn != null && !assignInsn.isConstInsn()) { assignInsn.add(AFlag.DONT_INLINE); } diff --git a/jadx-core/src/main/java/jadx/core/utils/DebugChecks.java b/jadx-core/src/main/java/jadx/core/utils/DebugChecks.java index 42dc1b848..f58f52f7a 100644 --- a/jadx-core/src/main/java/jadx/core/utils/DebugChecks.java +++ b/jadx-core/src/main/java/jadx/core/utils/DebugChecks.java @@ -67,6 +67,7 @@ public class DebugChecks { } } checkSSAVars(mth); + quickCheckPhiInsn(mth); // checkPHI(mth); } @@ -228,6 +229,34 @@ public class DebugChecks { } } + public static void quickCheckPhiInsn(MethodNode mth) { + if (mth.getSVars().isEmpty()) { + return; + } + for (BlockNode block : mth.getBasicBlocks()) { + PhiListAttr phiListAttr = block.get(AType.PHI_LIST); + if (phiListAttr != null) { + for (PhiInsn phiInsn : phiListAttr.getList()) { + checkPhiArg(mth, phiInsn, phiInsn.getResult(), () -> "result"); + int argsCount = phiInsn.getArgsCount(); + for (int i = 0; i < argsCount; i++) { + int argNum = i; + checkPhiArg(mth, phiInsn, phiInsn.getArg(argNum), () -> "arg_" + argNum); + } + } + } + } + } + + private static void checkPhiArg(MethodNode mth, PhiInsn phiInsn, RegisterArg arg, Supplier argName) { + if (arg == null) { + throw new JadxRuntimeException("Null " + argName.get() + " in PHI insn: " + phiInsn); + } + if (arg.getSVar() == null) { + throw new JadxRuntimeException("Null SSA variable in " + argName.get() + " in PHI insn: " + phiInsn); + } + } + private static void checkPHI(MethodNode mth) { for (BlockNode block : mth.getBasicBlocks()) { List phis = new ArrayList<>(); diff --git a/jadx-core/src/main/java/jadx/core/utils/InsnRemover.java b/jadx-core/src/main/java/jadx/core/utils/InsnRemover.java index eb737e25f..3b93d9f79 100644 --- a/jadx-core/src/main/java/jadx/core/utils/InsnRemover.java +++ b/jadx-core/src/main/java/jadx/core/utils/InsnRemover.java @@ -10,6 +10,8 @@ import org.jetbrains.annotations.Nullable; import jadx.core.Consts; import jadx.core.dex.attributes.AFlag; +import jadx.core.dex.attributes.AType; +import jadx.core.dex.attributes.nodes.PhiListAttr; import jadx.core.dex.instructions.InsnType; import jadx.core.dex.instructions.PhiInsn; import jadx.core.dex.instructions.args.InsnArg; @@ -277,4 +279,13 @@ public class InsnRemover { unbindInsn(mth, instructions.get(index)); instructions.remove(index); } + + public static void delistPhi(MethodNode mth, PhiInsn phiInsn) { + for (BlockNode block : mth.getBasicBlocks()) { + PhiListAttr phiListAttr = block.get(AType.PHI_LIST); + if (phiListAttr != null) { + phiListAttr.getList().removeIf(i -> i == phiInsn); + } + } + } } diff --git a/jadx-core/src/test/java/jadx/tests/integration/variables/TestThisBranchDup.java b/jadx-core/src/test/java/jadx/tests/integration/variables/TestThisBranchDup.java new file mode 100644 index 000000000..742e58371 --- /dev/null +++ b/jadx-core/src/test/java/jadx/tests/integration/variables/TestThisBranchDup.java @@ -0,0 +1,19 @@ +package jadx.tests.integration.variables; + +import org.junit.jupiter.api.Test; + +import jadx.tests.api.SmaliTest; + +import static jadx.tests.api.utils.assertj.JadxAssertions.assertThat; + +public class TestThisBranchDup extends SmaliTest { + + @Test + public void test() { + disableCompilation(); + assertThat(getClassNodeFromSmali()) + .code() + .containsOne("this(") // constructor type correctly detected + .countString(6, "(i & "); // ternary used and inlined + } +} diff --git a/jadx-core/src/test/smali/variables/TestThisBranchDup.smali b/jadx-core/src/test/smali/variables/TestThisBranchDup.smali new file mode 100644 index 000000000..d99dfa0b4 --- /dev/null +++ b/jadx-core/src/test/smali/variables/TestThisBranchDup.smali @@ -0,0 +1,58 @@ +.class public final Lvariables/TestThisBranchDup; +.super Ljava/lang/Object; + +.method public constructor (ZZZLh3/t;ZZILkotlin/jvm/internal/DefaultConstructorMarker;)V + .registers 10 + + and-int/lit8 p8, p7, 0x1 + if-eqz p8, :cond_5 + const/4 p1, 0x0 + + :cond_5 + and-int/lit8 p8, p7, 0x2 + const/4 v0, 0x1 + if-eqz p8, :cond_b + move p2, v0 + + :cond_b + and-int/lit8 p8, p7, 0x4 + if-eqz p8, :cond_10 + move p3, v0 + + :cond_10 + and-int/lit8 p8, p7, 0x8 + if-eqz p8, :cond_16 + .line 11 + sget-object p4, Lh3/t;->Inherit:Lh3/t; + + :cond_16 + and-int/lit8 p8, p7, 0x10 + if-eqz p8, :cond_1b + move p5, v0 + + :cond_1b + and-int/lit8 p7, p7, 0x20 + if-eqz p7, :cond_27 + move p8, v0 + move-object p6, p4 + move p7, p5 + move p4, p2 + move p5, p3 + move-object p2, p0 + move p3, p1 + goto :goto_2e + + :cond_27 + move p8, p6 + move p7, p5 + move p5, p3 + move-object p6, p4 + move p3, p1 + move p4, p2 + move-object p2, p0 + + .line 12 + :goto_2e + invoke-direct/range {p2 .. p8}, Lvariables/TestThisBranchDup;->(ZZZLh3/t;ZZ)V + return-void +.end method