From 710245d5977860cb9c608863c66c644f1a4cef3d Mon Sep 17 00:00:00 2001 From: Skylot Date: Thu, 14 Feb 2019 21:17:31 +0300 Subject: [PATCH 1/3] fix: replace recursive analysis algorithms with iterations to avoid StackOverflow on big methods (#441) --- .../visitors/blocksmaker/BlockProcessor.java | 31 +++++++-- .../dex/visitors/ssa/LiveVarAnalysis.java | 23 ++++--- .../core/dex/visitors/ssa/RenameState.java | 59 +++++++++++++++++ .../core/dex/visitors/ssa/SSATransform.java | 64 ++++++++----------- 4 files changed, 121 insertions(+), 56 deletions(-) create mode 100644 jadx-core/src/main/java/jadx/core/dex/visitors/ssa/RenameState.java diff --git a/jadx-core/src/main/java/jadx/core/dex/visitors/blocksmaker/BlockProcessor.java b/jadx-core/src/main/java/jadx/core/dex/visitors/blocksmaker/BlockProcessor.java index 5ac645a4b..13152cacc 100644 --- a/jadx-core/src/main/java/jadx/core/dex/visitors/blocksmaker/BlockProcessor.java +++ b/jadx-core/src/main/java/jadx/core/dex/visitors/blocksmaker/BlockProcessor.java @@ -3,7 +3,9 @@ package jadx.core.dex.visitors.blocksmaker; import java.util.ArrayList; import java.util.BitSet; import java.util.Collections; +import java.util.Deque; import java.util.Iterator; +import java.util.LinkedList; import java.util.List; import org.slf4j.Logger; @@ -25,7 +27,6 @@ import jadx.core.dex.trycatch.ExceptionHandler; import jadx.core.dex.trycatch.TryCatchBlock; import jadx.core.dex.visitors.AbstractVisitor; import jadx.core.utils.BlockUtils; -import jadx.core.utils.exceptions.JadxOverflowException; import jadx.core.utils.exceptions.JadxRuntimeException; import static jadx.core.dex.visitors.blocksmaker.BlockSplitter.connect; @@ -220,7 +221,12 @@ public class BlockProcessor extends AbstractVisitor { markLoops(mth); // clear self dominance - basicBlocks.forEach(block -> block.getDoms().clear(block.getId())); + basicBlocks.forEach(block -> { + block.getDoms().clear(block.getId()); + if (block.getDoms().isEmpty()) { + block.setDoms(EMPTY); + } + }); // calculate immediate dominators for (BlockNode block : basicBlocks) { @@ -253,11 +259,20 @@ public class BlockProcessor extends AbstractVisitor { for (BlockNode exit : mth.getExitBlocks()) { exit.setDomFrontier(EMPTY); } - for (BlockNode block : mth.getBasicBlocks()) { + List domSortedBlocks = new ArrayList<>(mth.getBasicBlocks().size()); + Deque stack = new LinkedList<>(); + stack.push(mth.getEnterBlock()); + while (!stack.isEmpty()) { + BlockNode node = stack.pop(); + for (BlockNode dominated : node.getDominatesOn()) { + stack.push(dominated); + } + domSortedBlocks.add(node); + } + Collections.reverse(domSortedBlocks); + for (BlockNode block : domSortedBlocks) { try { computeBlockDF(mth, block); - } catch (StackOverflowError e) { - throw new JadxOverflowException("Failed compute block dominance frontier"); } catch (Exception e) { throw new JadxRuntimeException("Failed compute block dominance frontier", e); } @@ -268,7 +283,6 @@ public class BlockProcessor extends AbstractVisitor { if (block.getDomFrontier() != null) { return; } - block.getDominatesOn().forEach(domBlock -> computeBlockDF(mth, domBlock)); List blocks = mth.getBasicBlocks(); BitSet domFrontier = null; for (BlockNode s : block.getSuccessors()) { @@ -281,6 +295,9 @@ public class BlockProcessor extends AbstractVisitor { } for (BlockNode c : block.getDominatesOn()) { BitSet frontier = c.getDomFrontier(); + if (frontier == null) { + throw new JadxRuntimeException("Dominance frontier not calculated for dominated block: " + c + ", from: " + block); + } for (int p = frontier.nextSetBit(0); p >= 0; p = frontier.nextSetBit(p + 1)) { if (blocks.get(p).getIDom() != block) { if (domFrontier == null) { @@ -290,7 +307,7 @@ public class BlockProcessor extends AbstractVisitor { } } } - if (domFrontier == null || domFrontier.cardinality() == 0) { + if (domFrontier == null || domFrontier.isEmpty()) { domFrontier = EMPTY; } block.setDomFrontier(domFrontier); 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 ef1637a95..51f800444 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 @@ -80,35 +80,34 @@ public class LiveVarAnalysis { private void processLiveInfo() { int bbCount = mth.getBasicBlocks().size(); int regsCount = mth.getRegsCount(); - BitSet[] liveIn = initBitSetArray(bbCount, regsCount); + BitSet[] liveInBlocks = initBitSetArray(bbCount, regsCount); List blocks = mth.getBasicBlocks(); - int blocksSize = blocks.size(); + int blocksCount = blocks.size(); + int iterationsLimit = blocksCount * 10; boolean changed; int k = 0; do { changed = false; - for (int i = 0; i < blocksSize; i++) { - BlockNode block = blocks.get(i); + for (BlockNode block : blocks) { int blockId = block.getId(); - BitSet prevIn = liveIn[blockId]; + BitSet prevIn = liveInBlocks[blockId]; BitSet newIn = new BitSet(regsCount); - List successors = block.getSuccessors(); - for (int s = 0, successorsSize = successors.size(); s < successorsSize; s++) { - newIn.or(liveIn[successors.get(s).getId()]); + for (BlockNode successor : block.getSuccessors()) { + newIn.or(liveInBlocks[successor.getId()]); } newIn.andNot(defs[blockId]); newIn.or(uses[blockId]); if (!prevIn.equals(newIn)) { changed = true; - liveIn[blockId] = newIn; + liveInBlocks[blockId] = newIn; } } - if (k++ > 1000) { - throw new JadxRuntimeException("Live variable analysis reach iterations limit"); + if (k++ > iterationsLimit) { + throw new JadxRuntimeException("Live variable analysis reach iterations limit, blocks count: " + blocksCount); } } while (changed); - this.liveIn = liveIn; + this.liveIn = liveInBlocks; } private static BitSet[] initBitSetArray(int length, int bitsCount) { diff --git a/jadx-core/src/main/java/jadx/core/dex/visitors/ssa/RenameState.java b/jadx-core/src/main/java/jadx/core/dex/visitors/ssa/RenameState.java new file mode 100644 index 000000000..e5773d3bf --- /dev/null +++ b/jadx-core/src/main/java/jadx/core/dex/visitors/ssa/RenameState.java @@ -0,0 +1,59 @@ +package jadx.core.dex.visitors.ssa; + +import java.util.Arrays; + +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.MethodNode; + +final class RenameState { + private final MethodNode mth; + private final BlockNode block; + private final SSAVar[] vars; + private final int[] versions; + + public static RenameState init(MethodNode mth) { + int regsCount = mth.getRegsCount(); + RenameState state = new RenameState( + mth, + mth.getEnterBlock(), + new SSAVar[regsCount], + new int[regsCount] + ); + for (RegisterArg arg : mth.getArguments(true)) { + state.startVar(arg); + } + return state; + } + + public static RenameState copyFrom(RenameState state, BlockNode block) { + return new RenameState( + state.mth, + block, + Arrays.copyOf(state.vars, state.vars.length), + state.versions + ); + } + + private RenameState(MethodNode mth, BlockNode block, SSAVar[] vars, int[] versions) { + this.mth = mth; + this.block = block; + this.vars = vars; + this.versions = versions; + } + + public BlockNode getBlock() { + return block; + } + + public SSAVar getVar(int regNum) { + return vars[regNum]; + } + + public void startVar(RegisterArg regArg) { + int regNum = regArg.getRegNum(); + int version = versions[regNum]++; + vars[regNum] = mth.makeNewSVar(regNum, version, regArg); + } +} 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 a1de3bd21..914df9b1c 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 @@ -1,7 +1,6 @@ package jadx.core.dex.visitors.ssa; import java.util.ArrayList; -import java.util.Arrays; import java.util.BitSet; import java.util.Deque; import java.util.Iterator; @@ -120,35 +119,31 @@ public class SSATransform extends AbstractVisitor { if (!mth.getSVars().isEmpty()) { throw new JadxRuntimeException("SSA rename variables already executed"); } - int regsCount = mth.getRegsCount(); - SSAVar[] vars = new SSAVar[regsCount]; - int[] versions = new int[regsCount]; - // init method arguments - for (RegisterArg arg : mth.getArguments(true)) { - int regNum = arg.getRegNum(); - vars[regNum] = newSSAVar(mth, versions, arg, regNum); - } - BlockNode enterBlock = mth.getEnterBlock(); - initPhiInEnterBlock(vars, enterBlock); - renameVar(mth, vars, versions, enterBlock); - } + RenameState initState = RenameState.init(mth); + initPhiInEnterBlock(initState); - 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) { - for (PhiInsn phiInsn : phiList.getList()) { - bindPhiArg(vars, enterBlock, phiInsn); + Deque stack = new LinkedList<>(); + stack.push(initState); + while (!stack.isEmpty()) { + RenameState state = stack.pop(); + renameVarsInBlock(state); + for (BlockNode dominated : state.getBlock().getDominatesOn()) { + stack.push(RenameState.copyFrom(state, dominated)); } } } - private static void renameVar(MethodNode mth, SSAVar[] vars, int[] vers, BlockNode block) { - SSAVar[] inputVars = Arrays.copyOf(vars, vars.length); + private static void initPhiInEnterBlock(RenameState initState) { + PhiListAttr phiList = initState.getBlock().get(AType.PHI_LIST); + if (phiList != null) { + for (PhiInsn phiInsn : phiList.getList()) { + bindPhiArg(initState, phiInsn); + } + } + } + + private static void renameVarsInBlock(RenameState state) { + BlockNode block = state.getBlock(); for (InsnNode insn : block.getInstructions()) { if (insn.getType() != InsnType.PHI) { for (InsnArg arg : insn.getArguments()) { @@ -157,18 +152,17 @@ public class SSATransform extends AbstractVisitor { } RegisterArg reg = (RegisterArg) arg; int regNum = reg.getRegNum(); - SSAVar var = vars[regNum]; + SSAVar var = state.getVar(regNum); if (var == null) { throw new JadxRuntimeException("Not initialized variable reg: " + regNum - + ", insn: " + insn + ", block:" + block + ", method: " + mth); + + ", insn: " + insn + ", block:" + block); } var.use(reg); } } RegisterArg result = insn.getResult(); if (result != null) { - int regNum = result.getRegNum(); - vars[regNum] = newSSAVar(mth, vers, result, regNum); + state.startVar(result); } } for (BlockNode s : block.getSuccessors()) { @@ -177,22 +171,18 @@ public class SSATransform extends AbstractVisitor { continue; } for (PhiInsn phiInsn : phiList.getList()) { - bindPhiArg(vars, block, phiInsn); + bindPhiArg(state, phiInsn); } } - for (BlockNode domOn : block.getDominatesOn()) { - renameVar(mth, vars, vers, domOn); - } - System.arraycopy(inputVars, 0, vars, 0, vars.length); } - private static void bindPhiArg(SSAVar[] vars, BlockNode block, PhiInsn phiInsn) { + private static void bindPhiArg(RenameState state, PhiInsn phiInsn) { int regNum = phiInsn.getResult().getRegNum(); - SSAVar var = vars[regNum]; + SSAVar var = state.getVar(regNum); if (var == null) { return; } - RegisterArg arg = phiInsn.bindArg(block); + RegisterArg arg = phiInsn.bindArg(state.getBlock()); var.use(arg); var.setUsedInPhi(phiInsn); } From 7f4e641860733703feaf0893a37574ee442762ac Mon Sep 17 00:00:00 2001 From: Skylot Date: Fri, 15 Feb 2019 16:22:21 +0300 Subject: [PATCH 2/3] fix: skip duplicated block in complex if (#441) --- .../dex/visitors/regions/IfMakerHelper.java | 13 +- .../integration/conditions/TestComplexIf.java | 39 ++ .../test/smali/conditions/TestComplexIf.smali | 636 ++++++++++++++++++ 3 files changed, 687 insertions(+), 1 deletion(-) create mode 100644 jadx-core/src/test/java/jadx/tests/integration/conditions/TestComplexIf.java create mode 100644 jadx-core/src/test/smali/conditions/TestComplexIf.smali 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 38d947258..d2f56d9b2 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 @@ -263,7 +263,18 @@ public class IfMakerHelper { result.setIfBlock(first.getIfBlock()); result.merge(first, second); - BlockNode otherPathBlock = followThenBranch ? first.getElseBlock() : first.getThenBlock(); + BlockNode otherPathBlock; + if (followThenBranch) { + otherPathBlock = first.getElseBlock(); + if (!otherPathBlock.equals(result.getElseBlock())) { + result.getSkipBlocks().add(otherPathBlock); + } + } else { + otherPathBlock = first.getThenBlock(); + if (!otherPathBlock.equals(result.getThenBlock())) { + result.getSkipBlocks().add(otherPathBlock); + } + } skipSimplePath(otherPathBlock, result.getSkipBlocks()); return result; } diff --git a/jadx-core/src/test/java/jadx/tests/integration/conditions/TestComplexIf.java b/jadx-core/src/test/java/jadx/tests/integration/conditions/TestComplexIf.java new file mode 100644 index 000000000..5ac1dab3f --- /dev/null +++ b/jadx-core/src/test/java/jadx/tests/integration/conditions/TestComplexIf.java @@ -0,0 +1,39 @@ +package jadx.tests.integration.conditions; + +import org.junit.Test; + +import jadx.core.dex.nodes.ClassNode; +import jadx.tests.api.SmaliTest; + +import static jadx.tests.api.utils.JadxMatchers.containsOne; +import static org.junit.Assert.assertThat; + +public class TestComplexIf extends SmaliTest { + +/* + public final class TestComplexIf { + private String a; + private int b; + private float c; + + public final boolean test() { + if (this.a.equals("GT-P6200") || this.a.equals("GT-P6210") || ... ) { + return true; + } + if (this.a.equals("SM-T810") || this.a.equals("SM-T813") || ...) { + return false; + } + return this.c > 160.0f ? true : this.c <= 0.0f && ((this.b & 15) == 4 ? 1 : null) != null; + } + } + */ + + @Test + public void test() { + ClassNode cls = getClassNodeFromSmaliWithPkg("conditions", "TestComplexIf"); + String code = cls.getCode().toString(); + + assertThat(code, containsOne("if (this.a.equals(\"GT-P6200\") || this.a.equals(\"GT-P6210\") || this.a.equals(\"A100\") " + + "|| this.a.equals(\"A101\") || this.a.equals(\"LIFETAB_S786X\") || this.a.equals(\"VS890 4G\")) {")); + } +} diff --git a/jadx-core/src/test/smali/conditions/TestComplexIf.smali b/jadx-core/src/test/smali/conditions/TestComplexIf.smali new file mode 100644 index 000000000..678ebd42d --- /dev/null +++ b/jadx-core/src/test/smali/conditions/TestComplexIf.smali @@ -0,0 +1,636 @@ +.class public final Lconditions/TestComplexIf; +.super Ljava/lang/Object; + + +# instance fields +.field private a:Ljava/lang/String; + +.field private b:I + +.field private c:F + + +# direct methods +.method public constructor ()V + .locals 1 + return-void +.end method + +.method public final test()Z + .locals 5 + + iget-object v0, p0, Lconditions/TestComplexIf;->a:Ljava/lang/String; + + const-string v1, "GT-P6200" + + invoke-virtual {v0, v1}, Ljava/lang/String;->equals(Ljava/lang/Object;)Z + + move-result v0 + + const/4 v1, 0x1 + + if-nez v0, :cond_b + + iget-object v0, p0, Lconditions/TestComplexIf;->a:Ljava/lang/String; + + const-string v2, "GT-P6210" + + invoke-virtual {v0, v2}, Ljava/lang/String;->equals(Ljava/lang/Object;)Z + + move-result v0 + + if-nez v0, :cond_b + + iget-object v0, p0, Lconditions/TestComplexIf;->a:Ljava/lang/String; + + const-string v2, "A100" + + invoke-virtual {v0, v2}, Ljava/lang/String;->equals(Ljava/lang/Object;)Z + + move-result v0 + + if-nez v0, :cond_b + + iget-object v0, p0, Lconditions/TestComplexIf;->a:Ljava/lang/String; + + const-string v2, "A101" + + invoke-virtual {v0, v2}, Ljava/lang/String;->equals(Ljava/lang/Object;)Z + + move-result v0 + + if-nez v0, :cond_b + + iget-object v0, p0, Lconditions/TestComplexIf;->a:Ljava/lang/String; + + const-string v2, "LIFETAB_S786X" + + invoke-virtual {v0, v2}, Ljava/lang/String;->equals(Ljava/lang/Object;)Z + + move-result v0 + + if-eqz v0, :cond_0 + + goto/16 :goto_2 + + :cond_0 + iget-object v0, p0, Lconditions/TestComplexIf;->a:Ljava/lang/String; + + const-string v2, "VS890 4G" + + invoke-virtual {v0, v2}, Ljava/lang/String;->equals(Ljava/lang/Object;)Z + + move-result v0 + + if-eqz v0, :cond_1 + + return v1 + + :cond_1 + iget-object v0, p0, Lconditions/TestComplexIf;->a:Ljava/lang/String; + + const-string v2, "SM-T810" + + invoke-virtual {v0, v2}, Ljava/lang/String;->equals(Ljava/lang/Object;)Z + + move-result v0 + + const/4 v2, 0x0 + + if-nez v0, :cond_a + + iget-object v0, p0, Lconditions/TestComplexIf;->a:Ljava/lang/String; + + const-string v3, "SM-T813" + + invoke-virtual {v0, v3}, Ljava/lang/String;->equals(Ljava/lang/Object;)Z + + move-result v0 + + if-nez v0, :cond_a + + iget-object v0, p0, Lconditions/TestComplexIf;->a:Ljava/lang/String; + + const-string v3, "SM-T815" + + invoke-virtual {v0, v3}, Ljava/lang/String;->equals(Ljava/lang/Object;)Z + + move-result v0 + + if-nez v0, :cond_a + + iget-object v0, p0, Lconditions/TestComplexIf;->a:Ljava/lang/String; + + const-string v3, "SM-T815N0" + + invoke-virtual {v0, v3}, Ljava/lang/String;->equals(Ljava/lang/Object;)Z + + move-result v0 + + if-nez v0, :cond_a + + iget-object v0, p0, Lconditions/TestComplexIf;->a:Ljava/lang/String; + + const-string v3, "SM-T815Y" + + invoke-virtual {v0, v3}, Ljava/lang/String;->equals(Ljava/lang/Object;)Z + + move-result v0 + + if-nez v0, :cond_a + + iget-object v0, p0, Lconditions/TestComplexIf;->a:Ljava/lang/String; + + const-string v3, "SM-T820" + + invoke-virtual {v0, v3}, Ljava/lang/String;->equals(Ljava/lang/Object;)Z + + move-result v0 + + if-nez v0, :cond_a + + iget-object v0, p0, Lconditions/TestComplexIf;->a:Ljava/lang/String; + + const-string v3, "SM-T825" + + invoke-virtual {v0, v3}, Ljava/lang/String;->equals(Ljava/lang/Object;)Z + + move-result v0 + + if-nez v0, :cond_a + + iget-object v0, p0, Lconditions/TestComplexIf;->a:Ljava/lang/String; + + const-string v3, "SM-P585" + + invoke-virtual {v0, v3}, Ljava/lang/String;->equals(Ljava/lang/Object;)Z + + move-result v0 + + if-nez v0, :cond_a + + iget-object v0, p0, Lconditions/TestComplexIf;->a:Ljava/lang/String; + + const-string v3, "SM-P585N0" + + invoke-virtual {v0, v3}, Ljava/lang/String;->equals(Ljava/lang/Object;)Z + + move-result v0 + + if-nez v0, :cond_a + + iget-object v0, p0, Lconditions/TestComplexIf;->a:Ljava/lang/String; + + const-string v3, "SM-T561" + + invoke-virtual {v0, v3}, Ljava/lang/String;->equals(Ljava/lang/Object;)Z + + move-result v0 + + if-nez v0, :cond_a + + iget-object v0, p0, Lconditions/TestComplexIf;->a:Ljava/lang/String; + + const-string v3, "SM-T567V" + + invoke-virtual {v0, v3}, Ljava/lang/String;->equals(Ljava/lang/Object;)Z + + move-result v0 + + if-nez v0, :cond_a + + iget-object v0, p0, Lconditions/TestComplexIf;->a:Ljava/lang/String; + + const-string v3, "SM-T320" + + invoke-virtual {v0, v3}, Ljava/lang/String;->equals(Ljava/lang/Object;)Z + + move-result v0 + + if-nez v0, :cond_a + + iget-object v0, p0, Lconditions/TestComplexIf;->a:Ljava/lang/String; + + const-string v3, "SM-T321" + + invoke-virtual {v0, v3}, Ljava/lang/String;->equals(Ljava/lang/Object;)Z + + move-result v0 + + if-nez v0, :cond_a + + iget-object v0, p0, Lconditions/TestComplexIf;->a:Ljava/lang/String; + + const-string v3, "SM-T325" + + invoke-virtual {v0, v3}, Ljava/lang/String;->equals(Ljava/lang/Object;)Z + + move-result v0 + + if-nez v0, :cond_a + + iget-object v0, p0, Lconditions/TestComplexIf;->a:Ljava/lang/String; + + const-string v3, "SM-T700" + + invoke-virtual {v0, v3}, Ljava/lang/String;->equals(Ljava/lang/Object;)Z + + move-result v0 + + if-nez v0, :cond_a + + iget-object v0, p0, Lconditions/TestComplexIf;->a:Ljava/lang/String; + + const-string v3, "SM-T705" + + invoke-virtual {v0, v3}, Ljava/lang/String;->equals(Ljava/lang/Object;)Z + + move-result v0 + + if-nez v0, :cond_a + + iget-object v0, p0, Lconditions/TestComplexIf;->a:Ljava/lang/String; + + const-string v3, "SM-T705M" + + invoke-virtual {v0, v3}, Ljava/lang/String;->equals(Ljava/lang/Object;)Z + + move-result v0 + + if-nez v0, :cond_a + + iget-object v0, p0, Lconditions/TestComplexIf;->a:Ljava/lang/String; + + const-string v3, "SM-T705Y" + + invoke-virtual {v0, v3}, Ljava/lang/String;->equals(Ljava/lang/Object;)Z + + move-result v0 + + if-nez v0, :cond_a + + iget-object v0, p0, Lconditions/TestComplexIf;->a:Ljava/lang/String; + + const-string v3, "SC-03G" + + invoke-virtual {v0, v3}, Ljava/lang/String;->equals(Ljava/lang/Object;)Z + + move-result v0 + + if-nez v0, :cond_a + + iget-object v0, p0, Lconditions/TestComplexIf;->a:Ljava/lang/String; + + const-string v3, "GT-N5100" + + invoke-virtual {v0, v3}, Ljava/lang/String;->equals(Ljava/lang/Object;)Z + + move-result v0 + + if-nez v0, :cond_a + + iget-object v0, p0, Lconditions/TestComplexIf;->a:Ljava/lang/String; + + const-string v3, "GT-N5105" + + invoke-virtual {v0, v3}, Ljava/lang/String;->equals(Ljava/lang/Object;)Z + + move-result v0 + + if-nez v0, :cond_a + + iget-object v0, p0, Lconditions/TestComplexIf;->a:Ljava/lang/String; + + const-string v3, "GT-N5110" + + invoke-virtual {v0, v3}, Ljava/lang/String;->equals(Ljava/lang/Object;)Z + + move-result v0 + + if-nez v0, :cond_a + + iget-object v0, p0, Lconditions/TestComplexIf;->a:Ljava/lang/String; + + const-string v3, "GT-N5120" + + invoke-virtual {v0, v3}, Ljava/lang/String;->equals(Ljava/lang/Object;)Z + + move-result v0 + + if-nez v0, :cond_a + + iget-object v0, p0, Lconditions/TestComplexIf;->a:Ljava/lang/String; + + const-string v3, "SHW-M500W" + + invoke-virtual {v0, v3}, Ljava/lang/String;->equals(Ljava/lang/Object;)Z + + move-result v0 + + if-nez v0, :cond_a + + iget-object v0, p0, Lconditions/TestComplexIf;->a:Ljava/lang/String; + + const-string v3, "SM-T310" + + invoke-virtual {v0, v3}, Ljava/lang/String;->equals(Ljava/lang/Object;)Z + + move-result v0 + + if-nez v0, :cond_a + + iget-object v0, p0, Lconditions/TestComplexIf;->a:Ljava/lang/String; + + const-string v3, "SM-T311" + + invoke-virtual {v0, v3}, Ljava/lang/String;->equals(Ljava/lang/Object;)Z + + move-result v0 + + if-nez v0, :cond_a + + iget-object v0, p0, Lconditions/TestComplexIf;->a:Ljava/lang/String; + + const-string v3, "SM-T315" + + invoke-virtual {v0, v3}, Ljava/lang/String;->equals(Ljava/lang/Object;)Z + + move-result v0 + + if-nez v0, :cond_a + + iget-object v0, p0, Lconditions/TestComplexIf;->a:Ljava/lang/String; + + const-string v3, "SM-T330" + + invoke-virtual {v0, v3}, Ljava/lang/String;->equals(Ljava/lang/Object;)Z + + move-result v0 + + if-nez v0, :cond_a + + iget-object v0, p0, Lconditions/TestComplexIf;->a:Ljava/lang/String; + + const-string v3, "SM-T330NU" + + invoke-virtual {v0, v3}, Ljava/lang/String;->equals(Ljava/lang/Object;)Z + + move-result v0 + + if-nez v0, :cond_a + + iget-object v0, p0, Lconditions/TestComplexIf;->a:Ljava/lang/String; + + const-string v3, "SM-T331" + + invoke-virtual {v0, v3}, Ljava/lang/String;->equals(Ljava/lang/Object;)Z + + move-result v0 + + if-nez v0, :cond_a + + iget-object v0, p0, Lconditions/TestComplexIf;->a:Ljava/lang/String; + + const-string v3, "SM-T335" + + invoke-virtual {v0, v3}, Ljava/lang/String;->equals(Ljava/lang/Object;)Z + + move-result v0 + + if-nez v0, :cond_a + + iget-object v0, p0, Lconditions/TestComplexIf;->a:Ljava/lang/String; + + const-string v3, "SM-T337V" + + invoke-virtual {v0, v3}, Ljava/lang/String;->equals(Ljava/lang/Object;)Z + + move-result v0 + + if-nez v0, :cond_a + + iget-object v0, p0, Lconditions/TestComplexIf;->a:Ljava/lang/String; + + const-string v3, "SM-T710" + + invoke-virtual {v0, v3}, Ljava/lang/String;->equals(Ljava/lang/Object;)Z + + move-result v0 + + if-nez v0, :cond_a + + iget-object v0, p0, Lconditions/TestComplexIf;->a:Ljava/lang/String; + + const-string v3, "SM-T715" + + invoke-virtual {v0, v3}, Ljava/lang/String;->equals(Ljava/lang/Object;)Z + + move-result v0 + + if-nez v0, :cond_a + + iget-object v0, p0, Lconditions/TestComplexIf;->a:Ljava/lang/String; + + const-string v3, "SM-T715N0" + + invoke-virtual {v0, v3}, Ljava/lang/String;->equals(Ljava/lang/Object;)Z + + move-result v0 + + if-nez v0, :cond_a + + iget-object v0, p0, Lconditions/TestComplexIf;->a:Ljava/lang/String; + + const-string v3, "SM-T719" + + invoke-virtual {v0, v3}, Ljava/lang/String;->equals(Ljava/lang/Object;)Z + + move-result v0 + + if-nez v0, :cond_a + + iget-object v0, p0, Lconditions/TestComplexIf;->a:Ljava/lang/String; + + const-string v3, "GT-P6800" + + invoke-virtual {v0, v3}, Ljava/lang/String;->equals(Ljava/lang/Object;)Z + + move-result v0 + + if-nez v0, :cond_a + + iget-object v0, p0, Lconditions/TestComplexIf;->a:Ljava/lang/String; + + const-string v3, "SC-01E" + + invoke-virtual {v0, v3}, Ljava/lang/String;->equals(Ljava/lang/Object;)Z + + move-result v0 + + if-eqz v0, :cond_2 + + goto/16 :goto_1 + + :cond_2 + iget-object v0, p0, Lconditions/TestComplexIf;->a:Ljava/lang/String; + + const-string v3, "LG-V500" + + invoke-virtual {v0, v3}, Ljava/lang/String;->equals(Ljava/lang/Object;)Z + + move-result v0 + + if-nez v0, :cond_a + + iget-object v0, p0, Lconditions/TestComplexIf;->a:Ljava/lang/String; + + const-string v3, "LG-V930" + + invoke-virtual {v0, v3}, Ljava/lang/String;->equals(Ljava/lang/Object;)Z + + move-result v0 + + if-eqz v0, :cond_3 + + goto/16 :goto_1 + + :cond_3 + iget-object v0, p0, Lconditions/TestComplexIf;->a:Ljava/lang/String; + + const-string v3, "P01T_1" + + invoke-virtual {v0, v3}, Ljava/lang/String;->equals(Ljava/lang/Object;)Z + + move-result v0 + + if-nez v0, :cond_a + + iget-object v0, p0, Lconditions/TestComplexIf;->a:Ljava/lang/String; + + const-string v3, "P01MA" + + invoke-virtual {v0, v3}, Ljava/lang/String;->equals(Ljava/lang/Object;)Z + + move-result v0 + + if-nez v0, :cond_a + + iget-object v0, p0, Lconditions/TestComplexIf;->a:Ljava/lang/String; + + const-string v3, "Nexus 9" + + invoke-virtual {v0, v3}, Ljava/lang/String;->equals(Ljava/lang/Object;)Z + + move-result v0 + + if-nez v0, :cond_a + + iget-object v0, p0, Lconditions/TestComplexIf;->a:Ljava/lang/String; + + const-string v3, "ASUS_P00I" + + invoke-virtual {v0, v3}, Ljava/lang/String;->equals(Ljava/lang/Object;)Z + + move-result v0 + + if-eqz v0, :cond_4 + + goto :goto_1 + + :cond_4 + iget-object v0, p0, Lconditions/TestComplexIf;->a:Ljava/lang/String; + + const-string v3, "Lenovo YT3-X90X" + + invoke-virtual {v0, v3}, Ljava/lang/String;->equals(Ljava/lang/Object;)Z + + move-result v0 + + if-nez v0, :cond_a + + iget-object v0, p0, Lconditions/TestComplexIf;->a:Ljava/lang/String; + + const-string v3, "Lenovo YT-X703F" + + invoke-virtual {v0, v3}, Ljava/lang/String;->equals(Ljava/lang/Object;)Z + + move-result v0 + + if-eqz v0, :cond_5 + + goto :goto_1 + + :cond_5 + iget-object v0, p0, Lconditions/TestComplexIf;->a:Ljava/lang/String; + + const-string v3, "PMT3408_4G" + + invoke-virtual {v0, v3}, Ljava/lang/String;->equals(Ljava/lang/Object;)Z + + move-result v0 + + if-eqz v0, :cond_6 + + return v2 + + :cond_6 + iget-object v0, p0, Lconditions/TestComplexIf;->a:Ljava/lang/String; + + const-string v3, "MediaPad T2 10.0 Pro" + + invoke-virtual {v0, v3}, Ljava/lang/String;->equals(Ljava/lang/Object;)Z + + move-result v0 + + if-eqz v0, :cond_7 + + return v2 + + :cond_7 + iget-object v0, p0, Lconditions/TestComplexIf;->b:I + + and-int/lit8 v0, v0, 0xf + + const/4 v3, 0x4 + + if-ne v0, v3, :cond_8 + + const/4 v0, 0x1 + + goto :goto_0 + + :cond_8 + const/4 v0, 0x0 + + :goto_0 + iget v3, p0, Lconditions/TestComplexIf;->c:F + + const/high16 v4, 0x43200000 # 160.0f + + cmpl-float v3, v3, v4 + + if-lez v3, :cond_9 + + return v1 + + :cond_9 + iget v3, p0, Lconditions/TestComplexIf;->c:F + + const/4 v4, 0x0 + + cmpg-float v3, v3, v4 + + if-gtz v3, :cond_a + + if-eqz v0, :cond_a + + return v1 + + :cond_a + :goto_1 + return v2 + + :cond_b + :goto_2 + return v1 +.end method From db1b027da2d5559cb3917b0f2546cfa5d26e8162 Mon Sep 17 00:00:00 2001 From: Skylot Date: Sat, 16 Feb 2019 12:42:29 +0300 Subject: [PATCH 3/3] fix: improve bridge methods renaming (#397) --- .../java/jadx/core/codegen/MethodGen.java | 8 +- .../jadx/core/dex/visitors/ClassModifier.java | 16 +++- .../core/dex/visitors/FixAccessModifiers.java | 21 +++-- .../integration/inner/TestInner2Samples.java | 77 +++++++++++++++++++ .../inner/TestSyntheticMthRename.java | 44 +++++++++++ .../TestSyntheticMthRename/TestCls$A.smali | 66 ++++++++++++++++ .../TestSyntheticMthRename/TestCls$I.smali | 35 +++++++++ .../TestSyntheticMthRename/TestCls.smali | 24 ++++++ 8 files changed, 278 insertions(+), 13 deletions(-) create mode 100644 jadx-core/src/test/java/jadx/tests/integration/inner/TestInner2Samples.java create mode 100644 jadx-core/src/test/java/jadx/tests/integration/inner/TestSyntheticMthRename.java create mode 100644 jadx-core/src/test/smali/inner/TestSyntheticMthRename/TestCls$A.smali create mode 100644 jadx-core/src/test/smali/inner/TestSyntheticMthRename/TestCls$I.smali create mode 100644 jadx-core/src/test/smali/inner/TestSyntheticMthRename/TestCls.smali 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 2f343a652..efcbf2f62 100644 --- a/jadx-core/src/main/java/jadx/core/codegen/MethodGen.java +++ b/jadx-core/src/main/java/jadx/core/codegen/MethodGen.java @@ -4,15 +4,15 @@ import java.util.Iterator; import java.util.List; import com.android.dx.rop.code.AccessFlags; -import jadx.core.dex.info.ClassInfo; -import jadx.core.utils.Utils; import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import jadx.core.Consts; import jadx.core.dex.attributes.AFlag; import jadx.core.dex.attributes.AType; import jadx.core.dex.attributes.annotations.MethodParameters; import jadx.core.dex.info.AccessInfo; +import jadx.core.dex.info.ClassInfo; import jadx.core.dex.instructions.InsnType; import jadx.core.dex.instructions.args.ArgType; import jadx.core.dex.instructions.args.RegisterArg; @@ -24,6 +24,7 @@ import jadx.core.dex.visitors.DepthTraversal; import jadx.core.dex.visitors.FallbackModeVisitor; import jadx.core.utils.ErrorsCounter; import jadx.core.utils.InsnUtils; +import jadx.core.utils.Utils; import jadx.core.utils.exceptions.CodegenException; import jadx.core.utils.exceptions.DecodeException; @@ -85,6 +86,9 @@ public class MethodGen { } code.startLineWithNum(mth.getSourceLine()); code.add(ai.makeString()); + if (Consts.DEBUG) { + code.add(mth.isVirtual() ? "/* virtual */ " : "/* direct */ "); + } if (classGen.addGenericMap(code, mth.getGenericMap())) { code.add(' '); 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 5e01741f2..c7d148c7e 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 @@ -3,6 +3,8 @@ package jadx.core.dex.visitors; import java.util.List; import java.util.Objects; +import com.android.dx.rop.code.AccessFlags; + import jadx.core.dex.attributes.AFlag; import jadx.core.dex.attributes.AType; import jadx.core.dex.attributes.nodes.FieldReplaceAttr; @@ -31,7 +33,10 @@ import jadx.core.utils.exceptions.JadxException; @JadxVisitor( name = "ClassModifier", desc = "Remove synthetic classes, methods and fields", - runAfter = ModVisitor.class + runAfter = { + ModVisitor.class, + FixAccessModifiers.class + } ) public class ClassModifier extends AbstractVisitor { @@ -218,6 +223,10 @@ public class ClassModifier extends AbstractVisitor { MethodInfo callMth = ((InvokeNode) insn).getCallMth(); MethodNode wrappedMth = mth.root().deepResolveMethod(callMth); if (wrappedMth != null) { + AccessInfo wrappedAccFlags = wrappedMth.getAccessFlags(); + if (wrappedAccFlags.isStatic()) { + return false; + } if (callMth.getArgsCount() != mth.getMethodInfo().getArgsCount()) { return false; } @@ -235,8 +244,9 @@ public class ClassModifier extends AbstractVisitor { if (Objects.equals(wrappedMth.getAlias(), alias)) { return true; } - if (!wrappedMth.isVirtual()) { - return false; + if (!wrappedAccFlags.isPublic()) { + // must be public + FixAccessModifiers.changeVisibility(wrappedMth, AccessFlags.ACC_PUBLIC); } wrappedMth.getMethodInfo().setAlias(alias); return true; diff --git a/jadx-core/src/main/java/jadx/core/dex/visitors/FixAccessModifiers.java b/jadx-core/src/main/java/jadx/core/dex/visitors/FixAccessModifiers.java index d62143945..62a1808bb 100644 --- a/jadx-core/src/main/java/jadx/core/dex/visitors/FixAccessModifiers.java +++ b/jadx-core/src/main/java/jadx/core/dex/visitors/FixAccessModifiers.java @@ -26,22 +26,27 @@ public class FixAccessModifiers extends AbstractVisitor { if (respectAccessModifiers) { return; } - AccessInfo accessFlags = mth.getAccessFlags(); - int newVisFlag = fixVisibility(mth, accessFlags); + int newVisFlag = fixVisibility(mth); if (newVisFlag != 0) { - AccessInfo newAccFlags = accessFlags.changeVisibility(newVisFlag); - if (newAccFlags != accessFlags) { - mth.setAccFlags(newAccFlags); - mth.addAttr(AType.COMMENTS, "Access modifiers changed, original: " + accessFlags.rawString()); - } + changeVisibility(mth, newVisFlag); } } - private int fixVisibility(MethodNode mth, AccessInfo accessFlags) { + public static void changeVisibility(MethodNode mth, int newVisFlag) { + AccessInfo accessFlags = mth.getAccessFlags(); + AccessInfo newAccFlags = accessFlags.changeVisibility(newVisFlag); + if (newAccFlags != accessFlags) { + mth.setAccFlags(newAccFlags); + mth.addAttr(AType.COMMENTS, "access modifiers changed from: " + accessFlags.rawString()); + } + } + + private static int fixVisibility(MethodNode mth) { if (mth.isVirtual()) { // make virtual methods public return AccessFlags.ACC_PUBLIC; } else { + AccessInfo accessFlags = mth.getAccessFlags(); if (accessFlags.isAbstract()) { // make abstract methods public return AccessFlags.ACC_PUBLIC; diff --git a/jadx-core/src/test/java/jadx/tests/integration/inner/TestInner2Samples.java b/jadx-core/src/test/java/jadx/tests/integration/inner/TestInner2Samples.java new file mode 100644 index 000000000..30b467143 --- /dev/null +++ b/jadx-core/src/test/java/jadx/tests/integration/inner/TestInner2Samples.java @@ -0,0 +1,77 @@ +package jadx.tests.integration.inner; + +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 org.hamcrest.Matchers.containsString; +import static org.hamcrest.Matchers.not; +import static org.junit.Assert.assertThat; + +public class TestInner2Samples extends IntegrationTest { + + public static class TestInner2 { + + private String a; + + public class A { + public A() { + a = "a"; + } + + public String a() { + return a; + } + } + + private static String b; + + public static class B { + public B() { + b = "b"; + } + + public String b() { + return b; + } + } + + private String c; + + private void setC(String c) { + this.c = c; + } + + public class C { + public String c() { + setC("c"); + return c; + } + } + + private static String d; + + private static void setD(String s) { + d = s; + } + + public static class D { + public String d() { + setD("d"); + return d; + } + } + } + + @Test + public void test() { + ClassNode cls = getClassNode(TestInner2.class); + String code = cls.getCode().toString(); + + assertThat(code, containsOne("setD(\"d\");")); + assertThat(code, not(containsString("synthetic"))); + assertThat(code, not(containsString("access$"))); + } +} diff --git a/jadx-core/src/test/java/jadx/tests/integration/inner/TestSyntheticMthRename.java b/jadx-core/src/test/java/jadx/tests/integration/inner/TestSyntheticMthRename.java new file mode 100644 index 000000000..2ca61e986 --- /dev/null +++ b/jadx-core/src/test/java/jadx/tests/integration/inner/TestSyntheticMthRename.java @@ -0,0 +1,44 @@ +package jadx.tests.integration.inner; + +import org.junit.Test; + +import jadx.core.dex.nodes.ClassNode; +import jadx.tests.api.SmaliTest; + +import static jadx.tests.api.utils.JadxMatchers.containsOne; +import static org.hamcrest.Matchers.containsString; +import static org.hamcrest.Matchers.not; +import static org.junit.Assert.assertThat; + +/** + * Issue: https://github.com/skylot/jadx/issues/397 + */ +public class TestSyntheticMthRename extends SmaliTest { + +// public class TestCls { +// public interface I { +// R call(P... p); +// } +// +// public static final class A implements I { +// public /* synthetic */ /* virtual */ Object call(Object[] objArr) { +// return renamedCall((Runnable[]) objArr); +// } +// +// private /* varargs */ /* direct */ String renamedCall(Runnable... p) { +// return "str"; +// } +// } +// } + + @Test + public void test() { + ClassNode cls = getClassNodeFromSmaliFiles("inner", "TestSyntheticMthRename", "TestCls", + "TestCls", "TestCls$I", "TestCls$A" + ); + String code = cls.getCode().toString(); + + assertThat(code, containsOne("public String call(Runnable... p) {")); + assertThat(code, not(containsString("synthetic"))); + } +} diff --git a/jadx-core/src/test/smali/inner/TestSyntheticMthRename/TestCls$A.smali b/jadx-core/src/test/smali/inner/TestSyntheticMthRename/TestCls$A.smali new file mode 100644 index 000000000..9a09c0df2 --- /dev/null +++ b/jadx-core/src/test/smali/inner/TestSyntheticMthRename/TestCls$A.smali @@ -0,0 +1,66 @@ +.class public final Linner/TestCls$A; +.super Ljava/lang/Object; +.source "TestCls.java" + +# interfaces +.implements Linner/TestCls$I; + + +# annotations +.annotation system Ldalvik/annotation/EnclosingClass; + value = Linner/TestCls; +.end annotation + +.annotation system Ldalvik/annotation/InnerClass; + accessFlags = 0x19 + name = "A" +.end annotation + +.annotation system Ldalvik/annotation/Signature; + value = { + "Ljava/lang/Object;", + "Linner/TestCls$I", + "<", + "Ljava/lang/String;", + "Ljava/lang/Runnable;", + ">;" + } +.end annotation + + +# direct methods +.method public constructor ()V + .registers 1 + + .prologue + .line 9 + invoke-direct {p0}, Ljava/lang/Object;->()V + + return-void +.end method + +.method private varargs renamedCall([Ljava/lang/Runnable;)Ljava/lang/String; + .registers 3 + .param p1, "p" # [Ljava/lang/Runnable; + + .prologue + .line 12 + const-string v0, "str" + + return-object v0 +.end method + +# virtual methods +.method public synthetic call([Ljava/lang/Object;)Ljava/lang/Object; + .registers 3 + + .prologue + .line 9 + check-cast p1, [Ljava/lang/Runnable; + + invoke-virtual {p0, p1}, Linner/TestCls$A;->renamedCall([Ljava/lang/Runnable;)Ljava/lang/String; + + move-result-object v0 + + return-object v0 +.end method diff --git a/jadx-core/src/test/smali/inner/TestSyntheticMthRename/TestCls$I.smali b/jadx-core/src/test/smali/inner/TestSyntheticMthRename/TestCls$I.smali new file mode 100644 index 000000000..9899b9a96 --- /dev/null +++ b/jadx-core/src/test/smali/inner/TestSyntheticMthRename/TestCls$I.smali @@ -0,0 +1,35 @@ +.class public interface abstract Linner/TestCls$I; +.super Ljava/lang/Object; +.source "TestCls.java" + + +# annotations +.annotation system Ldalvik/annotation/EnclosingClass; + value = Linner/TestCls; +.end annotation + +.annotation system Ldalvik/annotation/InnerClass; + accessFlags = 0x609 + name = "I" +.end annotation + +.annotation system Ldalvik/annotation/Signature; + value = { + "", + "Ljava/lang/Object;" + } +.end annotation + + +# virtual methods +.method public varargs abstract call([Ljava/lang/Object;)Ljava/lang/Object; + .annotation system Ldalvik/annotation/Signature; + value = { + "([TP;)TR;" + } + .end annotation +.end method diff --git a/jadx-core/src/test/smali/inner/TestSyntheticMthRename/TestCls.smali b/jadx-core/src/test/smali/inner/TestSyntheticMthRename/TestCls.smali new file mode 100644 index 000000000..fa403ef0c --- /dev/null +++ b/jadx-core/src/test/smali/inner/TestSyntheticMthRename/TestCls.smali @@ -0,0 +1,24 @@ +.class public Linner/TestCls; +.super Ljava/lang/Object; +.source "TestCls.java" + + +# annotations +.annotation system Ldalvik/annotation/MemberClasses; + value = { + Linner/TestCls$A;, + Linner/TestCls$I; + } +.end annotation + + +# direct methods +.method public constructor ()V + .registers 1 + + .prologue + .line 3 + invoke-direct {p0}, Ljava/lang/Object;->()V + + return-void +.end method