From 066b5a895d551ec04faeecb7eb064bb476fab925 Mon Sep 17 00:00:00 2001 From: Skylot Date: Sat, 22 Mar 2014 17:19:01 +0400 Subject: [PATCH] core: refactor 'if' regions processing --- .../dex/visitors/ConstInlinerVisitor.java | 13 +- .../visitors/regions/ProcessReturnInsns.java | 8 ++ .../dex/visitors/regions/RegionMaker.java | 134 ++++++++++-------- .../main/java/jadx/core/utils/BlockUtils.java | 17 +++ .../src/main/java/jadx/core/utils/Utils.java | 8 +- .../test/java/jadx/api/InternalJadxTest.java | 7 +- 6 files changed, 113 insertions(+), 74 deletions(-) diff --git a/jadx-core/src/main/java/jadx/core/dex/visitors/ConstInlinerVisitor.java b/jadx-core/src/main/java/jadx/core/dex/visitors/ConstInlinerVisitor.java index 46c18f4a..b916d17f 100644 --- a/jadx-core/src/main/java/jadx/core/dex/visitors/ConstInlinerVisitor.java +++ b/jadx-core/src/main/java/jadx/core/dex/visitors/ConstInlinerVisitor.java @@ -11,9 +11,9 @@ import jadx.core.dex.nodes.BlockNode; import jadx.core.dex.nodes.InsnNode; import jadx.core.dex.nodes.MethodNode; import jadx.core.utils.BlockUtils; +import jadx.core.utils.InstructionRemover; import jadx.core.utils.exceptions.JadxException; -import java.util.Iterator; import java.util.List; import java.util.Set; @@ -25,12 +25,13 @@ public class ConstInlinerVisitor extends AbstractVisitor { return; } for (BlockNode block : mth.getBasicBlocks()) { - for (Iterator it = block.getInstructions().iterator(); it.hasNext(); ) { - InsnNode insn = it.next(); + InstructionRemover remover = new InstructionRemover(block.getInstructions()); + for (InsnNode insn : block.getInstructions()) { if (checkInsn(mth, block, insn)) { - it.remove(); + remover.add(insn); } } + remover.perform(); } } @@ -53,9 +54,11 @@ public class ConstInlinerVisitor extends AbstractVisitor { private static boolean replaceConst(MethodNode mth, BlockNode block, InsnNode insn, long literal) { List use = insn.getResult().getTypedVar().getUseList(); int replaceCount = 0; + int assignCount = 0; for (InsnArg arg : use) { InsnNode useInsn = arg.getParentInsn(); if (arg == insn.getResult() || useInsn == null) { + assignCount++; continue; } BlockNode useBlock = BlockUtils.getBlockByInsn(mth, useInsn); @@ -76,7 +79,7 @@ public class ConstInlinerVisitor extends AbstractVisitor { } } } - return replaceCount == use.size() - 1; + return replaceCount == use.size() - assignCount; } private static boolean registerReassignOnPath(BlockNode block, BlockNode useBlock, InsnNode assignInsn) { 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 index ef2d9a6d..830dd707 100644 --- 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 @@ -7,7 +7,9 @@ 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; @@ -52,6 +54,12 @@ public class ProcessReturnInsns extends TracedRegionVisitor { 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()); diff --git a/jadx-core/src/main/java/jadx/core/dex/visitors/regions/RegionMaker.java b/jadx-core/src/main/java/jadx/core/dex/visitors/regions/RegionMaker.java index b4e9a84b..39bdc614 100644 --- a/jadx-core/src/main/java/jadx/core/dex/visitors/regions/RegionMaker.java +++ b/jadx-core/src/main/java/jadx/core/dex/visitors/regions/RegionMaker.java @@ -461,13 +461,8 @@ public class RegionMaker { } } - if (elseBlock != null) { - if (stack.containsExit(elseBlock)) { - elseBlock = null; - } else if (elseBlock.getAttributes().contains(AttributeFlag.RETURN)) { - out = elseBlock; - elseBlock = null; - } + if (elseBlock != null && stack.containsExit(elseBlock)) { + elseBlock = null; } stack.push(ifRegion); @@ -481,73 +476,88 @@ public class RegionMaker { } private IfInfo mergeNestedIfNodes(BlockNode block, BlockNode bThen, BlockNode bElse, List merged) { + IfInfo info = new IfInfo(); + info.setIfnode(block); + info.setCondition(IfCondition.fromIfBlock(block)); + info.setThenBlock(bThen); + info.setElseBlock(bElse); + return mergeNestedIfNodes(info, merged); + } + + private IfInfo mergeNestedIfNodes(IfInfo info, List merged) { + BlockNode bThen = info.getThenBlock(); + BlockNode bElse = info.getElseBlock(); if (bThen == bElse) { return null; } - boolean found; - IfCondition condition = IfCondition.fromIfBlock(block); - IfInfo result = null; - do { - found = false; - for (BlockNode succ : block.getSuccessors()) { - BlockNode nestedIfBlock = getIfNode(succ); - if (nestedIfBlock != null && nestedIfBlock != block) { - IfNode nestedIfInsn = (IfNode) nestedIfBlock.getInstructions().get(0); - BlockNode nbThen = nestedIfInsn.getThenBlock(); - BlockNode nbElse = nestedIfInsn.getElseBlock(); - - boolean inverted = false; - IfCondition nestedCondition = IfCondition.fromIfNode(nestedIfInsn); - if (isPathExists(bElse, nestedIfBlock)) { - // else branch - if (!isEqualPaths(bThen, nbThen)) { - if (!isEqualPaths(bThen, nbElse)) { - // not connected conditions - break; - } - nestedIfInsn.invertCondition(); - inverted = true; - } - condition = IfCondition.merge(Mode.OR, condition, nestedCondition); - } else { - // then branch - if (!isEqualPaths(bElse, nbElse)) { - if (!isEqualPaths(bElse, nbThen)) { - // not connected conditions - break; - } - nestedIfInsn.invertCondition(); - inverted = true; - } - condition = IfCondition.merge(Mode.AND, condition, nestedCondition); - } - result = new IfInfo(); - result.setCondition(condition); - nestedIfBlock.getAttributes().add(AttributeFlag.SKIP); - skipSimplePath(bThen); - - if (merged != null) { - merged.add(nestedIfBlock); - } + BlockNode ifBlock = info.getIfnode(); + BlockNode nestedIfBlock = getNextIfBlock(ifBlock); + if (nestedIfBlock == null) { + return null; + } - // set new blocks - result.setIfnode(nestedIfBlock); - result.setThenBlock(inverted ? nbElse : nbThen); - result.setElseBlock(inverted ? nbThen : nbElse); + IfNode nestedIfInsn = (IfNode) nestedIfBlock.getInstructions().get(0); + IfCondition nestedCondition = IfCondition.fromIfNode(nestedIfInsn); + BlockNode nbThen = nestedIfInsn.getThenBlock(); + BlockNode nbElse = nestedIfInsn.getElseBlock(); - found = true; - block = nestedIfBlock; - bThen = result.getThenBlock(); - bElse = result.getElseBlock(); - break; + IfCondition condition = info.getCondition(); + boolean inverted = false; + if (isPathExists(bElse, nestedIfBlock)) { + // else branch + if (!isEqualPaths(bThen, nbThen)) { + if (!isEqualPaths(bThen, nbElse)) { + // not connected conditions + return null; + } + nestedIfInsn.invertCondition(); + inverted = true; + } + condition = IfCondition.merge(Mode.OR, condition, nestedCondition); + } else { + // then branch + if (!isEqualPaths(bElse, nbElse)) { + if (!isEqualPaths(bElse, nbThen)) { + // not connected conditions + return null; } + nestedIfInsn.invertCondition(); + inverted = true; } - } while (found); + condition = IfCondition.merge(Mode.AND, condition, nestedCondition); + } + if (merged != null) { + merged.add(nestedIfBlock); + } + nestedIfBlock.getAttributes().add(AttributeFlag.SKIP); + BlockNode blockToNestedIfBlock = BlockUtils.getNextBlockToPath(ifBlock, nestedIfBlock); + skipSimplePath(BlockUtils.selectOther(blockToNestedIfBlock, ifBlock.getCleanSuccessors())); + + IfInfo result = new IfInfo(); + result.setIfnode(nestedIfBlock); + result.setCondition(condition); + result.setThenBlock(inverted ? nbElse : nbThen); + result.setElseBlock(inverted ? nbThen : nbElse); + // search next nested if block + IfInfo next = mergeNestedIfNodes(result, merged); + if (next != null) { + return next; + } return result; } + private BlockNode getNextIfBlock(BlockNode block) { + for (BlockNode succ : block.getSuccessors()) { + BlockNode nestedIfBlock = getIfNode(succ); + if (nestedIfBlock != null && nestedIfBlock != block) { + return nestedIfBlock; + } + } + return null; + } + private static BlockNode getIfNode(BlockNode block) { if (block != null && !block.getAttributes().contains(AttributeType.LOOP)) { List insns = block.getInstructions(); diff --git a/jadx-core/src/main/java/jadx/core/utils/BlockUtils.java b/jadx-core/src/main/java/jadx/core/utils/BlockUtils.java index 37911082..5dfa538b 100644 --- a/jadx-core/src/main/java/jadx/core/utils/BlockUtils.java +++ b/jadx-core/src/main/java/jadx/core/utils/BlockUtils.java @@ -131,6 +131,23 @@ public class BlockUtils { return s.isEmpty() ? null : s.get(0); } + /** + * Return successor on path to 'pathEnd' block + */ + public static BlockNode getNextBlockToPath(BlockNode block, BlockNode pathEnd) { + List successors = block.getCleanSuccessors(); + if (successors.contains(pathEnd)) { + return pathEnd; + } + Set path = getAllPathsBlocks(block, pathEnd); + for (BlockNode s : successors) { + if (path.contains(s)) { + return s; + } + } + return null; + } + /** * Collect blocks from all possible execution paths from 'start' to 'end' */ diff --git a/jadx-core/src/main/java/jadx/core/utils/Utils.java b/jadx-core/src/main/java/jadx/core/utils/Utils.java index 4ab8e53c..ae36280d 100644 --- a/jadx-core/src/main/java/jadx/core/utils/Utils.java +++ b/jadx-core/src/main/java/jadx/core/utils/Utils.java @@ -35,18 +35,18 @@ public class Utils { case '/': case ';': case '$': + case ' ': + case ',': case '<': - case '[': sb.append('_'); break; - case ']': + case '[': sb.append('A'); break; + case ']': case '>': - case ',': - case ' ': case '?': case '*': break; diff --git a/jadx-core/src/test/java/jadx/api/InternalJadxTest.java b/jadx-core/src/test/java/jadx/api/InternalJadxTest.java index ff9592a0..8eb0d568 100644 --- a/jadx-core/src/test/java/jadx/api/InternalJadxTest.java +++ b/jadx-core/src/test/java/jadx/api/InternalJadxTest.java @@ -1,7 +1,6 @@ package jadx.api; import jadx.core.Jadx; -import jadx.core.dex.attributes.AttributeFlag; import jadx.core.dex.nodes.ClassNode; import jadx.core.dex.nodes.MethodNode; import jadx.core.dex.visitors.DepthTraverser; @@ -20,9 +19,11 @@ import java.util.jar.JarEntry; import java.util.jar.JarOutputStream; import static junit.framework.Assert.assertEquals; -import static junit.framework.Assert.assertFalse; import static junit.framework.Assert.assertNotNull; import static junit.framework.Assert.fail; +import static org.hamcrest.CoreMatchers.containsString; +import static org.hamcrest.CoreMatchers.not; +import static org.junit.Assert.assertThat; public abstract class InternalJadxTest extends TestUtils { @@ -67,7 +68,7 @@ public abstract class InternalJadxTest extends TestUtils { for (IDexTreeVisitor visitor : passes) { DepthTraverser.visit(visitor, cls); } - assertFalse(cls.getAttributes().contains(AttributeFlag.INCONSISTENT_CODE)); + assertThat(cls.getCode().toString(), not(containsString("inconsistent"))); return cls; } catch (Exception e) { e.printStackTrace(); -- GitLab