Skip to content

Commit

Permalink
fix: don't override type of method parameter in const deboxing (#723)
Browse files Browse the repository at this point in the history
  • Loading branch information
skylot committed Jul 26, 2019
1 parent 472aa52 commit ddedb8d
Show file tree
Hide file tree
Showing 3 changed files with 127 additions and 9 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
import jadx.core.dex.instructions.args.ArgType;
import jadx.core.dex.instructions.args.InsnArg;
import jadx.core.dex.instructions.args.RegisterArg;
import jadx.core.dex.instructions.args.SSAVar;
import jadx.core.dex.nodes.BlockNode;
import jadx.core.dex.nodes.InsnNode;
import jadx.core.dex.nodes.MethodNode;
Expand Down Expand Up @@ -90,14 +91,16 @@ private InsnNode checkForReplace(InvokeNode insnNode) {
if (valueOfMths.contains(callMth)) {
RegisterArg resArg = insnNode.getResult();
InsnArg arg = insnNode.getArg(0);
if (arg.isLiteral() && checkArgUsage(resArg)) {
if (arg.isLiteral()) {
ArgType primitiveType = callMth.getArgumentsTypes().get(0);
ArgType boxType = callMth.getReturnType();
if (isNeedExplicitCast(resArg, primitiveType, boxType)) {
arg.add(AFlag.EXPLICIT_PRIMITIVE_TYPE);
}
resArg.setType(primitiveType);
arg.setType(primitiveType);
if (canChangeTypeToPrimitive(resArg)) {
resArg.setType(primitiveType);
}

InsnNode constInsn = new InsnNode(InsnType.CONST, 1);
constInsn.addArg(arg);
Expand All @@ -122,17 +125,23 @@ private boolean isNeedExplicitCast(RegisterArg resArg, ArgType primitiveType, Ar
return false;
}

private boolean checkArgUsage(RegisterArg arg) {
for (RegisterArg useArg : arg.getSVar().getUseList()) {
InsnNode parentInsn = useArg.getParentInsn();
if (parentInsn == null) {
private boolean canChangeTypeToPrimitive(RegisterArg arg) {
for (SSAVar ssaVar : arg.getSVar().getCodeVar().getSsaVars()) {
RegisterArg assignArg = ssaVar.getAssign();
if (assignArg.contains(AFlag.IMMUTABLE_TYPE)) {
return false;
}
if (parentInsn.getType() == InsnType.INVOKE) {
InvokeNode invokeNode = (InvokeNode) parentInsn;
if (useArg.equals(invokeNode.getInstanceArg())) {
for (RegisterArg useArg : ssaVar.getUseList()) {
InsnNode parentInsn = useArg.getParentInsn();
if (parentInsn == null) {
return false;
}
if (parentInsn.getType() == InsnType.INVOKE) {
InvokeNode invokeNode = (InvokeNode) parentInsn;
if (useArg.equals(invokeNode.getInstanceArg())) {
return false;
}
}
}
}
return true;
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
package jadx.tests.integration.others;

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 jadx.tests.api.utils.JadxMatchers.countString;
import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.Matchers.is;

public class TestDeboxing2 extends IntegrationTest {

public static class TestCls {
public long test(Long l) {
if (l == null) {
l = 0L;
}
return l;
}

public void check() {
assertThat(test(null), is(0L));
assertThat(test(0L), is(0L));
assertThat(test(7L), is(7L));
}
}

@Test
public void test() {
noDebugInfo();

ClassNode cls = getClassNode(TestCls.class);
String code = cls.getCode().toString();

assertThat(code, containsOne("long test(Long l)"));
assertThat(code, containsOne("if (l == null) {"));
assertThat(code, containsOne("l = 0L;"));

// checks for 'check' method
assertThat(code, containsOne("test(null)"));
assertThat(code, containsOne("test(0L)"));
assertThat(code, countString(2, "is(0L)"));
assertThat(code, containsOne("test(7L)"));
assertThat(code, containsOne("is(7L)"));

}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,60 @@
package jadx.tests.integration.others;

import java.util.HashMap;
import java.util.Map;

import org.junit.jupiter.api.Test;

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

import static jadx.tests.api.utils.JadxMatchers.containsOne;
import static org.hamcrest.MatcherAssert.assertThat;

public class TestDeboxing3 extends IntegrationTest {

public static class TestCls {

public static class Pair<F, S> {
public F first;
public S second;
}

private Map<String, Pair<Long, String>> cache = new HashMap<>();

public boolean test(String id, Long l) {
if (l == null) {
l = 900000L;
}
Pair<Long, String> pair = this.cache.get(id);
if (pair == null) {
return false;
}
return pair.first + l > System.currentTimeMillis();
}
}

@Test
public void test() {
noDebugInfo();

ClassNode cls = getClassNode(TestCls.class);
String code = cls.getCode().toString();

assertThat(code, containsOne("l = 900000L;"));
}

@Test
@NotYetImplemented("Full deboxing and generics propagation")
public void testFull() {
noDebugInfo();

ClassNode cls = getClassNode(TestCls.class);
String code = cls.getCode().toString();

assertThat(code, containsOne("Pair<Long, String> pair = this.cache.get(id);"));
assertThat(code, containsOne("return pair.first + l > System.currentTimeMillis();"));

}
}

0 comments on commit ddedb8d

Please sign in to comment.