fix: improve restructure of nested try/catch/finally blocks (#2033)

This commit is contained in:
Skylot
2023-10-15 16:03:22 +01:00
parent 816308e3ca
commit 1bd4526e4c
7 changed files with 258 additions and 27 deletions
@@ -33,6 +33,7 @@ public enum AFlag {
EXC_TOP_SPLITTER,
EXC_BOTTOM_SPLITTER,
FINALLY_INSNS,
IGNORE_THROW_SPLIT,
SKIP_FIRST_ARG,
SKIP_ARG, // skip argument in invoke call
@@ -6,6 +6,7 @@ import java.util.Collections;
import java.util.Comparator;
import java.util.Deque;
import java.util.HashMap;
import java.util.HashSet;
import java.util.List;
import java.util.Map;
import java.util.Objects;
@@ -283,14 +284,7 @@ public class BlockExceptionHandler {
boolean catchInTry = innerTryBlock.getBlocks().stream().anyMatch(isHandlersIntersects(outerTryBlock));
boolean blocksOutsideHandler = outerTryBlock.getBlocks().stream().anyMatch(b -> !handlerBlocks.contains(b));
boolean makeInner = catchInHandler && (catchInTry || blocksOutsideHandler);
if (makeInner && innerTryBlock.isAllHandler()) {
// inner try block can't have catch-all handler
outerTryBlock.setBlocks(Utils.concatDistinct(outerTryBlock.getBlocks(), innerTryBlock.getBlocks()));
innerTryBlock.clear();
return false;
}
if (makeInner) {
if (catchInHandler && (catchInTry || blocksOutsideHandler)) {
// convert to inner
List<BlockNode> mergedBlocks = Utils.concatDistinct(outerTryBlock.getBlocks(), innerTryBlock.getBlocks());
innerTryBlock.getHandlers().removeAll(outerTryBlock.getHandlers());
@@ -299,7 +293,8 @@ public class BlockExceptionHandler {
outerTryBlock.setBlocks(mergedBlocks);
return false;
}
if (innerTryBlock.getHandlers().containsAll(outerTryBlock.getHandlers())) {
Set<ExceptionHandler> innerHandlerSet = new HashSet<>(innerTryBlock.getHandlers());
if (innerHandlerSet.containsAll(outerTryBlock.getHandlers())) {
// merge
List<BlockNode> mergedBlocks = Utils.concatDistinct(outerTryBlock.getBlocks(), innerTryBlock.getBlocks());
List<ExceptionHandler> handlers = Utils.concatDistinct(outerTryBlock.getHandlers(), innerTryBlock.getHandlers());
@@ -1,8 +1,12 @@
package jadx.core.dex.visitors.blocks;
import java.util.ArrayList;
import java.util.Collections;
import java.util.HashMap;
import java.util.HashSet;
import java.util.LinkedHashSet;
import java.util.List;
import java.util.Map;
import java.util.Set;
import org.jetbrains.annotations.Nullable;
@@ -20,6 +24,7 @@ import jadx.core.dex.nodes.BlockNode;
import jadx.core.dex.nodes.Edge;
import jadx.core.dex.nodes.InsnNode;
import jadx.core.dex.nodes.MethodNode;
import jadx.core.dex.trycatch.ExcHandlerAttr;
import jadx.core.dex.trycatch.TryCatchBlockAttr;
import jadx.core.dex.visitors.AbstractVisitor;
import jadx.core.utils.BlockUtils;
@@ -264,7 +269,7 @@ public class BlockProcessor extends AbstractVisitor {
if (mergeConstReturn(mth)) {
return true;
}
return splitReturnBlocks(mth);
return splitExitBlocks(mth);
}
private static boolean mergeConstReturn(MethodNode mth) {
@@ -478,11 +483,13 @@ public class BlockProcessor extends AbstractVisitor {
return false;
}
private static boolean splitReturnBlocks(MethodNode mth) {
private static boolean splitExitBlocks(MethodNode mth) {
boolean changed = false;
for (BlockNode preExitBlock : mth.getPreExitBlocks()) {
if (splitReturn(mth, preExitBlock)) {
changed = true;
} else if (splitThrow(mth, preExitBlock)) {
changed = true;
}
}
if (changed) {
@@ -522,7 +529,7 @@ public class BlockProcessor extends AbstractVisitor {
}
if (returnInsn.getArgsCount() == 1
&& returnBlock.getInstructions().size() == 1
&& !isReturnArgAssignInPred(preds, returnInsn)) {
&& !isArgAssignInPred(preds, returnInsn.getArg(0))) {
return false;
}
@@ -546,11 +553,67 @@ public class BlockProcessor extends AbstractVisitor {
return true;
}
private static boolean isReturnArgAssignInPred(List<BlockNode> preds, InsnNode returnInsn) {
InsnArg retArg = returnInsn.getArg(0);
if (retArg.isRegister()) {
RegisterArg arg = (RegisterArg) retArg;
int regNum = arg.getRegNum();
private static boolean splitThrow(MethodNode mth, BlockNode exitBlock) {
if (exitBlock.contains(AFlag.IGNORE_THROW_SPLIT)) {
return false;
}
List<BlockNode> preds = exitBlock.getPredecessors();
if (preds.size() < 2) {
return false;
}
InsnNode throwInsn = BlockUtils.getLastInsn(exitBlock);
if (throwInsn == null || throwInsn.getType() != InsnType.THROW) {
return false;
}
// split only for several exception handlers
// traverse predecessors to exception handler
Map<BlockNode, ExcHandlerAttr> handlersMap = new HashMap<>(preds.size());
Set<BlockNode> handlers = new HashSet<>(preds.size());
for (BlockNode pred : preds) {
BlockUtils.visitPredecessorsUntil(mth, pred, block -> {
ExcHandlerAttr excHandlerAttr = block.get(AType.EXC_HANDLER);
if (excHandlerAttr == null) {
return false;
}
boolean correctHandler = excHandlerAttr.getHandler().getBlocks().contains(block);
if (correctHandler && isArgAssignInPred(Collections.singletonList(block), throwInsn.getArg(0))) {
handlersMap.put(pred, excHandlerAttr);
handlers.add(block);
}
return correctHandler;
});
}
if (handlers.size() == 1) {
exitBlock.add(AFlag.IGNORE_THROW_SPLIT);
return false;
}
boolean first = true;
for (BlockNode pred : new ArrayList<>(preds)) {
if (first) {
first = false;
} else {
BlockNode newThrowBlock = BlockSplitter.startNewBlock(mth, -1);
newThrowBlock.add(AFlag.SYNTHETIC);
for (InsnNode oldInsn : exitBlock.getInstructions()) {
InsnNode copyInsn = oldInsn.copyWithoutSsa();
copyInsn.add(AFlag.SYNTHETIC);
newThrowBlock.getInstructions().add(copyInsn);
}
newThrowBlock.copyAttributesFrom(exitBlock);
ExcHandlerAttr excHandlerAttr = handlersMap.get(pred);
if (excHandlerAttr != null) {
excHandlerAttr.getHandler().addBlock(newThrowBlock);
}
BlockSplitter.replaceConnection(pred, exitBlock, newThrowBlock);
}
}
return true;
}
private static boolean isArgAssignInPred(List<BlockNode> preds, InsnArg arg) {
if (arg.isRegister()) {
int regNum = ((RegisterArg) arg).getRegNum();
for (BlockNode pred : preds) {
for (InsnNode insnNode : pred.getInstructions()) {
RegisterArg result = insnNode.getResult();
@@ -44,7 +44,6 @@ import jadx.core.utils.blocks.BlockPair;
)
public class MarkFinallyVisitor extends AbstractVisitor {
private static final Logger LOG = LoggerFactory.getLogger(MarkFinallyVisitor.class);
// private static final Logger LOG = LoggerFactory.getLogger(MarkFinallyVisitor.class);
@Override
public void visit(MethodNode mth) {
@@ -164,15 +163,22 @@ public class MarkFinallyVisitor extends AbstractVisitor {
mergeInnerTryBlocks = false;
}
// remove 'finally' from 'try' blocks, check all up paths on each exit (connected with finally exit)
// remove 'finally' from 'try' blocks,
// check all up paths on each exit (connected with 'finally' exit)
List<BlockNode> tryBlocks = allHandler.getTryBlock().getBlocks();
BlockNode bottomBlock = BlockUtils.getBottomBlock(allHandler.getBlocks());
if (bottomBlock == null) {
if (Consts.DEBUG_FINALLY) {
LOG.warn("No bottom block for handler: {} and blocks: {}", allHandler, allHandler.getBlocks());
}
return false;
}
BlockNode bottomFinallyBlock = BlockUtils.followEmptyPath(bottomBlock);
BlockNode bottom = BlockUtils.getNextBlock(bottomFinallyBlock);
if (bottom == null) {
if (Consts.DEBUG_FINALLY) {
LOG.warn("Finally bottom block not found for: {} and: {}", bottomBlock, bottomFinallyBlock);
}
return false;
}
boolean found = false;
@@ -197,12 +203,15 @@ public class MarkFinallyVisitor extends AbstractVisitor {
}
}
}
if (!found) {
if (Consts.DEBUG_FINALLY) {
LOG.info("Dup not found for all handler: {}", allHandler);
}
return false;
}
if (Consts.DEBUG_FINALLY) {
LOG.debug("Result slices:\n{}", extractInfo);
}
if (!found) {
return false;
}
if (!checkSlices(extractInfo)) {
mth.addWarnComment("Finally extract failed");
return false;
@@ -512,22 +512,29 @@ public class BlockUtils {
bs.or(blocksToBitSet(mth, stopBlocks));
}
List<BlockNode> list = new ArrayList<>();
traversePredecessors(start, bs, list::add);
traversePredecessors(start, bs, block -> {
list.add(block);
return false;
});
return list;
}
public static void visitPredecessorsUntil(MethodNode mth, BlockNode start, Predicate<BlockNode> visitor) {
traversePredecessors(start, newBlocksBitSet(mth), visitor);
}
/**
* Up BFS
* Up BFS.
* To stop return true from predicate
*/
private static void traversePredecessors(BlockNode start, BitSet visited, Consumer<BlockNode> visitor) {
private static void traversePredecessors(BlockNode start, BitSet visited, Predicate<BlockNode> visitor) {
Queue<BlockNode> queue = new ArrayDeque<>();
queue.add(start);
while (true) {
BlockNode current = queue.poll();
if (current == null) {
if (current == null || visitor.test(current)) {
return;
}
visitor.accept(current);
for (BlockNode next : current.getPredecessors()) {
int id = next.getId();
if (!visited.get(id)) {
@@ -0,0 +1,19 @@
package jadx.tests.integration.trycatch;
import org.junit.jupiter.api.Test;
import jadx.tests.api.SmaliTest;
import static jadx.tests.api.utils.assertj.JadxAssertions.assertThat;
public class TestNestedTryCatch5 extends SmaliTest {
@Test
public void test() {
disableCompilation();
assertThat(getClassNodeFromSmali())
.code()
.doesNotContain("?? ")
.countString(3, "throw ");
}
}
@@ -0,0 +1,137 @@
.class public Ltrycatch/TestNestedTryCatch5;
.super Ljava/lang/Object;
.method public test(Landroid/database/sqlite/SQLiteDatabase;)V
.registers 13
const/4 v0, 0x0
invoke-static {p1, v0}, LX/7Yz;->A0I(Ljava/lang/Object;I)V
iget-boolean v0, p0, LX/00y;->A00:Z
if-nez v0, :cond_9e
:try_start_8
iget-object v8, p0, LX/00y;->A03:LX/0Ux;
iget-object v0, p0, LX/00y;->A04:LX/0Km;
invoke-static {p1, v0}, LX/00y;->A01(Landroid/database/sqlite/SQLiteDatabase;LX/0Km;)LX/0g9;
move-result-object v10
check-cast v8, LX/0AB;
invoke-virtual {v8, v10}, LX/0AB;->A05(LX/0wU;)V
iget-object v0, v8, LX/0AB;->A01:LX/0Z4;
iget-object v9, v0, LX/0Z4;->A00:Landroidx/work/impl/WorkDatabase_Impl;
iput-object v10, v9, LX/0Rt;->A0B:LX/0wU;
const-string v0, "PRAGMA foreign_keys = ON"
invoke-interface {v10, v0}, LX/0wU;->Auv(Ljava/lang/String;)V
iget-object v1, v9, LX/0Rt;->A06:LX/0Uj;
iget-object v2, v1, LX/0Uj;->A05:Ljava/lang/Object;
monitor-enter v2
:try_end_25
.catchall {:try_start_8 .. :try_end_25} :catchall_95
:try_start_25
iget-boolean v0, v1, LX/0Uj;->A0D:Z
if-eqz v0, :cond_31
const-string v1, "ROOM"
const-string v0, "Invalidation tracker is initialized twice :/."
invoke-static {v1, v0}, Landroid/util/Log;->e(Ljava/lang/String;Ljava/lang/String;)I
goto :goto_4e
:cond_31
const-string v0, "PRAGMA temp_store = MEMORY;"
invoke-interface {v10, v0}, LX/0wU;->Auv(Ljava/lang/String;)V
const-string v0, "PRAGMA recursive_triggers=\'ON\';"
invoke-interface {v10, v0}, LX/0wU;->Auv(Ljava/lang/String;)V
const-string v0, "CREATE TEMP TABLE room_table_modification_log (table_id INTEGER PRIMARY KEY, invalidated INTEGER NOT NULL DEFAULT 0)"
invoke-interface {v10, v0}, LX/0wU;->Auv(Ljava/lang/String;)V
invoke-virtual {v1, v10}, LX/0Uj;->A00(LX/0wU;)V
const-string v0, "UPDATE room_table_modification_log SET invalidated = 0 WHERE invalidated = 1"
invoke-interface {v10, v0}, LX/0wU;->ArR(Ljava/lang/String;)LX/0wJ;
move-result-object v0
iput-object v0, v1, LX/0Uj;->A0C:LX/0wJ;
const/4 v0, 0x1
iput-boolean v0, v1, LX/0Uj;->A0D:Z
:try_end_4e
.catchall {:try_start_25 .. :try_end_4e} :catchall_92
:goto_4e
:try_start_4e
monitor-exit v2
iget-object v0, v9, LX/0Rt;->A01:Ljava/util/List;
if-eqz v0, :cond_8e
invoke-interface {v0}, Ljava/util/List;->size()I
move-result v7
const/4 v6, 0x0
:goto_58
if-ge v6, v7, :cond_8e
iget-object v0, v9, LX/0Rt;->A01:Ljava/util/List;
invoke-interface {v0, v6}, Ljava/util/List;->get(I)Ljava/lang/Object;
iget-object v5, v10, LX/0g9;->A00:Landroid/database/sqlite/SQLiteDatabase;
invoke-virtual {v5}, Landroid/database/sqlite/SQLiteDatabase;->beginTransaction()V
:try_end_64
.catchall {:try_start_4e .. :try_end_64} :catchall_95
:try_start_64
invoke-static {}, LX/001;->A0r()Ljava/lang/StringBuilder;
move-result-object v4
const-string v0, "DELETE FROM workspec WHERE state IN (2, 3, 5) AND (last_enqueue_time + minimum_retention_duration) < "
invoke-virtual {v4, v0}, Ljava/lang/StringBuilder;->append(Ljava/lang/String;)Ljava/lang/StringBuilder;
invoke-static {}, Ljava/lang/System;->currentTimeMillis()J
move-result-wide v2
sget-wide v0, LX/0Jm;->A00:J
sub-long/2addr v2, v0
invoke-virtual {v4, v2, v3}, Ljava/lang/StringBuilder;->append(J)Ljava/lang/StringBuilder;
const-string v0, " AND (SELECT COUNT(*)=0 FROM dependency WHERE prerequisite_id=id AND work_spec_id NOT IN (SELECT id FROM workspec WHERE state IN (2, 3, 5)))"
invoke-static {v0, v4}, LX/000;->A0X(Ljava/lang/String;Ljava/lang/StringBuilder;)Ljava/lang/String;
move-result-object v0
invoke-interface {v10, v0}, LX/0wU;->Auv(Ljava/lang/String;)V
invoke-virtual {v5}, Landroid/database/sqlite/SQLiteDatabase;->setTransactionSuccessful()V
:try_end_83
.catchall {:try_start_64 .. :try_end_83} :catchall_89
:try_start_83
invoke-virtual {v5}, Landroid/database/sqlite/SQLiteDatabase;->endTransaction()V
add-int/lit8 v6, v6, 0x1
goto :goto_58
:catchall_89
move-exception v0
invoke-virtual {v5}, Landroid/database/sqlite/SQLiteDatabase;->endTransaction()V
goto :goto_94
:cond_8e
const/4 v0, 0x0
iput-object v0, v8, LX/0AB;->A00:LX/0N8;
goto :goto_9e
:catchall_92
move-exception v0
monitor-exit v2
:goto_94
throw v0
:try_end_95
.catchall {:try_start_83 .. :try_end_95} :catchall_95
:catchall_95
move-exception v2
sget-object v1, LX/0Fu;->A04:LX/0Fu;
new-instance v0, LX/0oe;
invoke-direct {v0, v1, v2}, LX/0oe;-><init>(LX/0Fu;Ljava/lang/Throwable;)V
throw v0
:cond_9e
:goto_9e
const/4 v0, 0x1
iput-boolean v0, p0, LX/00y;->A01:Z
return-void
.end method