From 0deafb768bef847cced6b8b81220e3c1fcb01b0d Mon Sep 17 00:00:00 2001 From: Skylot Date: Mon, 9 Nov 2020 20:46:03 +0000 Subject: [PATCH] fix: correct merge code variables across PHI instructions (#930) --- .../core/dex/instructions/args/SSAVar.java | 21 ++++ .../core/dex/visitors/InitCodeVariables.java | 13 ++- .../variables/TestVariablesInLoop.java | 19 ++++ .../smali/variables/TestVariablesInLoop.smali | 103 ++++++++++++++++++ 4 files changed, 151 insertions(+), 5 deletions(-) create mode 100644 jadx-core/src/test/java/jadx/tests/integration/variables/TestVariablesInLoop.java create mode 100644 jadx-core/src/test/smali/variables/TestVariablesInLoop.smali diff --git a/jadx-core/src/main/java/jadx/core/dex/instructions/args/SSAVar.java b/jadx-core/src/main/java/jadx/core/dex/instructions/args/SSAVar.java index f48d9ee3..4df8e83a 100644 --- a/jadx-core/src/main/java/jadx/core/dex/instructions/args/SSAVar.java +++ b/jadx-core/src/main/java/jadx/core/dex/instructions/args/SSAVar.java @@ -166,6 +166,27 @@ public class SSAVar { return usedInPhi; } + /** + * Concat assign PHI insn and usedInPhi + */ + public List getPhiList() { + InsnNode assignInsn = getAssign().getParentInsn(); + if (assignInsn != null && assignInsn.getType() == InsnType.PHI) { + PhiInsn assignPhi = (PhiInsn) assignInsn; + if (usedInPhi == null) { + return Collections.singletonList(assignPhi); + } + List list = new ArrayList<>(1 + usedInPhi.size()); + list.add(assignPhi); + list.addAll(usedInPhi); + return list; + } + if (usedInPhi == null) { + return Collections.emptyList(); + } + return usedInPhi; + } + public boolean isUsedInPhi() { return usedInPhi != null && !usedInPhi.isEmpty(); } diff --git a/jadx-core/src/main/java/jadx/core/dex/visitors/InitCodeVariables.java b/jadx-core/src/main/java/jadx/core/dex/visitors/InitCodeVariables.java index 9df3b689..fdae693b 100644 --- a/jadx-core/src/main/java/jadx/core/dex/visitors/InitCodeVariables.java +++ b/jadx-core/src/main/java/jadx/core/dex/visitors/InitCodeVariables.java @@ -70,11 +70,11 @@ public class InitCodeVariables extends AbstractVisitor { } private static void setCodeVar(SSAVar ssaVar, CodeVar codeVar) { - List usedInPhiList = ssaVar.getUsedInPhi(); - if (!usedInPhiList.isEmpty()) { + List phiList = ssaVar.getPhiList(); + if (!phiList.isEmpty()) { Set vars = new LinkedHashSet<>(); vars.add(ssaVar); - collectConnectedVars(usedInPhiList, vars); + collectConnectedVars(phiList, vars); setCodeVarType(codeVar, vars); vars.forEach(var -> { if (var.isCodeVarSet()) { @@ -105,15 +105,18 @@ public class InitCodeVariables extends AbstractVisitor { } private static void collectConnectedVars(List phiInsnList, Set vars) { + if (phiInsnList.isEmpty()) { + return; + } for (PhiInsn phiInsn : phiInsnList) { SSAVar resultVar = phiInsn.getResult().getSVar(); if (vars.add(resultVar)) { - collectConnectedVars(resultVar.getUsedInPhi(), vars); + collectConnectedVars(resultVar.getPhiList(), vars); } phiInsn.getArguments().forEach(arg -> { SSAVar sVar = ((RegisterArg) arg).getSVar(); if (vars.add(sVar)) { - collectConnectedVars(sVar.getUsedInPhi(), vars); + collectConnectedVars(sVar.getPhiList(), vars); } }); } diff --git a/jadx-core/src/test/java/jadx/tests/integration/variables/TestVariablesInLoop.java b/jadx-core/src/test/java/jadx/tests/integration/variables/TestVariablesInLoop.java new file mode 100644 index 00000000..6f0f387e --- /dev/null +++ b/jadx-core/src/test/java/jadx/tests/integration/variables/TestVariablesInLoop.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 TestVariablesInLoop extends SmaliTest { + + @Test + public void test() { + assertThat(getClassNodeFromSmali()) + .code() + .containsOne("int i;") + .countString(2, "i = 0;") + .doesNotContain("i3"); + } +} diff --git a/jadx-core/src/test/smali/variables/TestVariablesInLoop.smali b/jadx-core/src/test/smali/variables/TestVariablesInLoop.smali new file mode 100644 index 00000000..545d047b --- /dev/null +++ b/jadx-core/src/test/smali/variables/TestVariablesInLoop.smali @@ -0,0 +1,103 @@ +.class public abstract Lvariables/TestVariablesInLoop; +.super Ljava/lang/Object; +.source "SourceFile" + +.implements Ljava/util/List; + +.annotation system Ldalvik/annotation/Signature; + value = { + "Ljava/lang/Object;", + "Ljava/util/List<", + "Ljava/lang/Long;", + ">;" + } +.end annotation + +.method static test(Ljava/util/List;)I + .locals 5 + .annotation system Ldalvik/annotation/Signature; + value = { + "(", + "Ljava/util/List<", + "Ljava/lang/Long;", + ">;)I" + } + .end annotation + + invoke-interface {p0}, Ljava/util/List;->size()I + + move-result v0 + + const/4 v1, 0x0 + + if-nez v0, :cond_0 + + return v1 + + :cond_0 + instance-of v2, p0, Lvariables/TestVariablesInLoop; + + if-eqz v2, :cond_1 + + check-cast p0, Lvariables/TestVariablesInLoop; + + const/4 v2, 0x0 + + :goto_0 + if-ge v1, v0, :cond_2 + + invoke-virtual {p0, v1}, Lvariables/TestVariablesInLoop;->getLong(I)J + + move-result-wide v3 + + invoke-static {v3, v4}, Lvariables/TestVariablesInLoop;->mth(J)I + + move-result v3 + + add-int/2addr v2, v3 + + add-int/lit8 v1, v1, 0x1 + + goto :goto_0 + + :cond_1 + const/4 v2, 0x0 + + :goto_1 + if-ge v1, v0, :cond_2 + + invoke-interface {p0, v1}, Ljava/util/List;->get(I)Ljava/lang/Object; + + move-result-object v3 + + check-cast v3, Ljava/lang/Long; + + invoke-virtual {v3}, Ljava/lang/Long;->longValue()J + + move-result-wide v3 + + invoke-static {v3, v4}, Lvariables/TestVariablesInLoop;->mth(J)I + + move-result v3 + + add-int/2addr v2, v3 + + add-int/lit8 v1, v1, 0x1 + + goto :goto_1 + + :cond_2 + return v2 +.end method + +.method public final getLong(I)J + .locals 2 + const/16 v0, 0x0 + return-wide v0 +.end method + +.method static mth(J)I + .locals 2 + const/4 v0, 0x0 + return v0 +.end method -- GitLab