From 4e3d85887ca905956469688abaea576f6082e580 Mon Sep 17 00:00:00 2001 From: Skylot Date: Mon, 4 May 2020 14:55:11 +0100 Subject: [PATCH] fix: correctly process extended enums if class is not inner (#924) --- .../jadx/core/dex/visitors/EnumVisitor.java | 51 ++++++++++--------- .../java/jadx/core/utils/InsnRemover.java | 4 +- .../java/jadx/tests/api/IntegrationTest.java | 6 ++- 3 files changed, 34 insertions(+), 27 deletions(-) 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 fd331d4b2..2a81bfa0c 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 @@ -45,7 +45,6 @@ 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.Utils; import jadx.core.utils.exceptions.JadxException; import static jadx.core.utils.InsnUtils.checkInsnType; @@ -170,18 +169,9 @@ public class EnumVisitor extends AbstractVisitor { && cls.root().getArgs().isRenameValid()) { fieldNode.getFieldInfo().setAlias(name); } - if (!co.getClassType().equals(cls.getClassInfo())) { - // enum contains additional methods - for (ClassNode innerCls : cls.getInnerClasses()) { - processEnumInnerCls(co, enumField, innerCls); - } - } fieldNode.add(AFlag.DONT_GENERATE); + processConstructorInsn(cls, enumField, classInitMth, staticBlock); } - - List constrInsns = Utils.collectionMap(attr.getFields(), EnumField::getConstrInsn); - InsnRemover.removeAllWithoutUnbind(staticBlock, constrInsns); - valuesField.add(AFlag.DONT_GENERATE); InsnRemover.removeAllAndUnbind(classInitMth, staticBlock, toRemove); if (classInitMth.countInsns() == 0) { @@ -191,6 +181,28 @@ public class EnumVisitor extends AbstractVisitor { return true; } + private void processConstructorInsn(ClassNode cls, EnumField enumField, MethodNode classInitMth, BlockNode staticBlock) { + ConstructorInsn co = enumField.getConstrInsn(); + ClassInfo enumClsInfo = co.getClassType(); + if (!enumClsInfo.equals(cls.getClassInfo())) { + ClassNode enumCls = cls.root().resolveClass(enumClsInfo); + if (enumCls != null) { + processEnumCls(enumField, enumCls); + cls.addInlinedClass(enumCls); + } + } + List regs = new ArrayList<>(); + co.getRegisterArgs(regs); + if (!regs.isEmpty()) { + cls.addWarnComment("Init of enum " + enumField.getField().getName() + " can be incorrect"); + } + MethodNode ctrMth = cls.dex().resolveMethod(co.getCallMth()); + if (ctrMth != null) { + markArgsForSkip(ctrMth); + } + InsnRemover.removeWithoutUnbind(classInitMth, staticBlock, co); + } + private BlockInsnPair getValuesInitInsn(MethodNode classInitMth, FieldNode valuesField) { FieldInfo searchField = valuesField.getFieldInfo(); for (BlockNode blockNode : classInitMth.getBasicBlocks()) { @@ -288,18 +300,10 @@ public class EnumVisitor extends AbstractVisitor { if (!clsInfo.equals(cls.getClassInfo()) && !constrCls.getAccessFlags().isEnum()) { return null; } - MethodInfo callMth = co.getCallMth(); - MethodNode mth = cls.dex().resolveMethod(callMth); - if (mth == null) { + MethodNode ctrMth = cls.dex().resolveMethod(co.getCallMth()); + if (ctrMth == null) { return null; } - List regs = new ArrayList<>(); - co.getRegisterArgs(regs); - if (!regs.isEmpty()) { - cls.addWarnComment("Init of enum " + enumFieldNode.getName() + " can be incorrect"); - } - - markArgsForSkip(mth); return new EnumField(enumFieldNode, co); } @@ -378,10 +382,7 @@ public class EnumVisitor extends AbstractVisitor { return InsnUtils.searchInsn(mth, InsnType.SGET, insnTest) != null; } - private static void processEnumInnerCls(ConstructorInsn co, EnumField field, ClassNode innerCls) { - if (!innerCls.getClassInfo().equals(co.getClassType())) { - return; - } + private static void processEnumCls(EnumField field, ClassNode innerCls) { // remove constructor, because it is anonymous class for (MethodNode innerMth : innerCls.getMethods()) { if (innerMth.getAccessFlags().isConstructor()) { diff --git a/jadx-core/src/main/java/jadx/core/utils/InsnRemover.java b/jadx-core/src/main/java/jadx/core/utils/InsnRemover.java index 623d4a1dc..d42bb5aa1 100644 --- a/jadx-core/src/main/java/jadx/core/utils/InsnRemover.java +++ b/jadx-core/src/main/java/jadx/core/utils/InsnRemover.java @@ -210,7 +210,9 @@ public class InsnRemover { return true; } } - mth.addWarnComment("Failed to remove instruction: " + insn + " from block: " + block); + if (!insn.contains(AFlag.WRAPPED)) { + mth.addWarnComment("Failed to remove instruction: " + insn + " from block: " + block); + } return false; } diff --git a/jadx-core/src/test/java/jadx/tests/api/IntegrationTest.java b/jadx-core/src/test/java/jadx/tests/api/IntegrationTest.java index ada2ea51c..004c8d581 100644 --- a/jadx-core/src/test/java/jadx/tests/api/IntegrationTest.java +++ b/jadx-core/src/test/java/jadx/tests/api/IntegrationTest.java @@ -38,6 +38,7 @@ import jadx.core.dex.nodes.ClassNode; import jadx.core.dex.nodes.MethodNode; import jadx.core.dex.nodes.RootNode; import jadx.core.utils.DebugChecks; +import jadx.core.utils.Utils; import jadx.core.utils.files.FileUtils; import jadx.core.xmlgen.ResourceStorage; import jadx.core.xmlgen.entry.ResourceEntry; @@ -262,7 +263,10 @@ public abstract class IntegrationTest extends TestUtils { protected void checkCode(ClassNode cls) { assertFalse(hasErrors(cls), "Inconsistent cls: " + cls); for (MethodNode mthNode : cls.getMethods()) { - assertFalse(hasErrors(mthNode), "Method with problems: " + mthNode); + if (hasErrors(mthNode)) { + fail("Method with problems: " + mthNode + + "\n " + Utils.listToString(mthNode.getAttributesStringsList(), "\n ")); + } } assertThat(cls.getCode().toString(), not(containsString("inconsistent"))); }