Skip to content

Commit

Permalink
fix: optimize switch fallthrough (PR #2054)
Browse files Browse the repository at this point in the history
* cache post dom map between switch cases
* cache post dom map of whole methods
* calculate full post dom tree, fix switch out block detection

---------

Co-authored-by: Skylot <[email protected]>
  • Loading branch information
DanielFi and skylot authored Feb 14, 2024
1 parent 0143423 commit 0c33d72
Show file tree
Hide file tree
Showing 12 changed files with 237 additions and 236 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ public List<BlockNode> prepareBlocks() {
if (!block.contains(AFlag.EXC_BOTTOM_SPLITTER)) {
startLabel.set(block.getId());
}
if (prev.getSuccessors().size() == 1 && !mth.isPreExitBlocks(prev)) {
if (prev.getSuccessors().size() == 1 && !mth.isPreExitBlock(prev)) {
endGoto.set(prev.getId());
}
}
Expand All @@ -68,7 +68,7 @@ public List<BlockNode> prepareBlocks() {
if (block.contains(AType.EXC_HANDLER)) {
startLabel.set(block.getId());
}
if (nextBlock == null && !mth.isPreExitBlocks(block)) {
if (nextBlock == null && !mth.isPreExitBlock(block)) {
endGoto.set(block.getId());
}
prev = block;
Expand Down Expand Up @@ -145,7 +145,7 @@ public boolean isNeedEndGoto(BlockNode block) {
// DFS sort blocks to reduce goto count
private List<BlockNode> getSortedBlocks() {
List<BlockNode> list = new ArrayList<>(mth.getBasicBlocks().size());
BlockUtils.dfsVisit(mth, list::add);
BlockUtils.visitDFS(mth, list::add);
return list;
}
}
28 changes: 27 additions & 1 deletion jadx-core/src/main/java/jadx/core/dex/nodes/BlockNode.java
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,11 @@ public final class BlockNode extends AttrNode implements IBlock, Comparable<Bloc
*/
private BitSet doms = EmptyBitSet.EMPTY;

/**
* Post dominators, excluding self
*/
private BitSet postDoms = EmptyBitSet.EMPTY;

/**
* Dominance frontier
*/
Expand All @@ -56,6 +61,11 @@ public final class BlockNode extends AttrNode implements IBlock, Comparable<Bloc
*/
private BlockNode idom;

/**
* Immediate post dominator
*/
private BlockNode iPostDom;

/**
* Blocks on which dominates this block
*/
Expand Down Expand Up @@ -165,6 +175,14 @@ public void setDoms(BitSet doms) {
this.doms = doms;
}

public BitSet getPostDoms() {
return postDoms;
}

public void setPostDoms(BitSet postDoms) {
this.postDoms = postDoms;
}

public BitSet getDomFrontier() {
return domFrontier;
}
Expand All @@ -184,6 +202,14 @@ public void setIDom(BlockNode idom) {
this.idom = idom;
}

public BlockNode getIPostDom() {
return iPostDom;
}

public void setIPostDom(BlockNode iPostDom) {
this.iPostDom = iPostDom;
}

public List<BlockNode> getDominatesOn() {
return dominatesOn;
}
Expand Down Expand Up @@ -233,6 +259,6 @@ public String baseString() {

@Override
public String toString() {
return "B:" + cid + ':' + InsnUtils.formatOffset(startOffset);
return "B:" + id + ':' + InsnUtils.formatOffset(startOffset);
}
}
13 changes: 8 additions & 5 deletions jadx-core/src/main/java/jadx/core/dex/nodes/MethodNode.java
Original file line number Diff line number Diff line change
Expand Up @@ -360,10 +360,13 @@ public List<BlockNode> getBasicBlocks() {

public void setBasicBlocks(List<BlockNode> blocks) {
this.blocks = blocks;
int i = 0;
for (BlockNode block : blocks) {
block.setId(i);
i++;
updateBlockIds(blocks);
}

public void updateBlockIds(List<BlockNode> blocks) {
int count = blocks.size();
for (int i = 0; i < count; i++) {
blocks.get(i).setId(i);
}
}

Expand Down Expand Up @@ -391,7 +394,7 @@ public List<BlockNode> getPreExitBlocks() {
return exitBlock.getPredecessors();
}

public boolean isPreExitBlocks(BlockNode block) {
public boolean isPreExitBlock(BlockNode block) {
List<BlockNode> successors = block.getSuccessors();
if (successors.size() == 1) {
return successors.get(0).equals(exitBlock);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,12 @@

public final class SwitchRegion extends AbstractRegion implements IBranchRegion {

public static final Object DEFAULT_CASE_KEY = new Object();
public static final Object DEFAULT_CASE_KEY = new Object() {
@Override
public String toString() {
return "default";
}
};

private final BlockNode header;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -199,7 +199,7 @@ private void processBlock(MethodNode mth, BlockNode block, boolean error) {
dot.add("color=red,");
}
dot.add("label=\"{");
dot.add(String.valueOf(block.getCId())).add("\\:\\ ");
dot.add(String.valueOf(block.getId())).add("\\:\\ ");
dot.add(InsnUtils.formatOffset(block.getStartOffset()));
if (!attrs.isEmpty()) {
dot.add('|').add(attrs);
Expand All @@ -208,6 +208,8 @@ private void processBlock(MethodNode mth, BlockNode block, boolean error) {
dot.add('|');
dot.startLine("doms: ").add(escape(block.getDoms()));
dot.startLine("\\lidom: ").add(escape(block.getIDom()));
dot.startLine("\\lpost-doms: ").add(escape(block.getPostDoms()));
dot.startLine("\\lpost-idom: ").add(escape(block.getIPostDom()));
dot.startLine("\\ldom-f: ").add(escape(block.getDomFrontier()));
dot.startLine("\\ldoms-on: ").add(escape(Utils.listToString(block.getDominatesOn())));
dot.startLine("\\l");
Expand All @@ -230,10 +232,10 @@ private void processBlock(MethodNode mth, BlockNode block, boolean error) {

if (PRINT_DOMINATORS) {
for (BlockNode c : block.getDominatesOn()) {
conn.startLine(block.getCId() + " -> " + c.getCId() + "[color=green];");
conn.startLine(block.getId() + " -> " + c.getId() + "[color=green];");
}
for (BlockNode dom : BlockUtils.bitSetToBlocks(mth, block.getDomFrontier())) {
conn.startLine("f_" + block.getCId() + " -> f_" + dom.getCId() + "[color=blue];");
conn.startLine("f_" + block.getId() + " -> f_" + dom.getId() + "[color=blue];");
}
}
}
Expand Down Expand Up @@ -273,7 +275,7 @@ private String attributesString(IAttributeNode block) {
private String makeName(IContainer c) {
String name;
if (c instanceof BlockNode) {
name = "Node_" + ((BlockNode) c).getCId();
name = "Node_" + ((BlockNode) c).getId();
} else if (c instanceof IBlock) {
name = "Node_" + c.getClass().getSimpleName() + '_' + c.hashCode();
} else {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ public static boolean process(MethodNode mth) {
BlockProcessor.removeMarkedBlocks(mth);

BlockSet sorted = new BlockSet(mth);
BlockUtils.dfsVisit(mth, sorted::set);
BlockUtils.visitDFS(mth, sorted::set);
removeUnusedExcHandlers(mth, tryBlocks, sorted);
return true;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,8 @@ private static void processBlocksTree(MethodNode mth) {
registerLoops(mth);
processNestedLoops(mth);

PostDominatorTree.compute(mth);

updateCleanSuccessors(mth);
if (!mth.contains(AFlag.DISABLE_BLOCKS_LOCK)) {
mth.finishBasicBlocks();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,7 @@
import java.util.ArrayList;
import java.util.BitSet;
import java.util.List;

import org.jetbrains.annotations.NotNull;
import java.util.function.Function;

import jadx.core.dex.nodes.BlockNode;
import jadx.core.dex.nodes.MethodNode;
Expand All @@ -23,23 +22,22 @@ public class DominatorTree {

public static void compute(MethodNode mth) {
List<BlockNode> sorted = sortBlocks(mth);
BlockNode[] doms = build(sorted);
BlockNode[] doms = build(sorted, BlockNode::getPredecessors);
apply(sorted, doms);
}

private static List<BlockNode> sortBlocks(MethodNode mth) {
int blocksCount = mth.getBasicBlocks().size();
List<BlockNode> sorted = new ArrayList<>(blocksCount);
BlockUtils.dfsVisit(mth, sorted::add);
BlockUtils.visitDFS(mth, sorted::add);
if (sorted.size() != blocksCount) {
throw new JadxRuntimeException("Found unreachable blocks");
}
mth.setBasicBlocks(sorted);
return sorted;
}

@NotNull
private static BlockNode[] build(List<BlockNode> sorted) {
static BlockNode[] build(List<BlockNode> sorted, Function<BlockNode, List<BlockNode>> predFunc) {
int blocksCount = sorted.size();
BlockNode[] doms = new BlockNode[blocksCount];
doms[0] = sorted.get(0);
Expand All @@ -48,7 +46,7 @@ private static BlockNode[] build(List<BlockNode> sorted) {
changed = false;
for (int blockId = 1; blockId < blocksCount; blockId++) {
BlockNode b = sorted.get(blockId);
List<BlockNode> preds = b.getPredecessors();
List<BlockNode> preds = predFunc.apply(b);
int pickedPred = -1;
BlockNode newIDom = null;
for (BlockNode pred : preds) {
Expand All @@ -60,7 +58,7 @@ private static BlockNode[] build(List<BlockNode> sorted) {
}
}
if (newIDom == null) {
throw new JadxRuntimeException("No predecessors for block: " + b);
throw new JadxRuntimeException("No immediate dominator for block: " + b);
}
for (BlockNode predBlock : preds) {
int predId = predBlock.getId();
Expand Down Expand Up @@ -110,7 +108,7 @@ private static void apply(List<BlockNode> sorted, BlockNode[] doms) {
}
}

private static BitSet collectDoms(BlockNode[] doms, BlockNode idom) {
static BitSet collectDoms(BlockNode[] doms, BlockNode idom) {
BitSet domBS = new BitSet(doms.length);
BlockNode nextIDom = idom;
while (true) {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,65 @@
package jadx.core.dex.visitors.blocks;

import java.util.ArrayList;
import java.util.BitSet;
import java.util.List;

import jadx.core.dex.nodes.BlockNode;
import jadx.core.dex.nodes.MethodNode;
import jadx.core.utils.BlockUtils;
import jadx.core.utils.EmptyBitSet;

public class PostDominatorTree {

public static void compute(MethodNode mth) {
try {
int mthBlocksCount = mth.getBasicBlocks().size();
List<BlockNode> sorted = new ArrayList<>(mthBlocksCount);
BlockUtils.visitReverseDFS(mth, sorted::add);
// temporary set block ids to match reverse sorted order
// save old ids for later remapping
int blocksCount = sorted.size();
int[] ids = new int[mthBlocksCount];
for (int i = 0; i < blocksCount; i++) {
ids[i] = sorted.get(i).getId();
}
mth.updateBlockIds(sorted);

BlockNode[] postDoms = DominatorTree.build(sorted, BlockNode::getSuccessors);
BlockNode firstBlock = sorted.get(0);
firstBlock.setPostDoms(EmptyBitSet.EMPTY);
firstBlock.setIPostDom(null);
for (int i = 1; i < blocksCount; i++) {
BlockNode block = sorted.get(i);
BlockNode iPostDom = postDoms[i];
block.setIPostDom(iPostDom);
BitSet postDomBS = DominatorTree.collectDoms(postDoms, iPostDom);
block.setPostDoms(postDomBS);
}
for (int i = 1; i < blocksCount; i++) {
BlockNode block = sorted.get(i);
BitSet bs = new BitSet(blocksCount);
block.getPostDoms().stream().forEach(n -> bs.set(ids[n]));
bs.clear(ids[i]);
block.setPostDoms(bs);
}
// check for missing blocks in 'sorted' list
// can be caused by infinite loops
int blocksDelta = mthBlocksCount - blocksCount;
if (blocksDelta != 0) {
int insnsCount = 0;
for (BlockNode block : mth.getBasicBlocks()) {
if (block.getPostDoms() == null) {
block.setPostDoms(EmptyBitSet.EMPTY);
block.setIPostDom(null);
insnsCount += block.getInstructions().size();
}
}
mth.addInfoComment("Infinite loop detected, blocks: " + blocksDelta + ", insns: " + insnsCount);
}
} finally {
// revert block ids change
mth.updateBlockIds(mth.getBasicBlocks());
}
}
}
Loading

0 comments on commit 0c33d72

Please sign in to comment.