From db1b027da2d5559cb3917b0f2546cfa5d26e8162 Mon Sep 17 00:00:00 2001 From: Skylot Date: Sat, 16 Feb 2019 12:42:29 +0300 Subject: [PATCH] fix: improve bridge methods renaming (#397) --- .../java/jadx/core/codegen/MethodGen.java | 8 +- .../jadx/core/dex/visitors/ClassModifier.java | 16 +++- .../core/dex/visitors/FixAccessModifiers.java | 21 +++-- .../integration/inner/TestInner2Samples.java | 77 +++++++++++++++++++ .../inner/TestSyntheticMthRename.java | 44 +++++++++++ .../TestSyntheticMthRename/TestCls$A.smali | 66 ++++++++++++++++ .../TestSyntheticMthRename/TestCls$I.smali | 35 +++++++++ .../TestSyntheticMthRename/TestCls.smali | 24 ++++++ 8 files changed, 278 insertions(+), 13 deletions(-) create mode 100644 jadx-core/src/test/java/jadx/tests/integration/inner/TestInner2Samples.java create mode 100644 jadx-core/src/test/java/jadx/tests/integration/inner/TestSyntheticMthRename.java create mode 100644 jadx-core/src/test/smali/inner/TestSyntheticMthRename/TestCls$A.smali create mode 100644 jadx-core/src/test/smali/inner/TestSyntheticMthRename/TestCls$I.smali create mode 100644 jadx-core/src/test/smali/inner/TestSyntheticMthRename/TestCls.smali diff --git a/jadx-core/src/main/java/jadx/core/codegen/MethodGen.java b/jadx-core/src/main/java/jadx/core/codegen/MethodGen.java index 2f343a652..efcbf2f62 100644 --- a/jadx-core/src/main/java/jadx/core/codegen/MethodGen.java +++ b/jadx-core/src/main/java/jadx/core/codegen/MethodGen.java @@ -4,15 +4,15 @@ import java.util.Iterator; import java.util.List; import com.android.dx.rop.code.AccessFlags; -import jadx.core.dex.info.ClassInfo; -import jadx.core.utils.Utils; import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import jadx.core.Consts; import jadx.core.dex.attributes.AFlag; import jadx.core.dex.attributes.AType; import jadx.core.dex.attributes.annotations.MethodParameters; import jadx.core.dex.info.AccessInfo; +import jadx.core.dex.info.ClassInfo; import jadx.core.dex.instructions.InsnType; import jadx.core.dex.instructions.args.ArgType; import jadx.core.dex.instructions.args.RegisterArg; @@ -24,6 +24,7 @@ import jadx.core.dex.visitors.DepthTraversal; import jadx.core.dex.visitors.FallbackModeVisitor; import jadx.core.utils.ErrorsCounter; import jadx.core.utils.InsnUtils; +import jadx.core.utils.Utils; import jadx.core.utils.exceptions.CodegenException; import jadx.core.utils.exceptions.DecodeException; @@ -85,6 +86,9 @@ public class MethodGen { } code.startLineWithNum(mth.getSourceLine()); code.add(ai.makeString()); + if (Consts.DEBUG) { + code.add(mth.isVirtual() ? "/* virtual */ " : "/* direct */ "); + } if (classGen.addGenericMap(code, mth.getGenericMap())) { code.add(' '); 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 5e01741f2..c7d148c7e 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 @@ -3,6 +3,8 @@ package jadx.core.dex.visitors; import java.util.List; import java.util.Objects; +import com.android.dx.rop.code.AccessFlags; + import jadx.core.dex.attributes.AFlag; import jadx.core.dex.attributes.AType; import jadx.core.dex.attributes.nodes.FieldReplaceAttr; @@ -31,7 +33,10 @@ import jadx.core.utils.exceptions.JadxException; @JadxVisitor( name = "ClassModifier", desc = "Remove synthetic classes, methods and fields", - runAfter = ModVisitor.class + runAfter = { + ModVisitor.class, + FixAccessModifiers.class + } ) public class ClassModifier extends AbstractVisitor { @@ -218,6 +223,10 @@ public class ClassModifier extends AbstractVisitor { MethodInfo callMth = ((InvokeNode) insn).getCallMth(); MethodNode wrappedMth = mth.root().deepResolveMethod(callMth); if (wrappedMth != null) { + AccessInfo wrappedAccFlags = wrappedMth.getAccessFlags(); + if (wrappedAccFlags.isStatic()) { + return false; + } if (callMth.getArgsCount() != mth.getMethodInfo().getArgsCount()) { return false; } @@ -235,8 +244,9 @@ public class ClassModifier extends AbstractVisitor { if (Objects.equals(wrappedMth.getAlias(), alias)) { return true; } - if (!wrappedMth.isVirtual()) { - return false; + if (!wrappedAccFlags.isPublic()) { + // must be public + FixAccessModifiers.changeVisibility(wrappedMth, AccessFlags.ACC_PUBLIC); } wrappedMth.getMethodInfo().setAlias(alias); return true; diff --git a/jadx-core/src/main/java/jadx/core/dex/visitors/FixAccessModifiers.java b/jadx-core/src/main/java/jadx/core/dex/visitors/FixAccessModifiers.java index d62143945..62a1808bb 100644 --- a/jadx-core/src/main/java/jadx/core/dex/visitors/FixAccessModifiers.java +++ b/jadx-core/src/main/java/jadx/core/dex/visitors/FixAccessModifiers.java @@ -26,22 +26,27 @@ public class FixAccessModifiers extends AbstractVisitor { if (respectAccessModifiers) { return; } - AccessInfo accessFlags = mth.getAccessFlags(); - int newVisFlag = fixVisibility(mth, accessFlags); + int newVisFlag = fixVisibility(mth); if (newVisFlag != 0) { - AccessInfo newAccFlags = accessFlags.changeVisibility(newVisFlag); - if (newAccFlags != accessFlags) { - mth.setAccFlags(newAccFlags); - mth.addAttr(AType.COMMENTS, "Access modifiers changed, original: " + accessFlags.rawString()); - } + changeVisibility(mth, newVisFlag); } } - private int fixVisibility(MethodNode mth, AccessInfo accessFlags) { + public static void changeVisibility(MethodNode mth, int newVisFlag) { + AccessInfo accessFlags = mth.getAccessFlags(); + AccessInfo newAccFlags = accessFlags.changeVisibility(newVisFlag); + if (newAccFlags != accessFlags) { + mth.setAccFlags(newAccFlags); + mth.addAttr(AType.COMMENTS, "access modifiers changed from: " + accessFlags.rawString()); + } + } + + private static int fixVisibility(MethodNode mth) { if (mth.isVirtual()) { // make virtual methods public return AccessFlags.ACC_PUBLIC; } else { + AccessInfo accessFlags = mth.getAccessFlags(); if (accessFlags.isAbstract()) { // make abstract methods public return AccessFlags.ACC_PUBLIC; diff --git a/jadx-core/src/test/java/jadx/tests/integration/inner/TestInner2Samples.java b/jadx-core/src/test/java/jadx/tests/integration/inner/TestInner2Samples.java new file mode 100644 index 000000000..30b467143 --- /dev/null +++ b/jadx-core/src/test/java/jadx/tests/integration/inner/TestInner2Samples.java @@ -0,0 +1,77 @@ +package jadx.tests.integration.inner; + +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.hamcrest.Matchers.containsString; +import static org.hamcrest.Matchers.not; +import static org.junit.Assert.assertThat; + +public class TestInner2Samples extends IntegrationTest { + + public static class TestInner2 { + + private String a; + + public class A { + public A() { + a = "a"; + } + + public String a() { + return a; + } + } + + private static String b; + + public static class B { + public B() { + b = "b"; + } + + public String b() { + return b; + } + } + + private String c; + + private void setC(String c) { + this.c = c; + } + + public class C { + public String c() { + setC("c"); + return c; + } + } + + private static String d; + + private static void setD(String s) { + d = s; + } + + public static class D { + public String d() { + setD("d"); + return d; + } + } + } + + @Test + public void test() { + ClassNode cls = getClassNode(TestInner2.class); + String code = cls.getCode().toString(); + + assertThat(code, containsOne("setD(\"d\");")); + assertThat(code, not(containsString("synthetic"))); + assertThat(code, not(containsString("access$"))); + } +} diff --git a/jadx-core/src/test/java/jadx/tests/integration/inner/TestSyntheticMthRename.java b/jadx-core/src/test/java/jadx/tests/integration/inner/TestSyntheticMthRename.java new file mode 100644 index 000000000..2ca61e986 --- /dev/null +++ b/jadx-core/src/test/java/jadx/tests/integration/inner/TestSyntheticMthRename.java @@ -0,0 +1,44 @@ +package jadx.tests.integration.inner; + +import org.junit.Test; + +import jadx.core.dex.nodes.ClassNode; +import jadx.tests.api.SmaliTest; + +import static jadx.tests.api.utils.JadxMatchers.containsOne; +import static org.hamcrest.Matchers.containsString; +import static org.hamcrest.Matchers.not; +import static org.junit.Assert.assertThat; + +/** + * Issue: https://github.com/skylot/jadx/issues/397 + */ +public class TestSyntheticMthRename extends SmaliTest { + +// public class TestCls { +// public interface I { +// R call(P... p); +// } +// +// public static final class A implements I { +// public /* synthetic */ /* virtual */ Object call(Object[] objArr) { +// return renamedCall((Runnable[]) objArr); +// } +// +// private /* varargs */ /* direct */ String renamedCall(Runnable... p) { +// return "str"; +// } +// } +// } + + @Test + public void test() { + ClassNode cls = getClassNodeFromSmaliFiles("inner", "TestSyntheticMthRename", "TestCls", + "TestCls", "TestCls$I", "TestCls$A" + ); + String code = cls.getCode().toString(); + + assertThat(code, containsOne("public String call(Runnable... p) {")); + assertThat(code, not(containsString("synthetic"))); + } +} diff --git a/jadx-core/src/test/smali/inner/TestSyntheticMthRename/TestCls$A.smali b/jadx-core/src/test/smali/inner/TestSyntheticMthRename/TestCls$A.smali new file mode 100644 index 000000000..9a09c0df2 --- /dev/null +++ b/jadx-core/src/test/smali/inner/TestSyntheticMthRename/TestCls$A.smali @@ -0,0 +1,66 @@ +.class public final Linner/TestCls$A; +.super Ljava/lang/Object; +.source "TestCls.java" + +# interfaces +.implements Linner/TestCls$I; + + +# annotations +.annotation system Ldalvik/annotation/EnclosingClass; + value = Linner/TestCls; +.end annotation + +.annotation system Ldalvik/annotation/InnerClass; + accessFlags = 0x19 + name = "A" +.end annotation + +.annotation system Ldalvik/annotation/Signature; + value = { + "Ljava/lang/Object;", + "Linner/TestCls$I", + "<", + "Ljava/lang/String;", + "Ljava/lang/Runnable;", + ">;" + } +.end annotation + + +# direct methods +.method public constructor ()V + .registers 1 + + .prologue + .line 9 + invoke-direct {p0}, Ljava/lang/Object;->()V + + return-void +.end method + +.method private varargs renamedCall([Ljava/lang/Runnable;)Ljava/lang/String; + .registers 3 + .param p1, "p" # [Ljava/lang/Runnable; + + .prologue + .line 12 + const-string v0, "str" + + return-object v0 +.end method + +# virtual methods +.method public synthetic call([Ljava/lang/Object;)Ljava/lang/Object; + .registers 3 + + .prologue + .line 9 + check-cast p1, [Ljava/lang/Runnable; + + invoke-virtual {p0, p1}, Linner/TestCls$A;->renamedCall([Ljava/lang/Runnable;)Ljava/lang/String; + + move-result-object v0 + + return-object v0 +.end method diff --git a/jadx-core/src/test/smali/inner/TestSyntheticMthRename/TestCls$I.smali b/jadx-core/src/test/smali/inner/TestSyntheticMthRename/TestCls$I.smali new file mode 100644 index 000000000..9899b9a96 --- /dev/null +++ b/jadx-core/src/test/smali/inner/TestSyntheticMthRename/TestCls$I.smali @@ -0,0 +1,35 @@ +.class public interface abstract Linner/TestCls$I; +.super Ljava/lang/Object; +.source "TestCls.java" + + +# annotations +.annotation system Ldalvik/annotation/EnclosingClass; + value = Linner/TestCls; +.end annotation + +.annotation system Ldalvik/annotation/InnerClass; + accessFlags = 0x609 + name = "I" +.end annotation + +.annotation system Ldalvik/annotation/Signature; + value = { + "", + "Ljava/lang/Object;" + } +.end annotation + + +# virtual methods +.method public varargs abstract call([Ljava/lang/Object;)Ljava/lang/Object; + .annotation system Ldalvik/annotation/Signature; + value = { + "([TP;)TR;" + } + .end annotation +.end method diff --git a/jadx-core/src/test/smali/inner/TestSyntheticMthRename/TestCls.smali b/jadx-core/src/test/smali/inner/TestSyntheticMthRename/TestCls.smali new file mode 100644 index 000000000..fa403ef0c --- /dev/null +++ b/jadx-core/src/test/smali/inner/TestSyntheticMthRename/TestCls.smali @@ -0,0 +1,24 @@ +.class public Linner/TestCls; +.super Ljava/lang/Object; +.source "TestCls.java" + + +# annotations +.annotation system Ldalvik/annotation/MemberClasses; + value = { + Linner/TestCls$A;, + Linner/TestCls$I; + } +.end annotation + + +# direct methods +.method public constructor ()V + .registers 1 + + .prologue + .line 3 + invoke-direct {p0}, Ljava/lang/Object;->()V + + return-void +.end method