From 683c2dfbebbf0811bcf6673470f8f1adec2c004e Mon Sep 17 00:00:00 2001 From: Skylot Date: Fri, 7 Oct 2022 15:23:25 +0100 Subject: [PATCH] fix: improve ternary inline, resolve more enum cases (#1686) --- .../main/java/jadx/core/codegen/ClassGen.java | 2 +- .../java/jadx/core/dex/attributes/AFlag.java | 1 + .../dex/regions/conditions/IfCondition.java | 2 +- .../core/dex/visitors/ConstInlineVisitor.java | 66 +++---- .../jadx/core/dex/visitors/EnumVisitor.java | 162 +++++++++++------- .../regions/DepthRegionTraversal.java | 4 + .../core/dex/visitors/regions/TernaryMod.java | 71 +++++--- .../java/jadx/core/utils/InsnRemover.java | 13 ++ .../java/jadx/core/utils/RegionUtils.java | 12 ++ .../conditions/TestSwitchTryBreak.java | 41 ----- .../conditions/TestSwitchTryBreak2.java | 41 ----- .../enums/TestEnumsWithTernary.java | 39 +++++ 12 files changed, 253 insertions(+), 201 deletions(-) delete mode 100644 jadx-core/src/test/java/jadx/tests/integration/conditions/TestSwitchTryBreak.java delete mode 100644 jadx-core/src/test/java/jadx/tests/integration/conditions/TestSwitchTryBreak2.java create mode 100644 jadx-core/src/test/java/jadx/tests/integration/enums/TestEnumsWithTernary.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 d5de2750..b79be4a7 100644 --- a/jadx-core/src/main/java/jadx/core/codegen/ClassGen.java +++ b/jadx-core/src/main/java/jadx/core/codegen/ClassGen.java @@ -171,7 +171,7 @@ public class ClassGen { ArgType sup = cls.getSuperClass(); if (sup != null && !sup.equals(ArgType.OBJECT) - && !cls.isEnum()) { + && !cls.contains(AFlag.REMOVE_SUPER_CLASS)) { clsCode.add("extends "); useClass(clsCode, sup); clsCode.add(' '); diff --git a/jadx-core/src/main/java/jadx/core/dex/attributes/AFlag.java b/jadx-core/src/main/java/jadx/core/dex/attributes/AFlag.java index c43f8945..113888f0 100644 --- a/jadx-core/src/main/java/jadx/core/dex/attributes/AFlag.java +++ b/jadx-core/src/main/java/jadx/core/dex/attributes/AFlag.java @@ -21,6 +21,7 @@ public enum AFlag { DONT_GENERATE, // process as usual, but don't output to generated code COMMENT_OUT, // process as usual, but comment insn in generated code REMOVE, // can be completely removed + REMOVE_SUPER_CLASS, // don't add super class HIDDEN, // instruction used inside other instruction but not listed in args diff --git a/jadx-core/src/main/java/jadx/core/dex/regions/conditions/IfCondition.java b/jadx-core/src/main/java/jadx/core/dex/regions/conditions/IfCondition.java index 9f1d96be..118ad322 100644 --- a/jadx-core/src/main/java/jadx/core/dex/regions/conditions/IfCondition.java +++ b/jadx-core/src/main/java/jadx/core/dex/regions/conditions/IfCondition.java @@ -156,7 +156,7 @@ public final class IfCondition extends AttrNode { return i; } if (c.getOp() == IfOp.EQ && c.getB().isFalse()) { - cond = not(new IfCondition(c.invert())); + cond = new IfCondition(Mode.NOT, Collections.singletonList(new IfCondition(c.invert()))); } else { c.normalize(); } diff --git a/jadx-core/src/main/java/jadx/core/dex/visitors/ConstInlineVisitor.java b/jadx-core/src/main/java/jadx/core/dex/visitors/ConstInlineVisitor.java index 49032616..ab1d3a57 100644 --- a/jadx-core/src/main/java/jadx/core/dex/visitors/ConstInlineVisitor.java +++ b/jadx-core/src/main/java/jadx/core/dex/visitors/ConstInlineVisitor.java @@ -62,45 +62,47 @@ public class ConstInlineVisitor extends AbstractVisitor { || insn.getResult() == null) { return; } - SSAVar sVar = insn.getResult().getSVar(); InsnArg constArg; Runnable onSuccess = null; - - InsnType insnType = insn.getType(); - if (insnType == InsnType.CONST || insnType == InsnType.MOVE) { - constArg = insn.getArg(0); - if (!constArg.isLiteral()) { - return; - } - long lit = ((LiteralArg) constArg).getLiteral(); - if (lit == 0 && forbidNullInlines(sVar)) { - // all usages forbids inlining - return; + switch (insn.getType()) { + case CONST: + case MOVE: { + constArg = insn.getArg(0); + if (!constArg.isLiteral()) { + return; + } + long lit = ((LiteralArg) constArg).getLiteral(); + if (lit == 0 && forbidNullInlines(sVar)) { + // all usages forbids inlining + return; + } + break; } - } else if (insnType == InsnType.CONST_STR) { - if (sVar.isUsedInPhi()) { - return; + case CONST_STR: { + String s = ((ConstStringNode) insn).getString(); + FieldNode f = mth.getParentClass().getConstField(s); + if (f == null) { + InsnNode copy = insn.copyWithoutResult(); + constArg = InsnArg.wrapArg(copy); + } else { + InsnNode constGet = new IndexInsnNode(InsnType.SGET, f.getFieldInfo(), 0); + constArg = InsnArg.wrapArg(constGet); + constArg.setType(ArgType.STRING); + onSuccess = () -> f.addUseIn(mth); + } + break; } - String s = ((ConstStringNode) insn).getString(); - FieldNode f = mth.getParentClass().getConstField(s); - if (f == null) { - InsnNode copy = insn.copyWithoutResult(); - constArg = InsnArg.wrapArg(copy); - } else { - InsnNode constGet = new IndexInsnNode(InsnType.SGET, f.getFieldInfo(), 0); - constArg = InsnArg.wrapArg(constGet); - constArg.setType(ArgType.STRING); - onSuccess = () -> f.addUseIn(mth); + case CONST_CLASS: { + if (sVar.isUsedInPhi()) { + return; + } + constArg = InsnArg.wrapArg(insn.copyWithoutResult()); + constArg.setType(ArgType.CLASS); + break; } - } else if (insnType == InsnType.CONST_CLASS) { - if (sVar.isUsedInPhi()) { + default: return; - } - constArg = InsnArg.wrapArg(insn.copyWithoutResult()); - constArg.setType(ArgType.CLASS); - } else { - return; } // all check passed, run replace 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 5285b1d0..1fe9c4fd 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 @@ -40,13 +40,18 @@ import jadx.core.dex.nodes.FieldNode; import jadx.core.dex.nodes.InsnNode; import jadx.core.dex.nodes.MethodNode; import jadx.core.dex.nodes.RootNode; +import jadx.core.dex.regions.Region; +import jadx.core.dex.visitors.regions.CheckRegions; +import jadx.core.dex.visitors.regions.IfRegionVisitor; import jadx.core.dex.visitors.shrink.CodeShrinkVisitor; import jadx.core.utils.BlockInsnPair; import jadx.core.utils.BlockUtils; import jadx.core.utils.InsnRemover; import jadx.core.utils.InsnUtils; +import jadx.core.utils.ListUtils; import jadx.core.utils.Utils; import jadx.core.utils.exceptions.JadxException; +import jadx.core.utils.exceptions.JadxRuntimeException; import static jadx.core.utils.InsnUtils.checkInsnType; import static jadx.core.utils.InsnUtils.getSingleArg; @@ -55,8 +60,16 @@ import static jadx.core.utils.InsnUtils.getWrappedInsn; @JadxVisitor( name = "EnumVisitor", desc = "Restore enum classes", - runAfter = { CodeShrinkVisitor.class, ModVisitor.class, ReSugarCode.class }, - runBefore = { ExtractFieldInit.class } + runAfter = { + CodeShrinkVisitor.class, // all possible instructions already inlined + ModVisitor.class, + ReSugarCode.class, + IfRegionVisitor.class, // ternary operator inlined + CheckRegions.class // regions processing finished + }, + runBefore = { + ExtractFieldInit.class + } ) public class EnumVisitor extends AbstractVisitor { @@ -92,7 +105,7 @@ public class EnumVisitor extends AbstractVisitor { AccessInfo accessFlags = cls.getAccessFlags(); if (accessFlags.isEnum()) { cls.setAccessFlags(accessFlags.remove(AccessFlags.ENUM)); - cls.addWarnComment("Failed to restore enum class, 'enum' modifier removed"); + cls.addWarnComment("Failed to restore enum class, 'enum' modifier and super class removed"); } } return true; @@ -102,14 +115,24 @@ public class EnumVisitor extends AbstractVisitor { if (!cls.isEnum()) { return false; } + ArgType superType = cls.getSuperClass(); + if (superType != null && superType.getObject().equals(ArgType.ENUM.getObject())) { + cls.add(AFlag.REMOVE_SUPER_CLASS); + } MethodNode classInitMth = cls.getClassInitMth(); if (classInitMth == null) { cls.addWarnComment("Enum class init method not found"); return false; } - if (classInitMth.getBasicBlocks().isEmpty()) { + Region staticRegion = classInitMth.getRegion(); + if (staticRegion == null || classInitMth.getBasicBlocks().isEmpty()) { return false; } + if (!ListUtils.allMatch(staticRegion.getSubBlocks(), BlockNode.class::isInstance)) { + cls.addWarnComment("Unexpected branching instructions in enum static init block"); + return false; + } + List staticBlocks = ListUtils.map(staticRegion.getSubBlocks(), BlockNode.class::cast); ArgType clsType = cls.getClassInfo().getType(); // search "$VALUES" field (holds all enum values) @@ -137,7 +160,6 @@ public class EnumVisitor extends AbstractVisitor { return false; } FieldNode valuesField = valuesCandidates.get(0); - List toRemove = new ArrayList<>(); // search "$VALUES" array init and collect enum fields BlockInsnPair valuesInitPair = getValuesInitInsn(classInitMth, valuesField); @@ -147,16 +169,19 @@ public class EnumVisitor extends AbstractVisitor { BlockNode staticBlock = valuesInitPair.getBlock(); InsnNode valuesInitInsn = valuesInitPair.getInsn(); + EnumData enumData = new EnumData(cls, valuesField, staticBlocks); + List enumFields = null; InsnArg arrArg = valuesInitInsn.getArg(0); if (arrArg.isInsnWrap()) { InsnNode wrappedInsn = ((InsnWrapArg) arrArg).getWrapInsn(); - enumFields = extractEnumFieldsFromInsn(cls, staticBlock, wrappedInsn, toRemove); + enumFields = extractEnumFieldsFromInsn(enumData, wrappedInsn); } if (enumFields == null) { + cls.addWarnComment("Unknown enum class pattern. Please report as an issue!"); return false; } - toRemove.add(valuesInitInsn); + enumData.toRemove.add(valuesInitInsn); // all checks complete, perform transform EnumClassAttr attr = new EnumClassAttr(enumFields); @@ -176,59 +201,52 @@ public class EnumVisitor extends AbstractVisitor { fieldNode.getFieldInfo().setAlias(name); } fieldNode.add(AFlag.DONT_GENERATE); - processConstructorInsn(cls, enumField, classInitMth, staticBlock, toRemove); + processConstructorInsn(enumData, enumField, classInitMth); } valuesField.add(AFlag.DONT_GENERATE); - InsnRemover.removeAllAndUnbind(classInitMth, staticBlock, toRemove); + InsnRemover.removeAllAndUnbind(classInitMth, enumData.toRemove); if (classInitMth.countInsns() == 0) { classInitMth.add(AFlag.DONT_GENERATE); - } else if (!toRemove.isEmpty()) { + } else if (!enumData.toRemove.isEmpty()) { CodeShrinkVisitor.shrinkMethod(classInitMth); } removeEnumMethods(cls, clsType, valuesField); return true; } - private void processConstructorInsn(ClassNode cls, EnumField enumField, MethodNode classInitMth, - BlockNode staticBlock, List toRemove) { + private void processConstructorInsn(EnumData data, EnumField enumField, MethodNode classInitMth) { ConstructorInsn co = enumField.getConstrInsn(); ClassInfo enumClsInfo = co.getClassType(); - if (!enumClsInfo.equals(cls.getClassInfo())) { - ClassNode enumCls = cls.root().resolveClass(enumClsInfo); + if (!enumClsInfo.equals(data.cls.getClassInfo())) { + ClassNode enumCls = data.cls.root().resolveClass(enumClsInfo); if (enumCls != null) { - processEnumCls(cls, enumField, enumCls); + processEnumCls(data.cls, enumField, 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.root().resolveMethod(co.getCallMth()); + MethodNode ctrMth = data.cls.root().resolveMethod(co.getCallMth()); if (ctrMth != null) { markArgsForSkip(ctrMth); } RegisterArg coResArg = co.getResult(); if (coResArg == null || coResArg.getSVar().getUseList().size() <= 2) { - toRemove.add(co); + data.toRemove.add(co); } else { // constructor result used in other places -> replace constructor with enum field get (SGET) IndexInsnNode enumGet = new IndexInsnNode(InsnType.SGET, enumField.getField().getFieldInfo(), 0); enumGet.setResult(coResArg.duplicate()); - BlockUtils.replaceInsn(classInitMth, staticBlock, co, enumGet); + BlockUtils.replaceInsn(classInitMth, co, enumGet); } } @Nullable - private List extractEnumFieldsFromInsn(ClassNode cls, BlockNode staticBlock, - InsnNode wrappedInsn, List toRemove) { + private List extractEnumFieldsFromInsn(EnumData enumData, InsnNode wrappedInsn) { switch (wrappedInsn.getType()) { case FILLED_NEW_ARRAY: - return extractEnumFieldsFromFilledArray(cls, wrappedInsn, staticBlock, toRemove); + return extractEnumFieldsFromFilledArray(enumData, wrappedInsn); case INVOKE: // handle redirection of values array fill (added in java 15) - return extractEnumFieldsFromInvoke(cls, staticBlock, (InvokeNode) wrappedInsn, toRemove); + return extractEnumFieldsFromInvoke(enumData, (InvokeNode) wrappedInsn); case NEW_ARRAY: InsnArg arg = wrappedInsn.getArg(0); @@ -243,10 +261,9 @@ public class EnumVisitor extends AbstractVisitor { } } - private List extractEnumFieldsFromInvoke(ClassNode cls, BlockNode staticBlock, - InvokeNode invokeNode, List toRemove) { + private List extractEnumFieldsFromInvoke(EnumData enumData, InvokeNode invokeNode) { MethodInfo callMth = invokeNode.getCallMth(); - MethodNode valuesMth = cls.root().resolveMethod(callMth); + MethodNode valuesMth = enumData.cls.root().resolveMethod(callMth); if (valuesMth == null || valuesMth.isVoidReturn()) { return null; } @@ -256,7 +273,7 @@ public class EnumVisitor extends AbstractVisitor { if (wrappedInsn == null) { return null; } - List enumFields = extractEnumFieldsFromInsn(cls, staticBlock, wrappedInsn, toRemove); + List enumFields = extractEnumFieldsFromInsn(enumData, wrappedInsn); if (enumFields != null) { valuesMth.add(AFlag.DONT_GENERATE); } @@ -279,50 +296,49 @@ public class EnumVisitor extends AbstractVisitor { return null; } - private List extractEnumFieldsFromFilledArray(ClassNode cls, InsnNode arrFillInsn, BlockNode staticBlock, - List toRemove) { + private List extractEnumFieldsFromFilledArray(EnumData enumData, InsnNode arrFillInsn) { List enumFields = new ArrayList<>(); for (InsnArg arg : arrFillInsn.getArguments()) { EnumField field = null; if (arg.isInsnWrap()) { InsnNode wrappedInsn = ((InsnWrapArg) arg).getWrapInsn(); - field = processEnumFieldByWrappedInsn(cls, wrappedInsn, staticBlock, toRemove); + field = processEnumFieldByWrappedInsn(enumData, wrappedInsn); } else if (arg.isRegister()) { - field = processEnumFieldByRegister(cls, (RegisterArg) arg, staticBlock, toRemove); + field = processEnumFieldByRegister(enumData, (RegisterArg) arg); } if (field == null) { return null; } enumFields.add(field); } - toRemove.add(arrFillInsn); + enumData.toRemove.add(arrFillInsn); return enumFields; } - private EnumField processEnumFieldByWrappedInsn(ClassNode cls, InsnNode wrappedInsn, BlockNode staticBlock, List toRemove) { + private EnumField processEnumFieldByWrappedInsn(EnumData data, InsnNode wrappedInsn) { if (wrappedInsn.getType() == InsnType.SGET) { - return processEnumFieldByField(cls, wrappedInsn, staticBlock, toRemove); + return processEnumFieldByField(data, wrappedInsn); } ConstructorInsn constructorInsn = castConstructorInsn(wrappedInsn); if (constructorInsn != null) { - FieldNode enumFieldNode = createFakeField(cls, "EF" + constructorInsn.getOffset()); - cls.addField(enumFieldNode); - return createEnumFieldByConstructor(cls, enumFieldNode, constructorInsn); + FieldNode enumFieldNode = createFakeField(data.cls, "EF" + constructorInsn.getOffset()); + data.cls.addField(enumFieldNode); + return createEnumFieldByConstructor(data.cls, enumFieldNode, constructorInsn); } return null; } @Nullable - private EnumField processEnumFieldByField(ClassNode cls, InsnNode sgetInsn, BlockNode staticBlock, List toRemove) { + private EnumField processEnumFieldByField(EnumData data, InsnNode sgetInsn) { if (sgetInsn.getType() != InsnType.SGET) { return null; } FieldInfo fieldInfo = (FieldInfo) ((IndexInsnNode) sgetInsn).getIndex(); - FieldNode enumFieldNode = cls.searchField(fieldInfo); + FieldNode enumFieldNode = data.cls.searchField(fieldInfo); if (enumFieldNode == null) { return null; } - InsnNode sputInsn = searchFieldPutInsn(cls, staticBlock, enumFieldNode); + InsnNode sputInsn = searchFieldPutInsn(data, enumFieldNode); if (sputInsn == null) { return null; } @@ -333,17 +349,17 @@ public class EnumVisitor extends AbstractVisitor { } RegisterArg sgetResult = sgetInsn.getResult(); if (sgetResult == null || sgetResult.getSVar().getUseCount() == 1) { - toRemove.add(sgetInsn); + data.toRemove.add(sgetInsn); } - toRemove.add(sputInsn); - return createEnumFieldByConstructor(cls, enumFieldNode, co); + data.toRemove.add(sputInsn); + return createEnumFieldByConstructor(data.cls, enumFieldNode, co); } @Nullable - private EnumField processEnumFieldByRegister(ClassNode cls, RegisterArg arg, BlockNode staticBlock, List toRemove) { + private EnumField processEnumFieldByRegister(EnumData data, RegisterArg arg) { InsnNode assignInsn = arg.getAssignInsn(); if (assignInsn != null && assignInsn.getType() == InsnType.SGET) { - return processEnumFieldByField(cls, assignInsn, staticBlock, toRemove); + return processEnumFieldByField(data, assignInsn); } SSAVar ssaVar = arg.getSVar(); @@ -354,12 +370,12 @@ public class EnumVisitor extends AbstractVisitor { if (constrInsn == null || constrInsn.getType() != InsnType.CONSTRUCTOR) { return null; } - FieldNode enumFieldNode = searchEnumField(cls, ssaVar, toRemove); + FieldNode enumFieldNode = searchEnumField(data, ssaVar); if (enumFieldNode == null) { - enumFieldNode = createFakeField(cls, "EF" + arg.getRegNum()); - cls.addField(enumFieldNode); + enumFieldNode = createFakeField(data.cls, "EF" + arg.getRegNum()); + data.cls.addField(enumFieldNode); } - return createEnumFieldByConstructor(cls, enumFieldNode, (ConstructorInsn) constrInsn); + return createEnumFieldByConstructor(data.cls, enumFieldNode, (ConstructorInsn) constrInsn); } private FieldNode createFakeField(ClassNode cls, String name) { @@ -372,17 +388,17 @@ public class EnumVisitor extends AbstractVisitor { } @Nullable - private FieldNode searchEnumField(ClassNode cls, SSAVar ssaVar, List toRemove) { + private FieldNode searchEnumField(EnumData data, SSAVar ssaVar) { InsnNode sputInsn = ssaVar.getUseList().get(0).getParentInsn(); if (sputInsn == null || sputInsn.getType() != InsnType.SPUT) { return null; } FieldInfo fieldInfo = (FieldInfo) ((IndexInsnNode) sputInsn).getIndex(); - FieldNode enumFieldNode = cls.searchField(fieldInfo); + FieldNode enumFieldNode = data.cls.searchField(fieldInfo); if (enumFieldNode == null) { return null; } - toRemove.add(sputInsn); + data.toRemove.add(sputInsn); return enumFieldNode; } @@ -409,17 +425,24 @@ public class EnumVisitor extends AbstractVisitor { if (ctrMth == null) { return null; } + List regs = new ArrayList<>(); + co.getRegisterArgs(regs); + if (!regs.isEmpty()) { + throw new JadxRuntimeException("Init of enum " + enumFieldNode.getName() + " uses external variables"); + } return new EnumField(enumFieldNode, co); } @Nullable - private InsnNode searchFieldPutInsn(ClassNode cls, BlockNode staticBlock, FieldNode enumFieldNode) { - for (InsnNode sputInsn : staticBlock.getInstructions()) { - if (sputInsn != null && sputInsn.getType() == InsnType.SPUT) { - FieldInfo f = (FieldInfo) ((IndexInsnNode) sputInsn).getIndex(); - FieldNode fieldNode = cls.searchField(f); - if (Objects.equals(fieldNode, enumFieldNode)) { - return sputInsn; + private InsnNode searchFieldPutInsn(EnumData data, FieldNode enumFieldNode) { + for (BlockNode block : data.staticBlocks) { + for (InsnNode sputInsn : block.getInstructions()) { + if (sputInsn != null && sputInsn.getType() == InsnType.SPUT) { + FieldInfo f = (FieldInfo) ((IndexInsnNode) sputInsn).getIndex(); + FieldNode fieldNode = data.cls.searchField(f); + if (Objects.equals(fieldNode, enumFieldNode)) { + return sputInsn; + } } } } @@ -605,4 +628,17 @@ public class EnumVisitor extends AbstractVisitor { } return null; } + + private static class EnumData { + final ClassNode cls; + final FieldNode valuesField; + final List staticBlocks; + final List toRemove = new ArrayList<>(); + + public EnumData(ClassNode cls, FieldNode valuesField, List staticBlocks) { + this.cls = cls; + this.valuesField = valuesField; + this.staticBlocks = staticBlocks; + } + } } diff --git a/jadx-core/src/main/java/jadx/core/dex/visitors/regions/DepthRegionTraversal.java b/jadx-core/src/main/java/jadx/core/dex/visitors/regions/DepthRegionTraversal.java index bdf2a87f..2ec7d52d 100644 --- a/jadx-core/src/main/java/jadx/core/dex/visitors/regions/DepthRegionTraversal.java +++ b/jadx-core/src/main/java/jadx/core/dex/visitors/regions/DepthRegionTraversal.java @@ -19,6 +19,10 @@ public class DepthRegionTraversal { traverseInternal(mth, visitor, mth.getRegion()); } + public static void traverse(MethodNode mth, IContainer container, IRegionVisitor visitor) { + traverseInternal(mth, visitor, container); + } + public static void traverseIterative(MethodNode mth, IRegionIterativeVisitor visitor) { boolean repeat; int k = 0; diff --git a/jadx-core/src/main/java/jadx/core/dex/visitors/regions/TernaryMod.java b/jadx-core/src/main/java/jadx/core/dex/visitors/regions/TernaryMod.java index fd0a8e5d..38b77146 100644 --- a/jadx-core/src/main/java/jadx/core/dex/visitors/regions/TernaryMod.java +++ b/jadx-core/src/main/java/jadx/core/dex/visitors/regions/TernaryMod.java @@ -10,6 +10,7 @@ import jadx.core.dex.instructions.PhiInsn; 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.TernaryInsn; import jadx.core.dex.nodes.BlockNode; import jadx.core.dex.nodes.IContainer; @@ -73,11 +74,7 @@ public class TernaryMod extends AbstractRegionVisitor implements IRegionIterativ return false; } if (elseRegion == null) { - if (mth.isConstructor()) { - // force ternary conversion to inline all code in 'super' or 'this' calls - return processOneBranchTernary(mth, ifRegion); - } - return false; + return processOneBranchTernary(mth, ifRegion); } BlockNode tb = getTernaryInsnBlock(thenRegion); BlockNode eb = getTernaryInsnBlock(elseRegion); @@ -93,21 +90,8 @@ public class TernaryMod extends AbstractRegionVisitor implements IRegionIterativ InsnNode thenInsn = tb.getInstructions().get(0); InsnNode elseInsn = eb.getInstructions().get(0); - if (mth.contains(AFlag.USE_LINES_HINTS) - && thenInsn.getSourceLine() != elseInsn.getSourceLine()) { - if (thenInsn.getSourceLine() != 0 && elseInsn.getSourceLine() != 0) { - // sometimes source lines incorrect - if (!checkLineStats(thenInsn, elseInsn)) { - return false; - } - } else { - // no debug info - if (containsTernary(thenInsn) || containsTernary(elseInsn)) { - // don't make nested ternary by default - // TODO: add addition checks - return false; - } - } + if (!verifyLineHints(mth, thenInsn, elseInsn)) { + return false; } RegisterArg thenResArg = thenInsn.getResult(); @@ -184,6 +168,20 @@ public class TernaryMod extends AbstractRegionVisitor implements IRegionIterativ return false; } + private static boolean verifyLineHints(MethodNode mth, InsnNode thenInsn, InsnNode elseInsn) { + if (mth.contains(AFlag.USE_LINES_HINTS) + && thenInsn.getSourceLine() != elseInsn.getSourceLine()) { + if (thenInsn.getSourceLine() != 0 && elseInsn.getSourceLine() != 0) { + // sometimes source lines incorrect + return checkLineStats(thenInsn, elseInsn); + } + // don't make nested ternary by default + // TODO: add addition checks + return !containsTernary(thenInsn) && !containsTernary(elseInsn); + } + return true; + } + private static void clearConditionBlocks(List conditionBlocks, BlockNode header) { for (BlockNode block : conditionBlocks) { if (block != header) { @@ -277,6 +275,7 @@ public class TernaryMod extends AbstractRegionVisitor implements IRegionIterativ return false; } + @SuppressWarnings("StatementWithEmptyBody") private static void replaceWithTernary(MethodNode mth, IfRegion ifRegion, BlockNode block, InsnNode insn) { RegisterArg resArg = insn.getResult(); if (resArg.getSVar().getUseList().size() != 1) { @@ -296,17 +295,45 @@ public class TernaryMod extends AbstractRegionVisitor implements IRegionIterativ if (otherArg == null) { return; } + InsnNode elseAssign = otherArg.getAssignInsn(); + if (mth.isConstructor() || (mth.getParentClass().isEnum() && mth.getMethodInfo().isClassInit())) { + // forcing ternary inline for constructors (will help in moving super call to the top) and enums + // skip code style checks + } else { + if (elseAssign != null && elseAssign.isConstInsn()) { + if (!verifyLineHints(mth, insn, elseAssign)) { + return; + } + } else { + if (insn.getResult().sameCodeVar(otherArg)) { + // don't use same variable in else branch to prevent: l = (l == 0) ? 1 : l + return; + } + } + } // all checks passed BlockNode header = ifRegion.getConditionBlocks().get(0); if (!ifRegion.getParent().replaceSubBlock(ifRegion, header)) { return; } + InsnArg elseArg; + if (elseAssign != null && elseAssign.isConstInsn()) { + // inline constant + SSAVar elseVar = elseAssign.getResult().getSVar(); + if (elseVar.getUseCount() == 1 && elseVar.getOnlyOneUseInPhi() == phiInsn) { + InsnRemover.remove(mth, elseAssign); + } + elseArg = InsnArg.wrapInsnIntoArg(elseAssign); + } else { + elseArg = otherArg; + } TernaryInsn ternInsn = new TernaryInsn(ifRegion.getCondition(), - phiInsn.getResult(), InsnArg.wrapInsnIntoArg(insn), otherArg); + phiInsn.getResult(), InsnArg.wrapInsnIntoArg(insn), elseArg); + ternInsn.simplifyCondition(); + InsnRemover.unbindResult(mth, insn); InsnList.remove(block, insn); - InsnRemover.unbindAllArgs(mth, phiInsn); header.getInstructions().clear(); ternInsn.rebindArgs(); 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 39523133..607efb63 100644 --- a/jadx-core/src/main/java/jadx/core/utils/InsnRemover.java +++ b/jadx-core/src/main/java/jadx/core/utils/InsnRemover.java @@ -17,6 +17,7 @@ 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.nodes.BlockNode; +import jadx.core.dex.nodes.IContainer; import jadx.core.dex.nodes.InsnNode; import jadx.core.dex.nodes.MethodNode; import jadx.core.utils.exceptions.JadxRuntimeException; @@ -228,6 +229,18 @@ public class InsnRemover { removeAll(block.getInstructions(), insns); } + public static void removeAllAndUnbind(MethodNode mth, IContainer container, List insns) { + unbindInsns(mth, insns); + RegionUtils.visitBlocks(mth, container, b -> removeAll(b.getInstructions(), insns)); + } + + public static void removeAllAndUnbind(MethodNode mth, List insns) { + unbindInsns(mth, insns); + for (BlockNode block : mth.getBasicBlocks()) { + removeAll(block.getInstructions(), insns); + } + } + public static void removeAllWithoutUnbind(BlockNode block, List insns) { removeAll(block.getInstructions(), insns); } 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 b521dc64..87539cc1 100644 --- a/jadx-core/src/main/java/jadx/core/utils/RegionUtils.java +++ b/jadx-core/src/main/java/jadx/core/utils/RegionUtils.java @@ -4,6 +4,7 @@ import java.util.ArrayList; import java.util.Collections; import java.util.List; import java.util.Set; +import java.util.function.Consumer; import org.jetbrains.annotations.Nullable; @@ -26,6 +27,8 @@ import jadx.core.dex.regions.loops.LoopRegion; import jadx.core.dex.trycatch.CatchAttr; import jadx.core.dex.trycatch.ExceptionHandler; import jadx.core.dex.trycatch.TryCatchBlockAttr; +import jadx.core.dex.visitors.regions.AbstractRegionVisitor; +import jadx.core.dex.visitors.regions.DepthRegionTraversal; import jadx.core.utils.exceptions.JadxRuntimeException; public class RegionUtils { @@ -473,4 +476,13 @@ public class RegionUtils { } return "Unknown container type: " + container.getClass(); } + + public static void visitBlocks(MethodNode mth, IContainer container, Consumer visitor) { + DepthRegionTraversal.traverse(mth, container, new AbstractRegionVisitor() { + @Override + public void processBlock(MethodNode mth, IBlock block) { + visitor.accept(block); + } + }); + } } diff --git a/jadx-core/src/test/java/jadx/tests/integration/conditions/TestSwitchTryBreak.java b/jadx-core/src/test/java/jadx/tests/integration/conditions/TestSwitchTryBreak.java deleted file mode 100644 index 43200f8a..00000000 --- a/jadx-core/src/test/java/jadx/tests/integration/conditions/TestSwitchTryBreak.java +++ /dev/null @@ -1,41 +0,0 @@ -package jadx.tests.integration.conditions; - -import org.junit.jupiter.api.Test; - -import jadx.NotYetImplemented; -import jadx.tests.api.IntegrationTest; - -public class TestSwitchTryBreak extends IntegrationTest { - - public static class TestCls { - - public void test(int x) { - switch (x) { - case 0: - return; - case 1: - String res; - if ("android".equals(toString())) { - res = "hello"; - } else { - try { - if (String.CASE_INSENSITIVE_ORDER != null) { - break; - } - res = "hi"; - } catch (Exception e) { - break; - } - } - System.out.println(res); - } - System.out.println("returning"); - } - } - - @Test - @NotYetImplemented - public void test() { - getClassNode(TestCls.class); - } -} diff --git a/jadx-core/src/test/java/jadx/tests/integration/conditions/TestSwitchTryBreak2.java b/jadx-core/src/test/java/jadx/tests/integration/conditions/TestSwitchTryBreak2.java deleted file mode 100644 index d8c4c9ba..00000000 --- a/jadx-core/src/test/java/jadx/tests/integration/conditions/TestSwitchTryBreak2.java +++ /dev/null @@ -1,41 +0,0 @@ -package jadx.tests.integration.conditions; - -import org.junit.jupiter.api.Test; - -import jadx.NotYetImplemented; -import jadx.tests.api.IntegrationTest; - -public class TestSwitchTryBreak2 extends IntegrationTest { - - public static class TestCls { - - public void test(int x) { - switch (x) { - case 0: - return; - case 1: - String res; - if ("android".equals(toString())) { - res = "hello"; - } else { - try { - if (x == 5) { - break; - } - res = "hi"; - } catch (Exception e) { - break; - } - } - System.out.println(res); - } - System.out.println("returning"); - } - } - - @Test - @NotYetImplemented - public void test() { - getClassNode(TestCls.class); - } -} diff --git a/jadx-core/src/test/java/jadx/tests/integration/enums/TestEnumsWithTernary.java b/jadx-core/src/test/java/jadx/tests/integration/enums/TestEnumsWithTernary.java new file mode 100644 index 00000000..7684c49a --- /dev/null +++ b/jadx-core/src/test/java/jadx/tests/integration/enums/TestEnumsWithTernary.java @@ -0,0 +1,39 @@ +package jadx.tests.integration.enums; + +import jadx.tests.api.IntegrationTest; +import jadx.tests.api.extensions.profiles.TestProfile; +import jadx.tests.api.extensions.profiles.TestWithProfiles; + +import static jadx.tests.api.utils.assertj.JadxAssertions.assertThat; + +public class TestEnumsWithTernary extends IntegrationTest { + + public enum TestCls { + FIRST(useNumber() ? "1" : "A"), + SECOND(useNumber() ? "2" : "B"), + ANY(useNumber() ? "1" : "2"); + + private final String str; + + TestCls(String str) { + this.str = str; + } + + public String getStr() { + return str; + } + + public static boolean useNumber() { + return false; + } + } + + @TestWithProfiles({ TestProfile.DX_J8, TestProfile.D8_J8 }) + public void test() { + noDebugInfo(); + assertThat(getClassNode(TestCls.class)) + .code() + .containsOne("ANY(useNumber() ? \"1\" : \"2\");") + .doesNotContain("static {"); + } +} -- GitLab