From 4cb9f23a7ddaea5509db27f4e5ca57849546bbb6 Mon Sep 17 00:00:00 2001 From: Skylot Date: Sun, 14 Apr 2019 19:02:16 +0300 Subject: [PATCH] fix: inline anonymous classes with not default constructor (#450) --- .../main/java/jadx/core/codegen/InsnGen.java | 12 ++-- .../java/jadx/core/dex/nodes/ClassNode.java | 29 ++++++--- .../java/jadx/core/dex/nodes/MethodNode.java | 11 ++-- .../jadx/core/dex/visitors/ClassModifier.java | 39 ++++++++--- .../jadx/core/dex/visitors/ModVisitor.java | 26 ++++---- .../java/jadx/core/utils/RegionUtils.java | 9 ++- .../inner/TestAnonymousClass14.java | 58 +++++++++++++++++ .../inner/TestAnonymousClass15.java | 48 ++++++++++++++ .../TestAnonymousClass14/OuterCls$1.smali | 28 ++++++++ .../OuterCls$TestCls.smali | 39 +++++++++++ .../inner/TestAnonymousClass14/OuterCls.smali | 65 +++++++++++++++++++ 11 files changed, 319 insertions(+), 45 deletions(-) create mode 100644 jadx-core/src/test/java/jadx/tests/integration/inner/TestAnonymousClass14.java create mode 100644 jadx-core/src/test/java/jadx/tests/integration/inner/TestAnonymousClass15.java create mode 100644 jadx-core/src/test/smali/inner/TestAnonymousClass14/OuterCls$1.smali create mode 100644 jadx-core/src/test/smali/inner/TestAnonymousClass14/OuterCls$TestCls.smali create mode 100644 jadx-core/src/test/smali/inner/TestAnonymousClass14/OuterCls.smali diff --git a/jadx-core/src/main/java/jadx/core/codegen/InsnGen.java b/jadx-core/src/main/java/jadx/core/codegen/InsnGen.java index ba7383ea6..0301cae82 100644 --- a/jadx-core/src/main/java/jadx/core/codegen/InsnGen.java +++ b/jadx-core/src/main/java/jadx/core/codegen/InsnGen.java @@ -582,14 +582,14 @@ public class InsnGen { } else { parent = cls.getSuperClass(); } - MethodNode defCtr = cls.getDefaultConstructor(); - if (defCtr != null) { - if (RegionUtils.notEmpty(defCtr.getRegion())) { - defCtr.add(AFlag.ANONYMOUS_CONSTRUCTOR); - } else { - defCtr.add(AFlag.DONT_GENERATE); + // hide empty anonymous constructors + for (MethodNode ctor : cls.getMethods()) { + if (ctor.contains(AFlag.ANONYMOUS_CONSTRUCTOR) + && RegionUtils.isEmpty(ctor.getRegion())) { + ctor.add(AFlag.DONT_GENERATE); } } + code.add("new "); if (parent == null) { code.add("Object"); diff --git a/jadx-core/src/main/java/jadx/core/dex/nodes/ClassNode.java b/jadx-core/src/main/java/jadx/core/dex/nodes/ClassNode.java index 1a0b11714..5c7837e61 100644 --- a/jadx-core/src/main/java/jadx/core/dex/nodes/ClassNode.java +++ b/jadx-core/src/main/java/jadx/core/dex/nodes/ClassNode.java @@ -34,6 +34,7 @@ import jadx.core.dex.nodes.parser.AnnotationsParser; import jadx.core.dex.nodes.parser.FieldInitAttr; import jadx.core.dex.nodes.parser.SignatureParser; import jadx.core.dex.nodes.parser.StaticValuesParser; +import jadx.core.utils.RegionUtils; import jadx.core.utils.exceptions.DecodeException; import jadx.core.utils.exceptions.JadxRuntimeException; @@ -124,7 +125,7 @@ public class ClassNode extends LineAttrNode implements ILoadable, ICodeNode { accFlagsValue = cls.getAccessFlags(); } this.accessFlags = new AccessInfo(accFlagsValue, AFType.CLASS); - markAnonymousClass(this); + markAnonymousClass(); buildCache(); } catch (Exception e) { throw new JadxRuntimeException("Error decode class: " + clsInfo, e); @@ -403,10 +404,25 @@ public class ClassNode extends LineAttrNode implements ILoadable, ICodeNode { && getSuperClass().getObject().equals(ArgType.ENUM.getObject()); } + public boolean markAnonymousClass() { + if (isAnonymous() || isLambdaCls()) { + add(AFlag.ANONYMOUS_CLASS); + add(AFlag.DONT_GENERATE); + + for (MethodNode mth : getMethods()) { + if (mth.isConstructor()) { + mth.add(AFlag.ANONYMOUS_CONSTRUCTOR); + } + } + return true; + } + return false; + } + public boolean isAnonymous() { return clsInfo.isInner() - && clsInfo.getAlias().getShortName().startsWith(Consts.ANONYMOUS_CLASS_PREFIX) - && getDefaultConstructor() != null; + && Character.isDigit(clsInfo.getShortName().charAt(0)) + && methods.stream().filter(MethodNode::isConstructor).count() == 1; } public boolean isLambdaCls() { @@ -425,13 +441,6 @@ public class ClassNode extends LineAttrNode implements ILoadable, ICodeNode { return c; } - private static void markAnonymousClass(ClassNode cls) { - if (cls.isAnonymous() || cls.isLambdaCls()) { - cls.add(AFlag.ANONYMOUS_CLASS); - cls.add(AFlag.DONT_GENERATE); - } - } - @Nullable public MethodNode getClassInitMth() { return searchMethodByShortId("()V"); diff --git a/jadx-core/src/main/java/jadx/core/dex/nodes/MethodNode.java b/jadx-core/src/main/java/jadx/core/dex/nodes/MethodNode.java index a66b37aa2..dcfb1a759 100644 --- a/jadx-core/src/main/java/jadx/core/dex/nodes/MethodNode.java +++ b/jadx-core/src/main/java/jadx/core/dex/nodes/MethodNode.java @@ -552,9 +552,12 @@ public class MethodNode extends LineAttrNode implements ILoadable, ICodeNode { return false; } + public boolean isConstructor() { + return accFlags.isConstructor() && mthInfo.isConstructor(); + } + public boolean isDefaultConstructor() { - boolean result = false; - if (accFlags.isConstructor() && mthInfo.isConstructor()) { + if (isConstructor()) { int defaultArgCount = 0; // workaround for non-static inner class constructor, that has synthetic argument if (parentClass.getClassInfo().isInner() @@ -565,9 +568,9 @@ public class MethodNode extends LineAttrNode implements ILoadable, ICodeNode { defaultArgCount = 1; } } - result = argsList == null || argsList.size() == defaultArgCount; + return argsList == null || argsList.size() == defaultArgCount; } - return result; + return false; } public boolean isVirtual() { 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 9cd2f0f33..74ec8533f 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 @@ -51,10 +51,11 @@ public class ClassModifier extends AbstractVisitor { cls.add(AFlag.DONT_GENERATE); return false; } - markAnonymousClass(cls); + cls.markAnonymousClass(); removeSyntheticFields(cls); cls.getMethods().forEach(ClassModifier::removeSyntheticMethods); cls.getMethods().forEach(ClassModifier::removeEmptyMethods); + cls.getMethods().forEach(ClassModifier::cleanInsnsInAnonymousConstructor); return false; } @@ -65,13 +66,6 @@ public class ClassModifier extends AbstractVisitor { && cls.getInnerClasses().isEmpty(); } - private void markAnonymousClass(ClassNode cls) { - if (cls.isAnonymous()) { - cls.add(AFlag.ANONYMOUS_CLASS); - cls.add(AFlag.DONT_GENERATE); - } - } - /** * Remove synthetic fields if type is outer class or class will be inlined (anonymous) */ @@ -324,12 +318,37 @@ public class ClassModifier extends AbstractVisitor { } } + /** + * Remove super call and put into removed fields from anonymous constructor + */ + private static void cleanInsnsInAnonymousConstructor(MethodNode mth) { + if (!mth.contains(AFlag.ANONYMOUS_CONSTRUCTOR)) { + return; + } + for (BlockNode block : mth.getBasicBlocks()) { + for (InsnNode insn : block.getInstructions()) { + InsnType type = insn.getType(); + if (type == InsnType.CONSTRUCTOR) { + ConstructorInsn ctorInsn = (ConstructorInsn) insn; + if (ctorInsn.isSuper()) { + ctorInsn.add(AFlag.DONT_GENERATE); + } + } else if (type == InsnType.IPUT) { + FieldInfo fldInfo = (FieldInfo) ((IndexInsnNode) insn).getIndex(); + FieldNode fieldNode = mth.dex().resolveField(fldInfo); + if (fieldNode != null && fieldNode.contains(AFlag.DONT_GENERATE)) { + insn.add(AFlag.DONT_GENERATE); + } + } + } + } + } + private static boolean isNonDefaultConstructorExists(MethodNode defCtor) { ClassNode parentClass = defCtor.getParentClass(); for (MethodNode mth : parentClass.getMethods()) { if (mth != defCtor - && mth.getAccessFlags().isConstructor() - && mth.getMethodInfo().isConstructor() + && mth.isConstructor() && !mth.isDefaultConstructor()) { return true; } diff --git a/jadx-core/src/main/java/jadx/core/dex/visitors/ModVisitor.java b/jadx-core/src/main/java/jadx/core/dex/visitors/ModVisitor.java index 09d785806..8d08a131a 100644 --- a/jadx-core/src/main/java/jadx/core/dex/visitors/ModVisitor.java +++ b/jadx-core/src/main/java/jadx/core/dex/visitors/ModVisitor.java @@ -11,7 +11,6 @@ import org.slf4j.LoggerFactory; import jadx.core.dex.attributes.AFlag; import jadx.core.dex.attributes.AType; import jadx.core.dex.attributes.nodes.FieldReplaceAttr; -import jadx.core.dex.info.ClassInfo; import jadx.core.dex.info.FieldInfo; import jadx.core.dex.info.MethodInfo; import jadx.core.dex.instructions.ArithNode; @@ -39,8 +38,8 @@ import jadx.core.dex.trycatch.ExcHandlerAttr; import jadx.core.dex.trycatch.ExceptionHandler; import jadx.core.dex.visitors.shrink.CodeShrinkVisitor; import jadx.core.utils.ErrorsCounter; -import jadx.core.utils.InsnUtils; import jadx.core.utils.InsnRemover; +import jadx.core.utils.InsnUtils; import jadx.core.utils.exceptions.JadxRuntimeException; import static jadx.core.utils.BlockUtils.replaceInsn; @@ -207,23 +206,22 @@ public class ModVisitor extends AbstractVisitor { if (callMthNode == null) { return; } - ClassNode classNode = callMthNode.getParentClass(); - ClassInfo classInfo = classNode.getClassInfo(); - ClassNode parentClass = mth.getParentClass(); - if (!classInfo.isInner() - || !Character.isDigit(classInfo.getShortName().charAt(0)) - || !parentClass.getInnerClasses().contains(classNode)) { - return; - } - // TODO: calculate this constructor and other constructor usage Map argsMap = getArgsToFieldsMapping(callMthNode, co); if (argsMap.isEmpty() && !callMthNode.getArguments(true).isEmpty()) { return; } - // all checks passed - classNode.add(AFlag.ANONYMOUS_CLASS); - callMthNode.add(AFlag.DONT_GENERATE); + ClassNode classNode = callMthNode.getParentClass(); + if (!classNode.contains(AFlag.ANONYMOUS_CLASS)) { + // check if class can be anonymous but not yet marked due to dependency issues + if (!classNode.markAnonymousClass()) { + return; + } + } + if (!mth.getParentClass().getInnerClasses().contains(classNode)) { + return; + } + for (Map.Entry entry : argsMap.entrySet()) { FieldNode field = entry.getValue(); if (field == null) { diff --git a/jadx-core/src/main/java/jadx/core/utils/RegionUtils.java b/jadx-core/src/main/java/jadx/core/utils/RegionUtils.java index 014fbc11b..cd7504cab 100644 --- a/jadx-core/src/main/java/jadx/core/utils/RegionUtils.java +++ b/jadx-core/src/main/java/jadx/core/utils/RegionUtils.java @@ -7,6 +7,7 @@ import java.util.Set; import org.jetbrains.annotations.Nullable; +import jadx.core.dex.attributes.AFlag; import jadx.core.dex.attributes.AType; import jadx.core.dex.attributes.AttrList; import jadx.core.dex.attributes.nodes.LoopInfo; @@ -195,7 +196,13 @@ public class RegionUtils { return false; } if (container instanceof IBlock) { - return !((IBlock) container).getInstructions().isEmpty(); + List insnList = ((IBlock) container).getInstructions(); + for (InsnNode insnNode : insnList) { + if (!insnNode.contains(AFlag.DONT_GENERATE)) { + return true; + } + } + return false; } else if (container instanceof IRegion) { IRegion region = (IRegion) container; for (IContainer block : region.getSubBlocks()) { diff --git a/jadx-core/src/test/java/jadx/tests/integration/inner/TestAnonymousClass14.java b/jadx-core/src/test/java/jadx/tests/integration/inner/TestAnonymousClass14.java new file mode 100644 index 000000000..0b998531a --- /dev/null +++ b/jadx-core/src/test/java/jadx/tests/integration/inner/TestAnonymousClass14.java @@ -0,0 +1,58 @@ +package jadx.tests.integration.inner; + +import org.junit.jupiter.api.Test; + +import jadx.core.dex.nodes.ClassNode; +import jadx.tests.api.SmaliTest; + +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.containsString; +import static org.hamcrest.Matchers.not; + +public class TestAnonymousClass14 extends SmaliTest { + + /* + public class OuterCls implements Runnable { + class AnonymousClass1 { + AnonymousClass1(Runnable runnable) { + } + + public void someMethod() { + } + } + + class TestCls { + private TestCls() { + ArrayList arrayList = new ArrayList(); + } + + synthetic TestCls(OuterCls outerCls, AnonymousClass1 anonymousClass1) { + this(); + } + } + + public void makeAnonymousCls() { + AnonymousClass1 anonymousClass1 = new AnonymousClass1(this); + } + + public void makeTestCls() { + TestCls testCls = new TestCls(this, null); + } + + public void run() { + } + + public void use(AnonymousClass1 anonymousClass1) { + } + } + */ + + @Test + public void test() { + ClassNode clsNode = getClassNodeFromSmaliFiles("inner", "TestAnonymousClass14", "OuterCls"); + String code = clsNode.getCode().toString(); + + assertThat(code, not(containsString("AnonymousClass1"))); + assertThat(code, not(containsString("synthetic"))); + } +} diff --git a/jadx-core/src/test/java/jadx/tests/integration/inner/TestAnonymousClass15.java b/jadx-core/src/test/java/jadx/tests/integration/inner/TestAnonymousClass15.java new file mode 100644 index 000000000..d49f19469 --- /dev/null +++ b/jadx-core/src/test/java/jadx/tests/integration/inner/TestAnonymousClass15.java @@ -0,0 +1,48 @@ +package jadx.tests.integration.inner; + +import org.junit.jupiter.api.Test; + +import jadx.NotYetImplemented; +import jadx.core.dex.nodes.ClassNode; +import jadx.tests.api.IntegrationTest; + +import static jadx.tests.api.utils.JadxMatchers.containsOne; +import static jadx.tests.api.utils.JadxMatchers.countString; +import static org.hamcrest.MatcherAssert.assertThat; + +public class TestAnonymousClass15 extends IntegrationTest { + + public static class TestCls { + + public Thread test(Runnable run) { + return new Thread(run) { + @Override + public void run() { + System.out.println("run"); + super.run(); + } + }; + } + + public Thread test2(Runnable run) { + return new Thread(run) { + { + setName("run"); + } + + @Override + public void run() { + } + }; + } + } + + @Test + public void test() { + ClassNode classNode = getClassNode(TestCls.class); + String code = classNode.getCode().toString(); + + assertThat(code, countString(2, "return new Thread(run) {")); + assertThat(code, containsOne("setName(\"run\");")); + } +} diff --git a/jadx-core/src/test/smali/inner/TestAnonymousClass14/OuterCls$1.smali b/jadx-core/src/test/smali/inner/TestAnonymousClass14/OuterCls$1.smali new file mode 100644 index 000000000..022bc8d81 --- /dev/null +++ b/jadx-core/src/test/smali/inner/TestAnonymousClass14/OuterCls$1.smali @@ -0,0 +1,28 @@ +.class Linner/OuterCls$1; +.super Ljava/lang/Thread; +.source "SourceFile" + +.annotation system Ldalvik/annotation/InnerClass; + accessFlags = 0x0 + name = null +.end annotation + +.field final synthetic this$0:Linner/OuterCls; + + +# direct methods +.method constructor (Linner/OuterCls;Ljava/lang/Runnable;)V + .locals 0 + + iput-object p1, p0, Linner/OuterCls$1;->this$0:Linner/OuterCls; + + return-void +.end method + + +# virtual methods +.method public someMethod()V + .locals 3 + + return-void +.end method diff --git a/jadx-core/src/test/smali/inner/TestAnonymousClass14/OuterCls$TestCls.smali b/jadx-core/src/test/smali/inner/TestAnonymousClass14/OuterCls$TestCls.smali new file mode 100644 index 000000000..5b88af9d7 --- /dev/null +++ b/jadx-core/src/test/smali/inner/TestAnonymousClass14/OuterCls$TestCls.smali @@ -0,0 +1,39 @@ +.class Linner/OuterCls$TestCls; +.super Ljava/lang/Object; +.source "SourceFile" + + +# annotations +.annotation system Ldalvik/annotation/EnclosingClass; + value = Linner/OuterCls; +.end annotation + +.annotation system Ldalvik/annotation/InnerClass; + accessFlags = 0x0 + name = "TestCls" +.end annotation + +.field final synthetic this$0:Linner/OuterCls; + + +# direct methods +.method private constructor (Linner/OuterCls;)V + .locals 0 + + iput-object p1, p0, Linner/OuterCls$TestCls;->this$0:Linner/OuterCls; + + new-instance p1, Ljava/util/ArrayList; + + invoke-direct {p1}, Ljava/util/ArrayList;->()V + + return-void +.end method + +.method synthetic constructor (Linner/OuterCls;Linner/OuterCls$1;)V + .locals 0 + + invoke-direct {p0, p1}, Linner/OuterCls$TestCls;->(Linner/OuterCls;)V + + return-void +.end method + diff --git a/jadx-core/src/test/smali/inner/TestAnonymousClass14/OuterCls.smali b/jadx-core/src/test/smali/inner/TestAnonymousClass14/OuterCls.smali new file mode 100644 index 000000000..ae8a7143a --- /dev/null +++ b/jadx-core/src/test/smali/inner/TestAnonymousClass14/OuterCls.smali @@ -0,0 +1,65 @@ +.class public Linner/OuterCls; +.super Ljava/lang/Object; +.source "SourceFile" + +# interfaces +.implements Ljava/lang/Runnable; + + +# annotations +.annotation system Ldalvik/annotation/MemberClasses; + value = { + Linner/OuterCls$TestCls; + } +.end annotation + + +# direct methods +.method static constructor ()V + .locals 0 + + return-void +.end method + +.method public constructor ()V + .locals 1 + + return-void +.end method + +.method public makeTestCls()V + .locals 2 + + new-instance v1, Linner/OuterCls$TestCls; + + const/4 v0, 0x0 + + invoke-direct {v1, p0, v0}, Linner/OuterCls$TestCls;->(Linner/OuterCls;Linner/OuterCls$1;)V + + return-void +.end method + +.method public makeAnonymousCls()V + .locals 2 + + new-instance v1, Linner/OuterCls$1; + + invoke-direct {v1, p0, p0}, Linner/OuterCls$1;->(Linner/OuterCls;Ljava/lang/Runnable;)V + + invoke-direct {p0, v1}, Linner/OuterCls;->use(Ljava/lang/Thread;)V + + return-void +.end method + +.method public run()V + .locals 2 + + return-void +.end method + +.method public use(Ljava/lang/Thread;)V + .locals 2 + + return-void +.end method +