Skip to content

Commit

Permalink
fix: unbind unused ssa variable after ternary conversion (#708)
Browse files Browse the repository at this point in the history
  • Loading branch information
skylot committed Jul 16, 2019
1 parent 15d56ab commit aad2d24
Show file tree
Hide file tree
Showing 8 changed files with 233 additions and 21 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,7 @@ private static boolean makeTernaryInsn(MethodNode mth, IfRegion ifRegion) {
RegisterArg resArg;
if (thenPhi.getArgsCount() == 2) {
resArg = thenPhi.getResult();
InsnRemover.unbindResult(mth, thenInsn);
} else {
resArg = thenResArg;
thenPhi.removeArg(elseResArg);
Expand All @@ -107,6 +108,8 @@ private static boolean makeTernaryInsn(MethodNode mth, IfRegion ifRegion) {
resArg, InsnArg.wrapArg(thenInsn), InsnArg.wrapArg(elseInsn));
ternInsn.setSourceLine(thenInsn.getSourceLine());

InsnRemover.unbindResult(mth, elseInsn);

// remove 'if' instruction
header.getInstructions().clear();
header.getInstructions().add(ternInsn);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
import jadx.core.dex.attributes.AFlag;
import jadx.core.dex.attributes.AType;
import jadx.core.dex.attributes.nodes.DeclareVariablesAttr;
import jadx.core.dex.instructions.InsnType;
import jadx.core.dex.instructions.args.ArgType;
import jadx.core.dex.instructions.args.CodeVar;
import jadx.core.dex.instructions.args.RegisterArg;
Expand Down Expand Up @@ -228,7 +229,9 @@ private static boolean isContainerContainsUsePlace(IContainer subBlock, UsePlace
private static boolean checkDeclareAtAssign(SSAVar var) {
RegisterArg arg = var.getAssign();
InsnNode parentInsn = arg.getParentInsn();
if (parentInsn == null) {
if (parentInsn == null
|| parentInsn.contains(AFlag.WRAPPED)
|| parentInsn.getType() == InsnType.PHI) {
return false;
}
if (!arg.equals(parentInsn.getResult())) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
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(
Expand Down Expand Up @@ -67,7 +68,7 @@ private static void shrinkBlock(MethodNode mth, BlockNode block) {
}
if (!wrapList.isEmpty()) {
for (WrapInfo wrapInfo : wrapList) {
inline(wrapInfo.getArg(), wrapInfo.getInsn(), block);
inline(mth, wrapInfo.getArg(), wrapInfo.getInsn(), block);
}
}
}
Expand Down Expand Up @@ -106,19 +107,20 @@ private static void checkInline(MethodNode mth, BlockNode block, InsnList insnLi
if (assignBlock != null
&& assignInsn != arg.getParentInsn()
&& canMoveBetweenBlocks(assignInsn, assignBlock, block, argsInfo.getInsn())) {
inline(arg, assignInsn, assignBlock);
inline(mth, arg, assignInsn, assignBlock);
}
}
}

private static boolean inline(RegisterArg arg, InsnNode insn, BlockNode block) {
private static boolean inline(MethodNode mth, RegisterArg arg, InsnNode insn, BlockNode block) {
InsnNode parentInsn = arg.getParentInsn();
if (parentInsn != null && parentInsn.getType() == InsnType.RETURN) {
parentInsn.setSourceLine(insn.getSourceLine());
}
boolean replaced = arg.wrapInstruction(insn) != null;
if (replaced) {
InsnList.remove(block, insn);
InsnRemover.unbindResult(mth, insn);
}
return replaced;
}
Expand Down
35 changes: 30 additions & 5 deletions jadx-core/src/main/java/jadx/core/utils/InsnRemover.java
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@

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;
Expand Down Expand Up @@ -66,7 +67,7 @@ public void perform() {
toRemove.clear();
}

public static void unbindInsn(MethodNode mth, InsnNode insn) {
public static void unbindInsn(@Nullable MethodNode mth, InsnNode insn) {
for (InsnArg arg : insn.getArguments()) {
unbindArgUsage(mth, arg);
}
Expand All @@ -81,17 +82,41 @@ public static void unbindInsn(MethodNode mth, InsnNode insn) {
insn.add(AFlag.REMOVE);
}

public static void unbindResult(MethodNode mth, InsnNode insn) {
public static void unbindResult(@Nullable MethodNode mth, InsnNode insn) {
RegisterArg r = insn.getResult();
if (r != null && r.getSVar() != null && mth != null) {
SSAVar ssaVar = r.getSVar();
if (ssaVar.getUseCount() == 0) {
mth.removeSVar(ssaVar);
removeSsaVar(mth, ssaVar);
}
}

private static void removeSsaVar(MethodNode mth, SSAVar ssaVar) {
int useCount = ssaVar.getUseCount();
if (useCount == 0) {
mth.removeSVar(ssaVar);
return;
}
// check if all usage only in PHI insns
boolean allPhis = true;
for (RegisterArg arg : ssaVar.getUseList()) {
InsnNode parentInsn = arg.getParentInsn();
if (parentInsn == null || parentInsn.getType() != InsnType.PHI) {
allPhis = false;
break;
}
}
if (allPhis) {
for (RegisterArg arg : new ArrayList<>(ssaVar.getUseList())) {
InsnNode parentInsn = arg.getParentInsn();
if (parentInsn != null) {
((PhiInsn) parentInsn).removeArg(arg);
}
}
mth.removeSVar(ssaVar);
}
}

public static void unbindArgUsage(MethodNode mth, InsnArg arg) {
public static void unbindArgUsage(@Nullable MethodNode mth, InsnArg arg) {
if (arg instanceof RegisterArg) {
RegisterArg reg = (RegisterArg) arg;
SSAVar sVar = reg.getSVar();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ public static boolean test(Object a, Object b) {
if (r) {
return false;
}
System.out.println("1");
System.out.println("r=" + r);
return true;
}
}
Expand All @@ -29,6 +29,6 @@ public void test() {

assertThat(code, containsOne("boolean r = a == null ? b != null : !a.equals(b);"));
assertThat(code, containsOne("if (r) {"));
assertThat(code, containsOne("System.out.println(\"1\");"));
assertThat(code, containsOne("System.out.println(\"r=\" + r);"));
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
package jadx.tests.integration.conditions;

import org.junit.jupiter.api.Test;

import jadx.core.dex.nodes.ClassNode;
import jadx.tests.api.SmaliTest;

import static org.hamcrest.CoreMatchers.containsString;
import static org.hamcrest.CoreMatchers.not;
import static org.hamcrest.MatcherAssert.assertThat;

public class TestTernary4 extends SmaliTest {

// @formatter:off
/*
private Set test(HashMap<String, Object> hashMap) {
boolean z;
HashSet hashSet = new HashSet();
synchronized (this.defaultValuesByPath) {
for (String next : this.defaultValuesByPath.keySet()) {
Object obj = hashMap.get(next);
if (obj != null) {
z = !getValueObject(next).equals(obj);
} else {
z = this.valuesByPath.get(next) != null;;
}
if (z) {
hashSet.add(next);
}
}
}
return hashSet;
}
*/
// @formatter:on

@Test
public void test() {
ClassNode cls = getClassNodeFromSmali();
String code = cls.getCode().toString();

assertThat(code, not(containsString("r5")));
assertThat(code, not(containsString("try")));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@

import org.junit.jupiter.api.Test;

import jadx.NotYetImplemented;
import jadx.core.dex.nodes.ClassNode;
import jadx.tests.api.IntegrationTest;

Expand All @@ -27,15 +26,6 @@ public void test() {
ClassNode cls = getClassNode(TestCls.class);
String code = cls.getCode().toString();

assertThat(code, containsOne("s1.length() == s2.length() ? 0 : s1.length() < s2.length() ? -1 : 1;"));
}

@Test
@NotYetImplemented
public void test3() {
ClassNode cls = getClassNode(TestCls.class);
String code = cls.getCode().toString();

assertThat(code, containsOne("return s1.length() == s2.length() ? 0 : s1.length() < s2.length() ? -1 : 1;"));
}

Expand Down
144 changes: 144 additions & 0 deletions jadx-core/src/test/smali/conditions/TestTernary4.smali
Original file line number Diff line number Diff line change
@@ -0,0 +1,144 @@
.class public final Lconditions/TestTernary4;
.super Ljava/lang/Object;

.field private defaultValuesByPath:Ljava/util/Map;
.annotation system Ldalvik/annotation/Signature;
value = {
"Ljava/util/Map<",
"Ljava/lang/String;",
"Ljava/lang/Object;",
">;"
}
.end annotation
.end field

.field private valuesByPath:Ljava/util/Map;
.annotation system Ldalvik/annotation/Signature;
value = {
"Ljava/util/Map<",
"Ljava/lang/String;",
"Ljava/lang/Object;",
">;"
}
.end annotation
.end field

.method private test(Ljava/util/HashMap;)Ljava/util/Set;
.registers 10
.annotation system Ldalvik/annotation/Signature;
value = {
"(",
"Ljava/util/HashMap<",
"Ljava/lang/String;",
"Ljava/lang/Object;",
">;)",
"Ljava/util/Set;"
}
.end annotation

.line 278
new-instance v0, Ljava/util/HashSet;

invoke-direct {v0}, Ljava/util/HashSet;-><init>()V

.line 280
iget-object v1, p0, Lconditions/TestTernary4;->defaultValuesByPath:Ljava/util/Map;

monitor-enter v1

.line 281
:try_start_14
iget-object v3, p0, Lconditions/TestTernary4;->defaultValuesByPath:Ljava/util/Map;

invoke-interface {v3}, Ljava/util/Map;->keySet()Ljava/util/Set;

move-result-object v3

invoke-interface {v3}, Ljava/util/Set;->iterator()Ljava/util/Iterator;

move-result-object v3

:cond_1e
:goto_1e
invoke-interface {v3}, Ljava/util/Iterator;->hasNext()Z

move-result v4

if-eqz v4, :cond_4c

invoke-interface {v3}, Ljava/util/Iterator;->next()Ljava/lang/Object;

move-result-object v4

check-cast v4, Ljava/lang/String;

.line 286
invoke-virtual {p1, v4}, Ljava/util/HashMap;->get(Ljava/lang/Object;)Ljava/lang/Object;

move-result-object v5

const/4 v6, 0x1

if-eqz v5, :cond_3b

.line 287
invoke-direct {p0, v4}, Lconditions/TestTernary4;->getValueObject(Ljava/lang/String;)Ljava/lang/Object;

move-result-object v7

invoke-virtual {v7, v5}, Ljava/lang/Object;->equals(Ljava/lang/Object;)Z

move-result v5

xor-int/2addr v5, v6

goto :goto_46

.line 289
:cond_3b
iget-object v5, p0, Lconditions/TestTernary4;->valuesByPath:Ljava/util/Map;

invoke-interface {v5, v4}, Ljava/util/Map;->get(Ljava/lang/Object;)Ljava/lang/Object;

move-result-object v5

if-eqz v5, :cond_45

const/4 v5, 0x1

goto :goto_46

:cond_45
const/4 v5, 0x0

:goto_46
if-eqz v5, :cond_1e

.line 293
invoke-interface {v0, v4}, Ljava/util/Set;->add(Ljava/lang/Object;)Z

goto :goto_1e

.line 296
:cond_4c
monitor-exit v1

return-object v0

:catchall_4e
move-exception p1

monitor-exit v1
:try_end_50
.catchall {:try_start_14 .. :try_end_50} :catchall_4e

throw p1

return-void
.end method

.method private getValueObject(Ljava/lang/String;)Ljava/lang/Object;
.locals 1
const/4 v0, 0x0
return-object v0
.end method

0 comments on commit aad2d24

Please sign in to comment.