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 8d84d3470..a6aa91b49 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 @@ -11,8 +11,10 @@ import jadx.core.dex.info.FieldInfo; import jadx.core.dex.info.MethodInfo; import jadx.core.dex.instructions.IndexInsnNode; import jadx.core.dex.instructions.InsnType; +import jadx.core.dex.instructions.InvokeNode; import jadx.core.dex.instructions.args.ArgType; import jadx.core.dex.instructions.args.InsnArg; +import jadx.core.dex.instructions.args.InsnWrapArg; import jadx.core.dex.instructions.args.RegisterArg; import jadx.core.dex.instructions.args.SSAVar; import jadx.core.dex.instructions.mods.ConstructorInsn; @@ -44,8 +46,8 @@ public class ClassModifier extends AbstractVisitor { return false; } removeSyntheticFields(cls); - removeSyntheticMethods(cls); - removeEmptyMethods(cls); + cls.getMethods().forEach(mth -> removeSyntheticMethods(cls, mth)); + cls.getMethods().forEach(ClassModifier::removeEmptyMethods); markAnonymousClass(cls); return false; @@ -122,24 +124,23 @@ public class ClassModifier extends AbstractVisitor { return true; } - private static void removeSyntheticMethods(ClassNode cls) { - for (MethodNode mth : cls.getMethods()) { - if (mth.isNoCode()) { - continue; - } - AccessInfo af = mth.getAccessFlags(); - // remove bridge methods - if (af.isBridge() && af.isSynthetic() && !isMethodUniq(cls, mth)) { - // TODO add more checks before method deletion - mth.add(AFlag.DONT_GENERATE); - } else { - // remove synthetic constructor for inner classes - if (af.isSynthetic() && af.isConstructor() && mth.getBasicBlocks().size() == 2) { - List args = mth.getArguments(false); - if (isRemovedClassInArgs(cls, args)) { - modifySyntheticMethod(cls, mth, args); - } - } + private static void removeSyntheticMethods(ClassNode cls, MethodNode mth) { + if (mth.isNoCode()) { + return; + } + AccessInfo af = mth.getAccessFlags(); + if (!af.isSynthetic()) { + return; + } + if (removeBridgeMethod(cls, mth)) { + mth.add(AFlag.DONT_GENERATE); + return; + } + // remove synthetic constructor for inner classes + if (af.isConstructor() && mth.getBasicBlocks().size() == 2) { + List args = mth.getArguments(false); + if (isRemovedClassInArgs(cls, args)) { + modifySyntheticMethod(cls, mth, args); } } } @@ -192,7 +193,40 @@ public class ClassModifier extends AbstractVisitor { } } - private static boolean isMethodUniq(ClassNode cls, MethodNode mth) { + private static boolean removeBridgeMethod(ClassNode cls, MethodNode mth) { + List allInsns = BlockUtils.collectAllInsns(mth.getBasicBlocks()); + if (allInsns.size() == 1) { + InsnNode wrappedInsn = allInsns.get(0); + if (wrappedInsn.getType() == InsnType.RETURN) { + InsnArg arg = wrappedInsn.getArg(0); + if (arg.isInsnWrap()) { + wrappedInsn = ((InsnWrapArg) arg).getWrapInsn(); + } + } + if (checkSyntheticWrapper(mth, wrappedInsn)) { + return true; + } + } + return !isMethodUnique(cls, mth); + } + + private static boolean checkSyntheticWrapper(MethodNode mth, InsnNode insn) { + InsnType insnType = insn.getType(); + if (insnType == InsnType.INVOKE) { + MethodInfo callMth = ((InvokeNode) insn).getCallMth(); + MethodNode wrappedMth = mth.root().deepResolveMethod(callMth); + if (wrappedMth != null) { + String alias = mth.getAlias(); + if (!wrappedMth.getAlias().equals(alias) && wrappedMth.isVirtual()) { + wrappedMth.getMethodInfo().setAlias(alias); + } + return true; + } + } + return false; + } + + private static boolean isMethodUnique(ClassNode cls, MethodNode mth) { MethodInfo mi = mth.getMethodInfo(); for (MethodNode otherMth : cls.getMethods()) { if (otherMth != mth) { @@ -207,19 +241,16 @@ public class ClassModifier extends AbstractVisitor { return true; } - private static void removeEmptyMethods(ClassNode cls) { - for (MethodNode mth : cls.getMethods()) { - AccessInfo af = mth.getAccessFlags(); - - // remove public empty constructors - if (af.isConstructor() - && (af.isPublic() || af.isStatic()) - && mth.getArguments(false).isEmpty() - && !mth.contains(AType.JADX_ERROR)) { - List bb = mth.getBasicBlocks(); - if (bb == null || bb.isEmpty() || BlockUtils.isAllBlocksEmpty(bb)) { - mth.add(AFlag.DONT_GENERATE); - } + private static void removeEmptyMethods(MethodNode mth) { + AccessInfo af = mth.getAccessFlags(); + // remove public empty constructors + if (af.isConstructor() + && (af.isPublic() || af.isStatic()) + && mth.getArguments(false).isEmpty() + && !mth.contains(AType.JADX_ERROR)) { + List bb = mth.getBasicBlocks(); + if (bb == null || bb.isEmpty() || BlockUtils.isAllBlocksEmpty(bb)) { + mth.add(AFlag.DONT_GENERATE); } } } diff --git a/jadx-core/src/main/java/jadx/core/utils/BlockUtils.java b/jadx-core/src/main/java/jadx/core/utils/BlockUtils.java index 5a7fc5a71..8d6edc031 100644 --- a/jadx-core/src/main/java/jadx/core/utils/BlockUtils.java +++ b/jadx-core/src/main/java/jadx/core/utils/BlockUtils.java @@ -550,4 +550,10 @@ public class BlockUtils { } return true; } + + public static List collectAllInsns(List blocks) { + List insns = new ArrayList<>(); + blocks.forEach(block -> insns.addAll(block.getInstructions())); + return insns; + } } diff --git a/jadx-core/src/test/java/jadx/tests/integration/inner/TestInnerClassSyntheticRename.java b/jadx-core/src/test/java/jadx/tests/integration/inner/TestInnerClassSyntheticRename.java new file mode 100644 index 000000000..6a2068a20 --- /dev/null +++ b/jadx-core/src/test/java/jadx/tests/integration/inner/TestInnerClassSyntheticRename.java @@ -0,0 +1,41 @@ +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/336 + */ +public class TestInnerClassSyntheticRename extends SmaliTest { + +// private class MyAsync extends AsyncTask> { +// @Override +// protected List doInBackground(Uri... uris) { +// Log.i("MyAsync", "doInBackground"); +// return null; +// } +// +// @Override +// protected void onPostExecute(List uris) { +// Log.i("MyAsync", "onPostExecute"); +// } +// } + + @Test + public void test() { + disableCompilation(); + ClassNode cls = getClassNodeFromSmali("inner/TestInnerClassSyntheticRename", "com.github.skylot.testasync.MyAsync"); + String code = cls.getCode().toString(); + + assertThat(code, containsOne("protected List doInBackground(Uri... uriArr) {")); + assertThat(code, containsOne("protected void onPostExecute(List list) {")); + assertThat(code, not(containsString("synthetic"))); + } +} diff --git a/jadx-core/src/test/smali/inner/TestInnerClassSyntheticRename.smali b/jadx-core/src/test/smali/inner/TestInnerClassSyntheticRename.smali new file mode 100644 index 000000000..42974288c --- /dev/null +++ b/jadx-core/src/test/smali/inner/TestInnerClassSyntheticRename.smali @@ -0,0 +1,93 @@ +.class Lcom/github/skylot/testasync/MyAsync; +.super Landroid/os/AsyncTask; + + +# annotations +.annotation system Ldalvik/annotation/Signature; + value = { + "Landroid/os/AsyncTask<", + "Landroid/net/Uri;", + "Landroid/net/Uri;", + "Ljava/util/List<", + "Landroid/net/Uri;", + ">;>;" + } +.end annotation + + +# direct methods +.method private constructor (Lcom/github/skylot/testasync/MainActivity;)V + .locals 0 + + invoke-direct {p0}, Landroid/os/AsyncTask;->()V + + return-void +.end method + + +# virtual methods +.method protected varargs a([Landroid/net/Uri;)Ljava/util/List; + .locals 1 + .annotation system Ldalvik/annotation/Signature; + value = { + "([", + "Landroid/net/Uri;", + ")", + "Ljava/util/List<", + "Landroid/net/Uri;", + ">;" + } + .end annotation + + const-string p1, "MyAsync" + + const-string v0, "doInBackground" + + invoke-static {p1, v0}, Landroid/util/Log;->i(Ljava/lang/String;Ljava/lang/String;)I + + const/4 p1, 0x0 + + return-object p1 +.end method + +.method protected a(Ljava/util/List;)V + .locals 1 + .annotation system Ldalvik/annotation/Signature; + value = { + "(", + "Ljava/util/List<", + "Landroid/net/Uri;", + ">;)V" + } + .end annotation + + const-string p1, "MyAsync" + + const-string v0, "onPostExecute" + + invoke-static {p1, v0}, Landroid/util/Log;->i(Ljava/lang/String;Ljava/lang/String;)I + + return-void +.end method + +.method protected synthetic doInBackground([Ljava/lang/Object;)Ljava/lang/Object; + .locals 0 + + check-cast p1, [Landroid/net/Uri; + + invoke-virtual {p0, p1}, Lcom/github/skylot/testasync/MyAsync;->a([Landroid/net/Uri;)Ljava/util/List; + + move-result-object p1 + + return-object p1 +.end method + +.method protected synthetic onPostExecute(Ljava/lang/Object;)V + .locals 0 + + check-cast p1, Ljava/util/List; + + invoke-virtual {p0, p1}, Lcom/github/skylot/testasync/MyAsync;->a(Ljava/util/List;)V + + return-void +.end method