From 57c28c61e035675763fd8b871f8586bf225262fb Mon Sep 17 00:00:00 2001 From: Skylot Date: Fri, 14 Feb 2020 18:05:48 +0000 Subject: [PATCH] fix: restore enum for several blocks in class init method --- jadx-core/src/main/java/jadx/core/Jadx.java | 2 +- .../java/jadx/core/dex/nodes/ClassNode.java | 1 + .../jadx/core/dex/visitors/EnumVisitor.java | 62 ++++++++++++------- .../core/dex/visitors/ExtractFieldInit.java | 8 +-- .../java/jadx/core/utils/BlockInsnPair.java | 46 ++++++++++++++ .../enums/TestEnumsWithAssert.java | 44 +++++++++++++ .../enums/TestEnumsWithStaticFields.java | 2 +- 7 files changed, 133 insertions(+), 32 deletions(-) create mode 100644 jadx-core/src/main/java/jadx/core/utils/BlockInsnPair.java create mode 100644 jadx-core/src/test/java/jadx/tests/integration/enums/TestEnumsWithAssert.java diff --git a/jadx-core/src/main/java/jadx/core/Jadx.java b/jadx-core/src/main/java/jadx/core/Jadx.java index 506c8a4ab..02fd8fe24 100644 --- a/jadx-core/src/main/java/jadx/core/Jadx.java +++ b/jadx-core/src/main/java/jadx/core/Jadx.java @@ -105,12 +105,12 @@ public class Jadx { passes.add(new SimplifyVisitor()); passes.add(new CheckRegions()); + passes.add(new EnumVisitor()); passes.add(new ExtractFieldInit()); passes.add(new FixAccessModifiers()); passes.add(new ProcessAnonymous()); passes.add(new ClassModifier()); passes.add(new MethodInlineVisitor()); - passes.add(new EnumVisitor()); passes.add(new LoopRegionVisitor()); passes.add(new ProcessVariables()); 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 84d61466b..9ba4da5a6 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 @@ -174,6 +174,7 @@ public class ClassNode extends LineAttrNode implements ILoadable, ICodeNode { private void loadStaticValues(ClassDef cls, List staticFields) throws DecodeException { for (FieldNode f : staticFields) { if (f.getAccessFlags().isFinal()) { + // incorrect initialization will be removed if assign found in constructor f.addAttr(FieldInitAttr.NULL_VALUE); } } 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 60d6a9fc1..dc653702f 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 @@ -37,6 +37,7 @@ import jadx.core.dex.nodes.FieldNode; import jadx.core.dex.nodes.InsnNode; import jadx.core.dex.nodes.MethodNode; import jadx.core.dex.visitors.shrink.CodeShrinkVisitor; +import jadx.core.utils.BlockInsnPair; import jadx.core.utils.InsnRemover; import jadx.core.utils.InsnUtils; import jadx.core.utils.exceptions.JadxException; @@ -44,7 +45,8 @@ import jadx.core.utils.exceptions.JadxException; @JadxVisitor( name = "EnumVisitor", desc = "Restore enum classes", - runAfter = { CodeShrinkVisitor.class, ModVisitor.class } + runAfter = { CodeShrinkVisitor.class, ModVisitor.class }, + runBefore = { ExtractFieldInit.class } ) public class EnumVisitor extends AbstractVisitor { @@ -72,8 +74,6 @@ public class EnumVisitor extends AbstractVisitor { if (classInitMth.getBasicBlocks().isEmpty()) { return false; } - BlockNode staticBlock = classInitMth.getBasicBlocks().get(0); - ArgType clsType = cls.getClassInfo().getType(); // search "$VALUES" field (holds all enum values) @@ -104,34 +104,32 @@ public class EnumVisitor extends AbstractVisitor { List toRemove = new ArrayList<>(); // search "$VALUES" array init and collect enum fields + BlockInsnPair valuesInitPair = getValuesInitInsn(classInitMth, valuesField); + if (valuesInitPair == null) { + return false; + } + BlockNode staticBlock = valuesInitPair.getBlock(); + InsnNode valuesInitInsn = valuesInitPair.getInsn(); + List enumFields = null; - for (InsnNode insn : staticBlock.getInstructions()) { - if (insn.getType() != InsnType.SPUT) { - continue; - } - FieldInfo f = (FieldInfo) ((IndexInsnNode) insn).getIndex(); - if (f.equals(valuesField.getFieldInfo())) { - InsnArg arrArg = insn.getArg(0); - if (arrArg.isInsnWrap()) { - InsnNode arrFillInsn = ((InsnWrapArg) arrArg).getWrapInsn(); - InsnType insnType = arrFillInsn.getType(); - if (insnType == InsnType.FILLED_NEW_ARRAY) { - enumFields = extractEnumFields(cls, arrFillInsn, staticBlock, toRemove); - } else if (insnType == InsnType.NEW_ARRAY) { - // empty enum - InsnArg arg = arrFillInsn.getArg(0); - if (arg.isLiteral() && ((LiteralArg) arg).getLiteral() == 0) { - enumFields = Collections.emptyList(); - } - } + InsnArg arrArg = valuesInitInsn.getArg(0); + if (arrArg.isInsnWrap()) { + InsnNode arrFillInsn = ((InsnWrapArg) arrArg).getWrapInsn(); + InsnType insnType = arrFillInsn.getType(); + if (insnType == InsnType.FILLED_NEW_ARRAY) { + enumFields = extractEnumFields(cls, arrFillInsn, staticBlock, toRemove); + } else if (insnType == InsnType.NEW_ARRAY) { + // empty enum + InsnArg arg = arrFillInsn.getArg(0); + if (arg.isLiteral() && ((LiteralArg) arg).getLiteral() == 0) { + enumFields = Collections.emptyList(); } - toRemove.add(insn); - break; } } if (enumFields == null) { return false; } + toRemove.add(valuesInitInsn); // all checks complete, perform transform EnumClassAttr attr = new EnumClassAttr(enumFields.size()); @@ -169,6 +167,22 @@ public class EnumVisitor extends AbstractVisitor { return true; } + private BlockInsnPair getValuesInitInsn(MethodNode classInitMth, FieldNode valuesField) { + FieldInfo searchField = valuesField.getFieldInfo(); + for (BlockNode blockNode : classInitMth.getBasicBlocks()) { + for (InsnNode insn : blockNode.getInstructions()) { + if (insn.getType() == InsnType.SPUT) { + IndexInsnNode indexInsnNode = (IndexInsnNode) insn; + FieldInfo f = (FieldInfo) indexInsnNode.getIndex(); + if (f.equals(searchField)) { + return new BlockInsnPair(blockNode, indexInsnNode); + } + } + } + } + return null; + } + private List extractEnumFields(ClassNode cls, InsnNode arrFillInsn, BlockNode staticBlock, List toRemove) { List enumFields = new ArrayList<>(); for (InsnArg arg : arrFillInsn.getArguments()) { diff --git a/jadx-core/src/main/java/jadx/core/dex/visitors/ExtractFieldInit.java b/jadx-core/src/main/java/jadx/core/dex/visitors/ExtractFieldInit.java index 1356cfb9e..d219f9979 100644 --- a/jadx-core/src/main/java/jadx/core/dex/visitors/ExtractFieldInit.java +++ b/jadx-core/src/main/java/jadx/core/dex/visitors/ExtractFieldInit.java @@ -35,9 +35,6 @@ public class ExtractFieldInit extends AbstractVisitor { @Override public boolean visit(ClassNode cls) throws JadxException { - if (cls.isEnum()) { - return false; - } for (ClassNode inner : cls.getInnerClasses()) { visit(inner); } @@ -66,12 +63,11 @@ public class ExtractFieldInit extends AbstractVisitor { } /** - * Remove final field in place initialization if it assign in class init method + * Remove a final field in place initialization if it an assign found in class init method */ private static void processStaticFieldAssign(ClassNode cls, IndexInsnNode insn) { FieldInfo field = (FieldInfo) insn.getIndex(); - String thisClass = cls.getClassInfo().getFullName(); - if (field.getDeclClass().getFullName().equals(thisClass)) { + if (field.getDeclClass().equals(cls.getClassInfo())) { FieldNode fn = cls.searchField(field); if (fn != null && fn.getAccessFlags().isFinal()) { fn.remove(AType.FIELD_INIT); diff --git a/jadx-core/src/main/java/jadx/core/utils/BlockInsnPair.java b/jadx-core/src/main/java/jadx/core/utils/BlockInsnPair.java new file mode 100644 index 000000000..44c56048d --- /dev/null +++ b/jadx-core/src/main/java/jadx/core/utils/BlockInsnPair.java @@ -0,0 +1,46 @@ +package jadx.core.utils; + +import java.util.Objects; + +import jadx.core.dex.nodes.BlockNode; +import jadx.core.dex.nodes.InsnNode; + +public class BlockInsnPair { + private final BlockNode block; + private final InsnNode insn; + + public BlockInsnPair(BlockNode block, InsnNode insn) { + this.block = block; + this.insn = insn; + } + + public BlockNode getBlock() { + return block; + } + + public InsnNode getInsn() { + return insn; + } + + @Override + public boolean equals(Object o) { + if (this == o) { + return true; + } + if (!(o instanceof BlockInsnPair)) { + return false; + } + BlockInsnPair that = (BlockInsnPair) o; + return block.equals(that.block) && insn.equals(that.insn); + } + + @Override + public int hashCode() { + return Objects.hash(block, insn); + } + + @Override + public String toString() { + return "BlockInsnPair{" + block + ": " + insn + '}'; + } +} diff --git a/jadx-core/src/test/java/jadx/tests/integration/enums/TestEnumsWithAssert.java b/jadx-core/src/test/java/jadx/tests/integration/enums/TestEnumsWithAssert.java new file mode 100644 index 000000000..5051efa50 --- /dev/null +++ b/jadx-core/src/test/java/jadx/tests/integration/enums/TestEnumsWithAssert.java @@ -0,0 +1,44 @@ +package jadx.tests.integration.enums; + +import org.junit.jupiter.api.Test; + +import jadx.NotYetImplemented; +import jadx.tests.api.IntegrationTest; + +import static jadx.tests.api.utils.assertj.JadxAssertions.assertThat; + +public class TestEnumsWithAssert extends IntegrationTest { + + public static class TestCls { + public enum Numbers { + ONE(1), TWO(2), THREE(3); + + private final int num; + + Numbers(int n) { + this.num = n; + } + + public int getNum() { + assert num > 0; + return num; + } + } + } + + @Test + public void test() { + assertThat(getClassNode(TestCls.class)).code() + .containsOne("ONE(1)") + .doesNotContain("Failed to restore enum class"); + } + + @NotYetImplemented("handle java assert") + @Test + public void testNYI() { + assertThat(getClassNode(TestCls.class)).code() + .containsOne("assert num > 0;") + .doesNotContain("$assertionsDisabled") + .doesNotContain("throw new AssertionError()"); + } +} diff --git a/jadx-core/src/test/java/jadx/tests/integration/enums/TestEnumsWithStaticFields.java b/jadx-core/src/test/java/jadx/tests/integration/enums/TestEnumsWithStaticFields.java index f4c669bbb..c7c15d1a5 100644 --- a/jadx-core/src/test/java/jadx/tests/integration/enums/TestEnumsWithStaticFields.java +++ b/jadx-core/src/test/java/jadx/tests/integration/enums/TestEnumsWithStaticFields.java @@ -14,7 +14,7 @@ public class TestEnumsWithStaticFields extends SmaliTest { assertThat(getClassNodeFromSmali()) .code() .containsOnlyOnce("INSTANCE;") - .containsOnlyOnce("private static c sB;") + .containsOnlyOnce("private static c sB") .doesNotContain(" sA") .doesNotContain(" sC") .doesNotContain("private TestEnumsWithStaticFields(String str) {");