Commit 6c61ce52 authored by Skylot's avatar Skylot

fix: handle cases with SSA variable used in several PHI's (#667)

parent 1830c273
......@@ -12,7 +12,6 @@ import jadx.core.dex.instructions.args.InsnArg;
import jadx.core.dex.instructions.args.RegisterArg;
import jadx.core.dex.nodes.BlockNode;
import jadx.core.dex.nodes.InsnNode;
import jadx.core.utils.InsnRemover;
import jadx.core.utils.Utils;
import jadx.core.utils.exceptions.JadxRuntimeException;
......@@ -76,7 +75,7 @@ public final class PhiInsn extends InsnNode {
protected RegisterArg removeArg(int index) {
RegisterArg reg = (RegisterArg) super.removeArg(index);
blockBinds.remove(index);
InsnRemover.fixUsedInPhiFlag(reg);
reg.getSVar().updateUsedInPhiList();
return reg;
}
......@@ -98,7 +97,7 @@ public final class PhiInsn extends InsnNode {
RegisterArg reg = (RegisterArg) to;
bindArg(reg, pred);
reg.getSVar().setUsedInPhi(this);
reg.getSVar().addUsedInPhi(this);
return true;
}
......
......@@ -12,7 +12,9 @@ import org.jetbrains.annotations.Nullable;
import jadx.core.dex.attributes.AType;
import jadx.core.dex.attributes.AttrNode;
import jadx.core.dex.attributes.nodes.RegDebugInfoAttr;
import jadx.core.dex.instructions.InsnType;
import jadx.core.dex.instructions.PhiInsn;
import jadx.core.dex.nodes.InsnNode;
import jadx.core.dex.nodes.MethodNode;
import jadx.core.dex.visitors.typeinference.TypeInfo;
import jadx.core.utils.StringUtils;
......@@ -24,8 +26,7 @@ public class SSAVar extends AttrNode {
private RegisterArg assign;
private final List<RegisterArg> useList = new ArrayList<>(2);
@Nullable
private PhiInsn usedInPhi;
private List<PhiInsn> usedInPhi = null;
private TypeInfo typeInfo = new TypeInfo();
......@@ -85,24 +86,60 @@ public class SSAVar extends AttrNode {
useList.removeIf(registerArg -> registerArg == arg);
}
public void setUsedInPhi(@Nullable PhiInsn usedInPhi) {
this.usedInPhi = usedInPhi;
public void addUsedInPhi(PhiInsn phiInsn) {
if (usedInPhi == null) {
usedInPhi = new ArrayList<>(1);
}
usedInPhi.add(phiInsn);
}
public void removeUsedInPhi(PhiInsn phiInsn) {
if (usedInPhi != null) {
usedInPhi.removeIf(insn -> insn == phiInsn);
if (usedInPhi.isEmpty()) {
usedInPhi = null;
}
}
}
public void updateUsedInPhiList() {
this.usedInPhi = null;
for (RegisterArg reg : useList) {
InsnNode parentInsn = reg.getParentInsn();
if (parentInsn != null && parentInsn.getType() == InsnType.PHI) {
addUsedInPhi((PhiInsn) parentInsn);
}
}
}
@Nullable
public PhiInsn getUsedInPhi() {
public PhiInsn getOnlyOneUseInPhi() {
if (usedInPhi != null && usedInPhi.size() == 1) {
return usedInPhi.get(0);
}
return null;
}
public List<PhiInsn> getUsedInPhi() {
if (usedInPhi == null) {
return Collections.emptyList();
}
return usedInPhi;
}
public boolean isUsedInPhi() {
return usedInPhi != null;
return usedInPhi != null && !usedInPhi.isEmpty();
}
public int getVariableUseCount() {
int count = useList.size();
if (usedInPhi == null) {
return useList.size();
return count;
}
for (PhiInsn phiInsn : usedInPhi) {
count += phiInsn.getResult().getSVar().getUseCount();
}
return useList.size() + usedInPhi.getResult().getSVar().getUseCount();
return count;
}
public void setName(String name) {
......
......@@ -58,11 +58,11 @@ public class InitCodeVariables extends AbstractVisitor {
}
private static void setCodeVar(SSAVar ssaVar, CodeVar codeVar) {
PhiInsn usedInPhi = ssaVar.getUsedInPhi();
if (usedInPhi != null) {
List<PhiInsn> usedInPhiList = ssaVar.getUsedInPhi();
if (!usedInPhiList.isEmpty()) {
Set<SSAVar> vars = new LinkedHashSet<>();
vars.add(ssaVar);
collectConnectedVars(usedInPhi, vars);
collectConnectedVars(usedInPhiList, vars);
setCodeVarType(codeVar, vars);
vars.forEach(var -> {
if (var.isCodeVarSet()) {
......@@ -92,19 +92,18 @@ public class InitCodeVariables extends AbstractVisitor {
}
}
private static void collectConnectedVars(PhiInsn phiInsn, Set<SSAVar> vars) {
if (phiInsn == null) {
return;
}
SSAVar resultVar = phiInsn.getResult().getSVar();
if (vars.add(resultVar)) {
collectConnectedVars(resultVar.getUsedInPhi(), vars);
}
phiInsn.getArguments().forEach(arg -> {
SSAVar sVar = ((RegisterArg) arg).getSVar();
if (vars.add(sVar)) {
collectConnectedVars(sVar.getUsedInPhi(), vars);
private static void collectConnectedVars(List<PhiInsn> phiInsnList, Set<SSAVar> vars) {
for (PhiInsn phiInsn : phiInsnList) {
SSAVar resultVar = phiInsn.getResult().getSVar();
if (vars.add(resultVar)) {
collectConnectedVars(resultVar.getUsedInPhi(), vars);
}
});
phiInsn.getArguments().forEach(arg -> {
SSAVar sVar = ((RegisterArg) arg).getSVar();
if (vars.add(sVar)) {
collectConnectedVars(sVar.getUsedInPhi(), vars);
}
});
}
}
}
......@@ -203,8 +203,7 @@ public class DebugInfoApplyVisitor extends AbstractVisitor {
private static void fixNamesForPhiInsns(MethodNode mth) {
mth.getSVars().forEach(ssaVar -> {
PhiInsn phiInsn = ssaVar.getUsedInPhi();
if (phiInsn != null) {
for (PhiInsn phiInsn : ssaVar.getUsedInPhi()) {
Set<String> names = new HashSet<>(1 + phiInsn.getArgsCount());
addArgName(phiInsn.getResult(), names);
phiInsn.getArguments().forEach(arg -> addArgName(arg, names));
......
......@@ -89,9 +89,12 @@ public class LoopRegionVisitor extends AbstractVisitor implements IRegionVisitor
|| !incrArg.getSVar().isUsedInPhi()) {
return false;
}
PhiInsn phiInsn = incrArg.getSVar().getUsedInPhi();
if (phiInsn == null
|| phiInsn.getArgsCount() != 2
List<PhiInsn> phiInsnList = incrArg.getSVar().getUsedInPhi();
if (phiInsnList.size() != 1) {
return false;
}
PhiInsn phiInsn = phiInsnList.get(0);
if (phiInsn.getArgsCount() != 2
|| !phiInsn.containsArg(incrArg)
|| incrArg.getSVar().getUseCount() != 1) {
return false;
......
......@@ -62,8 +62,8 @@ public class TernaryMod {
RegisterArg thenResArg = thenInsn.getResult();
RegisterArg elseResArg = elseInsn.getResult();
if (thenResArg != null && elseResArg != null) {
PhiInsn thenPhi = thenResArg.getSVar().getUsedInPhi();
PhiInsn elsePhi = elseResArg.getSVar().getUsedInPhi();
PhiInsn thenPhi = thenResArg.getSVar().getOnlyOneUseInPhi();
PhiInsn elsePhi = elseResArg.getSVar().getOnlyOneUseInPhi();
if (thenPhi == null || thenPhi != elsePhi) {
return false;
}
......@@ -165,8 +165,8 @@ public class TernaryMod {
if (t.getResult() == null || e.getResult() == null) {
return false;
}
PhiInsn tPhi = t.getResult().getSVar().getUsedInPhi();
PhiInsn ePhi = e.getResult().getSVar().getUsedInPhi();
PhiInsn tPhi = t.getResult().getSVar().getOnlyOneUseInPhi();
PhiInsn ePhi = e.getResult().getSVar().getOnlyOneUseInPhi();
if (ePhi == null || tPhi != ePhi) {
return false;
}
......
......@@ -187,7 +187,7 @@ public class SSATransform extends AbstractVisitor {
}
RegisterArg arg = phiInsn.bindArg(state.getBlock());
var.use(arg);
var.setUsedInPhi(phiInsn);
var.addUsedInPhi(phiInsn);
}
/**
......@@ -323,7 +323,7 @@ public class SSATransform extends AbstractVisitor {
}
SSAVar sVar = ((RegisterArg) arg).getSVar();
if (sVar != null) {
sVar.setUsedInPhi(null);
sVar.removeUsedInPhi(phiInsn);
}
}
InsnRemover.remove(mth, block, phiInsn);
......@@ -347,13 +347,13 @@ public class SSATransform extends AbstractVisitor {
SSAVar argVar = arg.getSVar();
if (argVar != null) {
argVar.removeUse(arg);
argVar.setUsedInPhi(null);
argVar.removeUsedInPhi(phi);
}
// try inline
if (inlinePhiInsn(mth, block, phi)) {
insns.remove(phiIndex);
} else {
assign.setUsedInPhi(null);
assign.removeUsedInPhi(phi);
InsnNode m = new InsnNode(InsnType.MOVE, 1);
m.add(AFlag.SYNTHETIC);
......
......@@ -196,8 +196,7 @@ public final class TypeInferenceVisitor extends AbstractVisitor {
}
private void mergePhiBounds(SSAVar ssaVar) {
PhiInsn usedInPhi = ssaVar.getUsedInPhi();
if (usedInPhi != null) {
for (PhiInsn usedInPhi : ssaVar.getUsedInPhi()) {
Set<ITypeBound> bounds = ssaVar.getTypeInfo().getBounds();
bounds.addAll(usedInPhi.getResult().getSVar().getTypeInfo().getBounds());
for (InsnArg arg : usedInPhi.getArguments()) {
......@@ -307,8 +306,8 @@ public final class TypeInferenceVisitor extends AbstractVisitor {
if (var.getTypeInfo().getType().isTypeKnown()) {
return false;
}
PhiInsn phiInsn = var.getUsedInPhi();
if (phiInsn == null) {
List<PhiInsn> usedInPhiList = var.getUsedInPhi();
if (usedInPhiList.isEmpty()) {
return false;
}
if (var.getUseCount() == 1) {
......@@ -317,7 +316,15 @@ public final class TypeInferenceVisitor extends AbstractVisitor {
return false;
}
}
for (PhiInsn phiInsn : usedInPhiList) {
if (!insertMoveForPhi(mth, phiInsn, var)) {
return false;
}
}
return true;
}
private boolean insertMoveForPhi(MethodNode mth, PhiInsn phiInsn, SSAVar var) {
int argsCount = phiInsn.getArgsCount();
for (int argIndex = 0; argIndex < argsCount; argIndex++) {
RegisterArg reg = phiInsn.getArg(argIndex);
......
......@@ -193,8 +193,7 @@ public class DebugUtils {
}
}
for (SSAVar ssaVar : mth.getSVars()) {
PhiInsn usedInPhi = ssaVar.getUsedInPhi();
if (usedInPhi != null) {
for (PhiInsn usedInPhi : ssaVar.getUsedInPhi()) {
boolean found = false;
for (RegisterArg useArg : ssaVar.getUseList()) {
InsnNode parentInsn = useArg.getParentInsn();
......
......@@ -6,7 +6,6 @@ import java.util.List;
import jadx.core.dex.attributes.AFlag;
import jadx.core.dex.instructions.InsnType;
import jadx.core.dex.instructions.PhiInsn;
import jadx.core.dex.instructions.args.InsnArg;
import jadx.core.dex.instructions.args.InsnWrapArg;
import jadx.core.dex.instructions.args.RegisterArg;
......@@ -65,7 +64,7 @@ public class InsnRemover {
if (insn.getType() == InsnType.PHI) {
for (InsnArg arg : insn.getArguments()) {
if (arg instanceof RegisterArg) {
fixUsedInPhiFlag((RegisterArg) arg);
((RegisterArg) arg).getSVar().updateUsedInPhiList();
}
}
}
......@@ -73,19 +72,6 @@ public class InsnRemover {
insn.add(AFlag.REMOVE);
}
public static void fixUsedInPhiFlag(RegisterArg useReg) {
PhiInsn usedIn = null;
for (RegisterArg reg : useReg.getSVar().getUseList()) {
InsnNode parentInsn = reg.getParentInsn();
if (parentInsn != null
&& parentInsn.getType() == InsnType.PHI
&& parentInsn.containsArg(useReg)) {
usedIn = (PhiInsn) parentInsn;
}
}
useReg.getSVar().setUsedInPhi(usedIn);
}
public static void unbindResult(MethodNode mth, InsnNode insn) {
RegisterArg r = insn.getResult();
if (r != null && r.getSVar() != null && mth != null) {
......
package jadx.tests.integration.variables;
import java.util.List;
import org.junit.jupiter.api.Test;
import jadx.core.dex.nodes.ClassNode;
import jadx.tests.api.IntegrationTest;
import static jadx.tests.api.utils.JadxMatchers.containsOne;
import static org.hamcrest.CoreMatchers.containsString;
import static org.hamcrest.CoreMatchers.not;
import static org.hamcrest.MatcherAssert.assertThat;
public class TestVariablesDefinitions2 extends IntegrationTest {
public static class TestCls {
public static int test(List<String> list) {
int i = 0;
if (list != null) {
for (String str : list) {
if (str.isEmpty()) {
i++;
}
}
}
return i;
}
}
@Test
public void test() {
ClassNode cls = getClassNode(TestCls.class);
String code = cls.getCode().toString();
assertThat(code, containsOne("int i = 0;"));
assertThat(code, containsOne("i++;"));
assertThat(code, containsOne("return i;"));
assertThat(code, not(containsString("i2;")));
}
}
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment