From d55969bc652c1b34421441e400f429795e3976a0 Mon Sep 17 00:00:00 2001 From: Skylot Date: Sat, 5 Sep 2015 20:55:37 +0300 Subject: [PATCH] core: fix some 'try/catch/finally' cases --- .../main/java/jadx/core/codegen/ClassGen.java | 6 +- .../main/java/jadx/core/codegen/InsnGen.java | 13 +- .../java/jadx/core/codegen/MethodGen.java | 2 +- .../jadx/core/dex/instructions/InsnType.java | 3 + .../dex/instructions/args/RegisterArg.java | 10 +- .../java/jadx/core/dex/nodes/MethodNode.java | 16 +- .../core/dex/visitors/ConstInlineVisitor.java | 4 +- .../blocksmaker/BlockFinallyExtract.java | 147 +++++++++++++++--- .../dex/visitors/regions/IfMakerHelper.java | 4 +- .../visitors/regions/ProcessVariables.java | 6 +- .../dex/visitors/regions/RegionMaker.java | 42 ++++- .../visitors/regions/RegionMakerVisitor.java | 5 +- .../dex/visitors/ssa/EliminatePhiNodes.java | 77 +++++++++ .../dex/visitors/ssa/LiveVarAnalysis.java | 13 ++ .../core/dex/visitors/ssa/SSATransform.java | 12 +- .../typeinference/PostTypeInference.java | 15 +- .../main/java/jadx/core/utils/DebugUtils.java | 6 +- .../java/jadx/core/utils/RegionUtils.java | 16 ++ .../trycatch/TestTryCatchFinally6.java | 46 ++++++ 19 files changed, 392 insertions(+), 51 deletions(-) create mode 100644 jadx-core/src/test/java/jadx/tests/integration/trycatch/TestTryCatchFinally6.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 201f53e0c..e807d6551 100644 --- a/jadx-core/src/main/java/jadx/core/codegen/ClassGen.java +++ b/jadx-core/src/main/java/jadx/core/codegen/ClassGen.java @@ -264,8 +264,10 @@ public class ClassGen { try { addMethod(code, mth); } catch (Exception e) { - String msg = ErrorsCounter.methodError(mth, "Method generation error", e); - code.startLine("/* " + msg + CodeWriter.NL + Utils.getStackTrace(e) + " */"); + code.newLine().add("/*"); + code.newLine().add(ErrorsCounter.methodError(mth, "Method generation error", e)); + code.newLine().add(Utils.getStackTrace(e)); + code.newLine().add("*/"); } } } diff --git a/jadx-core/src/main/java/jadx/core/codegen/InsnGen.java b/jadx-core/src/main/java/jadx/core/codegen/InsnGen.java index 299a25435..db6d52cd8 100644 --- a/jadx-core/src/main/java/jadx/core/codegen/InsnGen.java +++ b/jadx-core/src/main/java/jadx/core/codegen/InsnGen.java @@ -510,7 +510,18 @@ public class InsnGen { case NEW_INSTANCE: // only fallback - make new instance in constructor invoke fallbackOnlyInsn(insn); - code.add("new " + insn.getResult().getType()); + code.add("new ").add(insn.getResult().getType().toString()); + break; + + case PHI: + case MERGE: + fallbackOnlyInsn(insn); + code.add(insn.getType().toString()).add("("); + for (InsnArg insnArg : insn.getArguments()) { + addArg(code, insnArg); + code.add(' '); + } + code.add(")"); break; default: diff --git a/jadx-core/src/main/java/jadx/core/codegen/MethodGen.java b/jadx-core/src/main/java/jadx/core/codegen/MethodGen.java index 44708f51d..b38936901 100644 --- a/jadx-core/src/main/java/jadx/core/codegen/MethodGen.java +++ b/jadx-core/src/main/java/jadx/core/codegen/MethodGen.java @@ -167,7 +167,7 @@ public class MethodGen { if (cause != null) { code.newLine(); code.add("/*"); - code.startLine("Error: ").add(Utils.getStackTrace(cause)); + code.newLine().add("Error: ").add(Utils.getStackTrace(cause)); code.add("*/"); } } diff --git a/jadx-core/src/main/java/jadx/core/dex/instructions/InsnType.java b/jadx-core/src/main/java/jadx/core/dex/instructions/InsnType.java index c43eebd5e..7465f8261 100644 --- a/jadx-core/src/main/java/jadx/core/dex/instructions/InsnType.java +++ b/jadx-core/src/main/java/jadx/core/dex/instructions/InsnType.java @@ -65,6 +65,9 @@ public enum InsnType { ONE_ARG, PHI, + // merge all arguments in one + MERGE, + // TODO: now multidimensional arrays created using Array.newInstance function NEW_MULTIDIM_ARRAY } diff --git a/jadx-core/src/main/java/jadx/core/dex/instructions/args/RegisterArg.java b/jadx-core/src/main/java/jadx/core/dex/instructions/args/RegisterArg.java index 1b9695be7..a0ada798d 100644 --- a/jadx-core/src/main/java/jadx/core/dex/instructions/args/RegisterArg.java +++ b/jadx-core/src/main/java/jadx/core/dex/instructions/args/RegisterArg.java @@ -77,8 +77,14 @@ public class RegisterArg extends InsnArg implements Named { } public RegisterArg duplicate() { - RegisterArg dup = new RegisterArg(getRegNum(), getType()); - dup.setSVar(sVar); + return duplicate(getRegNum(), sVar); + } + + public RegisterArg duplicate(int regNum, SSAVar sVar) { + RegisterArg dup = new RegisterArg(regNum, getType()); + if (sVar != null) { + dup.setSVar(sVar); + } dup.copyAttributesFrom(this); return dup; } diff --git a/jadx-core/src/main/java/jadx/core/dex/nodes/MethodNode.java b/jadx-core/src/main/java/jadx/core/dex/nodes/MethodNode.java index d41240966..e0fd69017 100644 --- a/jadx-core/src/main/java/jadx/core/dex/nodes/MethodNode.java +++ b/jadx-core/src/main/java/jadx/core/dex/nodes/MethodNode.java @@ -554,9 +554,8 @@ public class MethodNode extends LineAttrNode implements ILoadable { return debugInfoOffset; } - public SSAVar makeNewSVar(int regNum, int[] versions, @NotNull RegisterArg arg) { - SSAVar var = new SSAVar(regNum, versions[regNum], arg); - versions[regNum]++; + public SSAVar makeNewSVar(int regNum, int version, @NotNull RegisterArg assignArg) { + SSAVar var = new SSAVar(regNum, version, assignArg); if (sVars.isEmpty()) { sVars = new ArrayList(); } @@ -564,6 +563,17 @@ public class MethodNode extends LineAttrNode implements ILoadable { return var; } + public int getNextSVarVersion(int regNum) { + int v = -1; + for (SSAVar sVar : sVars) { + if (sVar.getRegNum() == regNum) { + v = Math.max(v, sVar.getVersion()); + } + } + v++; + return v; + } + public void removeSVar(SSAVar var) { sVars.remove(var); } 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 6d59cbd94..100d0092c 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 @@ -103,7 +103,9 @@ public class ConstInlineVisitor extends AbstractVisitor { int replaceCount = 0; for (RegisterArg arg : use) { InsnNode useInsn = arg.getParentInsn(); - if (useInsn == null || useInsn.getType() == InsnType.PHI) { + if (useInsn == null + || useInsn.getType() == InsnType.PHI + || useInsn.getType() == InsnType.MERGE) { continue; } LiteralArg litArg; diff --git a/jadx-core/src/main/java/jadx/core/dex/visitors/blocksmaker/BlockFinallyExtract.java b/jadx-core/src/main/java/jadx/core/dex/visitors/blocksmaker/BlockFinallyExtract.java index a37a53a4c..bb9a3151e 100644 --- a/jadx-core/src/main/java/jadx/core/dex/visitors/blocksmaker/BlockFinallyExtract.java +++ b/jadx-core/src/main/java/jadx/core/dex/visitors/blocksmaker/BlockFinallyExtract.java @@ -162,7 +162,7 @@ public class BlockFinallyExtract extends AbstractVisitor { return false; } - /* 'finally' extract confirmed, run remove steps */ + // 'finally' extract confirmed, run remove steps LiveVarAnalysis laBefore = null; boolean runReMap = isReMapNeeded(removes); @@ -191,7 +191,7 @@ public class BlockFinallyExtract extends AbstractVisitor { RegisterArg resArg = me.getResult(); BlockNode succ = handlerBlock.getCleanSuccessors().get(0); - if (laAfter.isLive(succ.getId(), resArg.getRegNum())) { + if (laAfter.isLive(succ, resArg.getRegNum())) { // kill variable InsnNode kill = new InsnNode(InsnType.NOP, 0); kill.setResult(resArg); @@ -224,34 +224,59 @@ public class BlockFinallyExtract extends AbstractVisitor { BitSet processed = new BitSet(mth.getRegsCount()); for (BlocksRemoveInfo removeInfo : removes) { processed.clear(); - BlockNode insertBlock = removeInfo.getStart().getSecond(); + BlocksPair start = removeInfo.getStart(); + BlockNode insertBlockBefore = start.getFirst(); + BlockNode insertBlock = start.getSecond(); if (removeInfo.getRegMap().isEmpty() || insertBlock == null) { continue; } for (Map.Entry entry : removeInfo.getRegMap().entrySet()) { - RegisterArg from = entry.getKey(); - int regNum = from.getRegNum(); - if (!processed.get(regNum)) { - if (laBefore.isLive(insertBlock.getId(), regNum)) { + RegisterArg fromReg = entry.getKey(); + RegisterArg toReg = entry.getValue(); + int fromRegNum = fromReg.getRegNum(); + int toRegNum = toReg.getRegNum(); + if (!processed.get(fromRegNum)) { + boolean liveFromBefore = laBefore.isLive(insertBlockBefore, fromRegNum); + boolean liveFromAfter = laAfter.isLive(insertBlock, fromRegNum); + // boolean liveToBefore = laBefore.isLive(insertBlock, toRegNum); + boolean liveToAfter = laAfter.isLive(insertBlock, toRegNum); + if (liveToAfter && liveFromBefore) { + // merge 'to' and 'from' registers + InsnNode merge = new InsnNode(InsnType.MERGE, 2); + merge.setResult(toReg.duplicate()); + merge.addArg(toReg.duplicate()); + merge.addArg(fromReg.duplicate()); + injectInsn(mth, insertBlock, merge); + } else if (liveFromBefore) { // remap variable - RegisterArg to = entry.getValue(); InsnNode move = new InsnNode(InsnType.MOVE, 1); - move.setResult(to); - move.addArg(from); - insertBlock.getInstructions().add(move); - } else if (laAfter.isLive(insertBlock.getId(), regNum)) { + move.setResult(toReg.duplicate()); + move.addArg(fromReg.duplicate()); + injectInsn(mth, insertBlock, move); + } else if (liveFromAfter) { // kill variable InsnNode kill = new InsnNode(InsnType.NOP, 0); - kill.setResult(from); + kill.setResult(fromReg.duplicate()); kill.add(AFlag.REMOVE); - insertBlock.getInstructions().add(0, kill); + injectInsn(mth, insertBlock, kill); } - processed.set(regNum); + processed.set(fromRegNum); } } } } + private static void injectInsn(MethodNode mth, BlockNode insertBlock, InsnNode insn) { + insn.add(AFlag.SYNTHETIC); + if (insertBlock.getInstructions().isEmpty()) { + insertBlock.getInstructions().add(insn); + } else { + BlockNode succBlock = splitBlock(mth, insertBlock, 0); + BlockNode predBlock = succBlock.getPredecessors().get(0); + predBlock.getInstructions().add(insn); + } + } + private static boolean isReMapNeeded(List removes) { for (BlocksRemoveInfo removeInfo : removes) { if (!removeInfo.getRegMap().isEmpty()) { @@ -270,11 +295,41 @@ public class BlockFinallyExtract extends AbstractVisitor { if (removeInfo == null) { return null; } - if (removeInfo.getOuts().size() != 1) { - LOG.debug("Unexpected finally block outs count: {}", removeInfo.getOuts()); - return null; + Set outs = removeInfo.getOuts(); + if (outs.size() == 1) { + return removeInfo; } - return removeInfo; + // check if several 'return' blocks maps to one out + if (mergeReturns(mth, outs)) { + return removeInfo; + } + LOG.debug("Unexpected finally block outs count: {}", outs); + return null; + } + + private static boolean mergeReturns(MethodNode mth, Set outs) { + Set rightOuts = new HashSet(); + boolean allReturns = true; + for (BlocksPair outPair : outs) { + BlockNode first = outPair.getFirst(); + if (!first.isReturnBlock()) { + allReturns = false; + } + rightOuts.add(outPair.getSecond()); + } + if (!allReturns || rightOuts.size() != 1) { + return false; + } + Iterator it = outs.iterator(); + while (it.hasNext()) { + BlocksPair out = it.next(); + BlockNode returnBlock = out.getFirst(); + if (!returnBlock.contains(AFlag.ORIG_RETURN)) { + markForRemove(mth, returnBlock); + it.remove(); + } + } + return true; } private static BlocksRemoveInfo checkFromFirstBlock(BlockNode remBlock, BlockNode startBlock, BitSet bs) { @@ -453,7 +508,7 @@ public class BlockFinallyExtract extends AbstractVisitor { // change start block in removeInfo removeInfo.getProcessed().remove(removeInfo.getStart()); BlocksPair newStart = new BlocksPair(remBlock, startBlock); - removeInfo.setStart(newStart); +// removeInfo.setStart(newStart); removeInfo.getProcessed().add(newStart); } // split end block @@ -529,6 +584,11 @@ public class BlockFinallyExtract extends AbstractVisitor { return true; } + /** + * Split one block into connected 2 blocks with same connections. + * + * @return new successor block + */ private static BlockNode splitBlock(MethodNode mth, BlockNode block, int splitIndex) { BlockNode newBlock = BlockSplitter.startNewBlock(mth, -1); @@ -654,6 +714,53 @@ public class BlockFinallyExtract extends AbstractVisitor { markForRemove(mth, mb); edgeAttr.getBlocks().remove(mb); } + mergeSyntheticPredecessors(mth, origReturnBlock); + } + + private static void mergeSyntheticPredecessors(MethodNode mth, BlockNode block) { + List preds = new ArrayList(block.getPredecessors()); + Iterator it = preds.iterator(); + while (it.hasNext()) { + BlockNode predBlock = it.next(); + if (!predBlock.isSynthetic()) { + it.remove(); + } + } + if (preds.size() < 2) { + return; + } + BlockNode commonBlock = null; + for (BlockNode predBlock : preds) { + List successors = predBlock.getSuccessors(); + if (successors.size() != 2) { + return; + } + BlockNode cmnBlk = BlockUtils.selectOtherSafe(block, successors); + if (commonBlock == null) { + commonBlock = cmnBlk; + } else if (cmnBlk != commonBlock) { + return; + } + if (!predBlock.contains(AType.IGNORE_EDGE)) { + return; + } + } + if (commonBlock == null) { + return; + } + + // merge confirmed + BlockNode mergeBlock = null; + for (BlockNode predBlock : preds) { + if (mergeBlock == null) { + mergeBlock = predBlock; + continue; + } + for (BlockNode remPred : predBlock.getPredecessors()) { + connect(remPred, mergeBlock); + } + markForRemove(mth, predBlock); + } } private static BlockNode getFinallyOutBlock(List exitBlocks) { diff --git a/jadx-core/src/main/java/jadx/core/dex/visitors/regions/IfMakerHelper.java b/jadx-core/src/main/java/jadx/core/dex/visitors/regions/IfMakerHelper.java index 3d44007d0..e0d3dde35 100644 --- a/jadx-core/src/main/java/jadx/core/dex/visitors/regions/IfMakerHelper.java +++ b/jadx-core/src/main/java/jadx/core/dex/visitors/regions/IfMakerHelper.java @@ -24,7 +24,7 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; import static jadx.core.dex.visitors.regions.RegionMaker.isEqualPaths; -import static jadx.core.dex.visitors.regions.RegionMaker.isReturnBlocks; +import static jadx.core.dex.visitors.regions.RegionMaker.isEqualReturnBlocks; import static jadx.core.utils.BlockUtils.getNextBlock; import static jadx.core.utils.BlockUtils.isPathExists; @@ -277,7 +277,7 @@ public class IfMakerHelper { } private static boolean isSameBlocks(BlockNode first, BlockNode second) { - return first == second || isReturnBlocks(first, second); + return first == second || isEqualReturnBlocks(first, second); } static void confirmMerge(IfInfo info) { diff --git a/jadx-core/src/main/java/jadx/core/dex/visitors/regions/ProcessVariables.java b/jadx-core/src/main/java/jadx/core/dex/visitors/regions/ProcessVariables.java index eed93c967..f4fc2a21d 100644 --- a/jadx-core/src/main/java/jadx/core/dex/visitors/regions/ProcessVariables.java +++ b/jadx-core/src/main/java/jadx/core/dex/visitors/regions/ProcessVariables.java @@ -256,17 +256,17 @@ public class ProcessVariables extends AbstractVisitor { continue; } IRegion parent = region; - boolean declare = false; + boolean declared = false; while (parent != null) { if (canDeclareInRegion(u, region, regionsOrder)) { declareVar(region, u.getArg()); - declare = true; + declared = true; break; } region = parent; parent = region.getParent(); } - if (!declare) { + if (!declared) { declareVar(mth.getRegion(), u.getArg()); } } 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 340595d2f..3f0895cfc 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 @@ -12,6 +12,8 @@ import jadx.core.dex.instructions.SwitchNode; import jadx.core.dex.instructions.args.InsnArg; import jadx.core.dex.nodes.BlockNode; import jadx.core.dex.nodes.Edge; +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; @@ -899,7 +901,7 @@ public class RegionMaker { } } - public void processTryCatchBlocks(MethodNode mth) { + public IRegion processTryCatchBlocks(MethodNode mth) { Set tcs = new HashSet(); for (ExceptionHandler handler : mth.getExceptionHandlers()) { tcs.add(handler.getTryBlock()); @@ -935,6 +937,40 @@ public class RegionMaker { processExcHandler(handler, exits); } } + return processHandlersOutBlocks(mth, tcs); + } + + /** + * Search handlers successor blocks not included in any region. + */ + protected IRegion processHandlersOutBlocks(MethodNode mth, Set tcs) { + Set allRegionBlocks = new HashSet(); + RegionUtils.getAllRegionBlocks(mth.getRegion(), allRegionBlocks); + + Set succBlocks = new HashSet(); + for (TryCatchBlock tc : tcs) { + for (ExceptionHandler handler : tc.getHandlers()) { + IContainer region = handler.getHandlerRegion(); + if (region != null) { + IBlock lastBlock = RegionUtils.getLastBlock(region); + if (lastBlock instanceof BlockNode) { + succBlocks.addAll(((BlockNode) lastBlock).getSuccessors()); + } + RegionUtils.getAllRegionBlocks(region, allRegionBlocks); + } + } + } + succBlocks.removeAll(allRegionBlocks); + if (succBlocks.isEmpty()) { + return null; + } + Region excOutRegion = new Region(mth.getRegion()); + for (IBlock block : succBlocks) { + if (block instanceof BlockNode) { + excOutRegion.add(makeRegion((BlockNode) block, new RegionStack(mth))); + } + } + return excOutRegion; } private void processExcHandler(ExceptionHandler handler, Set exits) { @@ -980,7 +1016,7 @@ public class RegionMaker { if (b1 == null || b2 == null) { return false; } - return isReturnBlocks(b1, b2) || isSyntheticPath(b1, b2); + return isEqualReturnBlocks(b1, b2) || isSyntheticPath(b1, b2); } private static boolean isSyntheticPath(BlockNode b1, BlockNode b2) { @@ -989,7 +1025,7 @@ public class RegionMaker { return (n1 != b1 || n2 != b2) && isEqualPaths(n1, n2); } - public static boolean isReturnBlocks(BlockNode b1, BlockNode b2) { + public static boolean isEqualReturnBlocks(BlockNode b1, BlockNode b2) { if (!b1.isReturnBlock() || !b2.isReturnBlock()) { return false; } 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 3ab7638d7..e6995a85b 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 @@ -49,7 +49,10 @@ public class RegionMakerVisitor extends AbstractVisitor { mth.setRegion(rm.makeRegion(mth.getEnterBlock(), state)); if (!mth.isNoExceptionHandlers()) { - rm.processTryCatchBlocks(mth); + IRegion expOutBlock = rm.processTryCatchBlocks(mth); + if (expOutBlock != null) { + mth.getRegion().add(expOutBlock); + } } postProcessRegions(mth); diff --git a/jadx-core/src/main/java/jadx/core/dex/visitors/ssa/EliminatePhiNodes.java b/jadx-core/src/main/java/jadx/core/dex/visitors/ssa/EliminatePhiNodes.java index b686e96eb..20fccb790 100644 --- a/jadx-core/src/main/java/jadx/core/dex/visitors/ssa/EliminatePhiNodes.java +++ b/jadx-core/src/main/java/jadx/core/dex/visitors/ssa/EliminatePhiNodes.java @@ -2,12 +2,17 @@ package jadx.core.dex.visitors.ssa; import jadx.core.dex.attributes.AType; import jadx.core.dex.attributes.nodes.PhiListAttr; +import jadx.core.dex.instructions.InsnType; import jadx.core.dex.instructions.PhiInsn; +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.InsnNode; import jadx.core.dex.nodes.MethodNode; import jadx.core.dex.visitors.AbstractVisitor; +import jadx.core.utils.BlockUtils; import jadx.core.utils.exceptions.JadxException; +import jadx.core.utils.exceptions.JadxRuntimeException; import java.util.Iterator; import java.util.List; @@ -23,6 +28,7 @@ public class EliminatePhiNodes extends AbstractVisitor { if (mth.isNoCode()) { return; } + replaceMergeInstructions(mth); removePhiInstructions(mth); } @@ -50,4 +56,75 @@ public class EliminatePhiNodes extends AbstractVisitor { } LOG.warn("Phi node not removed: {}, mth: {}", phiInsn, mth); } + + private void replaceMergeInstructions(MethodNode mth) { + for (BlockNode block : mth.getBasicBlocks()) { + List insns = block.getInstructions(); + if (insns.isEmpty()) { + continue; + } + InsnNode insn = insns.get(0); + if (insn.getType() == InsnType.MERGE) { + replaceMerge(mth, block, insn); + } + } + } + + /** + * Replace 'MERGE' with 'PHI' insn. + */ + private void replaceMerge(MethodNode mth, BlockNode block, InsnNode insn) { + if (insn.getArgsCount() != 2) { + throw new JadxRuntimeException("Unexpected count of arguments in merge insn: " + insn); + } + RegisterArg oldArg = (RegisterArg) insn.getArg(1); + RegisterArg newArg = (RegisterArg) insn.getArg(0); + int newRegNum = newArg.getRegNum(); + if (oldArg.getRegNum() == newRegNum) { + throw new JadxRuntimeException("Unexpected register number in merge insn: " + insn); + } + SSAVar oldSVar = oldArg.getSVar(); + RegisterArg assignArg = oldSVar.getAssign(); + + InsnNode assignParentInsn = assignArg.getParentInsn(); + BlockNode assignBlock = BlockUtils.getBlockByInsn(mth, assignParentInsn); + if (assignBlock == null) { + throw new JadxRuntimeException("Unknown assign block for " + assignParentInsn); + } + BlockNode assignPred = null; + for (BlockNode pred : block.getPredecessors()) { + if (BlockUtils.isPathExists(assignBlock, pred)) { + assignPred = pred; + break; + } + } + if (assignPred == null) { + throw new JadxRuntimeException("Assign predecessor not found for " + assignBlock + " from " + block); + } + + // all checks passed + RegisterArg newAssignArg = oldArg.duplicate(newRegNum, null); + SSAVar newSVar = mth.makeNewSVar(newRegNum, mth.getNextSVarVersion(newRegNum), newAssignArg); + newSVar.setName(oldSVar.getName()); + newSVar.setType(assignArg.getType()); + + if (assignParentInsn != null) { + assignParentInsn.setResult(newAssignArg); + } + for (RegisterArg useArg : oldSVar.getUseList()) { + RegisterArg newUseArg = useArg.duplicate(newRegNum, newSVar); + InsnNode parentInsn = useArg.getParentInsn(); + if (parentInsn != null) { + newSVar.use(newUseArg); + parentInsn.replaceArg(useArg, newUseArg); + } + } + block.getInstructions().remove(0); + PhiInsn phiInsn = SSATransform.addPhi(mth, block, newRegNum); + phiInsn.setResult(insn.getResult()); + + phiInsn.bindArg(newAssignArg.duplicate(), assignPred); + phiInsn.bindArg(newArg.duplicate(), + BlockUtils.selectOtherSafe(assignPred, block.getPredecessors())); + } } diff --git a/jadx-core/src/main/java/jadx/core/dex/visitors/ssa/LiveVarAnalysis.java b/jadx-core/src/main/java/jadx/core/dex/visitors/ssa/LiveVarAnalysis.java index fcfa0c26b..b1caee067 100644 --- a/jadx-core/src/main/java/jadx/core/dex/visitors/ssa/LiveVarAnalysis.java +++ b/jadx-core/src/main/java/jadx/core/dex/visitors/ssa/LiveVarAnalysis.java @@ -10,7 +10,12 @@ import jadx.core.utils.exceptions.JadxRuntimeException; import java.util.BitSet; import java.util.List; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + public class LiveVarAnalysis { + private static final Logger LOG = LoggerFactory.getLogger(LiveVarAnalysis.class); + private final MethodNode mth; private BitSet[] uses; @@ -37,9 +42,17 @@ public class LiveVarAnalysis { } public boolean isLive(int blockId, int regNum) { + if (blockId >= liveIn.length) { + LOG.warn("LiveVarAnalysis: out of bounds block: {}, max: {}", blockId, liveIn.length); + return false; + } return liveIn[blockId].get(regNum); } + public boolean isLive(BlockNode block, int regNum) { + return isLive(block.getId(), regNum); + } + private void fillBasicBlockInfo() { for (BlockNode block : mth.getBasicBlocks()) { int blockId = block.getId(); diff --git a/jadx-core/src/main/java/jadx/core/dex/visitors/ssa/SSATransform.java b/jadx-core/src/main/java/jadx/core/dex/visitors/ssa/SSATransform.java index 8807aba16..037f3fa65 100644 --- a/jadx-core/src/main/java/jadx/core/dex/visitors/ssa/SSATransform.java +++ b/jadx-core/src/main/java/jadx/core/dex/visitors/ssa/SSATransform.java @@ -93,7 +93,7 @@ public class SSATransform extends AbstractVisitor { } } - private static void addPhi(MethodNode mth, BlockNode block, int regNum) { + public static PhiInsn addPhi(MethodNode mth, BlockNode block, int regNum) { PhiListAttr phiList = block.get(AType.PHI_LIST); if (phiList == null) { phiList = new PhiListAttr(); @@ -112,6 +112,7 @@ public class SSATransform extends AbstractVisitor { phiList.getList().add(phiInsn); phiInsn.setOffset(block.getStartOffset()); block.getInstructions().add(0, phiInsn); + return phiInsn; } private static void renameVariables(MethodNode mth) { @@ -124,13 +125,18 @@ public class SSATransform extends AbstractVisitor { // init method arguments for (RegisterArg arg : mth.getArguments(true)) { int regNum = arg.getRegNum(); - vars[regNum] = mth.makeNewSVar(regNum, versions, arg); + vars[regNum] = newSSAVar(mth, versions, arg, regNum); } BlockNode enterBlock = mth.getEnterBlock(); initPhiInEnterBlock(vars, enterBlock); renameVar(mth, vars, versions, enterBlock); } + private static SSAVar newSSAVar(MethodNode mth, int[] versions, RegisterArg arg, int regNum) { + int version = versions[regNum]++; + return mth.makeNewSVar(regNum, version, arg); + } + private static void initPhiInEnterBlock(SSAVar[] vars, BlockNode enterBlock) { PhiListAttr phiList = enterBlock.get(AType.PHI_LIST); if (phiList != null) { @@ -168,7 +174,7 @@ public class SSATransform extends AbstractVisitor { RegisterArg result = insn.getResult(); if (result != null) { int regNum = result.getRegNum(); - vars[regNum] = mth.makeNewSVar(regNum, vers, result); + vars[regNum] = newSSAVar(mth, vers, result, regNum); } } for (BlockNode s : block.getSuccessors()) { diff --git a/jadx-core/src/main/java/jadx/core/dex/visitors/typeinference/PostTypeInference.java b/jadx-core/src/main/java/jadx/core/dex/visitors/typeinference/PostTypeInference.java index dc3d56478..88bf45bed 100644 --- a/jadx-core/src/main/java/jadx/core/dex/visitors/typeinference/PostTypeInference.java +++ b/jadx-core/src/main/java/jadx/core/dex/visitors/typeinference/PostTypeInference.java @@ -3,7 +3,6 @@ package jadx.core.dex.visitors.typeinference; import jadx.core.dex.info.MethodInfo; import jadx.core.dex.instructions.IndexInsnNode; import jadx.core.dex.instructions.InvokeNode; -import jadx.core.dex.instructions.PhiInsn; import jadx.core.dex.instructions.args.ArgType; import jadx.core.dex.instructions.args.InsnArg; import jadx.core.dex.instructions.args.LiteralArg; @@ -101,11 +100,11 @@ public class PostTypeInference { return true; } - case PHI: { - PhiInsn phi = (PhiInsn) insn; - ArgType type = phi.getResult().getType(); + case PHI: + case MERGE: { + ArgType type = insn.getResult().getType(); if (!type.isTypeKnown()) { - for (InsnArg arg : phi.getArguments()) { + for (InsnArg arg : insn.getArguments()) { if (arg.getType().isTypeKnown()) { type = arg.getType(); break; @@ -113,11 +112,11 @@ public class PostTypeInference { } } boolean changed = false; - if (updateType(phi.getResult(), type)) { + if (updateType(insn.getResult(), type)) { changed = true; } - for (int i = 0; i < phi.getArgsCount(); i++) { - RegisterArg arg = phi.getArg(i); + for (int i = 0; i < insn.getArgsCount(); i++) { + RegisterArg arg = (RegisterArg) insn.getArg(i); if (updateType(arg, type)) { changed = true; } diff --git a/jadx-core/src/main/java/jadx/core/utils/DebugUtils.java b/jadx-core/src/main/java/jadx/core/utils/DebugUtils.java index 6bf0498b6..6b5433405 100644 --- a/jadx-core/src/main/java/jadx/core/utils/DebugUtils.java +++ b/jadx-core/src/main/java/jadx/core/utils/DebugUtils.java @@ -36,7 +36,11 @@ public class DebugUtils { private static final Logger LOG = LoggerFactory.getLogger(DebugUtils.class); public static void dump(MethodNode mth) { - File out = new File("test-graph-tmp"); + dump(mth, ""); + } + + public static void dump(MethodNode mth, String desc) { + File out = new File("test-graph" + desc + "-tmp"); DotGraphVisitor.dump(out).visit(mth); DotGraphVisitor.dumpRaw(out).visit(mth); DotGraphVisitor.dumpRegions(out).visit(mth); 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 48c9b9afb..eb32f040b 100644 --- a/jadx-core/src/main/java/jadx/core/utils/RegionUtils.java +++ b/jadx-core/src/main/java/jadx/core/utils/RegionUtils.java @@ -72,6 +72,22 @@ public class RegionUtils { } } + public static IBlock getLastBlock(IContainer container) { + if (container instanceof IBlock) { + return (IBlock) container; + } else if (container instanceof IBranchRegion) { + return null; + } else if (container instanceof IRegion) { + List blocks = ((IRegion) container).getSubBlocks(); + if (blocks.isEmpty()) { + return null; + } + return getLastBlock(blocks.get(blocks.size() - 1)); + } else { + throw new JadxRuntimeException(unknownContainerType(container)); + } + } + /** * Return true if last block in region has no successors */ diff --git a/jadx-core/src/test/java/jadx/tests/integration/trycatch/TestTryCatchFinally6.java b/jadx-core/src/test/java/jadx/tests/integration/trycatch/TestTryCatchFinally6.java new file mode 100644 index 000000000..a31ff2f97 --- /dev/null +++ b/jadx-core/src/test/java/jadx/tests/integration/trycatch/TestTryCatchFinally6.java @@ -0,0 +1,46 @@ +package jadx.tests.integration.trycatch; + +import jadx.core.dex.nodes.ClassNode; +import jadx.tests.api.IntegrationTest; + +import java.io.FileInputStream; +import java.io.IOException; +import java.io.InputStream; + +import org.junit.Test; + +import static jadx.tests.api.utils.JadxMatchers.containsLines; +import static org.junit.Assert.assertThat; + +public class TestTryCatchFinally6 extends IntegrationTest { + + public static class TestCls { + public static void test() throws IOException { + InputStream is = null; + try { + is = new FileInputStream("1.txt"); + } finally { + if (is != null) { + is.close(); + } + } + } + } + + @Test + public void test() { + ClassNode cls = getClassNode(TestCls.class); + String code = cls.getCode().toString(); + + assertThat(code, containsLines(2, + "InputStream is = null;", + "try {", + indent(1) + "is = new FileInputStream(\"1.txt\");", + "} finally {", + indent(1) + "if (is != null) {", + indent(2) + "is.close();", + indent(1) + "}", + "}" + )); + } +}