From b9fffa149be2e47b150d340645035ea441d98a17 Mon Sep 17 00:00:00 2001 From: Skylot Date: Thu, 6 Dec 2018 16:27:32 +0300 Subject: [PATCH] fix: allow override type with wider one only from debug info (#403) --- .../core/dex/instructions/args/ArgType.java | 5 ++ .../debuginfo/DebugInfoApplyVisitor.java | 15 +++--- .../visitors/typeinference/TypeCompare.java | 3 -- .../visitors/typeinference/TypeUpdate.java | 29 ++++++++++++ .../trycatch/TestInlineInCatch.java | 3 ++ .../integration/types/TestPrimitivesInIf.java | 47 +++++++++++++++++++ 6 files changed, 92 insertions(+), 10 deletions(-) create mode 100644 jadx-core/src/test/java/jadx/tests/integration/types/TestPrimitivesInIf.java 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 8e9663323..541b1b456 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 @@ -526,6 +526,11 @@ public abstract class ArgType { return isArray() || (!isTypeKnown() && contains(PrimitiveType.ARRAY)); } + public boolean canBePrimitive(PrimitiveType primitiveType) { + return (isPrimitive() && getPrimitiveType() == primitiveType) + || (!isTypeKnown() && contains(primitiveType)); + } + public static ArgType convertFromPrimitiveType(PrimitiveType primitiveType) { switch (primitiveType) { case BOOLEAN: diff --git a/jadx-core/src/main/java/jadx/core/dex/visitors/debuginfo/DebugInfoApplyVisitor.java b/jadx-core/src/main/java/jadx/core/dex/visitors/debuginfo/DebugInfoApplyVisitor.java index 806a449ee..5d4ca45d5 100644 --- a/jadx-core/src/main/java/jadx/core/dex/visitors/debuginfo/DebugInfoApplyVisitor.java +++ b/jadx-core/src/main/java/jadx/core/dex/visitors/debuginfo/DebugInfoApplyVisitor.java @@ -9,6 +9,7 @@ import java.util.Set; import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import jadx.core.Consts; import jadx.core.deobf.NameMapper; import jadx.core.dex.attributes.AFlag; import jadx.core.dex.attributes.AType; @@ -34,8 +35,8 @@ import jadx.core.utils.ErrorsCounter; import jadx.core.utils.exceptions.JadxException; @JadxVisitor( - name = "Debug Info Parser", - desc = "Parse debug information (variable names and types, instruction lines)", + name = "Debug Info Apply", + desc = "Apply debug info to registers (type and names)", runAfter = { SSATransform.class, TypeInferenceVisitor.class, @@ -101,7 +102,7 @@ public class DebugInfoApplyVisitor extends AbstractVisitor { int startAddr = localVar.getStartAddr(); int endAddr = localVar.getEndAddr(); if (isInside(startOffset, startAddr, endAddr) || isInside(endOffset, startAddr, endAddr)) { - if (LOG.isDebugEnabled()) { + if (Consts.DEBUG && LOG.isDebugEnabled()) { LOG.debug("Apply debug info by offset for: {} to {}", ssaVar, localVar); } applyDebugInfo(mth, ssaVar, localVar.getType(), localVar.getName()); @@ -126,15 +127,15 @@ public class DebugInfoApplyVisitor extends AbstractVisitor { } public static void applyDebugInfo(MethodNode mth, SSAVar ssaVar, ArgType type, String varName) { - TypeUpdateResult result = mth.root().getTypeUpdate().apply(ssaVar, type); + if (NameMapper.isValidIdentifier(varName)) { + ssaVar.setName(varName); + } + TypeUpdateResult result = mth.root().getTypeUpdate().applyDebug(ssaVar, type); if (result == TypeUpdateResult.REJECT) { if (LOG.isDebugEnabled()) { LOG.debug("Reject debug info of type: {} and name: '{}' for {}, mth: {}", type, varName, ssaVar, mth); } } else { - if (NameMapper.isValidIdentifier(varName)) { - ssaVar.setName(varName); - } detachDebugInfo(ssaVar.getAssign()); ssaVar.getUseList().forEach(DebugInfoApplyVisitor::detachDebugInfo); } diff --git a/jadx-core/src/main/java/jadx/core/dex/visitors/typeinference/TypeCompare.java b/jadx-core/src/main/java/jadx/core/dex/visitors/typeinference/TypeCompare.java index 5f650bd8b..578d60f56 100644 --- a/jadx-core/src/main/java/jadx/core/dex/visitors/typeinference/TypeCompare.java +++ b/jadx-core/src/main/java/jadx/core/dex/visitors/typeinference/TypeCompare.java @@ -193,9 +193,6 @@ public class TypeCompare { return comparator; } - /** - * - */ private final class ArgTypeComparator implements Comparator { @Override public int compare(ArgType a, ArgType b) { diff --git a/jadx-core/src/main/java/jadx/core/dex/visitors/typeinference/TypeUpdate.java b/jadx-core/src/main/java/jadx/core/dex/visitors/typeinference/TypeUpdate.java index 0af2cac9f..7fc82bb99 100644 --- a/jadx-core/src/main/java/jadx/core/dex/visitors/typeinference/TypeUpdate.java +++ b/jadx-core/src/main/java/jadx/core/dex/visitors/typeinference/TypeUpdate.java @@ -33,11 +33,22 @@ public final class TypeUpdate { private final TypeUpdateRegistry listenerRegistry; private final TypeCompare comparator; + private ThreadLocal applyDebug = new ThreadLocal<>(); + public TypeUpdate(RootNode root) { this.listenerRegistry = initListenerRegistry(); this.comparator = new TypeCompare(root); } + public TypeUpdateResult applyDebug(SSAVar ssaVar, ArgType candidateType) { + try { + applyDebug.set(true); + return apply(ssaVar, candidateType); + } finally { + applyDebug.set(false); + } + } + public TypeUpdateResult apply(SSAVar ssaVar, ArgType candidateType) { if (candidateType == null) { return REJECT; @@ -71,6 +82,16 @@ public final class TypeUpdate { if (arg.isTypeImmutable() && currentType != ArgType.UNKNOWN) { return REJECT; } + TypeCompareEnum compareResult = comparator.compareTypes(candidateType, currentType); + if (compareResult == TypeCompareEnum.CONFLICT) { + return REJECT; + } + if (compareResult == TypeCompareEnum.WIDER || compareResult == TypeCompareEnum.WIDER_BY_GENERIC) { + // allow wider types for apply from debug info + if (applyDebug.get() != Boolean.TRUE) { + return REJECT; + } + } if (arg instanceof RegisterArg) { RegisterArg reg = (RegisterArg) arg; return updateTypeForSsaVar(updateInfo, reg.getSVar(), candidateType); @@ -316,6 +337,14 @@ public final class TypeUpdate { if (candidateType.isArray() && updateArgType.canBeArray()) { return SAME; } + if (candidateType.isPrimitive()) { + if (updateArgType.canBePrimitive(candidateType.getPrimitiveType())) { + return SAME; + } + if (updateArgType.isTypeKnown() && candidateType.getRegCount() == updateArgType.getRegCount()) { + return SAME; + } + } } return result; } diff --git a/jadx-core/src/test/java/jadx/tests/integration/trycatch/TestInlineInCatch.java b/jadx-core/src/test/java/jadx/tests/integration/trycatch/TestInlineInCatch.java index 00bfd297e..e87d5ed01 100644 --- a/jadx-core/src/test/java/jadx/tests/integration/trycatch/TestInlineInCatch.java +++ b/jadx-core/src/test/java/jadx/tests/integration/trycatch/TestInlineInCatch.java @@ -19,6 +19,9 @@ public class TestInlineInCatch extends IntegrationTest { File output = null; try { output = File.createTempFile("f", "a", dir); + if (!output.exists()) { + return 1; + } return 0; } catch (Exception e) { if (output != null) { diff --git a/jadx-core/src/test/java/jadx/tests/integration/types/TestPrimitivesInIf.java b/jadx-core/src/test/java/jadx/tests/integration/types/TestPrimitivesInIf.java new file mode 100644 index 000000000..bd738c35a --- /dev/null +++ b/jadx-core/src/test/java/jadx/tests/integration/types/TestPrimitivesInIf.java @@ -0,0 +1,47 @@ +package jadx.tests.integration.types; + +import org.junit.Test; + +import jadx.core.dex.nodes.ClassNode; +import jadx.tests.api.IntegrationTest; + +import static jadx.tests.api.utils.JadxMatchers.containsOne; +import static org.junit.Assert.assertThat; +import static org.junit.Assert.assertTrue; + +public class TestPrimitivesInIf extends IntegrationTest { + + public static class TestCls { + + public boolean test(String str) { + short sh = Short.parseShort(str); + int i = Integer.parseInt(str); + System.out.println(sh + " vs " + i); + return sh == i; + } + + public void check() { + assertTrue(test("1")); + } + } + + @Test + public void test() { + ClassNode cls = getClassNode(TestCls.class); + String code = cls.getCode().toString(); + + assertThat(code, containsOne("short sh = Short.parseShort(str);")); + assertThat(code, containsOne("int i = Integer.parseInt(str);")); + assertThat(code, containsOne("return sh == i;")); + } + + @Test + public void test2() { + setOutputCFG(); + noDebugInfo(); + ClassNode cls = getClassNode(TestCls.class); + String code = cls.getCode().toString(); + + assertThat(code, containsOne("short parseShort = Short.parseShort(str);")); + } +}