From a62de839beb3303daa70075fbf1d24b0eccaf2d6 Mon Sep 17 00:00:00 2001 From: Skylot Date: Tue, 24 Aug 2021 13:32:06 +0100 Subject: [PATCH] fix: handle unbound type variables in type inference (#1237) --- .../core/dex/visitors/SignatureProcessor.java | 38 ++++++++++++------- .../visitors/typeinference/TypeCompare.java | 6 +-- .../typeinference/TypeCompareTest.java | 2 +- .../integration/types/TestTypeResolver18.java | 37 ++++++++++++++++++ .../types/TestPrimitiveConversion2.smali | 2 +- 5 files changed, 67 insertions(+), 18 deletions(-) create mode 100644 jadx-core/src/test/java/jadx/tests/integration/types/TestTypeResolver18.java diff --git a/jadx-core/src/main/java/jadx/core/dex/visitors/SignatureProcessor.java b/jadx-core/src/main/java/jadx/core/dex/visitors/SignatureProcessor.java index 5aae30448..e22e11248 100644 --- a/jadx-core/src/main/java/jadx/core/dex/visitors/SignatureProcessor.java +++ b/jadx-core/src/main/java/jadx/core/dex/visitors/SignatureProcessor.java @@ -1,6 +1,7 @@ package jadx.core.dex.visitors; import java.util.ArrayList; +import java.util.Collections; import java.util.List; import java.util.Objects; @@ -16,8 +17,6 @@ import jadx.core.dex.visitors.typeinference.TypeCompareEnum; import jadx.core.utils.Utils; import jadx.core.utils.exceptions.JadxException; -import static java.util.Collections.unmodifiableList; - public class SignatureProcessor extends AbstractVisitor { private RootNode root; @@ -106,25 +105,38 @@ public class SignatureProcessor extends AbstractVisitor { List parsedArgTypes = sp.consumeMethodArgs(mth.getMethodInfo().getArgsCount()); ArgType parsedRetType = sp.consumeType(); - if (!validateParsedType(parsedRetType, mth.getMethodInfo().getReturnType())) { - mth.addWarnComment("Incorrect return type in method signature: " + sp.getSignature()); - return; - } - List checkedArgTypes = checkArgTypes(mth, sp, parsedArgTypes); - if (checkedArgTypes == null) { - return; - } mth.updateTypeParameters(typeParameters); // apply before expand args - TypeUtils typeUtils = root.getTypeUtils(); ArgType retType = typeUtils.expandTypeVariables(mth, parsedRetType); - List resultArgTypes = Utils.collectionMap(checkedArgTypes, t -> typeUtils.expandTypeVariables(mth, t)); - mth.updateTypes(unmodifiableList(resultArgTypes), retType); + List argTypes = Utils.collectionMap(parsedArgTypes, t -> typeUtils.expandTypeVariables(mth, t)); + + if (!validateAndApplyTypes(mth, sp, retType, argTypes)) { + // bad types -> reset typed parameters + mth.updateTypeParameters(Collections.emptyList()); + } } catch (Exception e) { mth.addWarnComment("Failed to parse method signature: " + sp.getSignature(), e); } } + private boolean validateAndApplyTypes(MethodNode mth, SignatureParser sp, ArgType retType, List argTypes) { + try { + if (!validateParsedType(retType, mth.getMethodInfo().getReturnType())) { + mth.addWarnComment("Incorrect return type in method signature: " + sp.getSignature()); + return false; + } + List checkedArgTypes = checkArgTypes(mth, sp, argTypes); + if (checkedArgTypes == null) { + return false; + } + mth.updateTypes(Utils.lockList(checkedArgTypes), retType); + return true; + } catch (Exception e) { + mth.addWarnComment("Type validation failed for signature: " + sp.getSignature(), e); + return false; + } + } + private List checkArgTypes(MethodNode mth, SignatureParser sp, List parsedArgTypes) { MethodInfo mthInfo = mth.getMethodInfo(); List mthArgTypes = mthInfo.getArgumentsTypes(); 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 49f3dce5f..b4001bcf1 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 @@ -245,12 +245,12 @@ public class TypeCompare { if (objType.isGenericType()) { return compareTypeVariables(genericType, objType); } - + boolean rootObject = objType.equals(ArgType.OBJECT); List extendTypes = genericType.getExtendTypes(); if (extendTypes.isEmpty()) { - return NARROW; + return rootObject ? NARROW : CONFLICT; } - if (extendTypes.contains(objType) || objType.equals(ArgType.OBJECT)) { + if (extendTypes.contains(objType) || rootObject) { return NARROW; } for (ArgType extendType : extendTypes) { diff --git a/jadx-core/src/test/java/jadx/core/dex/visitors/typeinference/TypeCompareTest.java b/jadx-core/src/test/java/jadx/core/dex/visitors/typeinference/TypeCompareTest.java index 7166f44e7..38b9d54b4 100644 --- a/jadx-core/src/test/java/jadx/core/dex/visitors/typeinference/TypeCompareTest.java +++ b/jadx-core/src/test/java/jadx/core/dex/visitors/typeinference/TypeCompareTest.java @@ -138,7 +138,7 @@ public class TypeCompareTest { public void compareGenericTypes() { ArgType vType = genericType("V"); check(vType, OBJECT, TypeCompareEnum.NARROW); - check(vType, STRING, TypeCompareEnum.NARROW); + check(vType, STRING, TypeCompareEnum.CONFLICT); ArgType rType = genericType("R"); check(vType, rType, TypeCompareEnum.CONFLICT); diff --git a/jadx-core/src/test/java/jadx/tests/integration/types/TestTypeResolver18.java b/jadx-core/src/test/java/jadx/tests/integration/types/TestTypeResolver18.java new file mode 100644 index 000000000..1b817950e --- /dev/null +++ b/jadx-core/src/test/java/jadx/tests/integration/types/TestTypeResolver18.java @@ -0,0 +1,37 @@ +package jadx.tests.integration.types; + +import java.io.Closeable; +import java.io.IOException; +import java.util.concurrent.atomic.AtomicReference; + +import org.junit.jupiter.api.Test; + +import jadx.tests.api.IntegrationTest; + +import static jadx.tests.api.utils.assertj.JadxAssertions.assertThat; + +public class TestTypeResolver18 extends IntegrationTest { + + public static class TestCls { + private final AtomicReference reference = new AtomicReference<>(); + + public void test() { + T t = this.reference.get(); + if (t instanceof Closeable) { + try { + ((Closeable) t).close(); + } catch (IOException unused) { + // ignore + } + } + this.reference.set(null); + } + } + + @Test + public void test() { + assertThat(getClassNode(TestCls.class)) + .code() + .containsOne("((Closeable) t).close();"); + } +} diff --git a/jadx-core/src/test/smali/types/TestPrimitiveConversion2.smali b/jadx-core/src/test/smali/types/TestPrimitiveConversion2.smali index 0eeccb5a9..a00a83092 100644 --- a/jadx-core/src/test/smali/types/TestPrimitiveConversion2.smali +++ b/jadx-core/src/test/smali/types/TestPrimitiveConversion2.smali @@ -12,7 +12,7 @@ "Lapp/ItemCurrency;", "Lapp/ItemCurrency;", "Lapp/ItemCurrency;", - "ZTItem;)Z" + "ZLapp/SearchListItem;)Z" } .end annotation