From b66293a2f76e58a2eb8daca38b4ed73c07935e58 Mon Sep 17 00:00:00 2001 From: Skylot Date: Mon, 18 Apr 2022 16:09:33 +0100 Subject: [PATCH] fix: handle wildcard in invoke type resolver (#1238) --- .../core/dex/instructions/args/SSAVar.java | 5 ++ .../core/dex/visitors/MoveInlineVisitor.java | 7 ++ .../TypeBoundCheckCastAssign.java | 4 + .../typeinference/TypeBoundInvokeAssign.java | 18 ++++- .../typeinference/TypeInferenceVisitor.java | 57 +++++++++++++- .../main/java/jadx/core/utils/ListUtils.java | 6 +- .../integration/types/TestTypeResolver20.java | 75 +++++++++++++++++++ .../types/TestTypeResolver20/Sequence.smali | 22 ++++++ .../TestTypeResolver20.smali | 61 +++++++++++++++ 9 files changed, 249 insertions(+), 6 deletions(-) create mode 100644 jadx-core/src/test/java/jadx/tests/integration/types/TestTypeResolver20.java create mode 100644 jadx-core/src/test/smali/types/TestTypeResolver20/Sequence.smali create mode 100644 jadx-core/src/test/smali/types/TestTypeResolver20/TestTypeResolver20.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 a6b5111af..f0e3c250f 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 @@ -192,6 +192,11 @@ public class SSAVar { return usedInPhi; } + public boolean isAssignInPhi() { + InsnNode assignInsn = getAssignInsn(); + return assignInsn != null && assignInsn.getType() == InsnType.PHI; + } + public boolean isUsedInPhi() { return usedInPhi != null && !usedInPhi.isEmpty(); } diff --git a/jadx-core/src/main/java/jadx/core/dex/visitors/MoveInlineVisitor.java b/jadx-core/src/main/java/jadx/core/dex/visitors/MoveInlineVisitor.java index 8c597927c..0c4c77dce 100644 --- a/jadx-core/src/main/java/jadx/core/dex/visitors/MoveInlineVisitor.java +++ b/jadx-core/src/main/java/jadx/core/dex/visitors/MoveInlineVisitor.java @@ -52,6 +52,13 @@ public class MoveInlineVisitor extends AbstractVisitor { if (resultArg.sameRegAndSVar(moveArg)) { return true; } + if (moveArg.isRegister()) { + RegisterArg moveReg = (RegisterArg) moveArg; + if (moveReg.getSVar().isAssignInPhi()) { + // don't mix already merged variables + return false; + } + } SSAVar ssaVar = resultArg.getSVar(); if (ssaVar.isUsedInPhi()) { return deleteMove(mth, move); diff --git a/jadx-core/src/main/java/jadx/core/dex/visitors/typeinference/TypeBoundCheckCastAssign.java b/jadx-core/src/main/java/jadx/core/dex/visitors/typeinference/TypeBoundCheckCastAssign.java index ec1d88e07..7359bc084 100644 --- a/jadx-core/src/main/java/jadx/core/dex/visitors/typeinference/TypeBoundCheckCastAssign.java +++ b/jadx-core/src/main/java/jadx/core/dex/visitors/typeinference/TypeBoundCheckCastAssign.java @@ -46,6 +46,10 @@ public final class TypeBoundCheckCastAssign implements ITypeBoundDynamic { return insn.getResult(); } + public IndexInsnNode getInsn() { + return insn; + } + @Override public String toString() { return "CHECK_CAST_ASSIGN{(" + insn.getIndex() + ") " + insn.getArg(0).getType() + "}"; diff --git a/jadx-core/src/main/java/jadx/core/dex/visitors/typeinference/TypeBoundInvokeAssign.java b/jadx-core/src/main/java/jadx/core/dex/visitors/typeinference/TypeBoundInvokeAssign.java index d391ab795..64e49459c 100644 --- a/jadx-core/src/main/java/jadx/core/dex/visitors/typeinference/TypeBoundInvokeAssign.java +++ b/jadx-core/src/main/java/jadx/core/dex/visitors/typeinference/TypeBoundInvokeAssign.java @@ -1,5 +1,7 @@ package jadx.core.dex.visitors.typeinference; +import org.jetbrains.annotations.Nullable; + import jadx.core.dex.instructions.InvokeNode; import jadx.core.dex.instructions.args.ArgType; import jadx.core.dex.instructions.args.InsnArg; @@ -48,12 +50,24 @@ public final class TypeBoundInvokeAssign implements ITypeBoundDynamic { mthDeclType = instanceType; } ArgType resultGeneric = root.getTypeUtils().replaceClassGenerics(instanceType, mthDeclType, genericReturnType); - if (resultGeneric != null && !resultGeneric.isWildcard()) { - return resultGeneric; + ArgType result = processResultType(resultGeneric); + if (result != null) { + return result; } return invokeNode.getCallMth().getReturnType(); } + @Nullable + private ArgType processResultType(@Nullable ArgType resultGeneric) { + if (resultGeneric == null) { + return null; + } + if (!resultGeneric.isWildcard()) { + return resultGeneric; + } + return resultGeneric.getWildcardType(); + } + private InsnArg getInstanceArg() { return invokeNode.getArg(0); } diff --git a/jadx-core/src/main/java/jadx/core/dex/visitors/typeinference/TypeInferenceVisitor.java b/jadx-core/src/main/java/jadx/core/dex/visitors/typeinference/TypeInferenceVisitor.java index 0de65a3fb..89031707a 100644 --- a/jadx-core/src/main/java/jadx/core/dex/visitors/typeinference/TypeInferenceVisitor.java +++ b/jadx-core/src/main/java/jadx/core/dex/visitors/typeinference/TypeInferenceVisitor.java @@ -57,6 +57,7 @@ import jadx.core.dex.visitors.ssa.SSATransform; import jadx.core.utils.BlockUtils; import jadx.core.utils.InsnList; import jadx.core.utils.InsnUtils; +import jadx.core.utils.ListUtils; import jadx.core.utils.Utils; import jadx.core.utils.exceptions.JadxOverflowException; @@ -83,6 +84,7 @@ public final class TypeInferenceVisitor extends AbstractVisitor { this.resolvers = Arrays.asList( this::initTypeBounds, this::runTypePropagation, + this::tryRestoreTypeVarCasts, this::tryInsertCasts, this::tryDeduceTypes, this::trySplitConstInsns, @@ -520,7 +522,60 @@ public final class TypeInferenceVisitor extends AbstractVisitor { return false; } - @SuppressWarnings("ForLoopReplaceableByWhile") + /** + * Fix check casts to type var extend type: + *
+ * {@code T var = (Comparable) obj; => T var = (T) obj; } + */ + private boolean tryRestoreTypeVarCasts(MethodNode mth) { + int changed = 0; + List mthSVars = mth.getSVars(); + for (SSAVar var : mthSVars) { + changed += restoreTypeVarCasts(var); + } + if (changed == 0) { + return false; + } + if (Consts.DEBUG_TYPE_INFERENCE) { + mth.addDebugComment("Restore " + changed + " type vars casts"); + } + initTypeBounds(mth); + return runTypePropagation(mth); + } + + private int restoreTypeVarCasts(SSAVar var) { + TypeInfo typeInfo = var.getTypeInfo(); + Set bounds = typeInfo.getBounds(); + if (!ListUtils.anyMatch(bounds, t -> t.getType().isGenericType())) { + return 0; + } + List casts = ListUtils.filter(bounds, TypeBoundCheckCastAssign.class::isInstance); + if (casts.isEmpty()) { + return 0; + } + ArgType bestType = selectBestTypeFromBounds(bounds).orElse(ArgType.UNKNOWN); + if (!bestType.isGenericType()) { + return 0; + } + List extendTypes = bestType.getExtendTypes(); + if (extendTypes.size() != 1) { + return 0; + } + int fixed = 0; + ArgType extendType = extendTypes.get(0); + for (ITypeBound bound : casts) { + TypeBoundCheckCastAssign cast = (TypeBoundCheckCastAssign) bound; + ArgType castType = cast.getType(); + TypeCompareEnum result = typeUpdate.getTypeCompare().compareTypes(extendType, castType); + if (result.isEqual() || result == TypeCompareEnum.NARROW_BY_GENERIC) { + cast.getInsn().updateIndex(bestType); + fixed++; + } + } + return fixed; + } + + @SuppressWarnings({ "ForLoopReplaceableByWhile", "ForLoopReplaceableByForEach" }) private boolean tryInsertCasts(MethodNode mth) { int added = 0; List mthSVars = mth.getSVars(); diff --git a/jadx-core/src/main/java/jadx/core/utils/ListUtils.java b/jadx-core/src/main/java/jadx/core/utils/ListUtils.java index 346461bbb..65ffb1f52 100644 --- a/jadx-core/src/main/java/jadx/core/utils/ListUtils.java +++ b/jadx-core/src/main/java/jadx/core/utils/ListUtils.java @@ -112,7 +112,7 @@ public class ListUtils { return list; } - public static List filter(List list, Predicate filter) { + public static List filter(Collection list, Predicate filter) { if (list == null || list.isEmpty()) { return Collections.emptyList(); } @@ -148,7 +148,7 @@ public class ListUtils { return found; } - public static boolean allMatch(List list, Predicate test) { + public static boolean allMatch(Collection list, Predicate test) { if (list == null || list.isEmpty()) { return false; } @@ -160,7 +160,7 @@ public class ListUtils { return true; } - public static boolean anyMatch(List list, Predicate test) { + public static boolean anyMatch(Collection list, Predicate test) { if (list == null || list.isEmpty()) { return false; } diff --git a/jadx-core/src/test/java/jadx/tests/integration/types/TestTypeResolver20.java b/jadx-core/src/test/java/jadx/tests/integration/types/TestTypeResolver20.java new file mode 100644 index 000000000..7a61a3198 --- /dev/null +++ b/jadx-core/src/test/java/jadx/tests/integration/types/TestTypeResolver20.java @@ -0,0 +1,75 @@ +package jadx.tests.integration.types; + +import java.util.Arrays; +import java.util.Iterator; +import java.util.List; + +import org.junit.jupiter.api.Test; + +import jadx.tests.api.SmaliTest; +import jadx.tests.api.extensions.profiles.TestProfile; +import jadx.tests.api.extensions.profiles.TestWithProfiles; + +import static jadx.tests.api.utils.assertj.JadxAssertions.assertThat; + +/** + * Issue 1238 + */ +public class TestTypeResolver20 extends SmaliTest { + + public static class TestCls { + public interface Sequence { + Iterator iterator(); + } + + public static > T max(Sequence seq) { + Iterator it = seq.iterator(); + if (!it.hasNext()) { + return null; + } + T t = it.next(); + while (it.hasNext()) { + T next = it.next(); + if (t.compareTo(next) < 0) { + t = next; + } + } + return t; + } + + private static class ArraySeq implements Sequence { + private final List list; + + @SafeVarargs + public ArraySeq(T... arr) { + this.list = Arrays.asList(arr); + } + + @Override + public Iterator iterator() { + return list.iterator(); + } + } + + public void check() { + assertThat(max(new ArraySeq<>(2, 5, 3, 4))).isEqualTo(5); + } + } + + @TestWithProfiles({ TestProfile.DX_J8, TestProfile.JAVA8 }) + public void test() { + noDebugInfo(); + assertThat(getClassNode(TestCls.class)) + .code() + .doesNotContain("next = next;") + .containsOne("T next = it.next();"); + } + + @Test + public void testSmali() { + assertThat(getClassNodeFromSmaliFiles()) + .code() + .containsOne("T next = it.next();") + .containsOne("T next2 = it.next();"); + } +} diff --git a/jadx-core/src/test/smali/types/TestTypeResolver20/Sequence.smali b/jadx-core/src/test/smali/types/TestTypeResolver20/Sequence.smali new file mode 100644 index 000000000..5a87f7a32 --- /dev/null +++ b/jadx-core/src/test/smali/types/TestTypeResolver20/Sequence.smali @@ -0,0 +1,22 @@ +.class public interface abstract Lkotlin/sequences/Sequence; +.super Ljava/lang/Object; +.source "SourceFile" + +.annotation system Ldalvik/annotation/Signature; + value = { + "", + "Ljava/lang/Object;" + } +.end annotation + +.method public abstract iterator()Ljava/util/Iterator; + .annotation system Ldalvik/annotation/Signature; + value = { + "()", + "Ljava/util/Iterator<", + "TT;>;" + } + .end annotation +.end method diff --git a/jadx-core/src/test/smali/types/TestTypeResolver20/TestTypeResolver20.smali b/jadx-core/src/test/smali/types/TestTypeResolver20/TestTypeResolver20.smali new file mode 100644 index 000000000..d2bbae5a1 --- /dev/null +++ b/jadx-core/src/test/smali/types/TestTypeResolver20/TestTypeResolver20.smali @@ -0,0 +1,61 @@ +.class public Ltypes/TestTypeResolver20; +.super Ljava/lang/Object; +.source "SourceFile" + + +.method public static final max(Lkotlin/sequences/Sequence;)Ljava/lang/Comparable; + .registers 4 + .annotation system Ldalvik/annotation/Signature; + value = { + ";>(", + "Lkotlin/sequences/Sequence<", + "+TT;>;)TT;" + } + .end annotation + + .line 1147 + invoke-interface {p0}, Lkotlin/sequences/Sequence;->iterator()Ljava/util/Iterator; + move-result-object p0 + + .line 1148 + invoke-interface {p0}, Ljava/util/Iterator;->hasNext()Z + move-result v0 + + if-nez v0, :cond_11 + + const/4 p0, 0x0 + return-object p0 + + .line 1149 + :cond_11 + invoke-interface {p0}, Ljava/util/Iterator;->next()Ljava/lang/Object; + move-result-object v0 + check-cast v0, Ljava/lang/Comparable; + + .line 1150 + :cond_17 + :goto_17 + invoke-interface {p0}, Ljava/util/Iterator;->hasNext()Z + move-result v1 + + if-eqz v1, :cond_2b + + .line 1151 + invoke-interface {p0}, Ljava/util/Iterator;->next()Ljava/lang/Object; + move-result-object v1 + check-cast v1, Ljava/lang/Comparable; + + .line 1152 + invoke-interface {v0, v1}, Ljava/lang/Comparable;->compareTo(Ljava/lang/Object;)I + move-result v2 + + if-gez v2, :cond_17 + + move-object v0, v1 + goto :goto_17 + + :cond_2b + return-object v0 +.end method