From f3cd4e38d764de87e8848425c653b717cfd3d487 Mon Sep 17 00:00:00 2001 From: Skylot Date: Sun, 3 May 2020 18:16:59 +0100 Subject: [PATCH] fix: check enum constructor content before removing (#922) --- .../main/java/jadx/core/codegen/ClassGen.java | 20 ++++++- .../dex/attributes/nodes/EnumClassAttr.java | 8 +-- .../attributes/nodes/SkipMethodArgsAttr.java | 4 ++ .../jadx/core/dex/visitors/EnumVisitor.java | 38 +++++++++--- .../tests/integration/enums/TestEnums2a.java | 60 +++++++++++++++++++ .../tests/integration/enums/TestEnums6.java | 46 ++++++++++++++ .../tests/integration/enums/TestEnums7.java | 41 +++++++++++++ 7 files changed, 198 insertions(+), 19 deletions(-) create mode 100644 jadx-core/src/test/java/jadx/tests/integration/enums/TestEnums2a.java create mode 100644 jadx-core/src/test/java/jadx/tests/integration/enums/TestEnums6.java create mode 100644 jadx-core/src/test/java/jadx/tests/integration/enums/TestEnums7.java diff --git a/jadx-core/src/main/java/jadx/core/codegen/ClassGen.java b/jadx-core/src/main/java/jadx/core/codegen/ClassGen.java index 5044fa649..11d30f048 100644 --- a/jadx-core/src/main/java/jadx/core/codegen/ClassGen.java +++ b/jadx-core/src/main/java/jadx/core/codegen/ClassGen.java @@ -9,6 +9,8 @@ import java.util.List; import java.util.Objects; import java.util.Set; +import org.jetbrains.annotations.Nullable; + import com.android.dx.rop.code.AccessFlags; import com.google.common.collect.Streams; @@ -21,6 +23,7 @@ import jadx.core.dex.attributes.nodes.EnumClassAttr; import jadx.core.dex.attributes.nodes.EnumClassAttr.EnumField; import jadx.core.dex.attributes.nodes.JadxError; import jadx.core.dex.attributes.nodes.LineAttrNode; +import jadx.core.dex.attributes.nodes.SkipMethodArgsAttr; import jadx.core.dex.info.AccessInfo; import jadx.core.dex.info.ClassInfo; import jadx.core.dex.instructions.args.ArgType; @@ -420,12 +423,13 @@ public class ClassGen { EnumField f = it.next(); code.startLine(f.getField().getAlias()); ConstructorInsn constrInsn = f.getConstrInsn(); - if (constrInsn.getArgsCount() > f.getStartArg()) { + MethodNode callMth = cls.dex().resolveMethod(constrInsn.getCallMth()); + int skipCount = getEnumCtrSkipArgsCount(callMth); + if (constrInsn.getArgsCount() > skipCount) { if (igen == null) { igen = makeInsnGen(enumFields.getStaticMethod()); } - MethodNode callMth = cls.dex().resolveMethod(constrInsn.getCallMth()); - igen.generateMethodArguments(code, constrInsn, f.getStartArg(), callMth); + igen.generateMethodArguments(code, constrInsn, 0, callMth); } if (f.getCls() != null) { code.add(' '); @@ -446,6 +450,16 @@ public class ClassGen { } } + private int getEnumCtrSkipArgsCount(@Nullable MethodNode callMth) { + if (callMth != null) { + SkipMethodArgsAttr skipArgsAttr = callMth.get(AType.SKIP_MTH_ARGS); + if (skipArgsAttr != null) { + return skipArgsAttr.getSkipCount(); + } + } + return 0; + } + private InsnGen makeInsnGen(MethodNode mth) { MethodGen mthGen = new MethodGen(this, mth); return new InsnGen(mthGen, false); diff --git a/jadx-core/src/main/java/jadx/core/dex/attributes/nodes/EnumClassAttr.java b/jadx-core/src/main/java/jadx/core/dex/attributes/nodes/EnumClassAttr.java index dd9f24c50..122d73ca4 100644 --- a/jadx-core/src/main/java/jadx/core/dex/attributes/nodes/EnumClassAttr.java +++ b/jadx-core/src/main/java/jadx/core/dex/attributes/nodes/EnumClassAttr.java @@ -14,13 +14,11 @@ public class EnumClassAttr implements IAttribute { public static class EnumField { private final FieldNode field; private final ConstructorInsn constrInsn; - private final int startArg; private ClassNode cls; - public EnumField(FieldNode field, ConstructorInsn co, int startArg) { + public EnumField(FieldNode field, ConstructorInsn co) { this.field = field; this.constrInsn = co; - this.startArg = startArg; } public FieldNode getField() { @@ -31,10 +29,6 @@ public class EnumClassAttr implements IAttribute { return constrInsn; } - public int getStartArg() { - return startArg; - } - public ClassNode getCls() { return cls; } diff --git a/jadx-core/src/main/java/jadx/core/dex/attributes/nodes/SkipMethodArgsAttr.java b/jadx-core/src/main/java/jadx/core/dex/attributes/nodes/SkipMethodArgsAttr.java index 0580e4bd4..9709e6523 100644 --- a/jadx-core/src/main/java/jadx/core/dex/attributes/nodes/SkipMethodArgsAttr.java +++ b/jadx-core/src/main/java/jadx/core/dex/attributes/nodes/SkipMethodArgsAttr.java @@ -55,6 +55,10 @@ public class SkipMethodArgsAttr implements IAttribute { return skipArgs.get(argNum); } + public int getSkipCount() { + return skipArgs.cardinality(); + } + @Override public AType getType() { return AType.SKIP_MTH_ARGS; diff --git a/jadx-core/src/main/java/jadx/core/dex/visitors/EnumVisitor.java b/jadx-core/src/main/java/jadx/core/dex/visitors/EnumVisitor.java index a5f8b428c..60889f9a6 100644 --- a/jadx-core/src/main/java/jadx/core/dex/visitors/EnumVisitor.java +++ b/jadx-core/src/main/java/jadx/core/dex/visitors/EnumVisitor.java @@ -19,6 +19,7 @@ import jadx.core.dex.attributes.AFlag; import jadx.core.dex.attributes.AType; import jadx.core.dex.attributes.nodes.EnumClassAttr; import jadx.core.dex.attributes.nodes.EnumClassAttr.EnumField; +import jadx.core.dex.attributes.nodes.SkipMethodArgsAttr; import jadx.core.dex.info.AccessInfo; import jadx.core.dex.info.ClassInfo; import jadx.core.dex.info.FieldInfo; @@ -287,8 +288,13 @@ public class EnumVisitor extends AbstractVisitor { if (!clsInfo.equals(cls.getClassInfo()) && !constrCls.getAccessFlags().isEnum()) { return null; } - int startArg = co.getArgsCount() == 1 ? 1 : 2; - return new EnumField(enumFieldNode, co, startArg); + MethodInfo callMth = co.getCallMth(); + MethodNode mth = cls.dex().resolveMethod(callMth); + if (mth == null) { + return null; + } + markArgsForSkip(mth); + return new EnumField(enumFieldNode, co); } @Nullable @@ -306,10 +312,7 @@ public class EnumVisitor extends AbstractVisitor { } private void removeEnumMethods(ClassNode cls, ArgType clsType, FieldNode valuesField) { - String enumConstructor = "(Ljava/lang/String;I)V"; - String enumConstructorAlt = "(Ljava/lang/String;)V"; String valuesMethod = "values()" + TypeGen.signature(ArgType.array(clsType)); - FieldInfo valuesFieldInfo = valuesField.getFieldInfo(); // remove compiler generated methods @@ -319,12 +322,11 @@ public class EnumVisitor extends AbstractVisitor { continue; } String shortId = mi.getShortId(); - boolean isSynthetic = mth.getAccessFlags().isSynthetic(); - if (mi.isConstructor() && !isSynthetic) { - if (shortId.equals(enumConstructor) - || shortId.equals(enumConstructorAlt)) { + if (mi.isConstructor()) { + if (isDefaultConstructor(mth, shortId)) { mth.add(AFlag.DONT_GENERATE); } + markArgsForSkip(mth); } else if (shortId.equals(valuesMethod) || usesValuesField(mth, valuesFieldInfo) || simpleValueOfMth(mth, clsType)) { @@ -333,6 +335,24 @@ public class EnumVisitor extends AbstractVisitor { } } + private void markArgsForSkip(MethodNode mth) { + // skip first and second args + SkipMethodArgsAttr.skipArg(mth, 0); + if (mth.getMethodInfo().getArgsCount() > 1) { + SkipMethodArgsAttr.skipArg(mth, 1); + } + } + + private boolean isDefaultConstructor(MethodNode mth, String shortId) { + boolean defaultId = shortId.equals("(Ljava/lang/String;I)V") + || shortId.equals("(Ljava/lang/String;)V"); + if (defaultId) { + // check content + return mth.countInsns() == 0; + } + return false; + } + private boolean simpleValueOfMth(MethodNode mth, ArgType clsType) { InsnNode returnInsn = InsnUtils.searchSingleReturnInsn(mth, insn -> insn.getArgsCount() == 1); if (returnInsn == null) { diff --git a/jadx-core/src/test/java/jadx/tests/integration/enums/TestEnums2a.java b/jadx-core/src/test/java/jadx/tests/integration/enums/TestEnums2a.java new file mode 100644 index 000000000..7532850a8 --- /dev/null +++ b/jadx-core/src/test/java/jadx/tests/integration/enums/TestEnums2a.java @@ -0,0 +1,60 @@ +package jadx.tests.integration.enums; + +import org.junit.jupiter.api.Test; + +import jadx.tests.api.IntegrationTest; + +import static jadx.tests.api.utils.assertj.JadxAssertions.assertThat; +import static jadx.tests.integration.enums.TestEnums2a.TestCls.DoubleOperations.DIVIDE; +import static jadx.tests.integration.enums.TestEnums2a.TestCls.DoubleOperations.TIMES; + +public class TestEnums2a extends IntegrationTest { + + public static class TestCls { + + public interface IOps { + double apply(double x, double y); + } + + public enum DoubleOperations implements IOps { + TIMES("*") { + @Override + public double apply(double x, double y) { + return x * y; + } + }, + DIVIDE("/") { + @Override + public double apply(double x, double y) { + return x / y; + } + }; + + private final String op; + + DoubleOperations(String op) { + this.op = op; + } + + public String getOp() { + return op; + } + } + + public void check() { + assertThat(TIMES.getOp()).isEqualTo("*"); + assertThat(DIVIDE.getOp()).isEqualTo("/"); + + assertThat(TIMES.apply(2, 3)).isEqualTo(6); + assertThat(DIVIDE.apply(10, 5)).isEqualTo(2); + } + } + + @Test + public void test() { + assertThat(getClassNode(TestCls.class)) + .code() + .containsOne("TIMES(\"*\") {") + .containsOne("DIVIDE(\"/\")"); + } +} diff --git a/jadx-core/src/test/java/jadx/tests/integration/enums/TestEnums6.java b/jadx-core/src/test/java/jadx/tests/integration/enums/TestEnums6.java new file mode 100644 index 000000000..bc88c0437 --- /dev/null +++ b/jadx-core/src/test/java/jadx/tests/integration/enums/TestEnums6.java @@ -0,0 +1,46 @@ +package jadx.tests.integration.enums; + +import org.assertj.core.api.Assertions; +import org.junit.jupiter.api.Test; + +import jadx.tests.api.IntegrationTest; + +import static jadx.tests.api.utils.assertj.JadxAssertions.assertThat; + +public class TestEnums6 extends IntegrationTest { + + public static class TestCls { + public enum Numbers { + ZERO, + ONE(1); + + private final int n; + + Numbers() { + this(0); + } + + Numbers(int n) { + this.n = n; + } + + public int getN() { + return n; + } + } + + public void check() { + Assertions.assertThat(TestCls.Numbers.ZERO.getN()).isEqualTo(0); + Assertions.assertThat(TestCls.Numbers.ONE.getN()).isEqualTo(1); + } + } + + @Test + public void test() { + assertThat(getClassNode(TestCls.class)) + .code() + .containsOne("ZERO,") + .containsOne("Numbers() {") + .containsOne("ONE(1);"); + } +} diff --git a/jadx-core/src/test/java/jadx/tests/integration/enums/TestEnums7.java b/jadx-core/src/test/java/jadx/tests/integration/enums/TestEnums7.java new file mode 100644 index 000000000..8248e0c36 --- /dev/null +++ b/jadx-core/src/test/java/jadx/tests/integration/enums/TestEnums7.java @@ -0,0 +1,41 @@ +package jadx.tests.integration.enums; + +import org.junit.jupiter.api.Test; + +import jadx.tests.api.IntegrationTest; + +import static jadx.tests.api.utils.assertj.JadxAssertions.assertThat; + +public class TestEnums7 extends IntegrationTest { + + public static class TestCls { + public enum Numbers { + ZERO, + ONE; + + private final int n; + + Numbers() { + this.n = this.name().equals("ZERO") ? 0 : 1; + } + + public int getN() { + return n; + } + } + + public void check() { + assertThat(Numbers.ZERO.getN()).isEqualTo(0); + assertThat(Numbers.ONE.getN()).isEqualTo(1); + } + } + + @Test + public void test() { + assertThat(getClassNode(TestCls.class)) + .code() + .containsOne("ZERO,") + .containsOne("ONE;") + .containsOne("Numbers() {"); + } +}