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 46c18f4aba934811617c3d462e95fa54f3d7df9a..b916d17fe7b312e297ee2283c55b457d33a79d02 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 ef2d9a6df25b2ebe09e412ec06965fac65734df5..830dd70742f194d2617fc507edcdf063609a0ecc 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 b4e9a84bcd4f89e86d203e56deb5e68ec72d8cb2..39bdc6141d0eb84c46b6f295dae243f7be90d00f 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 3791108298d955e7437db324afaea490cf7d7815..5dfa538b9c612eb7e102c6642c3c714233d11e62 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 4ab8e53cd1e9f6843e6f6eff3ae6d953c39c4d53..ae36280d5590ead9198b4f815c5d7f09f4980082 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 ff9592a00ac52d793abd55b8dc207c02d9e1943a..8eb0d568c549129d08df574cc8693cad50d48758 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();