diff --git a/jadx-core/src/main/java/jadx/core/codegen/TypeGen.java b/jadx-core/src/main/java/jadx/core/codegen/TypeGen.java index b14877102..dfb9441d3 100644 --- a/jadx-core/src/main/java/jadx/core/codegen/TypeGen.java +++ b/jadx-core/src/main/java/jadx/core/codegen/TypeGen.java @@ -4,6 +4,7 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; import jadx.api.JadxArgs; +import jadx.core.deobf.NameMapper; import jadx.core.dex.instructions.args.ArgType; import jadx.core.dex.instructions.args.PrimitiveType; import jadx.core.dex.nodes.IDexNode; @@ -57,7 +58,11 @@ public class TypeGen { case BOOLEAN: return lit == 0 ? "false" : "true"; case CHAR: - return stringUtils.unescapeChar((char) lit); + char ch = (char) lit; + if (!NameMapper.isPrintableChar(ch)) { + return Integer.toString(ch); + } + return stringUtils.unescapeChar(ch); case BYTE: return formatByte((byte) lit); case SHORT: @@ -171,5 +176,4 @@ public class TypeGen { } return Float.toString(f) + "f"; } - } 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 eaa6b4a50..1bf06c41f 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 @@ -1,7 +1,6 @@ package jadx.core.dex.visitors.regions; import java.util.ArrayList; -import java.util.HashMap; import java.util.Iterator; import java.util.LinkedHashMap; import java.util.LinkedHashSet; @@ -10,9 +9,6 @@ import java.util.Map; import java.util.Map.Entry; import java.util.Set; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; - import jadx.core.dex.attributes.AFlag; import jadx.core.dex.attributes.AType; import jadx.core.dex.attributes.nodes.DeclareVariablesAttr; @@ -21,7 +17,6 @@ import jadx.core.dex.instructions.args.ArgType; import jadx.core.dex.instructions.args.RegisterArg; import jadx.core.dex.instructions.args.VarName; import jadx.core.dex.nodes.IBlock; -import jadx.core.dex.nodes.IBranchRegion; import jadx.core.dex.nodes.IContainer; import jadx.core.dex.nodes.IRegion; import jadx.core.dex.nodes.InsnNode; @@ -34,7 +29,6 @@ import jadx.core.utils.RegionUtils; import jadx.core.utils.exceptions.JadxException; public class ProcessVariables extends AbstractVisitor { - private static final Logger LOG = LoggerFactory.getLogger(ProcessVariables.class); private static class Variable { private final int regNum; @@ -213,16 +207,13 @@ public class ProcessVariables extends AbstractVisitor { return; } - Map regionsOrder = new HashMap<>(); - calculateOrder(mth.getRegion(), regionsOrder, 0, true); - for (Iterator> it = usageMap.entrySet().iterator(); it.hasNext(); ) { Entry entry = it.next(); Usage u = entry.getValue(); // check if variable can be declared at current assigns for (IRegion assignRegion : u.getAssigns()) { if (u.getArgRegion() == assignRegion - && canDeclareInRegion(u, assignRegion, regionsOrder) + && canDeclareInRegion(u, assignRegion) && declareAtAssign(u)) { it.remove(); break; @@ -258,7 +249,7 @@ public class ProcessVariables extends AbstractVisitor { IRegion parent = region; boolean declared = false; while (parent != null) { - if (canDeclareInRegion(u, region, regionsOrder)) { + if (canDeclareInRegion(u, region)) { declareVar(region, u.getArg()); declared = true; break; @@ -311,38 +302,7 @@ public class ProcessVariables extends AbstractVisitor { dv.addVar(arg); } - private static int calculateOrder(IContainer container, Map regionsOrder, int id, boolean inc) { - if (!(container instanceof IRegion)) { - return id; - } - IRegion region = (IRegion) container; - Integer previous = regionsOrder.put(region, id); - if (previous != null) { - return id; - } - for (IContainer c : region.getSubBlocks()) { - if (c instanceof IBranchRegion) { - // on branch set for all inner regions same order id - id = calculateOrder(c, regionsOrder, inc ? id + 1 : id, false); - } else { - List handlers = RegionUtils.getExcHandlersForRegion(c); - if (!handlers.isEmpty()) { - for (IContainer handler : handlers) { - id = calculateOrder(handler, regionsOrder, inc ? id + 1 : id, inc); - } - } - id = calculateOrder(c, regionsOrder, inc ? id + 1 : id, inc); - } - } - return id; - } - - private static boolean canDeclareInRegion(Usage u, IRegion region, Map regionsOrder) { - Integer pos = regionsOrder.get(region); - if (pos == null) { - LOG.debug("TODO: Not found order for region {} for {}", region, u); - return false; - } + private static boolean canDeclareInRegion(Usage u, IRegion region) { // workaround for declare variables used in several loops if (region instanceof LoopRegion) { for (IRegion r : u.getAssigns()) { @@ -355,32 +315,12 @@ public class ProcessVariables extends AbstractVisitor { if (region.contains(AFlag.ELSE_IF_CHAIN)) { return false; } - return isAllRegionsAfter(region, pos, u.getAssigns(), regionsOrder) - && isAllRegionsAfter(region, pos, u.getUseRegions(), regionsOrder); + // TODO: make index for faster search + return isAllRegionsAfter(region, u.getAssigns()) + && isAllRegionsAfter(region, u.getUseRegions()); } - private static boolean isAllRegionsAfter(IRegion region, int pos, - Set regions, Map regionsOrder) { - for (IRegion r : regions) { - if (r == region) { - continue; - } - Integer rPos = regionsOrder.get(r); - if (rPos == null) { - LOG.debug("TODO: Not found order for region {} in {}", r, regionsOrder); - return false; - } - if (pos > rPos) { - return false; - } - if (pos == rPos) { - return isAllRegionsAfterRecursive(region, regions); - } - } - return true; - } - - private static boolean isAllRegionsAfterRecursive(IRegion region, Set others) { + private static boolean isAllRegionsAfter(IRegion region, Set others) { for (IRegion r : others) { if (!RegionUtils.isRegionContainsRegion(region, r)) { return false; 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 c7014045d..fc5e6c69f 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 @@ -192,10 +192,9 @@ public class RegionMaker { if (loopExit != null) { // add 'break' instruction before path cross between main loop exit and sub-exit for (Edge exitEdge : loop.getExitEdges()) { - if (!exitBlocks.contains(exitEdge.getSource())) { - continue; + if (exitBlocks.contains(exitEdge.getSource())) { + insertBreak(stack, loopExit, exitEdge); } - insertBreak(stack, loopExit, exitEdge); } } } @@ -207,7 +206,8 @@ public class RegionMaker { loopStart.remove(AType.LOOP); loop.getEnd().add(AFlag.SKIP); stack.addExit(loop.getEnd()); - loopRegion.setBody(makeRegion(loopStart, stack)); + Region body = makeRegion(loopStart, stack); + loopRegion.setBody(body); loopStart.addAttr(AType.LOOP, loop); loop.getEnd().remove(AFlag.SKIP); } else { @@ -303,16 +303,31 @@ public class RegionMaker { loopStart.remove(AType.LOOP); stack.push(loopRegion); - BlockNode loopExit = null; + BlockNode out = null; // insert 'break' for exits List exitEdges = loop.getExitEdges(); - for (Edge exitEdge : exitEdges) { + if (exitEdges.size() == 1) { + Edge exitEdge = exitEdges.get(0); BlockNode exit = exitEdge.getTarget(); if (insertBreak(stack, exit, exitEdge)) { BlockNode nextBlock = getNextBlock(exit); if (nextBlock != null) { stack.addExit(nextBlock); - loopExit = nextBlock; + out = nextBlock; + } + } + } else { + for (Edge exitEdge : exitEdges) { + BlockNode exit = exitEdge.getTarget(); + List blocks = BlockUtils.bitSetToBlocks(mth, exit.getDomFrontier()); + for (BlockNode block : blocks) { + if (BlockUtils.isPathExists(exit, block)) { + stack.addExit(block); + insertBreak(stack, block, exitEdge); + out = block; + } else { + insertBreak(stack, exit, exitEdge); + } } } } @@ -326,13 +341,13 @@ public class RegionMaker { } loopRegion.setBody(body); - if (loopExit == null) { + if (out == null) { BlockNode next = getNextBlock(loopEnd); - loopExit = RegionUtils.isRegionContainsBlock(body, next) ? null : next; + out = RegionUtils.isRegionContainsBlock(body, next) ? null : next; } stack.pop(); loopStart.addAttr(AType.LOOP, loop); - return loopExit; + return out; } private boolean inExceptionHandlerBlocks(BlockNode loopEnd) { @@ -463,7 +478,7 @@ public class RegionMaker { } private static boolean canInsertContinue(BlockNode pred, List predecessors, BlockNode loopEnd, - Set loopExitNodes) { + Set loopExitNodes) { if (!pred.contains(AFlag.SYNTHETIC) || BlockUtils.checkLastInsnType(pred, InsnType.CONTINUE)) { return false; @@ -553,8 +568,8 @@ public class RegionMaker { /** * Traverse from monitor-enter thru successors and collect blocks contains monitor-exit */ - private static void traverseMonitorExits(SynchronizedRegion region, InsnArg arg, BlockNode block, - Set exits, Set visited) { + private static void traverseMonitorExits(SynchronizedRegion region, InsnArg arg, BlockNode block, Set exits, + Set visited) { visited.add(block); for (InsnNode insn : block.getInstructions()) { if (insn.getType() == InsnType.MONITOR_EXIT @@ -828,7 +843,7 @@ public class RegionMaker { } private boolean isBadCasesOrder(Map> blocksMap, - Map fallThroughCases) { + Map fallThroughCases) { BlockNode nextCaseBlock = null; for (BlockNode caseBlock : blocksMap.keySet()) { if (nextCaseBlock != null && !caseBlock.equals(nextCaseBlock)) { 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 b3e7e8cfe..401766594 100644 --- a/jadx-core/src/main/java/jadx/core/utils/DebugUtils.java +++ b/jadx-core/src/main/java/jadx/core/utils/DebugUtils.java @@ -99,7 +99,7 @@ public class DebugUtils { CodeWriter code = new CodeWriter(); ig.makeInsn(insn, code); String insnStr = code.toString().substring(CodeWriter.NL.length()); - LOG.debug("{}> {}", indent, insnStr); + LOG.debug("{}> {}\t{}", indent, insnStr, insn.getAttributesString()); } catch (CodegenException e) { LOG.debug("{}>!! {}", indent, insn); } diff --git a/jadx-core/src/test/java/jadx/tests/integration/loops/TestBreakInLoop2.java b/jadx-core/src/test/java/jadx/tests/integration/loops/TestBreakInLoop2.java index 181f02c9a..8f964ac2a 100644 --- a/jadx-core/src/test/java/jadx/tests/integration/loops/TestBreakInLoop2.java +++ b/jadx-core/src/test/java/jadx/tests/integration/loops/TestBreakInLoop2.java @@ -9,6 +9,7 @@ import jadx.tests.api.IntegrationTest; import static jadx.tests.api.utils.JadxMatchers.containsOne; import static org.junit.Assert.assertThat; +import static org.hamcrest.Matchers.anyOf; public class TestBreakInLoop2 extends IntegrationTest { @@ -43,7 +44,7 @@ public class TestBreakInLoop2 extends IntegrationTest { String code = cls.getCode().toString(); assertThat(code, containsOne("while (true) {")); - assertThat(code, containsOne("break;")); + assertThat(code, anyOf(containsOne("break;"), containsOne("return;"))); assertThat(code, containsOne("throw ex;")); assertThat(code, containsOne("data.clear();")); assertThat(code, containsOne("Thread.sleep(100);")); diff --git a/jadx-core/src/test/java/jadx/tests/integration/loops/TestLoopConditionInvoke.java b/jadx-core/src/test/java/jadx/tests/integration/loops/TestLoopConditionInvoke.java index 21cc65917..dbfca8f9d 100644 --- a/jadx-core/src/test/java/jadx/tests/integration/loops/TestLoopConditionInvoke.java +++ b/jadx-core/src/test/java/jadx/tests/integration/loops/TestLoopConditionInvoke.java @@ -37,7 +37,7 @@ public class TestLoopConditionInvoke extends IntegrationTest { String code = cls.getCode().toString(); assertThat(code, containsOne("do {")); - assertThat(code, containsOne("if (ch == '\\u0000') {")); + assertThat(code, containsOne("if (ch == 0) {")); assertThat(code, containsOne("this.pos = startPos;")); assertThat(code, containsOne("return false;")); assertThat(code, containsOne("} while (ch != lastChar);")); diff --git a/jadx-core/src/test/java/jadx/tests/integration/loops/TestSequentialLoops.java b/jadx-core/src/test/java/jadx/tests/integration/loops/TestSequentialLoops.java index 083608ed3..e064369ef 100644 --- a/jadx-core/src/test/java/jadx/tests/integration/loops/TestSequentialLoops.java +++ b/jadx-core/src/test/java/jadx/tests/integration/loops/TestSequentialLoops.java @@ -14,7 +14,7 @@ import static org.junit.Assert.assertThat; public class TestSequentialLoops extends IntegrationTest { public static class TestCls { - public int test7(int a, int b) { + public int test(int a, int b) { int c = b; int z; diff --git a/jadx-core/src/test/java/jadx/tests/integration/loops/TestSequentialLoops2.java b/jadx-core/src/test/java/jadx/tests/integration/loops/TestSequentialLoops2.java new file mode 100644 index 000000000..4bc68418b --- /dev/null +++ b/jadx-core/src/test/java/jadx/tests/integration/loops/TestSequentialLoops2.java @@ -0,0 +1,51 @@ +package jadx.tests.integration.loops; + +import org.junit.Test; + +import jadx.core.dex.nodes.ClassNode; +import jadx.tests.api.IntegrationTest; + +import static jadx.tests.api.utils.JadxMatchers.containsOne; +import static jadx.tests.api.utils.JadxMatchers.countString; +import static org.hamcrest.CoreMatchers.containsString; +import static org.junit.Assert.assertThat; + +public class TestSequentialLoops2 extends IntegrationTest { + + public static class TestCls { + private static char[] lowercases = new char[]{'a'}; + + public static String asciiToLowerCase(String s) { + char[] c = null; + int i = s.length(); + while (i-- > 0) { + char c1 = s.charAt(i); + if (c1 <= 127) { + char c2 = lowercases[c1]; + if (c1 != c2) { + c = s.toCharArray(); + c[i] = c2; + break; + } + } + } + while (i-- > 0) { + if (c[i] <= 127) { + c[i] = lowercases[c[i]]; + } + } + return c == null ? s : new String(c); + } + } + + @Test + public void test() { + ClassNode cls = getClassNode(TestCls.class); + String code = cls.getCode().toString(); + + assertThat(code, countString(2, "while (")); + assertThat(code, containsString("break;")); + assertThat(code, containsOne("return c")); + assertThat(code, countString(2, "<= 127")); + } +}