jessikitty/jadx
1
0
Fork 0
Code Issues Pull Requests 1 Actions 1 Packages Projects Releases Wiki Activity

fix: remove useless PHI for duplicate moves (#2813)

Browse Source
This commit is contained in:
Skylot
2026-03-13 18:41:03 +00:00
parent 00196e412b
commit ff64da705c
6 changed files with 174 additions and 11 deletions
Download Patch File Download Diff File Expand all files Collapse all files
jadx-core/src/main/java/jadx/core/dex/visitors/regions/TernaryMod.java
+1
View File
@@ -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();
jadx-core/src/main/java/jadx/core/dex/visitors/ssa/SSATransform.java
+56 -11
View File
@@ -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<PhiInsn> 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<RegisterArg> 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);
}
jadx-core/src/main/java/jadx/core/utils/DebugChecks.java
+29
View File
@@ -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<String> 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<PhiInsn> phis = new ArrayList<>();
jadx-core/src/main/java/jadx/core/utils/InsnRemover.java
+11
View File
@@ -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);
}
}
}
}
jadx-core/src/test/java/jadx/tests/integration/variables/TestThisBranchDup.java
+19
View File
@@ -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
}
}
jadx-core/src/test/smali/variables/TestThisBranchDup.smali
+58
View File
@@ -0,0 +1,58 @@
.class public final Lvariables/TestThisBranchDup;
.super Ljava/lang/Object;
.method public constructor <init>(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;-><init>(ZZZLh3/t;ZZ)V
return-void
.end method