From 3d920725aa7fc4cb13fd62e689f6c395b4701fe5 Mon Sep 17 00:00:00 2001 From: Skylot Date: Wed, 29 Jun 2022 19:18:53 +0100 Subject: [PATCH] fix: check synthetic methods before remove/inline (#1560) --- .../jadx/core/dex/visitors/ClassModifier.java | 22 ++------- .../dex/visitors/rename/RenameVisitor.java | 7 --- .../inline/TestOverlapSyntheticMethods.java | 47 +++++++++++++++++++ .../inline/TestOverlapSyntheticMethods.smali | 41 ++++++++++++++++ 4 files changed, 91 insertions(+), 26 deletions(-) create mode 100644 jadx-core/src/test/java/jadx/tests/integration/inline/TestOverlapSyntheticMethods.java create mode 100644 jadx-core/src/test/smali/inline/TestOverlapSyntheticMethods.smali diff --git a/jadx-core/src/main/java/jadx/core/dex/visitors/ClassModifier.java b/jadx-core/src/main/java/jadx/core/dex/visitors/ClassModifier.java index f7e22800..79666f16 100644 --- a/jadx-core/src/main/java/jadx/core/dex/visitors/ClassModifier.java +++ b/jadx-core/src/main/java/jadx/core/dex/visitors/ClassModifier.java @@ -225,7 +225,7 @@ public class ClassModifier extends AbstractVisitor { } private static boolean removeBridgeMethod(ClassNode cls, MethodNode mth) { - if (cls.root().getArgs().isRenameValid()) { + if (cls.root().getArgs().isInlineMethods()) { // simple wrapper remove is same as inline List allInsns = BlockUtils.collectAllInsns(mth.getBasicBlocks()); if (allInsns.size() == 1) { InsnNode wrappedInsn = allInsns.get(0); @@ -235,12 +235,10 @@ public class ClassModifier extends AbstractVisitor { wrappedInsn = ((InsnWrapArg) arg).getWrapInsn(); } } - if (checkSyntheticWrapper(mth, wrappedInsn)) { - return true; - } + return checkSyntheticWrapper(mth, wrappedInsn); } } - return !isMethodUnique(cls, mth); + return false; } private static boolean checkSyntheticWrapper(MethodNode mth, InsnNode insn) { @@ -299,20 +297,6 @@ public class ClassModifier extends AbstractVisitor { return false; } - private static boolean isMethodUnique(ClassNode cls, MethodNode mth) { - MethodInfo mi = mth.getMethodInfo(); - for (MethodNode otherMth : cls.getMethods()) { - if (otherMth != mth) { - MethodInfo omi = otherMth.getMethodInfo(); - if (omi.getName().equals(mi.getName()) - && Objects.equals(omi.getArgumentsTypes(), mi.getArgumentsTypes())) { - return false; - } - } - } - return true; - } - /** * Remove public empty constructors (static or default) */ diff --git a/jadx-core/src/main/java/jadx/core/dex/visitors/rename/RenameVisitor.java b/jadx-core/src/main/java/jadx/core/dex/visitors/rename/RenameVisitor.java index fc2b2f0b..ec1a9d57 100644 --- a/jadx-core/src/main/java/jadx/core/dex/visitors/rename/RenameVisitor.java +++ b/jadx-core/src/main/java/jadx/core/dex/visitors/rename/RenameVisitor.java @@ -14,9 +14,7 @@ import jadx.core.Consts; import jadx.core.codegen.json.JsonMappingGen; import jadx.core.deobf.Deobfuscator; import jadx.core.deobf.NameMapper; -import jadx.core.dex.attributes.AFlag; import jadx.core.dex.attributes.nodes.RenameReasonAttr; -import jadx.core.dex.info.AccessInfo; import jadx.core.dex.info.ClassInfo; import jadx.core.dex.info.FieldInfo; import jadx.core.dex.nodes.ClassNode; @@ -196,11 +194,6 @@ public class RenameVisitor extends AbstractVisitor { if (args.isRenameValid()) { Set names = new HashSet<>(methods.size()); for (MethodNode mth : methods) { - AccessInfo accessFlags = mth.getAccessFlags(); - if (accessFlags.isBridge() || accessFlags.isSynthetic() - || mth.contains(AFlag.DONT_GENERATE) /* this flag not set yet */) { - continue; - } String signature = mth.getMethodInfo().makeSignature(true, false); if (!names.add(signature)) { deobfuscator.forceRenameMethod(mth); diff --git a/jadx-core/src/test/java/jadx/tests/integration/inline/TestOverlapSyntheticMethods.java b/jadx-core/src/test/java/jadx/tests/integration/inline/TestOverlapSyntheticMethods.java new file mode 100644 index 00000000..9dfda0fd --- /dev/null +++ b/jadx-core/src/test/java/jadx/tests/integration/inline/TestOverlapSyntheticMethods.java @@ -0,0 +1,47 @@ +package jadx.tests.integration.inline; + +import java.util.Collections; + +import org.junit.jupiter.api.Test; + +import jadx.tests.api.SmaliTest; + +import static jadx.tests.api.utils.assertj.JadxAssertions.assertThat; + +@SuppressWarnings("CommentedOutCode") +public class TestOverlapSyntheticMethods extends SmaliTest { + // @formatter:off + /* + public String test(int i) { + return a(i) + "|" + a(i); + } + + public int a(int i) { + return i; + } + + public String a(int i) { + return "i:" + i; + } + */ + // @formatter:on + + @Test + public void testSmali() { + assertThat(getClassNodeFromSmali()) + .code() + .containsOne("int a(int i) {") + .containsOne("String m0a(int i) {"); + } + + @Test + public void testSmaliNoRename() { + getArgs().setRenameFlags(Collections.emptySet()); + disableCompilation(); + assertThat(getClassNodeFromSmali()) + .code() + .containsOne("int a(int i) {") + .containsOne("String a(int i) {") + .containsOne("return a(i) + \"|\" + a(i);"); + } +} diff --git a/jadx-core/src/test/smali/inline/TestOverlapSyntheticMethods.smali b/jadx-core/src/test/smali/inline/TestOverlapSyntheticMethods.smali new file mode 100644 index 00000000..07450e2b --- /dev/null +++ b/jadx-core/src/test/smali/inline/TestOverlapSyntheticMethods.smali @@ -0,0 +1,41 @@ +.class public Linline/TestOverlapSyntheticMethods; +.super Ljava/lang/Object; + +.method public synthetic a(I)I + .registers 2 + return p1 +.end method + +.method public synthetic a(I)Ljava/lang/String; + .registers 4 + new-instance v0, Ljava/lang/StringBuilder; + invoke-direct {v0}, Ljava/lang/StringBuilder;->()V + const-string v1, "i:" + invoke-virtual {v0, v1}, Ljava/lang/StringBuilder;->append(Ljava/lang/String;)Ljava/lang/StringBuilder; + move-result-object v0 + invoke-virtual {v0, p1}, Ljava/lang/StringBuilder;->append(I)Ljava/lang/StringBuilder; + move-result-object v0 + invoke-virtual {v0}, Ljava/lang/StringBuilder;->toString()Ljava/lang/String; + move-result-object v0 + return-object v0 +.end method + +.method public test(I)Ljava/lang/String; + .registers 4 + new-instance v0, Ljava/lang/StringBuilder; + invoke-direct {v0}, Ljava/lang/StringBuilder;->()V + invoke-virtual {p0, p1}, Linline/TestOverlapSyntheticMethods;->a(I)I + move-result v1 + invoke-virtual {v0, v1}, Ljava/lang/StringBuilder;->append(I)Ljava/lang/StringBuilder; + move-result-object v0 + const-string v1, "|" + invoke-virtual {v0, v1}, Ljava/lang/StringBuilder;->append(Ljava/lang/String;)Ljava/lang/StringBuilder; + move-result-object v0 + invoke-virtual {p0, p1}, Linline/TestOverlapSyntheticMethods;->a(I)Ljava/lang/String; + move-result-object v1 + invoke-virtual {v0, v1}, Ljava/lang/StringBuilder;->append(Ljava/lang/String;)Ljava/lang/StringBuilder; + move-result-object v0 + invoke-virtual {v0}, Ljava/lang/StringBuilder;->toString()Ljava/lang/String; + move-result-object v0 + return-object v0 +.end method -- GitLab