Skip to content

Commit

Permalink
fix: process try/catch without move-exception instruction (#395)
Browse files Browse the repository at this point in the history
  • Loading branch information
@skylot
skylot committed Nov 26, 2018
1 parent 3a798cb commit 6d59f77
Show file tree
Hide file tree
Showing 4 changed files with 141 additions and 69 deletions.
60 changes: 17 additions & 43 deletions jadx-core/src/main/java/jadx/core/dex/visitors/blocksmaker/BlockExceptionHandler.java
View file
Original file line number Diff line number Diff line change
@@ -1,8 +1,5 @@
package jadx.core.dex.visitors.blocksmaker;

import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

import jadx.core.dex.attributes.AFlag;
import jadx.core.dex.attributes.AType;
import jadx.core.dex.instructions.InsnType;
Expand All @@ -23,8 +20,6 @@

public class BlockExceptionHandler extends AbstractVisitor {

private static final Logger LOG = LoggerFactory.getLogger(BlockExceptionHandler.class);

@Override
public void visit(MethodNode mth) {
if (mth.isNoCode()) {
Expand All @@ -40,35 +35,34 @@ public void visit(MethodNode mth) {
processExceptionHandlers(mth, block);
}
for (BlockNode block : mth.getBasicBlocks()) {
processTryCatchBlocks(mth, block);
processTryCatchBlocks(block);
}
}

/**
* Set exception handler attribute for whole block
*/
private static void markExceptionHandlers(BlockNode block) {
if (block.getInstructions().isEmpty()) {
return;
}
InsnNode me = block.getInstructions().get(0);
ExcHandlerAttr handlerAttr = me.get(AType.EXC_HANDLER);
ExcHandlerAttr handlerAttr = block.get(AType.EXC_HANDLER);
if (handlerAttr == null) {
return;
}
ExceptionHandler excHandler = handlerAttr.getHandler();
block.addAttr(handlerAttr);
ArgType argType = excHandler.isCatchAll() ? ArgType.THROWABLE : excHandler.getCatchType().getType();
if (me.getType() == InsnType.MOVE_EXCEPTION) {
// set correct type for 'move-exception' operation
RegisterArg resArg = InsnArg.reg(me.getResult().getRegNum(), argType);
me.setResult(resArg);
me.add(AFlag.DONT_INLINE);
excHandler.setArg(resArg);
} else {
// handler arguments not used
excHandler.setArg(new NamedArg("unused", argType));
if (!block.getInstructions().isEmpty()) {
InsnNode me = block.getInstructions().get(0);
if (me.getType() == InsnType.MOVE_EXCEPTION) {
// set correct type for 'move-exception' operation
RegisterArg resArg = InsnArg.reg(me.getResult().getRegNum(), argType);
resArg.copyAttributesFrom(me);
me.setResult(resArg);
me.add(AFlag.DONT_INLINE);
excHandler.setArg(resArg);
return;
}
}
// handler arguments not used
excHandler.setArg(new NamedArg("unused", argType));
}

private static void processExceptionHandlers(MethodNode mth, BlockNode block) {
Expand Down Expand Up @@ -113,17 +107,15 @@ private static void processExceptionHandlers(MethodNode mth, BlockNode block) {
private static boolean onlyAllHandler(TryCatchBlock tryBlock) {
if (tryBlock.getHandlersCount() == 1) {
ExceptionHandler eh = tryBlock.getHandlers().iterator().next();
if (eh.isCatchAll() || eh.isFinally()) {
return true;
}
return eh.isCatchAll() || eh.isFinally();
}
return false;
}

/**
* If all instructions in block have same 'catch' attribute mark it as 'TryCatch' block.
*/
private static void processTryCatchBlocks(MethodNode mth, BlockNode block) {
private static void processTryCatchBlocks(BlockNode block) {
CatchAttr commonCatchAttr = null;
for (InsnNode insn : block.getInstructions()) {
CatchAttr catchAttr = insn.get(AType.CATCH_BLOCK);
Expand All @@ -139,24 +131,6 @@ private static void processTryCatchBlocks(MethodNode mth, BlockNode block) {
}
if (commonCatchAttr != null) {
block.addAttr(commonCatchAttr);
// connect handler to block
for (ExceptionHandler handler : commonCatchAttr.getTryBlock().getHandlers()) {
connectHandler(mth, handler);
}
}
}

private static void connectHandler(MethodNode mth, ExceptionHandler handler) {
int addr = handler.getHandleOffset();
for (BlockNode block : mth.getBasicBlocks()) {
ExcHandlerAttr bh = block.get(AType.EXC_HANDLER);
if (bh != null && bh.getHandler().getHandleOffset() == addr) {
handler.setHandlerBlock(block);
break;
}
}
if (handler.getHandlerBlock() == null) {
LOG.warn("Exception handler block not set for {}, mth: {}", handler, mth);
}
}
}
95 changes: 69 additions & 26 deletions jadx-core/src/main/java/jadx/core/dex/visitors/blocksmaker/BlockSplitter.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
import jadx.core.dex.nodes.InsnNode;
import jadx.core.dex.nodes.MethodNode;
import jadx.core.dex.trycatch.CatchAttr;
import jadx.core.dex.trycatch.ExcHandlerAttr;
import jadx.core.dex.trycatch.ExceptionHandler;
import jadx.core.dex.trycatch.SplitterBlockAttr;
import jadx.core.dex.visitors.AbstractVisitor;
Expand Down Expand Up @@ -81,11 +82,11 @@ private static void splitBasicBlocks(MethodNode mth) {
if (type == InsnType.RETURN || type == InsnType.THROW) {
mth.addExitBlock(curBlock);
}
BlockNode block = startNewBlock(mth, insn.getOffset());
BlockNode newBlock = startNewBlock(mth, insn.getOffset());
if (type == InsnType.MONITOR_ENTER || type == InsnType.MONITOR_EXIT) {
connect(curBlock, block);
connect(curBlock, newBlock);
}
curBlock = block;
curBlock = newBlock;
startNew = true;
} else {
startNew = isSplitByJump(prevInsn, insn)
Expand All @@ -95,40 +96,79 @@ private static void splitBasicBlocks(MethodNode mth) {
|| prevInsn.contains(AFlag.TRY_LEAVE)
|| prevInsn.getType() == InsnType.MOVE_EXCEPTION;
if (startNew) {
BlockNode block = startNewBlock(mth, insn.getOffset());
connect(curBlock, block);
curBlock = block;
curBlock = connectNewBlock(mth, curBlock, insn.getOffset());
}
}
}
// for try/catch make empty block for connect handlers
if (insn.contains(AFlag.TRY_ENTER)) {
BlockNode block;
if (insn.getOffset() != 0 && !startNew) {
block = startNewBlock(mth, insn.getOffset());
connect(curBlock, block);
curBlock = block;
}
curBlock = insertSplitterBlock(mth, blocksMap, curBlock, insn, startNew);
} else if (insn.contains(AType.EXC_HANDLER)) {
processExceptionHandler(mth, curBlock, insn);
blocksMap.put(insn.getOffset(), curBlock);

// add this insn in new block
block = startNewBlock(mth, -1);
curBlock.add(AFlag.SYNTHETIC);
SplitterBlockAttr splitter = new SplitterBlockAttr(curBlock);
block.addAttr(splitter);
curBlock.addAttr(splitter);
connect(curBlock, block);
curBlock = block;
curBlock.getInstructions().add(insn);
} else {
blocksMap.put(insn.getOffset(), curBlock);
curBlock.getInstructions().add(insn);
}
curBlock.getInstructions().add(insn);
prevInsn = insn;
}
// setup missing connections
setupConnections(mth, blocksMap);
}

/**
* Make separate block for exception handler. New block already added if MOVE_EXCEPTION insn exists.
* Also link ExceptionHandler with current block.
*/
private static void processExceptionHandler(MethodNode mth, BlockNode curBlock, InsnNode insn) {
ExcHandlerAttr excHandlerAttr = insn.get(AType.EXC_HANDLER);
insn.remove(AType.EXC_HANDLER);

BlockNode excHandlerBlock;
if (insn.getType() == InsnType.MOVE_EXCEPTION) {
excHandlerBlock = curBlock;
} else {
BlockNode newBlock = startNewBlock(mth, -1);
newBlock.add(AFlag.SYNTHETIC);
connect(newBlock, curBlock);

excHandlerBlock = newBlock;
}
excHandlerBlock.addAttr(excHandlerAttr);
excHandlerAttr.getHandler().setHandlerBlock(excHandlerBlock);
}

/**
* For try/catch make empty (splitter) block for connect handlers
*/
private static BlockNode insertSplitterBlock(MethodNode mth, Map<Integer, BlockNode> blocksMap,
BlockNode curBlock, InsnNode insn, boolean startNew) {
BlockNode splitterBlock;
if (insn.getOffset() == 0 || startNew) {
splitterBlock = curBlock;
} else {
splitterBlock = connectNewBlock(mth, curBlock, insn.getOffset());
}
blocksMap.put(insn.getOffset(), splitterBlock);

SplitterBlockAttr splitterAttr = new SplitterBlockAttr(splitterBlock);
splitterBlock.add(AFlag.SYNTHETIC);
splitterBlock.addAttr(splitterAttr);

// add this insn in new block
BlockNode newBlock = startNewBlock(mth, -1);
newBlock.getInstructions().add(insn);
newBlock.addAttr(splitterAttr);
connect(splitterBlock, newBlock);
return newBlock;
}

private static BlockNode connectNewBlock(MethodNode mth, BlockNode curBlock, int offset) {
BlockNode block = startNewBlock(mth, offset);
connect(curBlock, block);
return block;
}

static BlockNode startNewBlock(MethodNode mth, int offset) {
BlockNode block = new BlockNode(mth.getBasicBlocks().size(), offset);
mth.getBasicBlocks().add(block);
Expand Down Expand Up @@ -183,12 +223,12 @@ private static void setupConnections(MethodNode mth, Map<Integer, BlockNode> blo
BlockNode thisBlock = getBlock(jump.getDest(), blocksMap);
connect(srcBlock, thisBlock);
}
connectExceptionHandlers(blocksMap, block, insn);
connectExceptionHandlers(block, insn);
}
}
}

private static void connectExceptionHandlers(Map<Integer, BlockNode> blocksMap, BlockNode block, InsnNode insn) {
private static void connectExceptionHandlers(BlockNode block, InsnNode insn) {
CatchAttr catches = insn.get(AType.CATCH_BLOCK);
SplitterBlockAttr spl = block.get(AType.SPLITTER_BLOCK);
if (catches == null || spl == null) {
Expand All @@ -197,7 +237,7 @@ private static void connectExceptionHandlers(Map<Integer, BlockNode> blocksMap,
BlockNode splitterBlock = spl.getBlock();
boolean tryEnd = insn.contains(AFlag.TRY_LEAVE);
for (ExceptionHandler h : catches.getTryBlock().getHandlers()) {
BlockNode handlerBlock = getBlock(h.getHandleOffset(), blocksMap);
BlockNode handlerBlock = h.getHandlerBlock();
// skip self loop in handler
if (splitterBlock != handlerBlock) {
if (!handlerBlock.contains(AType.SPLITTER_BLOCK)) {
Expand Down Expand Up @@ -248,6 +288,9 @@ private static BlockNode getBlock(int offset, Map<Integer, BlockNode> blocksMap)
private static void removeInsns(MethodNode mth) {
for (BlockNode block : mth.getBasicBlocks()) {
block.getInstructions().removeIf(insn -> {
if (!insn.isAttrStorageEmpty()) {
return false;
}
InsnType insnType = insn.getType();
return insnType == InsnType.GOTO || insnType == InsnType.NOP;
});
Expand Down
39 changes: 39 additions & 0 deletions jadx-core/src/test/java/jadx/tests/integration/trycatch/TestTryCatchNoMoveExc2.java
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
package jadx.tests.integration.trycatch;

import org.junit.Test;

import jadx.core.dex.nodes.ClassNode;
import jadx.tests.api.SmaliTest;

import static jadx.tests.api.utils.JadxMatchers.containsLines;
import static jadx.tests.api.utils.JadxMatchers.containsOne;
import static org.junit.Assert.assertThat;

/**
* Issue: https://github.com/skylot/jadx/issues/395
*/
public class TestTryCatchNoMoveExc2 extends SmaliTest {

// private static void test(AutoCloseable closeable) {
// if (closeable != null) {
// try {
// closeable.close();
// } catch (Exception unused) {
// }
// System.nanoTime();
// }
// }

@Test
public void test() {
ClassNode cls = getClassNodeFromSmaliWithPath("trycatch", "TestTryCatchNoMoveExc2");
String code = cls.getCode().toString();

assertThat(code, containsOne("try {"));
assertThat(code, containsLines(2,
"} catch (Exception unused) {",
"}",
"System.nanoTime();"
));
}
}
16 changes: 16 additions & 0 deletions jadx-core/src/test/smali/trycatch/TestTryCatchNoMoveExc2.smali
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
.class public LTestTryCatchNoMoveExc2;
.super Ljava/lang/Object;

.method private static test(Ljava/lang/AutoCloseable;)V
.locals 0

:try_start_0
invoke-interface {p0}, Ljava/lang/AutoCloseable;->close()V
:try_end_0
.catch Ljava/lang/Exception; {:try_start_0 .. :try_end_0} :catch_0

:catch_0
invoke-static {}, Ljava/lang/System;->nanoTime()J

return-void
.end method

0 comments on commit 6d59f77

Please sign in to comment.