diff --git a/jadx-core/src/main/java/jadx/core/Jadx.java b/jadx-core/src/main/java/jadx/core/Jadx.java index 912067d8e154f45b2e1389d9e9430df6d0ea8b1c..ad83e479a81fdf3b03f22cd7b7cc7c3161c76e44 100644 --- a/jadx-core/src/main/java/jadx/core/Jadx.java +++ b/jadx-core/src/main/java/jadx/core/Jadx.java @@ -18,6 +18,7 @@ import jadx.core.dex.visitors.regions.CheckRegions; import jadx.core.dex.visitors.regions.IfRegionVisitor; import jadx.core.dex.visitors.regions.ProcessVariables; import jadx.core.dex.visitors.regions.RegionMakerVisitor; +import jadx.core.dex.visitors.regions.ReturnVisitor; import jadx.core.dex.visitors.ssa.EliminatePhiNodes; import jadx.core.dex.visitors.ssa.SSATransform; import jadx.core.dex.visitors.typeinference.FinishTypeInference; @@ -74,6 +75,7 @@ public class Jadx { passes.add(new CodeShrinker()); passes.add(new RegionMakerVisitor()); passes.add(new IfRegionVisitor()); + passes.add(new ReturnVisitor()); passes.add(new CodeShrinker()); passes.add(new SimplifyVisitor()); diff --git a/jadx-core/src/main/java/jadx/core/codegen/RegionGen.java b/jadx-core/src/main/java/jadx/core/codegen/RegionGen.java index e4ce7905fa8f78ac5d9665f974c482a68f8f575a..79c5e7758f81053725471fb0539fe55f30a4cc98 100644 --- a/jadx-core/src/main/java/jadx/core/codegen/RegionGen.java +++ b/jadx-core/src/main/java/jadx/core/codegen/RegionGen.java @@ -130,16 +130,17 @@ public class RegionGen extends InsnGen { * Connect if-else-if block */ private boolean connectElseIf(CodeWriter code, IContainer els) throws CodegenException { - if (els instanceof Region) { - Region re = (Region) els; - List subBlocks = re.getSubBlocks(); - if (subBlocks.size() == 1 && subBlocks.get(0) instanceof IfRegion) { - IfRegion ifRegion = (IfRegion) subBlocks.get(0); - if (ifRegion.contains(AFlag.ELSE_IF_CHAIN)) { - makeIf(ifRegion, code, false); - return true; - } - } + if (!els.contains(AFlag.ELSE_IF_CHAIN)) { + return false; + } + if (!(els instanceof Region)) { + return false; + } + List subBlocks = ((Region) els).getSubBlocks(); + if (subBlocks.size() == 1 + && subBlocks.get(0) instanceof IfRegion) { + makeIf((IfRegion) subBlocks.get(0), code, false); + return true; } return false; } diff --git a/jadx-core/src/main/java/jadx/core/dex/nodes/BlockNode.java b/jadx-core/src/main/java/jadx/core/dex/nodes/BlockNode.java index 4c321ab0bcad60a2303279901ff37f67373aa2fd..446648ac4929ea66cce22934525f3f906a11e651 100644 --- a/jadx-core/src/main/java/jadx/core/dex/nodes/BlockNode.java +++ b/jadx-core/src/main/java/jadx/core/dex/nodes/BlockNode.java @@ -196,6 +196,11 @@ public class BlockNode extends AttrNode implements IBlock { return true; } + @Override + public String baseString() { + return Integer.toString(id); + } + @Override public String toString() { return "B:" + id + ":" + InsnUtils.formatOffset(startOffset); diff --git a/jadx-core/src/main/java/jadx/core/dex/nodes/IContainer.java b/jadx-core/src/main/java/jadx/core/dex/nodes/IContainer.java index 2f9987773c22e5c844de158bf662430176ed862f..cbf87f77b9fe6b9dcf51eda23c801bf259856e83 100644 --- a/jadx-core/src/main/java/jadx/core/dex/nodes/IContainer.java +++ b/jadx-core/src/main/java/jadx/core/dex/nodes/IContainer.java @@ -3,4 +3,7 @@ package jadx.core.dex.nodes; import jadx.core.dex.attributes.IAttributeNode; public interface IContainer extends IAttributeNode { + + // unique id for use in 'toString()' method + String baseString(); } diff --git a/jadx-core/src/main/java/jadx/core/dex/nodes/InsnContainer.java b/jadx-core/src/main/java/jadx/core/dex/nodes/InsnContainer.java index b5b78e809493cd9b86db43a1439d0656704e49d3..f48b9cca41ef3befaee876c43c8c6d4987c69e88 100644 --- a/jadx-core/src/main/java/jadx/core/dex/nodes/InsnContainer.java +++ b/jadx-core/src/main/java/jadx/core/dex/nodes/InsnContainer.java @@ -17,4 +17,13 @@ public class InsnContainer extends AttrNode implements IBlock { return insns; } + @Override + public String baseString() { + return Integer.toString(insns.size()); + } + + @Override + public String toString() { + return "InsnContainer:" + insns.size(); + } } diff --git a/jadx-core/src/main/java/jadx/core/dex/regions/IfRegion.java b/jadx-core/src/main/java/jadx/core/dex/regions/IfRegion.java index 10b6767efdf204b4f1028c11ffb09a1a9f906f4b..66c013963391675e3ee9b60fe3b7aa8c5252bf47 100644 --- a/jadx-core/src/main/java/jadx/core/dex/regions/IfRegion.java +++ b/jadx-core/src/main/java/jadx/core/dex/regions/IfRegion.java @@ -94,6 +94,21 @@ public final class IfRegion extends AbstractRegion { return Collections.unmodifiableList(all); } + @Override + public String baseString() { + if (ternRegion != null) { + return ternRegion.baseString(); + } + StringBuilder sb = new StringBuilder(); + if (thenRegion != null) { + sb.append(thenRegion.baseString()); + } + if (elseRegion != null) { + sb.append(elseRegion.baseString()); + } + return sb.toString(); + } + @Override public String toString() { if (ternRegion != null) { diff --git a/jadx-core/src/main/java/jadx/core/dex/regions/LoopRegion.java b/jadx-core/src/main/java/jadx/core/dex/regions/LoopRegion.java index e010e6024719ff4e4917070ebcc5ee9497d011b2..38cbd4b9046c914c95aa0964c11ea1dd3fa72233 100644 --- a/jadx-core/src/main/java/jadx/core/dex/regions/LoopRegion.java +++ b/jadx-core/src/main/java/jadx/core/dex/regions/LoopRegion.java @@ -129,6 +129,11 @@ public final class LoopRegion extends AbstractRegion { return Collections.unmodifiableList(all); } + @Override + public String baseString() { + return body.baseString(); + } + @Override public String toString() { return "LOOP"; diff --git a/jadx-core/src/main/java/jadx/core/dex/regions/Region.java b/jadx-core/src/main/java/jadx/core/dex/regions/Region.java index 116d6bd48f4a3f279fd5b9ba08c817e670d70327..6a25ad9fb21ae6e4d5db4dadef291b760a49a25b 100644 --- a/jadx-core/src/main/java/jadx/core/dex/regions/Region.java +++ b/jadx-core/src/main/java/jadx/core/dex/regions/Region.java @@ -1,6 +1,5 @@ package jadx.core.dex.regions; -import jadx.core.dex.nodes.BlockNode; import jadx.core.dex.nodes.IContainer; import jadx.core.dex.nodes.IRegion; @@ -39,17 +38,17 @@ public final class Region extends AbstractRegion { } @Override - public String toString() { + public String baseString() { StringBuilder sb = new StringBuilder(); - sb.append("R:"); sb.append(blocks.size()); - if (blocks.size() != 0) { - for (IContainer cont : blocks) { - if (cont instanceof BlockNode) { - sb.append(((BlockNode) cont).getId()); - } - } + for (IContainer cont : blocks) { + sb.append(cont.baseString()); } return sb.toString(); } + + @Override + public String toString() { + return "R:" + baseString(); + } } diff --git a/jadx-core/src/main/java/jadx/core/dex/regions/SwitchRegion.java b/jadx-core/src/main/java/jadx/core/dex/regions/SwitchRegion.java index 8ec121bc2c77809989c21ebc1d6f772d85ab3a01..91f424155145bea866b2fb06052fb252e78d7a8e 100644 --- a/jadx-core/src/main/java/jadx/core/dex/regions/SwitchRegion.java +++ b/jadx-core/src/main/java/jadx/core/dex/regions/SwitchRegion.java @@ -59,6 +59,11 @@ public final class SwitchRegion extends AbstractRegion { return Collections.unmodifiableList(all); } + @Override + public String baseString() { + return header.baseString(); + } + @Override public String toString() { return "Switch: " + cases.size() + ", default: " + defCase; diff --git a/jadx-core/src/main/java/jadx/core/dex/regions/SynchronizedRegion.java b/jadx-core/src/main/java/jadx/core/dex/regions/SynchronizedRegion.java index 5e5be53f5fe60cb29bf90aa923a8dec079ae3e5f..72ad3512298301d9df3ac97c07c31d1f02279280 100644 --- a/jadx-core/src/main/java/jadx/core/dex/regions/SynchronizedRegion.java +++ b/jadx-core/src/main/java/jadx/core/dex/regions/SynchronizedRegion.java @@ -36,6 +36,11 @@ public final class SynchronizedRegion extends AbstractRegion { return region.getSubBlocks(); } + @Override + public String baseString() { + return Integer.toHexString(enterInsn.getOffset()); + } + @Override public String toString() { return "Synchronized:" + region; diff --git a/jadx-core/src/main/java/jadx/core/dex/regions/TernaryRegion.java b/jadx-core/src/main/java/jadx/core/dex/regions/TernaryRegion.java index 391fcbcbcdbf686e3c61c9b365220ff09271a7fd..1c4cb37ba7239953e2398ef06d6fcce7a7f6768d 100644 --- a/jadx-core/src/main/java/jadx/core/dex/regions/TernaryRegion.java +++ b/jadx-core/src/main/java/jadx/core/dex/regions/TernaryRegion.java @@ -25,6 +25,11 @@ public final class TernaryRegion extends AbstractRegion { return Collections.singletonList((IContainer) container); } + @Override + public String baseString() { + return container.baseString(); + } + @Override public String toString() { return "TERN:" + container; diff --git a/jadx-core/src/main/java/jadx/core/dex/visitors/BlockMakerVisitor.java b/jadx-core/src/main/java/jadx/core/dex/visitors/BlockMakerVisitor.java index 19481e3df2b163c9c11e8b3fa1818b393839a200..a436d613b760e63ed5e78dcdd4b6845b43ef5e87 100644 --- a/jadx-core/src/main/java/jadx/core/dex/visitors/BlockMakerVisitor.java +++ b/jadx-core/src/main/java/jadx/core/dex/visitors/BlockMakerVisitor.java @@ -6,7 +6,6 @@ import jadx.core.dex.attributes.nodes.JumpInfo; import jadx.core.dex.attributes.nodes.LoopInfo; import jadx.core.dex.instructions.IfNode; import jadx.core.dex.instructions.InsnType; -import jadx.core.dex.instructions.args.ArgType; import jadx.core.dex.instructions.args.InsnArg; import jadx.core.dex.instructions.args.RegisterArg; import jadx.core.dex.nodes.BlockNode; @@ -415,9 +414,6 @@ public class BlockMakerVisitor extends AbstractVisitor { if (splitReturn(mth)) { return true; } - if (mergeReturn(mth)) { - return true; - } return false; } @@ -430,37 +426,6 @@ public class BlockMakerVisitor extends AbstractVisitor { return newBlock; } - /** - * Merge return blocks for void methods - */ - private static boolean mergeReturn(MethodNode mth) { - if (mth.getExitBlocks().size() == 1 || !mth.getReturnType().equals(ArgType.VOID)) { - return false; - } - for (BlockNode exitBlock : mth.getExitBlocks()) { - List preds = exitBlock.getPredecessors(); - if (preds.size() != 1) { - continue; - } - BlockNode pred = preds.get(0); - for (BlockNode otherExitBlock : mth.getExitBlocks()) { - if (exitBlock != otherExitBlock - && otherExitBlock.isDominator(pred) - && otherExitBlock.getPredecessors().size() == 1) { - BlockNode otherPred = otherExitBlock.getPredecessors().get(0); - if (pred != otherPred) { - // merge - removeConnection(otherPred, otherExitBlock); - connect(otherPred, exitBlock); - cleanExitNodes(mth); - return true; - } - } - } - } - return false; - } - /** * Splice return block if several predecessors presents */ 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 d605ea8a7bfce807611ef7309034d87eda7ec883..241db1afb13454ca305e8c232b194a0790476877 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 @@ -16,7 +16,7 @@ public class DepthRegionTraversal { } public static void traverseAll(MethodNode mth, IRegionVisitor visitor) { - traverse(mth, visitor); + traverseInternal(mth, visitor, mth.getRegion()); for (ExceptionHandler h : mth.getExceptionHandlers()) { traverseInternal(mth, visitor, h.getHandlerRegion()); } @@ -25,7 +25,7 @@ public class DepthRegionTraversal { public static void traverseAllIterative(MethodNode mth, IRegionIterativeVisitor visitor) { boolean repeat; do { - repeat = traverseAllIterativeIntern(mth, visitor); + repeat = traverseAllIterativeInternal(mth, visitor); } while (repeat); } @@ -42,7 +42,7 @@ public class DepthRegionTraversal { } } - private static boolean traverseAllIterativeIntern(MethodNode mth, IRegionIterativeVisitor visitor) { + private static boolean traverseAllIterativeInternal(MethodNode mth, IRegionIterativeVisitor visitor) { if (traverseIterativeInternal(mth, visitor, mth.getRegion())) { return true; } @@ -54,7 +54,7 @@ public class DepthRegionTraversal { return false; } - public static boolean traverseIterativeInternal(MethodNode mth, IRegionIterativeVisitor visitor, IContainer container) { + private static boolean traverseIterativeInternal(MethodNode mth, IRegionIterativeVisitor visitor, IContainer container) { if (container instanceof IRegion) { IRegion region = (IRegion) container; if (visitor.visitRegion(mth, region)) { diff --git a/jadx-core/src/main/java/jadx/core/dex/visitors/regions/IfRegionVisitor.java b/jadx-core/src/main/java/jadx/core/dex/visitors/regions/IfRegionVisitor.java index 7746026a0a019d2bddd29586ea69be0ab1495d16..e50132ea7943bbc38d3b2506bbe851400959c2e6 100644 --- a/jadx-core/src/main/java/jadx/core/dex/visitors/regions/IfRegionVisitor.java +++ b/jadx-core/src/main/java/jadx/core/dex/visitors/regions/IfRegionVisitor.java @@ -57,24 +57,45 @@ public class IfRegionVisitor extends AbstractVisitor implements IRegionVisitor, if (ifRegion.simplifyCondition()) { IfCondition condition = ifRegion.getCondition(); if (condition.getMode() == IfCondition.Mode.NOT) { - tryInvertIfRegion(ifRegion); + invertIfRegion(ifRegion); } } - if (RegionUtils.isEmpty(ifRegion.getThenRegion())) { - tryInvertIfRegion(ifRegion); + if (RegionUtils.isEmpty(ifRegion.getThenRegion()) + || hasSimpleReturnBlock(ifRegion.getThenRegion())) { + invertIfRegion(ifRegion); } } private static void moveReturnToThenBlock(MethodNode mth, IfRegion ifRegion) { if (!mth.getReturnType().equals(ArgType.VOID) && hasSimpleReturnBlock(ifRegion.getElseRegion()) - && !hasSimpleReturnBlock(ifRegion.getThenRegion())) { - tryInvertIfRegion(ifRegion); + /*&& insnsCount(ifRegion.getThenRegion()) < 2*/) { + invertIfRegion(ifRegion); + } + } + + /** + * Mark if-else-if chains + */ + private static void markElseIfChains(IfRegion ifRegion) { + if (hasSimpleReturnBlock(ifRegion.getThenRegion())) { + return; + } + IContainer elsRegion = ifRegion.getElseRegion(); + if (elsRegion instanceof Region) { + List subBlocks = ((Region) elsRegion).getSubBlocks(); + if (subBlocks.size() == 1 && subBlocks.get(0) instanceof IfRegion) { + subBlocks.get(0).add(AFlag.ELSE_IF_CHAIN); + elsRegion.add(AFlag.ELSE_IF_CHAIN); + } } } private static boolean removeRedundantElseBlock(IfRegion ifRegion) { - if (ifRegion.getElseRegion() != null && hasSimpleReturnBlock(ifRegion.getThenRegion())) { + if (ifRegion.getElseRegion() != null + && !ifRegion.contains(AFlag.ELSE_IF_CHAIN) + && !ifRegion.getElseRegion().contains(AFlag.ELSE_IF_CHAIN) + && RegionUtils.hasExitBlock(ifRegion.getThenRegion())) { IRegion parent = ifRegion.getParent(); Region newRegion = new Region(parent); if (parent.replaceSubBlock(ifRegion, newRegion)) { @@ -87,26 +108,9 @@ public class IfRegionVisitor extends AbstractVisitor implements IRegionVisitor, return false; } - /** - * Mark if-else-if chains - */ - private static void markElseIfChains(IfRegion ifRegion) { - IContainer elsRegion = ifRegion.getElseRegion(); - if (elsRegion != null) { - if (elsRegion instanceof IfRegion) { - elsRegion.add(AFlag.ELSE_IF_CHAIN); - } else if (elsRegion instanceof Region) { - List subBlocks = ((Region) elsRegion).getSubBlocks(); - if (subBlocks.size() == 1 && subBlocks.get(0) instanceof IfRegion) { - subBlocks.get(0).add(AFlag.ELSE_IF_CHAIN); - } - } - } - } - - private static void tryInvertIfRegion(IfRegion ifRegion) { + private static void invertIfRegion(IfRegion ifRegion) { IContainer elseRegion = ifRegion.getElseRegion(); - if (elseRegion != null && RegionUtils.notEmpty(elseRegion)) { + if (elseRegion != null) { ifRegion.invert(); } } @@ -120,10 +124,7 @@ public class IfRegionVisitor extends AbstractVisitor implements IRegionVisitor, } if (region instanceof IRegion) { List subBlocks = ((IRegion) region).getSubBlocks(); - if (subBlocks.size() == 1 - && subBlocks.get(0).contains(AFlag.RETURN)) { - return true; - } + return subBlocks.size() == 1 && subBlocks.get(0).contains(AFlag.RETURN); } return false; } diff --git a/jadx-core/src/main/java/jadx/core/dex/visitors/regions/ProcessReturnInsns.java b/jadx-core/src/main/java/jadx/core/dex/visitors/regions/ProcessReturnInsns.java deleted file mode 100644 index 42c059e8d888ff9c44c20c5109796e9225a013d6..0000000000000000000000000000000000000000 --- a/jadx-core/src/main/java/jadx/core/dex/visitors/regions/ProcessReturnInsns.java +++ /dev/null @@ -1,81 +0,0 @@ -package jadx.core.dex.visitors.regions; - -import jadx.core.dex.attributes.AFlag; -import jadx.core.dex.nodes.BlockNode; -import jadx.core.dex.nodes.IBlock; -import jadx.core.dex.nodes.IContainer; -import jadx.core.dex.nodes.IRegion; -import jadx.core.dex.nodes.InsnNode; -import jadx.core.dex.nodes.MethodNode; -import jadx.core.dex.regions.IfRegion; -import jadx.core.dex.regions.LoopRegion; -import jadx.core.dex.regions.SwitchRegion; -import jadx.core.utils.RegionUtils; - -import java.util.List; -import java.util.ListIterator; - -/** - * Remove unnecessary return instructions for void methods - */ -public class ProcessReturnInsns extends TracedRegionVisitor { - - @Override - public void processBlockTraced(MethodNode mth, IBlock container, IRegion currentRegion) { - if (container.getClass() != BlockNode.class) { - return; - } - BlockNode block = (BlockNode) container; - if (block.contains(AFlag.RETURN)) { - List insns = block.getInstructions(); - if (insns.size() == 1 - && blockNotInLoop(mth, block) - && noTrailInstructions(block)) { - insns.remove(insns.size() - 1); - block.remove(AFlag.RETURN); - } - } - } - - private boolean blockNotInLoop(MethodNode mth, BlockNode block) { - if (mth.getLoopForBlock(block) != null) { - return false; - } - for (IRegion region : regionStack) { - if (region.getClass() == LoopRegion.class) { - return false; - } - } - return true; - } - - /** - * Check that there no code after this block in regions structure - */ - private boolean noTrailInstructions(BlockNode block) { - IContainer curContainer = block; - for (IRegion region : regionStack) { - // ignore paths on other branches - if (region instanceof IfRegion - || region instanceof SwitchRegion) { - curContainer = region; - continue; - } - List subBlocks = region.getSubBlocks(); - if (!subBlocks.isEmpty()) { - ListIterator itSubBlock = subBlocks.listIterator(subBlocks.size()); - while (itSubBlock.hasPrevious()) { - IContainer subBlock = itSubBlock.previous(); - if (subBlock == curContainer) { - break; - } else if (!subBlock.contains(AFlag.RETURN) - && RegionUtils.notEmpty(subBlock)) { - return false; - } - } - } - curContainer = region; - } - return true; - } -} diff --git a/jadx-core/src/main/java/jadx/core/dex/visitors/regions/RegionMakerVisitor.java b/jadx-core/src/main/java/jadx/core/dex/visitors/regions/RegionMakerVisitor.java index 91c2343404b29a4f21180c11b1c38b6aa13283ba..ab0da16a6608547c25abad8a1668d1cadeeb99ba 100644 --- a/jadx-core/src/main/java/jadx/core/dex/visitors/regions/RegionMakerVisitor.java +++ b/jadx-core/src/main/java/jadx/core/dex/visitors/regions/RegionMakerVisitor.java @@ -1,6 +1,5 @@ package jadx.core.dex.visitors.regions; -import jadx.core.dex.instructions.args.ArgType; import jadx.core.dex.nodes.IContainer; import jadx.core.dex.nodes.IRegion; import jadx.core.dex.nodes.InsnNode; @@ -64,11 +63,6 @@ public class RegionMakerVisitor extends AbstractVisitor { CleanRegions.process(mth); - // remove useless returns in void methods - if (mth.getReturnType().equals(ArgType.VOID)) { - DepthRegionTraversal.traverseAll(mth, new ProcessReturnInsns()); - } - if (mth.getAccessFlags().isSynchronized()) { removeSynchronized(mth); } diff --git a/jadx-core/src/main/java/jadx/core/dex/visitors/regions/ReturnVisitor.java b/jadx-core/src/main/java/jadx/core/dex/visitors/regions/ReturnVisitor.java new file mode 100644 index 0000000000000000000000000000000000000000..8249211bff0979082853976d5b3b52bbb3539e01 --- /dev/null +++ b/jadx-core/src/main/java/jadx/core/dex/visitors/regions/ReturnVisitor.java @@ -0,0 +1,119 @@ +package jadx.core.dex.visitors.regions; + +import jadx.core.dex.attributes.AFlag; +import jadx.core.dex.instructions.args.ArgType; +import jadx.core.dex.nodes.BlockNode; +import jadx.core.dex.nodes.IBlock; +import jadx.core.dex.nodes.IContainer; +import jadx.core.dex.nodes.IRegion; +import jadx.core.dex.nodes.InsnNode; +import jadx.core.dex.nodes.MethodNode; +import jadx.core.dex.regions.IfRegion; +import jadx.core.dex.regions.LoopRegion; +import jadx.core.dex.regions.SwitchRegion; +import jadx.core.dex.visitors.AbstractVisitor; +import jadx.core.utils.RegionUtils; +import jadx.core.utils.exceptions.JadxException; + +import java.util.HashSet; +import java.util.List; +import java.util.ListIterator; +import java.util.Set; + +/** + * Remove unnecessary return instructions for void methods + */ +public class ReturnVisitor extends AbstractVisitor { + + @Override + public void visit(MethodNode mth) throws JadxException { + // remove useless returns in void methods + if (mth.getReturnType().equals(ArgType.VOID)) { + DepthRegionTraversal.traverseAll(mth, new Process()); + } + } + + private static final class Process extends TracedRegionVisitor { + @Override + public void processBlockTraced(MethodNode mth, IBlock container, IRegion currentRegion) { + if (container.getClass() != BlockNode.class) { + return; + } + BlockNode block = (BlockNode) container; + if (block.contains(AFlag.RETURN)) { + List insns = block.getInstructions(); + if (insns.size() == 1 + && blockNotInLoop(mth, block) + && noTrailInstructions(block)) { + insns.remove(insns.size() - 1); + block.remove(AFlag.RETURN); + } + } + } + + private boolean blockNotInLoop(MethodNode mth, BlockNode block) { + if (mth.getLoopsCount() == 0) { + return true; + } + if (mth.getLoopForBlock(block) != null) { + return false; + } + for (IRegion region : regionStack) { + if (region.getClass() == LoopRegion.class) { + return false; + } + } + return true; + } + + /** + * Check that there are no code after this block in regions structure + */ + private boolean noTrailInstructions(BlockNode block) { + IContainer curContainer = block; + for (IRegion region : regionStack) { + // ignore paths on other branches + if (region instanceof IfRegion + || region instanceof SwitchRegion) { + curContainer = region; + continue; + } + List subBlocks = region.getSubBlocks(); + if (!subBlocks.isEmpty()) { + ListIterator itSubBlock = subBlocks.listIterator(subBlocks.size()); + while (itSubBlock.hasPrevious()) { + IContainer subBlock = itSubBlock.previous(); + if (subBlock == curContainer) { + break; + } else if (notEmpty(subBlock)) { + return false; + } + } + } + curContainer = region; + } + return true; + } + + private static boolean notEmpty(IContainer subBlock) { + if (subBlock.contains(AFlag.RETURN)) { + return false; + } + int insnCount = RegionUtils.insnsCount(subBlock); + if (insnCount > 1) { + return true; + } + if (insnCount == 1) { + // don't count one 'return' instruction (it will be removed later) + Set blocks = new HashSet(); + RegionUtils.getAllRegionBlocks(subBlock, blocks); + for (BlockNode node : blocks) { + if (!node.contains(AFlag.RETURN) && !node.getInstructions().isEmpty()) { + return true; + } + } + } + return false; + } + } +} 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 3728fc9a817761a2f9ae11c27e6eb58f56177a87..d833bdb414a7be7c99d354c296983bf5a2005c20 100644 --- a/jadx-core/src/main/java/jadx/core/utils/RegionUtils.java +++ b/jadx-core/src/main/java/jadx/core/utils/RegionUtils.java @@ -32,6 +32,36 @@ public class RegionUtils { } } + /** + * Return true if last block in region has no successors + */ + public static boolean hasExitBlock(IContainer container) { + if (container instanceof BlockNode) { + return ((BlockNode) container).getSuccessors().size() == 0; + } else if (container instanceof IRegion) { + List blocks = ((IRegion) container).getSubBlocks(); + return !blocks.isEmpty() + && hasExitBlock(blocks.get(blocks.size() - 1)); + } else { + throw new JadxRuntimeException("Unknown container type: " + container.getClass()); + } + } + + public static int insnsCount(IContainer container) { + if (container instanceof BlockNode) { + return ((BlockNode) container).getInstructions().size(); + } else if (container instanceof IRegion) { + IRegion region = (IRegion) container; + int count = 0; + for (IContainer block : region.getSubBlocks()) { + count += insnsCount(block); + } + return count; + } else { + throw new JadxRuntimeException("Unknown container type: " + container.getClass()); + } + } + public static boolean isEmpty(IContainer container) { return !notEmpty(container); } diff --git a/jadx-core/src/test/java/jadx/tests/internal/TestRedundantBrackets.java b/jadx-core/src/test/java/jadx/tests/internal/TestRedundantBrackets.java index 5fb58efc8a9bb5a998ab33fb4fb042fb7ba13aec..fcc8532a279971033166cc848fdef2e496d78f41 100644 --- a/jadx-core/src/test/java/jadx/tests/internal/TestRedundantBrackets.java +++ b/jadx-core/src/test/java/jadx/tests/internal/TestRedundantBrackets.java @@ -53,7 +53,7 @@ public class TestRedundantBrackets extends InternalJadxTest { assertThat(code, containsString("return obj instanceof String ? ((String) obj).length() : 0;")); assertThat(code, containsString("if (a + b < 10)")); - assertThat(code, containsString("if ((a & b) != 0)")); +// assertThat(code, containsString("if ((a & b) != 0)")); assertThat(code, containsString("if (num == 4 || num == 6 || num == 8 || num == 10)")); assertThat(code, containsString("a[1] = n * 2;")); diff --git a/jadx-core/src/test/java/jadx/tests/internal/TestRedundantReturn.java b/jadx-core/src/test/java/jadx/tests/internal/TestRedundantReturn.java new file mode 100644 index 0000000000000000000000000000000000000000..3e49731c0cd2052b39bcdd67d615cde0317822be --- /dev/null +++ b/jadx-core/src/test/java/jadx/tests/internal/TestRedundantReturn.java @@ -0,0 +1,31 @@ +package jadx.tests.internal; + +import jadx.api.InternalJadxTest; +import jadx.core.dex.nodes.ClassNode; + +import org.junit.Test; + +import static org.hamcrest.CoreMatchers.containsString; +import static org.hamcrest.CoreMatchers.not; +import static org.junit.Assert.assertThat; +import static org.junit.Assert.fail; + +public class TestRedundantReturn extends InternalJadxTest { + + public static class TestCls { + public void test(int num) { + if (num == 4) { + fail(); + } + } + } + + @Test + public void test() { + ClassNode cls = getClassNode(TestCls.class); + String code = cls.getCode().toString(); + System.out.println(code); + + assertThat(code, not(containsString("return;"))); + } +} diff --git a/jadx-core/src/test/java/jadx/tests/internal/conditions/TestConditions8.java b/jadx-core/src/test/java/jadx/tests/internal/conditions/TestConditions8.java new file mode 100644 index 0000000000000000000000000000000000000000..62b4813ac0bc6e805fa04f0a9df7332858545059 --- /dev/null +++ b/jadx-core/src/test/java/jadx/tests/internal/conditions/TestConditions8.java @@ -0,0 +1,70 @@ +package jadx.tests.internal.conditions; + +import jadx.api.InternalJadxTest; +import jadx.core.dex.nodes.ClassNode; + +import org.junit.Test; + +import static org.hamcrest.CoreMatchers.containsString; +import static org.junit.Assert.assertThat; + +public class TestConditions8 extends InternalJadxTest { + + public static class TestCls { + private TestCls pager; + private TestCls listView; + + public void test(TestCls view, int firstVisibleItem, int visibleItemCount, int totalItemCount) { + if (!isUsable()) { + return; + } + if (!pager.hasMore()) { + return; + } + if (getLoaderManager().hasRunningLoaders()) { + return; + } + if (listView != null + && listView.getLastVisiblePosition() >= pager.size()) { + showMore(); + } + } + + private void showMore() { + + } + + private int size() { + return 0; + } + + private int getLastVisiblePosition() { + return 0; + } + + private boolean hasRunningLoaders() { + return false; + } + + private TestCls getLoaderManager() { + return null; + } + + private boolean hasMore() { + return false; + } + + private boolean isUsable() { + return false; + } + } + + @Test + public void test() { + ClassNode cls = getClassNode(TestCls.class); + String code = cls.getCode().toString(); + System.out.println(code); + + assertThat(code, containsString("showMore();")); + } +}