fix: process try/catch without move-exception instruction (#395)

This commit is contained in:
Skylot 2018-11-26 14:31:49 +03:00
parent 3a798cb21c
commit 6d59f77165
4 changed files with 141 additions and 69 deletions

View File

@ -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;
@ -23,8 +20,6 @@ import jadx.core.utils.InstructionRemover;
public class BlockExceptionHandler extends AbstractVisitor {
private static final Logger LOG = LoggerFactory.getLogger(BlockExceptionHandler.class);
@Override
public void visit(MethodNode mth) {
if (mth.isNoCode()) {
@ -40,7 +35,7 @@ public class BlockExceptionHandler extends AbstractVisitor {
processExceptionHandlers(mth, block);
}
for (BlockNode block : mth.getBasicBlocks()) {
processTryCatchBlocks(mth, block);
processTryCatchBlocks(block);
}
}
@ -48,27 +43,26 @@ public class BlockExceptionHandler extends AbstractVisitor {
* 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) {
@ -113,9 +107,7 @@ public class BlockExceptionHandler extends AbstractVisitor {
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;
}
@ -123,7 +115,7 @@ public class BlockExceptionHandler extends AbstractVisitor {
/**
* 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);
@ -139,24 +131,6 @@ public class BlockExceptionHandler extends AbstractVisitor {
}
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);
}
}
}

View File

@ -16,6 +16,7 @@ import jadx.core.dex.nodes.BlockNode;
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;
@ -81,11 +82,11 @@ public class BlockSplitter extends AbstractVisitor {
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)
@ -95,40 +96,79 @@ public class BlockSplitter extends AbstractVisitor {
|| 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);
@ -183,12 +223,12 @@ public class BlockSplitter extends AbstractVisitor {
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) {
@ -197,7 +237,7 @@ public class BlockSplitter extends AbstractVisitor {
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)) {
@ -248,6 +288,9 @@ public class BlockSplitter extends AbstractVisitor {
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;
});

View File

@ -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();"
));
}
}

View File

@ -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