From 988628a2e7ed3a4d81c42438b9d0295c08b124ef Mon Sep 17 00:00:00 2001 From: Skylot Date: Sun, 9 Nov 2014 14:55:33 +0300 Subject: [PATCH] core: fix variable declaration used in several loops --- jadx-core/src/main/java/jadx/core/Jadx.java | 2 +- .../visitors/regions/ProcessVariables.java | 120 +++++++++++++----- .../integration/names/TestNameAssign2.java | 64 ++++++++++ 3 files changed, 152 insertions(+), 34 deletions(-) create mode 100644 jadx-core/src/test/java/jadx/tests/integration/names/TestNameAssign2.java diff --git a/jadx-core/src/main/java/jadx/core/Jadx.java b/jadx-core/src/main/java/jadx/core/Jadx.java index b8d1ed06c..f42c62bd6 100644 --- a/jadx-core/src/main/java/jadx/core/Jadx.java +++ b/jadx-core/src/main/java/jadx/core/Jadx.java @@ -82,7 +82,6 @@ public class Jadx { passes.add(new CodeShrinker()); passes.add(new SimplifyVisitor()); - passes.add(new ProcessVariables()); passes.add(new CheckRegions()); if (args.isCFGOutput()) { @@ -93,6 +92,7 @@ public class Jadx { passes.add(new ClassModifier()); passes.add(new PrepareForCodeGen()); passes.add(new LoopRegionVisitor()); + passes.add(new ProcessVariables()); } passes.add(new CodeGen(args)); return passes; 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 c3eaef466..173c5409e 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 @@ -15,6 +15,9 @@ import jadx.core.dex.nodes.InsnNode; import jadx.core.dex.nodes.MethodNode; import jadx.core.dex.regions.SwitchRegion; import jadx.core.dex.regions.conditions.IfRegion; +import jadx.core.dex.regions.loops.ForLoop; +import jadx.core.dex.regions.loops.LoopRegion; +import jadx.core.dex.regions.loops.LoopType; import jadx.core.dex.visitors.AbstractVisitor; import jadx.core.utils.RegionUtils; import jadx.core.utils.exceptions.JadxException; @@ -112,45 +115,77 @@ public class ProcessVariables extends AbstractVisitor { } } + private static class CollectUsageRegionVisitor extends TracedRegionVisitor { + private final List args; + private final Map usageMap; + + public CollectUsageRegionVisitor(Map usageMap) { + this.usageMap = usageMap; + args = new ArrayList(); + } + + @Override + public void processBlockTraced(MethodNode mth, IBlock container, IRegion curRegion) { + regionProcess(mth, curRegion); + int len = container.getInstructions().size(); + for (int i = 0; i < len; i++) { + InsnNode insn = container.getInstructions().get(i); + if (insn.contains(AFlag.SKIP)) { + continue; + } + args.clear(); + processInsn(insn, curRegion); + } + } + + private void regionProcess(MethodNode mth, IRegion region) { + if (region instanceof LoopRegion) { + LoopRegion loopRegion = (LoopRegion) region; + LoopType loopType = loopRegion.getType(); + if (loopType instanceof ForLoop) { + ForLoop forLoop = (ForLoop) loopType; + processInsn(forLoop.getInitInsn(), region); + processInsn(forLoop.getIncrInsn(), region); + } + } + } + + void processInsn(InsnNode insn, IRegion curRegion) { + if (insn == null) { + return; + } + // result + RegisterArg result = insn.getResult(); + if (result != null && result.isRegister()) { + Usage u = addToUsageMap(result, usageMap); + if (u.getArg() == null) { + u.setArg(result); + u.setArgRegion(curRegion); + } + u.getAssigns().add(curRegion); + } + // args + args.clear(); + insn.getRegisterArgs(args); + for (RegisterArg arg : args) { + Usage u = addToUsageMap(arg, usageMap); + u.getUseRegions().add(curRegion); + } + } + } + @Override public void visit(MethodNode mth) throws JadxException { if (mth.isNoCode()) { return; } - final Map usageMap = new LinkedHashMap(); for (RegisterArg arg : mth.getArguments(true)) { addToUsageMap(arg, usageMap); } // collect all variables usage - IRegionVisitor collect = new TracedRegionVisitor() { - @Override - public void processBlockTraced(MethodNode mth, IBlock container, IRegion curRegion) { - int len = container.getInstructions().size(); - List args = new ArrayList(); - for (int i = 0; i < len; i++) { - InsnNode insn = container.getInstructions().get(i); - // result - RegisterArg result = insn.getResult(); - if (result != null && result.isRegister()) { - Usage u = addToUsageMap(result, usageMap); - if (u.getArg() == null) { - u.setArg(result); - u.setArgRegion(curRegion); - } - u.getAssigns().add(curRegion); - } - // args - args.clear(); - insn.getRegisterArgs(args); - for (RegisterArg arg : args) { - Usage u = addToUsageMap(arg, usageMap); - u.getUseRegions().add(curRegion); - } - } - } - }; + IRegionVisitor collect = new CollectUsageRegionVisitor(usageMap); DepthRegionTraversal.traverseAll(mth, collect); // reduce assigns map @@ -189,10 +224,10 @@ public class ProcessVariables extends AbstractVisitor { for (IRegion assignRegion : u.getAssigns()) { if (u.getArgRegion() == assignRegion && canDeclareInRegion(u, assignRegion, regionsOrder)) { - u.getArg().getParentInsn().add(AFlag.DECLARE_VAR); - processVar(u.getArg()); - it.remove(); - break; + if (declareAtAssign(u)) { + it.remove(); + break; + } } } } @@ -239,7 +274,7 @@ public class ProcessVariables extends AbstractVisitor { } } - Usage addToUsageMap(RegisterArg arg, Map usageMap) { + private static Usage addToUsageMap(RegisterArg arg, Map usageMap) { Variable varId = new Variable(arg); Usage usage = usageMap.get(varId); if (usage == null) { @@ -260,6 +295,17 @@ public class ProcessVariables extends AbstractVisitor { return usage; } + private static boolean declareAtAssign(Usage u) { + RegisterArg arg = u.getArg(); + InsnNode parentInsn = arg.getParentInsn(); + if (!arg.equals(parentInsn.getResult())) { + return false; + } + parentInsn.add(AFlag.DECLARE_VAR); + processVar(arg); + return true; + } + private static void declareVar(IContainer region, RegisterArg arg) { DeclareVariablesAttr dv = region.get(AType.DECLARE_VARIABLES); if (dv == null) { @@ -308,6 +354,14 @@ public class ProcessVariables extends AbstractVisitor { LOG.debug("TODO: Not found order for region {} for {}", region, u); return false; } + // workaround for declare variables used in several loops + if (region instanceof LoopRegion) { + for (IRegion r : u.getAssigns()) { + if (!RegionUtils.isRegionContainsRegion(region, r)) { + return false; + } + } + } return isAllRegionsAfter(region, pos, u.getAssigns(), regionsOrder) && isAllRegionsAfter(region, pos, u.getUseRegions(), regionsOrder); } diff --git a/jadx-core/src/test/java/jadx/tests/integration/names/TestNameAssign2.java b/jadx-core/src/test/java/jadx/tests/integration/names/TestNameAssign2.java new file mode 100644 index 000000000..ad521de4e --- /dev/null +++ b/jadx-core/src/test/java/jadx/tests/integration/names/TestNameAssign2.java @@ -0,0 +1,64 @@ +package jadx.tests.integration.names; + +import jadx.core.dex.nodes.BlockNode; +import jadx.core.dex.nodes.ClassNode; +import jadx.core.dex.nodes.MethodNode; +import jadx.core.dex.visitors.ssa.LiveVarAnalysis; +import jadx.tests.api.IntegrationTest; + +import java.util.BitSet; +import java.util.Deque; +import java.util.LinkedList; +import java.util.List; + +import org.junit.Test; + +import static jadx.tests.api.utils.JadxMatchers.containsOne; +import static org.junit.Assert.assertThat; + +public class TestNameAssign2 extends IntegrationTest { + + public static class TestCls { + + private static void test(MethodNode mth, int regNum, LiveVarAnalysis la) { + List blocks = mth.getBasicBlocks(); + int blocksCount = blocks.size(); + BitSet hasPhi = new BitSet(blocksCount); + BitSet processed = new BitSet(blocksCount); + Deque workList = new LinkedList(); + + BitSet assignBlocks = la.getAssignBlocks(regNum); + for (int id = assignBlocks.nextSetBit(0); id >= 0; id = assignBlocks.nextSetBit(id + 1)) { + processed.set(id); + workList.add(blocks.get(id)); + } + while (!workList.isEmpty()) { + BlockNode block = workList.pop(); + BitSet domFrontier = block.getDomFrontier(); + for (int id = domFrontier.nextSetBit(0); id >= 0; id = domFrontier.nextSetBit(id + 1)) { + if (!hasPhi.get(id) && la.isLive(id, regNum)) { + BlockNode df = blocks.get(id); + addPhi(df, regNum); + hasPhi.set(id); + if (!processed.get(id)) { + processed.set(id); + workList.add(df); + } + } + } + } + } + + private static void addPhi(BlockNode df, int regNum) { + } + } + + @Test + public void test() { + ClassNode cls = getClassNode(TestCls.class); + String code = cls.getCode().toString(); + + // TODO: + assertThat(code, containsOne("int id;")); + } +}