From 0a36bfb088ed67295b8e905ad45c531bee0d8b83 Mon Sep 17 00:00:00 2001 From: Skylot Date: Sat, 24 May 2014 19:55:29 +0400 Subject: [PATCH] core: fix try-catch blocks processing --- jadx-core/src/main/java/jadx/core/Jadx.java | 8 +- .../src/main/java/jadx/core/ProcessClass.java | 3 - .../dex/instructions/args/RegisterArg.java | 7 ++ .../core/dex/instructions/args/SSAVar.java | 6 +- .../java/jadx/core/dex/nodes/ClassNode.java | 2 +- .../java/jadx/core/dex/nodes/MethodNode.java | 8 +- .../jadx/core/dex/regions/LoopRegion.java | 2 +- .../jadx/core/dex/visitors/ClassModifier.java | 5 +- .../jadx/core/dex/visitors/ModVisitor.java | 4 +- .../core/dex/visitors/PrepareForCodeGen.java | 10 +++ .../regions/ProcessTryCatchRegions.java | 6 +- .../dex/visitors/regions/RegionMaker.java | 5 -- .../visitors/regions/RegionMakerVisitor.java | 7 +- .../core/dex/visitors/ssa/SSATransform.java | 2 +- .../main/java/jadx/core/utils/BlockUtils.java | 10 +-- .../jadx/core/utils/InstructionRemover.java | 2 +- .../test/java/jadx/api/InternalJadxTest.java | 76 +++++++++---------- .../internal/TestVariablesDefinitions.java | 3 +- .../internal/trycatch/TestTryCatch4.java | 45 +++++++++++ 19 files changed, 136 insertions(+), 75 deletions(-) create mode 100644 jadx-core/src/test/java/jadx/tests/internal/trycatch/TestTryCatch4.java diff --git a/jadx-core/src/main/java/jadx/core/Jadx.java b/jadx-core/src/main/java/jadx/core/Jadx.java index ad83e479a..8a3717d1a 100644 --- a/jadx-core/src/main/java/jadx/core/Jadx.java +++ b/jadx-core/src/main/java/jadx/core/Jadx.java @@ -68,10 +68,6 @@ public class Jadx { passes.add(new ModVisitor()); passes.add(new EnumVisitor()); - if (args.isCFGOutput()) { - passes.add(new DotGraphVisitor(outDir, false)); - } - passes.add(new CodeShrinker()); passes.add(new RegionMakerVisitor()); passes.add(new IfRegionVisitor()); @@ -89,6 +85,10 @@ public class Jadx { passes.add(new MethodInlineVisitor()); passes.add(new ClassModifier()); passes.add(new PrepareForCodeGen()); + + if (args.isCFGOutput()) { + passes.add(new DotGraphVisitor(outDir, false)); + } } passes.add(new CodeGen(args)); return passes; diff --git a/jadx-core/src/main/java/jadx/core/ProcessClass.java b/jadx-core/src/main/java/jadx/core/ProcessClass.java index af42dc5a0..f23255294 100644 --- a/jadx-core/src/main/java/jadx/core/ProcessClass.java +++ b/jadx-core/src/main/java/jadx/core/ProcessClass.java @@ -3,7 +3,6 @@ package jadx.core; import jadx.core.dex.nodes.ClassNode; import jadx.core.dex.visitors.DepthTraversal; import jadx.core.dex.visitors.IDexTreeVisitor; -import jadx.core.utils.exceptions.DecodeException; import java.util.List; @@ -22,8 +21,6 @@ public final class ProcessClass { for (IDexTreeVisitor visitor : passes) { DepthTraversal.visit(visitor, cls); } - } catch (DecodeException e) { - LOG.error("Decode exception: " + cls, e); } catch (Exception e) { LOG.error("Class process exception: " + cls, e); } finally { 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 4d52a0dd8..25ac1082b 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 @@ -66,6 +66,13 @@ public class RegisterArg extends InsnArg implements Named { return name; } + public boolean isNameEquals(InsnArg arg) { + if (name == null || !(arg instanceof Named)) { + return false; + } + return name.equals(((Named) arg).getName()); + } + public void setName(String newName) { this.name = newName; } diff --git a/jadx-core/src/main/java/jadx/core/dex/instructions/args/SSAVar.java b/jadx-core/src/main/java/jadx/core/dex/instructions/args/SSAVar.java index 61860832d..8dba96ead 100644 --- a/jadx-core/src/main/java/jadx/core/dex/instructions/args/SSAVar.java +++ b/jadx-core/src/main/java/jadx/core/dex/instructions/args/SSAVar.java @@ -47,6 +47,10 @@ public class SSAVar { return useList; } + public int getUseCount() { + return useList.size(); + } + public void use(RegisterArg arg) { mergeName(arg); if (arg.getSVar() != null) { @@ -81,7 +85,7 @@ public class SSAVar { if (!isUsedInPhi()) { return useList.size(); } - return useList.size() + usedInPhi.getResult().getSVar().getUseList().size(); + return useList.size() + usedInPhi.getResult().getSVar().getUseCount(); } public ArgType getType() { diff --git a/jadx-core/src/main/java/jadx/core/dex/nodes/ClassNode.java b/jadx-core/src/main/java/jadx/core/dex/nodes/ClassNode.java index 8e75eff1a..9c70ac94f 100644 --- a/jadx-core/src/main/java/jadx/core/dex/nodes/ClassNode.java +++ b/jadx-core/src/main/java/jadx/core/dex/nodes/ClassNode.java @@ -205,7 +205,7 @@ public class ClassNode extends LineAttrNode implements ILoadable { } @Override - public void load() throws DecodeException { + public void load() { for (MethodNode mth : getMethods()) { try { mth.load(); 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 5b16dea63..0ecb2912b 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 @@ -446,8 +446,12 @@ public class MethodNode extends LineAttrNode implements ILoadable { return handler; } - public List getExceptionHandlers() { - return Collections.unmodifiableList(exceptionHandlers); + public Iterable getExceptionHandlers() { + return exceptionHandlers; + } + + public boolean isNoExceptionHandlers() { + return exceptionHandlers.isEmpty(); } /** 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 38cbd4b90..7a294ea1d 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 @@ -79,7 +79,7 @@ public final class LoopRegion extends AbstractRegion { return false; } else { RegisterArg res = insn.getResult(); - if (res.getSVar().getUseList().size() > 1) { + if (res.getSVar().getUseCount() > 1) { return false; } boolean found = false; diff --git a/jadx-core/src/main/java/jadx/core/dex/visitors/ClassModifier.java b/jadx-core/src/main/java/jadx/core/dex/visitors/ClassModifier.java index 55f24e6b5..f03fd1a5c 100644 --- a/jadx-core/src/main/java/jadx/core/dex/visitors/ClassModifier.java +++ b/jadx-core/src/main/java/jadx/core/dex/visitors/ClassModifier.java @@ -99,11 +99,10 @@ public class ClassModifier extends AbstractVisitor { mth.removeFirstArgument(); InstructionRemover.remove(mth, block, insn); // other arg usage -> wrap with IGET insn - List useList = arg.getSVar().getUseList(); - if (useList.size() > 0) { + if (arg.getSVar().getUseCount() != 0) { InsnNode iget = new IndexInsnNode(InsnType.IGET, fieldInfo, 1); iget.addArg(insn.getArg(1)); - for (InsnArg insnArg : useList) { + for (InsnArg insnArg : arg.getSVar().getUseList()) { insnArg.wrapInstruction(iget); } } diff --git a/jadx-core/src/main/java/jadx/core/dex/visitors/ModVisitor.java b/jadx-core/src/main/java/jadx/core/dex/visitors/ModVisitor.java index 546ad6166..7e9126385 100644 --- a/jadx-core/src/main/java/jadx/core/dex/visitors/ModVisitor.java +++ b/jadx-core/src/main/java/jadx/core/dex/visitors/ModVisitor.java @@ -229,7 +229,7 @@ public class ModVisitor extends AbstractVisitor { if (blockInsns.size() > 0) { InsnNode insn = blockInsns.get(0); if (insn.getType() == InsnType.MOVE_EXCEPTION - && insn.getResult().getSVar().getUseList().isEmpty()) { + && insn.getResult().getSVar().getUseCount() == 0) { InstructionRemover.remove(mth, block, 0); } } @@ -257,7 +257,7 @@ public class ModVisitor extends AbstractVisitor { * In argument not used in other instructions then remove assign instruction. */ private static void removeInsnForArg(InstructionRemover remover, RegisterArg arg) { - if (arg.getSVar().getUseList().isEmpty() + if (arg.getSVar().getUseCount() == 0 && arg.getAssignInsn() != null) { remover.add(arg.getAssignInsn()); } diff --git a/jadx-core/src/main/java/jadx/core/dex/visitors/PrepareForCodeGen.java b/jadx-core/src/main/java/jadx/core/dex/visitors/PrepareForCodeGen.java index 19ab15e79..973116471 100644 --- a/jadx-core/src/main/java/jadx/core/dex/visitors/PrepareForCodeGen.java +++ b/jadx-core/src/main/java/jadx/core/dex/visitors/PrepareForCodeGen.java @@ -54,6 +54,16 @@ public class PrepareForCodeGen extends AbstractVisitor { it.remove(); } break; + + case MOVE: + // remove redundant moves: + // unused result and same args names (a = a;) + RegisterArg result = insn.getResult(); + if (result.getSVar().getUseCount() == 0 + && result.isNameEquals(insn.getArg(0))) { + it.remove(); + } + break; } } } diff --git a/jadx-core/src/main/java/jadx/core/dex/visitors/regions/ProcessTryCatchRegions.java b/jadx-core/src/main/java/jadx/core/dex/visitors/regions/ProcessTryCatchRegions.java index 37d226c97..77e0bd98e 100644 --- a/jadx-core/src/main/java/jadx/core/dex/visitors/regions/ProcessTryCatchRegions.java +++ b/jadx-core/src/main/java/jadx/core/dex/visitors/regions/ProcessTryCatchRegions.java @@ -40,7 +40,7 @@ public class ProcessTryCatchRegions extends AbstractRegionVisitor { private final Map tryBlocksMap = new HashMap(2); public ProcessTryCatchRegions(MethodNode mth) { - if (mth.isNoCode() || mth.getExceptionHandlers().isEmpty()) { + if (mth.isNoCode() || mth.isNoExceptionHandlers()) { return; } @@ -84,7 +84,7 @@ public class ProcessTryCatchRegions extends AbstractRegionVisitor { TryCatchBlock prevTB = tryBlocksMap.put(domBlock, tb); if (prevTB != null) { - LOG.info("!!! TODO merge try blocks in " + mth); + LOG.info("!!! TODO: merge try blocks in " + mth); } } @@ -134,7 +134,7 @@ public class ProcessTryCatchRegions extends AbstractRegionVisitor { } } } - if (newRegion.getSubBlocks().size() != 0) { + if (!newRegion.getSubBlocks().isEmpty()) { if (DEBUG) { LOG.debug("ProcessTryCatchRegions mark: {}", newRegion); } 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 c20f84053..d797cc25f 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 @@ -685,11 +685,6 @@ public class RegionMaker { LOG.debug(ErrorsCounter.formatErrorMsg(mth, "No exception handler block: " + handler)); return; } - - BlockNode out = BlockUtils.traverseWhileDominates(start, start); - if (out != null) { - stack.addExit(out); - } // TODO extract finally part which exists in all handlers from same try block // TODO add blocks common for several handlers to some region handler.setHandlerRegion(makeRegion(start, stack)); 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 ab0da16a6..74c8faa23 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 @@ -34,7 +34,7 @@ public class RegionMakerVisitor extends AbstractVisitor { // fill region structure mth.setRegion(rm.makeRegion(mth.getEnterBlock(), state)); - if (!mth.getExceptionHandlers().isEmpty()) { + if (!mth.isNoExceptionHandlers()) { state = new RegionStack(mth); for (ExceptionHandler handler : mth.getExceptionHandlers()) { rm.processExcHandler(handler, state); @@ -46,8 +46,9 @@ public class RegionMakerVisitor extends AbstractVisitor { private static void postProcessRegions(MethodNode mth) { // make try-catch regions - DepthRegionTraversal.traverse(mth, new ProcessTryCatchRegions(mth)); - + if (!mth.isNoExceptionHandlers()) { + DepthRegionTraversal.traverse(mth, new ProcessTryCatchRegions(mth)); + } // merge conditions in loops if (mth.getLoopsCount() != 0) { DepthRegionTraversal.traverseAll(mth, new AbstractRegionVisitor() { 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 e846c713e..63816a93e 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 @@ -146,7 +146,7 @@ public class SSATransform extends AbstractVisitor { List insnToRemove = new ArrayList(); for (SSAVar var : mth.getSVars()) { // phi result not used - if (var.getUseList().isEmpty()) { + if (var.getUseCount() == 0) { InsnNode assignInsn = var.getAssign().getParentInsn(); if (assignInsn != null && assignInsn.getType() == InsnType.PHI) { insnToRemove.add((PhiInsn) assignInsn); 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 8e5f5fd90..ba01e22f0 100644 --- a/jadx-core/src/main/java/jadx/core/utils/BlockUtils.java +++ b/jadx-core/src/main/java/jadx/core/utils/BlockUtils.java @@ -229,14 +229,14 @@ public class BlockUtils { } /** - * Search for first node which not dominated by block, starting from child + * Search for first node which not dominated by dom, starting from start */ - public static BlockNode traverseWhileDominates(BlockNode block, BlockNode child) { - for (BlockNode node : child.getCleanSuccessors()) { - if (!node.isDominator(block)) { + public static BlockNode traverseWhileDominates(BlockNode dom, BlockNode start) { + for (BlockNode node : start.getCleanSuccessors()) { + if (!node.isDominator(dom)) { return node; } else { - BlockNode out = traverseWhileDominates(block, node); + BlockNode out = traverseWhileDominates(dom, node); if (out != null) { return out; } diff --git a/jadx-core/src/main/java/jadx/core/utils/InstructionRemover.java b/jadx-core/src/main/java/jadx/core/utils/InstructionRemover.java index 7f9bf0c1f..39bc385cc 100644 --- a/jadx-core/src/main/java/jadx/core/utils/InstructionRemover.java +++ b/jadx-core/src/main/java/jadx/core/utils/InstructionRemover.java @@ -59,7 +59,7 @@ public class InstructionRemover { public static void unbindInsn(MethodNode mth, InsnNode insn) { RegisterArg r = insn.getResult(); if (r != null && r.getSVar() != null) { - if (Consts.DEBUG && !r.getSVar().getUseList().isEmpty()) { + if (Consts.DEBUG && r.getSVar().getUseCount() != 0) { LOG.debug("Unbind insn with result: {}", insn); } mth.removeSVar(r.getSVar()); diff --git a/jadx-core/src/test/java/jadx/api/InternalJadxTest.java b/jadx-core/src/test/java/jadx/api/InternalJadxTest.java index 7fe343a3e..1ad278c65 100644 --- a/jadx-core/src/test/java/jadx/api/InternalJadxTest.java +++ b/jadx-core/src/test/java/jadx/api/InternalJadxTest.java @@ -1,6 +1,8 @@ package jadx.api; import jadx.core.Jadx; +import jadx.core.dex.attributes.AFlag; +import jadx.core.dex.attributes.AType; import jadx.core.dex.nodes.ClassNode; import jadx.core.dex.nodes.MethodNode; import jadx.core.dex.visitors.DepthTraversal; @@ -23,6 +25,7 @@ import static org.hamcrest.CoreMatchers.not; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertThat; +import static org.junit.Assert.assertTrue; import static org.junit.Assert.fail; public abstract class InternalJadxTest extends TestUtils { @@ -33,48 +36,45 @@ public abstract class InternalJadxTest extends TestUtils { protected String outDir = "test-out-tmp"; public ClassNode getClassNode(Class clazz) { + Decompiler d = new Decompiler(); try { - File temp = getJarForClass(clazz); - Decompiler d = new Decompiler(); - try { - d.loadFile(temp); - } catch (Exception e) { - fail(e.getMessage()); - } - List classes = d.getRoot().getClasses(false); - String clsName = clazz.getName(); - ClassNode cls = null; - for (ClassNode aClass : classes) { - if (aClass.getFullName().equals(clsName)) { - cls = aClass; - } - } - assertNotNull("Class not found: " + clsName, cls); - - assertEquals(cls.getFullName(), clazz.getName()); - - cls.load(); - List passes = Jadx.getPassesList(new DefaultJadxArgs() { - @Override - public boolean isCFGOutput() { - return outputCFG; - } - - @Override - public boolean isRawCFGOutput() { - return outputCFG; - } - }, new File(outDir)); - for (IDexTreeVisitor visitor : passes) { - DepthTraversal.visit(visitor, cls); - } - assertThat(cls.getCode().toString(), not(containsString("inconsistent"))); - return cls; + d.loadFile(getJarForClass(clazz)); } catch (Exception e) { - e.printStackTrace(); fail(e.getMessage()); - return null; } + String clsName = clazz.getName(); + ClassNode cls = d.getRoot().searchClassByName(clsName); + assertNotNull("Class not found: " + clsName, cls); + assertEquals(cls.getFullName(), clazz.getName()); + + cls.load(); + for (IDexTreeVisitor visitor : getPasses()) { + DepthTraversal.visit(visitor, cls); + } + // don't unload class + + assertTrue("Inconsistent cls: " + cls, + !cls.contains(AFlag.INCONSISTENT_CODE) && !cls.contains(AType.JADX_ERROR)); + for (MethodNode mthNode : cls.getMethods()) { + assertTrue("Inconsistent method: " + mthNode, + !mthNode.contains(AFlag.INCONSISTENT_CODE) && !mthNode.contains(AType.JADX_ERROR)); + } + assertThat(cls.getCode().toString(), not(containsString("inconsistent"))); + return cls; + } + + protected List getPasses() { + return Jadx.getPassesList(new DefaultJadxArgs() { + @Override + public boolean isCFGOutput() { + return outputCFG; + } + + @Override + public boolean isRawCFGOutput() { + return outputCFG; + } + }, new File(outDir)); } protected MethodNode getMethod(ClassNode cls, String method) { diff --git a/jadx-core/src/test/java/jadx/tests/internal/TestVariablesDefinitions.java b/jadx-core/src/test/java/jadx/tests/internal/TestVariablesDefinitions.java index 7c0ef4731..bcbec24eb 100644 --- a/jadx-core/src/test/java/jadx/tests/internal/TestVariablesDefinitions.java +++ b/jadx-core/src/test/java/jadx/tests/internal/TestVariablesDefinitions.java @@ -4,7 +4,6 @@ import jadx.api.InternalJadxTest; import jadx.core.dex.nodes.ClassNode; import jadx.core.dex.visitors.DepthTraversal; import jadx.core.dex.visitors.IDexTreeVisitor; -import jadx.core.utils.exceptions.DecodeException; import java.util.Iterator; import java.util.List; @@ -30,7 +29,7 @@ public class TestVariablesDefinitions extends InternalJadxTest { while (iterator.hasNext()) { DepthTraversal.visit(iterator.next(), cls); } - } catch (DecodeException e) { + } catch (Exception e) { LOG.error("Decode exception: " + cls, e); } } diff --git a/jadx-core/src/test/java/jadx/tests/internal/trycatch/TestTryCatch4.java b/jadx-core/src/test/java/jadx/tests/internal/trycatch/TestTryCatch4.java new file mode 100644 index 000000000..44de0f7c0 --- /dev/null +++ b/jadx-core/src/test/java/jadx/tests/internal/trycatch/TestTryCatch4.java @@ -0,0 +1,45 @@ +package jadx.tests.internal.trycatch; + +import jadx.api.InternalJadxTest; +import jadx.core.dex.nodes.ClassNode; + +import java.io.File; +import java.io.FileNotFoundException; +import java.io.FileOutputStream; + +import org.junit.Test; + +import static org.hamcrest.CoreMatchers.containsString; +import static org.hamcrest.CoreMatchers.not; +import static org.junit.Assert.assertThat; + +public class TestTryCatch4 extends InternalJadxTest { + + public static class TestCls { + private Object test(Object obj) { + FileOutputStream output = null; + try { + output = new FileOutputStream(new File("f")); + return new Object(); + } catch (FileNotFoundException e) { + System.out.println("Exception"); + return null; + } + } + } + + @Test + public void test() { + ClassNode cls = getClassNode(TestCls.class); + String code = cls.getCode().toString(); + System.out.println(code); + + assertThat(code, containsString("try {")); + assertThat(code, containsString("output = new FileOutputStream(new File(\"f\"));")); + assertThat(code, containsString("return new Object();")); + assertThat(code, containsString("} catch (FileNotFoundException e) {")); + assertThat(code, containsString("System.out.println(\"Exception\");")); + assertThat(code, containsString("return null;")); + assertThat(code, not(containsString("output = output;"))); + } +}