fix: limit 'if' region out block to current scope (#2791)
This commit is contained in:
@@ -5,6 +5,8 @@ import java.util.HashSet;
|
||||
import java.util.List;
|
||||
import java.util.Set;
|
||||
|
||||
import org.jetbrains.annotations.Nullable;
|
||||
|
||||
import jadx.core.dex.nodes.BlockNode;
|
||||
import jadx.core.dex.nodes.InsnNode;
|
||||
import jadx.core.dex.nodes.MethodNode;
|
||||
@@ -18,7 +20,7 @@ public final class IfInfo {
|
||||
private final BlockNode elseBlock;
|
||||
private final Set<BlockNode> skipBlocks;
|
||||
private final List<InsnNode> forceInlineInsns;
|
||||
private BlockNode outBlock;
|
||||
private @Nullable BlockNode outBlock;
|
||||
|
||||
public IfInfo(MethodNode mth, IfCondition condition, BlockNode thenBlock, BlockNode elseBlock) {
|
||||
this(mth, condition, thenBlock, elseBlock, BlockSet.empty(mth), new HashSet<>(), new ArrayList<>());
|
||||
@@ -84,11 +86,11 @@ public final class IfInfo {
|
||||
return elseBlock;
|
||||
}
|
||||
|
||||
public BlockNode getOutBlock() {
|
||||
public @Nullable BlockNode getOutBlock() {
|
||||
return outBlock;
|
||||
}
|
||||
|
||||
public void setOutBlock(BlockNode outBlock) {
|
||||
public void setOutBlock(@Nullable BlockNode outBlock) {
|
||||
this.outBlock = outBlock;
|
||||
}
|
||||
|
||||
|
||||
@@ -89,7 +89,7 @@ public class ProcessTryCatchRegions extends AbstractRegionVisitor {
|
||||
// this block/region has a path from an exception handler so is after the end of the try block
|
||||
continue;
|
||||
}
|
||||
tryRegion.getSubBlocks().add(cont);
|
||||
tryRegion.add(cont);
|
||||
}
|
||||
}
|
||||
if (tryRegion.getSubBlocks().isEmpty()) {
|
||||
|
||||
@@ -55,13 +55,12 @@ final class IfRegionMaker {
|
||||
this.regionMaker = regionMaker;
|
||||
}
|
||||
|
||||
@Nullable
|
||||
BlockNode process(IRegion currentRegion, BlockNode block, IfNode ifnode, RegionStack stack) {
|
||||
|
||||
if (block.contains(AFlag.ADDED_TO_REGION)) {
|
||||
// block already included in other 'if' region
|
||||
return ifnode.getThenBlock();
|
||||
}
|
||||
|
||||
IfInfo currentIf = makeIfInfo(mth, block);
|
||||
if (currentIf == null) {
|
||||
return null;
|
||||
@@ -80,7 +79,7 @@ final class IfRegionMaker {
|
||||
block.add(AFlag.DONT_INVERT);
|
||||
}
|
||||
}
|
||||
IfInfo modifiedIf = restructureIf(mth, block, currentIf);
|
||||
IfInfo modifiedIf = restructureIf(block, currentIf);
|
||||
if (modifiedIf != null) {
|
||||
currentIf = modifiedIf;
|
||||
} else {
|
||||
@@ -88,7 +87,7 @@ final class IfRegionMaker {
|
||||
return null;
|
||||
}
|
||||
currentIf = makeIfInfo(mth, block);
|
||||
currentIf = restructureIf(mth, block, currentIf);
|
||||
currentIf = restructureIf(block, currentIf);
|
||||
if (currentIf == null) {
|
||||
// all attempts failed
|
||||
return null;
|
||||
@@ -173,7 +172,7 @@ final class IfRegionMaker {
|
||||
return info;
|
||||
}
|
||||
|
||||
static IfInfo restructureIf(MethodNode mth, BlockNode block, IfInfo info) {
|
||||
IfInfo restructureIf(BlockNode block, IfInfo info) {
|
||||
BlockNode thenBlock = info.getThenBlock();
|
||||
BlockNode elseBlock = info.getElseBlock();
|
||||
|
||||
@@ -211,8 +210,8 @@ final class IfRegionMaker {
|
||||
// getPathCross may not find outBlock (e.g. one branch has return, outBlock definitely is
|
||||
// null), so should check further
|
||||
if (info.getOutBlock() == null) {
|
||||
BlockNode scopeOutBlockThen = findScopeOutBlock(mth, info.getThenBlock());
|
||||
BlockNode scopeOutBlockElse = findScopeOutBlock(mth, info.getElseBlock());
|
||||
BlockNode scopeOutBlockThen = findScopeOutBlock(info.getThenBlock());
|
||||
BlockNode scopeOutBlockElse = findScopeOutBlock(info.getElseBlock());
|
||||
if (scopeOutBlockThen == null && scopeOutBlockElse != null) {
|
||||
info.setOutBlock(scopeOutBlockElse);
|
||||
} else if (scopeOutBlockThen != null && scopeOutBlockElse == null) {
|
||||
@@ -228,7 +227,7 @@ final class IfRegionMaker {
|
||||
return info;
|
||||
}
|
||||
|
||||
static BlockNode findOutBlock(MethodNode mth, BlockNode thenBlock, BlockNode elseBlock) {
|
||||
static @Nullable BlockNode findOutBlock(MethodNode mth, BlockNode thenBlock, BlockNode elseBlock) {
|
||||
if (thenBlock == elseBlock) {
|
||||
return thenBlock;
|
||||
}
|
||||
@@ -365,7 +364,7 @@ final class IfRegionMaker {
|
||||
/**
|
||||
* if startBlock is in a (try) scope, find the scope end as outBlock
|
||||
*/
|
||||
private @Nullable static BlockNode findScopeOutBlock(MethodNode mth, BlockNode startBlock) {
|
||||
private @Nullable BlockNode findScopeOutBlock(BlockNode startBlock) {
|
||||
if (startBlock == null) {
|
||||
return null;
|
||||
}
|
||||
@@ -385,7 +384,14 @@ final class IfRegionMaker {
|
||||
break;
|
||||
}
|
||||
}
|
||||
|
||||
if (scopeOutBlock != null) {
|
||||
// check if out block still inside scope limited by 'exit' blocks
|
||||
for (BlockNode exit : regionMaker.getStack().getExits()) {
|
||||
if (BlockUtils.isPathExists(exit, scopeOutBlock)) {
|
||||
return null;
|
||||
}
|
||||
}
|
||||
}
|
||||
return scopeOutBlock;
|
||||
}
|
||||
|
||||
|
||||
@@ -14,7 +14,6 @@ import jadx.core.dex.instructions.IfNode;
|
||||
import jadx.core.dex.instructions.InsnType;
|
||||
import jadx.core.dex.instructions.SwitchInsn;
|
||||
import jadx.core.dex.nodes.BlockNode;
|
||||
import jadx.core.dex.nodes.IRegion;
|
||||
import jadx.core.dex.nodes.InsnContainer;
|
||||
import jadx.core.dex.nodes.InsnNode;
|
||||
import jadx.core.dex.nodes.MethodNode;
|
||||
@@ -79,7 +78,7 @@ public class RegionMaker {
|
||||
/**
|
||||
* Recursively traverse all blocks from 'block' until block from 'exits'
|
||||
*/
|
||||
private @Nullable BlockNode traverse(IRegion r, BlockNode block) {
|
||||
private @Nullable BlockNode traverse(Region r, BlockNode block) {
|
||||
if (block.contains(AFlag.MTH_EXIT_BLOCK)) {
|
||||
return null;
|
||||
}
|
||||
@@ -125,7 +124,7 @@ public class RegionMaker {
|
||||
}
|
||||
}
|
||||
if (!processed) {
|
||||
r.getSubBlocks().add(block);
|
||||
r.add(block);
|
||||
next = getNextBlock(block);
|
||||
}
|
||||
if (next != null && !stack.containsExit(block) && !stack.containsExit(next)) {
|
||||
|
||||
@@ -106,6 +106,10 @@ final class RegionStack {
|
||||
return curState.exits.contains(exit);
|
||||
}
|
||||
|
||||
public Iterable<BlockNode> getExits() {
|
||||
return curState.exits;
|
||||
}
|
||||
|
||||
public IRegion peekRegion() {
|
||||
return curState.region;
|
||||
}
|
||||
|
||||
@@ -0,0 +1,50 @@
|
||||
package jadx.tests.integration.trycatch;
|
||||
|
||||
import java.io.BufferedReader;
|
||||
import java.io.IOException;
|
||||
import java.io.Reader;
|
||||
import java.io.StringReader;
|
||||
|
||||
import org.junit.jupiter.api.Test;
|
||||
|
||||
import jadx.tests.api.IntegrationTest;
|
||||
|
||||
import static jadx.tests.api.utils.assertj.JadxAssertions.assertThat;
|
||||
|
||||
public class TestLoopInTryCatch2 extends IntegrationTest {
|
||||
|
||||
public static class TestCls {
|
||||
public String test(Reader in) throws IOException {
|
||||
StringBuilder sb = new StringBuilder();
|
||||
try {
|
||||
BufferedReader bufferedReader = new BufferedReader(in);
|
||||
while (true) {
|
||||
String line = bufferedReader.readLine();
|
||||
if (line == null) {
|
||||
break;
|
||||
}
|
||||
sb.append(line);
|
||||
}
|
||||
bufferedReader.close();
|
||||
} catch (IOException e) {
|
||||
doSomething();
|
||||
}
|
||||
return sb.toString();
|
||||
}
|
||||
|
||||
private void doSomething() {
|
||||
}
|
||||
|
||||
public void check() throws IOException {
|
||||
String text = "line1\nline2";
|
||||
assertThat(test(new StringReader(text))).isEqualTo(text.replace("\n", ""));
|
||||
}
|
||||
}
|
||||
|
||||
@Test
|
||||
public void test() {
|
||||
assertThat(getClassNode(TestCls.class))
|
||||
.code()
|
||||
.containsOne("} catch (IOException e) {");
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user