From 9d2c0e4aea5fa2fb2882abd3560aa326dc3e7cc7 Mon Sep 17 00:00:00 2001 From: Skylot Date: Tue, 4 Nov 2014 15:48:05 +0300 Subject: [PATCH] core: fix type inference and const inline for arrays --- .../java/jadx/core/dex/attributes/AFlag.java | 1 + .../core/dex/instructions/InsnDecoder.java | 14 ++- .../jadx/core/dex/instructions/PhiInsn.java | 2 + .../core/dex/instructions/args/ArgType.java | 114 ++++++++++++------ .../core/dex/instructions/args/InsnArg.java | 4 + .../dex/visitors/BlockProcessingHelper.java | 2 + .../jadx/core/dex/visitors/CodeShrinker.java | 5 +- .../dex/visitors/ConstInlinerVisitor.java | 45 +++++-- .../visitors/regions/LoopRegionVisitor.java | 2 +- .../java/jadx/core/utils/ErrorsCounter.java | 2 +- .../main/java/jadx/core/utils/InsnUtils.java | 8 +- .../java/jadx/tests/api/IntegrationTest.java | 1 + .../jadx/tests/functional/TypeMergeTest.java | 20 +-- .../jadx/tests/integration/TestWrongCode.java | 11 +- .../tests/integration/arrays/TestArrays.java | 33 +++++ 15 files changed, 189 insertions(+), 75 deletions(-) create mode 100644 jadx-core/src/test/java/jadx/tests/integration/arrays/TestArrays.java diff --git a/jadx-core/src/main/java/jadx/core/dex/attributes/AFlag.java b/jadx-core/src/main/java/jadx/core/dex/attributes/AFlag.java index e90cdf712..6c3ac2bcc 100644 --- a/jadx-core/src/main/java/jadx/core/dex/attributes/AFlag.java +++ b/jadx-core/src/main/java/jadx/core/dex/attributes/AFlag.java @@ -15,6 +15,7 @@ public enum AFlag { DONT_WRAP, DONT_SHRINK, + DONT_INLINE, DONT_GENERATE, SKIP, diff --git a/jadx-core/src/main/java/jadx/core/dex/instructions/InsnDecoder.java b/jadx-core/src/main/java/jadx/core/dex/instructions/InsnDecoder.java index 3f750b52c..babf6a6bf 100644 --- a/jadx-core/src/main/java/jadx/core/dex/instructions/InsnDecoder.java +++ b/jadx-core/src/main/java/jadx/core/dex/instructions/InsnDecoder.java @@ -9,6 +9,7 @@ import jadx.core.dex.instructions.args.RegisterArg; import jadx.core.dex.nodes.DexNode; import jadx.core.dex.nodes.InsnNode; import jadx.core.dex.nodes.MethodNode; +import jadx.core.utils.InsnUtils; import jadx.core.utils.exceptions.DecodeException; import com.android.dex.Code; @@ -620,16 +621,19 @@ public class InsnDecoder { int resReg = getMoveResultRegister(insnArr, offset); ArgType arrType = dex.getType(insn.getIndex()); ArgType elType = arrType.getArrayElement(); - InsnArg[] regs = new InsnArg[insn.getRegisterCount()]; + boolean typeImmutable = elType.isPrimitive(); + int regsCount = insn.getRegisterCount(); + InsnArg[] regs = new InsnArg[regsCount]; if (isRange) { int r = insn.getA(); - for (int i = 0; i < insn.getRegisterCount(); i++) { - regs[i] = InsnArg.reg(r, elType); + for (int i = 0; i < regsCount; i++) { + regs[i] = InsnArg.reg(r, elType, typeImmutable); r++; } } else { - for (int i = 0; i < insn.getRegisterCount(); i++) { - regs[i] = InsnArg.reg(insn, i, elType); + for (int i = 0; i < regsCount; i++) { + int regNum = InsnUtils.getArg(insn, i); + regs[i] = InsnArg.reg(regNum, elType, typeImmutable); } } return insn(InsnType.FILLED_NEW_ARRAY, diff --git a/jadx-core/src/main/java/jadx/core/dex/instructions/PhiInsn.java b/jadx-core/src/main/java/jadx/core/dex/instructions/PhiInsn.java index 2161e6fb2..d25134921 100644 --- a/jadx-core/src/main/java/jadx/core/dex/instructions/PhiInsn.java +++ b/jadx-core/src/main/java/jadx/core/dex/instructions/PhiInsn.java @@ -1,5 +1,6 @@ package jadx.core.dex.instructions; +import jadx.core.dex.attributes.AFlag; import jadx.core.dex.instructions.args.ArgType; import jadx.core.dex.instructions.args.InsnArg; import jadx.core.dex.instructions.args.RegisterArg; @@ -14,6 +15,7 @@ public class PhiInsn extends InsnNode { for (int i = 0; i < predecessors; i++) { addReg(regNum, ArgType.UNKNOWN); } + add(AFlag.DONT_INLINE); } @Override diff --git a/jadx-core/src/main/java/jadx/core/dex/instructions/args/ArgType.java b/jadx-core/src/main/java/jadx/core/dex/instructions/args/ArgType.java index 4ff1b25ae..2e385ce2d 100644 --- a/jadx-core/src/main/java/jadx/core/dex/instructions/args/ArgType.java +++ b/jadx-core/src/main/java/jadx/core/dex/instructions/args/ArgType.java @@ -9,6 +9,8 @@ import java.util.ArrayList; import java.util.Arrays; import java.util.List; +import org.jetbrains.annotations.Nullable; + public abstract class ArgType { public static final ArgType INT = primitive(PrimitiveType.INT); @@ -93,10 +95,28 @@ public abstract class ArgType { } private abstract static class KnownType extends ArgType { + + private static final PrimitiveType[] EMPTY_POSSIBLES = new PrimitiveType[0]; + @Override public boolean isTypeKnown() { return true; } + + @Override + public boolean contains(PrimitiveType type) { + return getPrimitiveType() == type; + } + + @Override + public ArgType selectFirst() { + return null; + } + + @Override + public PrimitiveType[] getPossibleTypes() { + return EMPTY_POSSIBLES; + } } private static final class PrimitiveArg extends KnownType { @@ -269,6 +289,7 @@ public abstract class ArgType { } private static final class ArrayArg extends KnownType { + public static final PrimitiveType[] ARRAY_POSSIBLES = new PrimitiveType[]{PrimitiveType.ARRAY}; private final ArgType arrayElement; public ArrayArg(ArgType arrayElement) { @@ -291,6 +312,21 @@ public abstract class ArgType { return PrimitiveType.ARRAY; } + @Override + public boolean isTypeKnown() { + return arrayElement.isTypeKnown(); + } + + @Override + public ArgType selectFirst() { + return array(arrayElement.selectFirst()); + } + + @Override + public PrimitiveType[] getPossibleTypes() { + return ARRAY_POSSIBLES; + } + @Override public int getArrayDimension() { return 1 + arrayElement.getArrayDimension(); @@ -343,8 +379,10 @@ public abstract class ArgType { @Override public ArgType selectFirst() { PrimitiveType f = possibleTypes[0]; - if (f == PrimitiveType.OBJECT || f == PrimitiveType.ARRAY) { - return object(Consts.CLASS_OBJECT); + if (contains(PrimitiveType.OBJECT)) { + return OBJECT; + } else if (contains(PrimitiveType.ARRAY)) { + return array(OBJECT); } else { return primitive(f); } @@ -428,18 +466,13 @@ public abstract class ArgType { return this; } - public boolean contains(PrimitiveType type) { - throw new UnsupportedOperationException(); - } + public abstract boolean contains(PrimitiveType type); - public ArgType selectFirst() { - throw new UnsupportedOperationException(); - } + public abstract ArgType selectFirst(); - public PrimitiveType[] getPossibleTypes() { - return null; - } + public abstract PrimitiveType[] getPossibleTypes(); + @Nullable public static ArgType merge(ArgType a, ArgType b) { if (a == null || b == null) { return null; @@ -458,13 +491,18 @@ public abstract class ArgType { if (a == UNKNOWN) { return b; } + + if (a.isArray()) { + return mergeArrays((ArrayArg) a, b); + } else if (b.isArray()) { + return mergeArrays((ArrayArg) b, a); + } if (!a.isTypeKnown()) { if (b.isTypeKnown()) { if (a.contains(b.getPrimitiveType())) { return b; - } else { - return null; } + return null; } else { // both types unknown List types = new ArrayList(); @@ -475,7 +513,8 @@ public abstract class ArgType { } if (types.isEmpty()) { return null; - } else if (types.size() == 1) { + } + if (types.size() == 1) { PrimitiveType nt = types.get(0); if (nt == PrimitiveType.OBJECT || nt == PrimitiveType.ARRAY) { return unknown(nt); @@ -499,31 +538,15 @@ public abstract class ArgType { String bObj = b.getObject(); if (aObj.equals(bObj)) { return a.getGenericTypes() != null ? a : b; - } else if (aObj.equals(Consts.CLASS_OBJECT)) { + } + if (aObj.equals(Consts.CLASS_OBJECT)) { return b; - } else if (bObj.equals(Consts.CLASS_OBJECT)) { + } + if (bObj.equals(Consts.CLASS_OBJECT)) { return a; - } else { - // different objects - String obj = clsp.getCommonAncestor(aObj, bObj); - return obj == null ? null : object(obj); - } - } - if (a.isArray()) { - if (b.isArray()) { - ArgType ea = a.getArrayElement(); - ArgType eb = b.getArrayElement(); - if (ea.isPrimitive() && eb.isPrimitive()) { - return OBJECT; - } else { - ArgType res = merge(ea, eb); - return res == null ? null : array(res); - } - } else if (b.equals(OBJECT)) { - return OBJECT; - } else { - return null; } + String obj = clsp.getCommonAncestor(aObj, bObj); + return obj == null ? null : object(obj); } if (a.isPrimitive() && b.isPrimitive() && a.getRegCount() == b.getRegCount()) { return primitive(PrimitiveType.getSmaller(a.getPrimitiveType(), b.getPrimitiveType())); @@ -532,6 +555,25 @@ public abstract class ArgType { return null; } + private static ArgType mergeArrays(ArrayArg array, ArgType b) { + if (b.isArray()) { + ArgType ea = array.getArrayElement(); + ArgType eb = b.getArrayElement(); + if (ea.isPrimitive() && eb.isPrimitive()) { + return OBJECT; + } + ArgType res = merge(ea, eb); + return res == null ? null : array(res); + } + if (b.contains(PrimitiveType.ARRAY)) { + return array; + } + if (b.equals(OBJECT)) { + return OBJECT; + } + return null; + } + public static boolean isCastNeeded(ArgType from, ArgType to) { if (from.equals(to)) { return false; diff --git a/jadx-core/src/main/java/jadx/core/dex/instructions/args/InsnArg.java b/jadx-core/src/main/java/jadx/core/dex/instructions/args/InsnArg.java index cf49e4782..ff1429b1a 100644 --- a/jadx-core/src/main/java/jadx/core/dex/instructions/args/InsnArg.java +++ b/jadx-core/src/main/java/jadx/core/dex/instructions/args/InsnArg.java @@ -34,6 +34,10 @@ public abstract class InsnArg extends Typed { return new TypeImmutableArg(regNum, type); } + public static RegisterArg reg(int regNum, ArgType type, boolean typeImmutable) { + return typeImmutable ? new TypeImmutableArg(regNum, type) : new RegisterArg(regNum, type); + } + public static LiteralArg lit(long literal, ArgType type) { return new LiteralArg(literal, type); } diff --git a/jadx-core/src/main/java/jadx/core/dex/visitors/BlockProcessingHelper.java b/jadx-core/src/main/java/jadx/core/dex/visitors/BlockProcessingHelper.java index 46c0ae730..ed9d2fda1 100644 --- a/jadx-core/src/main/java/jadx/core/dex/visitors/BlockProcessingHelper.java +++ b/jadx-core/src/main/java/jadx/core/dex/visitors/BlockProcessingHelper.java @@ -1,5 +1,6 @@ package jadx.core.dex.visitors; +import jadx.core.dex.attributes.AFlag; import jadx.core.dex.attributes.AType; import jadx.core.dex.instructions.IfNode; import jadx.core.dex.instructions.InsnType; @@ -59,6 +60,7 @@ public class BlockProcessingHelper { RegisterArg resArg = me.getResult(); resArg = InsnArg.reg(resArg.getRegNum(), type); me.setResult(resArg); + me.add(AFlag.DONT_INLINE); excHandler.setArg(resArg); } diff --git a/jadx-core/src/main/java/jadx/core/dex/visitors/CodeShrinker.java b/jadx-core/src/main/java/jadx/core/dex/visitors/CodeShrinker.java index e5368fafb..c44364430 100644 --- a/jadx-core/src/main/java/jadx/core/dex/visitors/CodeShrinker.java +++ b/jadx-core/src/main/java/jadx/core/dex/visitors/CodeShrinker.java @@ -2,7 +2,6 @@ package jadx.core.dex.visitors; 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; @@ -213,9 +212,7 @@ public class CodeShrinker extends AbstractVisitor { continue; } InsnNode assignInsn = sVar.getAssign().getParentInsn(); - if (assignInsn == null - || assignInsn instanceof PhiInsn - || assignInsn.getType() == InsnType.MOVE_EXCEPTION) { + if (assignInsn == null || assignInsn.contains(AFlag.DONT_INLINE)) { continue; } int assignPos = insnList.getIndex(assignInsn); diff --git a/jadx-core/src/main/java/jadx/core/dex/visitors/ConstInlinerVisitor.java b/jadx-core/src/main/java/jadx/core/dex/visitors/ConstInlinerVisitor.java index a228732d5..8fc0d0cd0 100644 --- a/jadx-core/src/main/java/jadx/core/dex/visitors/ConstInlinerVisitor.java +++ b/jadx-core/src/main/java/jadx/core/dex/visitors/ConstInlinerVisitor.java @@ -1,5 +1,6 @@ package jadx.core.dex.visitors; +import jadx.core.dex.attributes.AFlag; import jadx.core.dex.info.FieldInfo; import jadx.core.dex.instructions.IndexInsnNode; import jadx.core.dex.instructions.InsnType; @@ -52,21 +53,13 @@ public class ConstInlinerVisitor extends AbstractVisitor { SSAVar sVar = insn.getResult().getSVar(); if (lit == 0) { - // don't inline null object if: - // - used as instance arg in invoke instruction - for (RegisterArg useArg : sVar.getUseList()) { - InsnNode parentInsn = useArg.getParentInsn(); - if (parentInsn != null) { - InsnType insnType = parentInsn.getType(); - if (insnType == InsnType.INVOKE) { - InvokeNode inv = (InvokeNode) parentInsn; - if (inv.getInvokeType() != InvokeType.STATIC - && inv.getArg(0) == useArg) { - return false; - } - } + if (checkObjectInline(sVar)) { + if (sVar.getUseCount() == 1) { + insn.getResult().getAssignInsn().add(AFlag.DONT_INLINE); } + return false; } + } ArgType resType = insn.getResult().getType(); // make sure arg has correct type @@ -76,6 +69,32 @@ public class ConstInlinerVisitor extends AbstractVisitor { return replaceConst(mth, sVar, lit); } + /** + * Don't inline null object if: + * - used as instance arg in invoke instruction + * - used in 'array.length' + */ + private static boolean checkObjectInline(SSAVar sVar) { + for (RegisterArg useArg : sVar.getUseList()) { + InsnNode insn = useArg.getParentInsn(); + if (insn != null) { + InsnType insnType = insn.getType(); + if (insnType == InsnType.INVOKE) { + InvokeNode inv = (InvokeNode) insn; + if (inv.getInvokeType() != InvokeType.STATIC + && inv.getArg(0) == useArg) { + return true; + } + } else if (insnType == InsnType.ARRAY_LENGTH) { + if (insn.getArg(0) == useArg) { + return true; + } + } + } + } + return false; + } + private static boolean replaceConst(MethodNode mth, SSAVar sVar, long literal) { List use = new ArrayList(sVar.getUseList()); int replaceCount = 0; diff --git a/jadx-core/src/main/java/jadx/core/dex/visitors/regions/LoopRegionVisitor.java b/jadx-core/src/main/java/jadx/core/dex/visitors/regions/LoopRegionVisitor.java index 7b04ab139..3aa88a55f 100644 --- a/jadx-core/src/main/java/jadx/core/dex/visitors/regions/LoopRegionVisitor.java +++ b/jadx-core/src/main/java/jadx/core/dex/visitors/regions/LoopRegionVisitor.java @@ -107,7 +107,7 @@ public class LoopRegionVisitor extends AbstractVisitor implements IRegionVisitor List args = new LinkedList(); incrInsn.getRegisterArgs(args); for (RegisterArg iArg : args) { - if (assignOnlyInLoop(mth, loopRegion, (RegisterArg) iArg)) { + if (assignOnlyInLoop(mth, loopRegion, iArg)) { return false; } } diff --git a/jadx-core/src/main/java/jadx/core/utils/ErrorsCounter.java b/jadx-core/src/main/java/jadx/core/utils/ErrorsCounter.java index b33ac654f..e9657453f 100644 --- a/jadx-core/src/main/java/jadx/core/utils/ErrorsCounter.java +++ b/jadx-core/src/main/java/jadx/core/utils/ErrorsCounter.java @@ -40,7 +40,7 @@ public class ErrorsCounter { if (e.getClass() == JadxOverflowException.class) { // don't print full stack trace e = new JadxOverflowException(e.getMessage()); - LOG.error(msg); + LOG.error(msg + ", message: " + e.getMessage()); } else { LOG.error(msg, e); } diff --git a/jadx-core/src/main/java/jadx/core/utils/InsnUtils.java b/jadx-core/src/main/java/jadx/core/utils/InsnUtils.java index a7774fdc9..ba155cfef 100644 --- a/jadx-core/src/main/java/jadx/core/utils/InsnUtils.java +++ b/jadx-core/src/main/java/jadx/core/utils/InsnUtils.java @@ -35,11 +35,7 @@ public class InsnUtils { } public static String insnTypeToString(InsnType type) { - return insnTypeToString(type.toString()); - } - - public static String insnTypeToString(String str) { - return String.format("%s ", str); + return type.toString() + " "; } public static String indexToString(Object index) { @@ -49,7 +45,7 @@ public class InsnUtils { if (index instanceof String) { return "\"" + index + "\""; } else { - return " " + index; + return index.toString(); } } } diff --git a/jadx-core/src/test/java/jadx/tests/api/IntegrationTest.java b/jadx-core/src/test/java/jadx/tests/api/IntegrationTest.java index 21c8fafef..a67b1963d 100644 --- a/jadx-core/src/test/java/jadx/tests/api/IntegrationTest.java +++ b/jadx-core/src/test/java/jadx/tests/api/IntegrationTest.java @@ -201,6 +201,7 @@ public abstract class IntegrationTest extends TestUtils { dynamicCompiler = new DynamicCompiler(cls); boolean result = dynamicCompiler.compile(); assertTrue("Compilation failed", result); + System.out.println("Compilation: PASSED"); } catch (Exception e) { e.printStackTrace(); fail(e.getMessage()); diff --git a/jadx-core/src/test/java/jadx/tests/functional/TypeMergeTest.java b/jadx-core/src/test/java/jadx/tests/functional/TypeMergeTest.java index 297fa38a6..e56c5930b 100644 --- a/jadx-core/src/test/java/jadx/tests/functional/TypeMergeTest.java +++ b/jadx-core/src/test/java/jadx/tests/functional/TypeMergeTest.java @@ -20,6 +20,7 @@ import static jadx.core.dex.instructions.args.ArgType.OBJECT; import static jadx.core.dex.instructions.args.ArgType.STRING; import static jadx.core.dex.instructions.args.ArgType.UNKNOWN; import static jadx.core.dex.instructions.args.ArgType.UNKNOWN_OBJECT; +import static jadx.core.dex.instructions.args.ArgType.array; import static jadx.core.dex.instructions.args.ArgType.genericType; import static jadx.core.dex.instructions.args.ArgType.object; import static jadx.core.dex.instructions.args.ArgType.unknown; @@ -59,13 +60,6 @@ public class TypeMergeTest { unknown(PrimitiveType.OBJECT, PrimitiveType.ARRAY), unknown(PrimitiveType.OBJECT)); - check(ArgType.array(INT), ArgType.array(BYTE), ArgType.OBJECT); - first(ArgType.array(INT), ArgType.array(INT)); - first(ArgType.array(STRING), ArgType.array(STRING)); - - first(OBJECT, ArgType.array(INT)); - first(OBJECT, ArgType.array(STRING)); - ArgType objExc = object("java.lang.Exception"); ArgType objThr = object("java.lang.Throwable"); ArgType objIO = object("java.io.IOException"); @@ -83,6 +77,18 @@ public class TypeMergeTest { first(generic, objExc); } + @Test + public void testArrayMerge() { + check(array(INT), array(BYTE), ArgType.OBJECT); + first(array(INT), array(INT)); + first(array(STRING), array(STRING)); + + first(OBJECT, array(INT)); + first(OBJECT, array(STRING)); + + first(array(unknown(PrimitiveType.INT, PrimitiveType.FLOAT)), unknown(PrimitiveType.ARRAY)); + } + private void first(ArgType t1, ArgType t2) { check(t1, t2, t1); } diff --git a/jadx-core/src/test/java/jadx/tests/integration/TestWrongCode.java b/jadx-core/src/test/java/jadx/tests/integration/TestWrongCode.java index 541fdc2b3..3d9d04a8d 100644 --- a/jadx-core/src/test/java/jadx/tests/integration/TestWrongCode.java +++ b/jadx-core/src/test/java/jadx/tests/integration/TestWrongCode.java @@ -5,6 +5,7 @@ import jadx.tests.api.IntegrationTest; import org.junit.Test; +import static jadx.tests.api.utils.JadxMatchers.containsOne; import static org.hamcrest.CoreMatchers.containsString; import static org.hamcrest.CoreMatchers.not; import static org.junit.Assert.assertThat; @@ -28,13 +29,19 @@ public class TestWrongCode extends IntegrationTest { @Test public void test() { - disableCompilation(); ClassNode cls = getClassNode(TestCls.class); String code = cls.getCode().toString(); assertThat(code, not(containsString("return false.length;"))); - assertThat(code, containsString("return null.length;")); + assertThat(code, containsOne("int[] a = null;")); + assertThat(code, containsOne("return a.length;")); assertThat(code, containsString("return a == 0 ? a : a;")); } + + @Test + public void testNoDebug() { + noDebugInfo(); + getClassNode(TestCls.class); + } } diff --git a/jadx-core/src/test/java/jadx/tests/integration/arrays/TestArrays.java b/jadx-core/src/test/java/jadx/tests/integration/arrays/TestArrays.java new file mode 100644 index 000000000..3c948e1c4 --- /dev/null +++ b/jadx-core/src/test/java/jadx/tests/integration/arrays/TestArrays.java @@ -0,0 +1,33 @@ +package jadx.tests.integration.arrays; + +import jadx.core.dex.nodes.ClassNode; +import jadx.tests.api.IntegrationTest; + +import org.junit.Test; + +import static jadx.tests.api.utils.JadxMatchers.containsOne; +import static org.junit.Assert.assertThat; + +public class TestArrays extends IntegrationTest { + public static class TestCls { + + public int test1(int i) { + int[] a = new int[]{1, 2, 3, 5}; + return a[i]; + } + + public int test2(int i) { + int[][] a = new int[i][i + 1]; + return a.length; + } + } + + @Test + public void test() { + noDebugInfo(); + ClassNode cls = getClassNode(TestCls.class); + String code = cls.getCode().toString(); + + assertThat(code, containsOne("return new int[]{1, 2, 3, 5}[i];")); + } +}