Commit eadf046b authored by Skylot's avatar Skylot

chore: show try/catch processing problems in code comments

parent e9591efd
...@@ -23,6 +23,6 @@ public class SplitterBlockAttr implements IAttribute { ...@@ -23,6 +23,6 @@ public class SplitterBlockAttr implements IAttribute {
@Override @Override
public String toString() { public String toString() {
return "Splitter: " + block; return "Splitter:" + block;
} }
} }
...@@ -26,7 +26,7 @@ public class BlockFinish extends AbstractVisitor { ...@@ -26,7 +26,7 @@ public class BlockFinish extends AbstractVisitor {
for (BlockNode block : mth.getBasicBlocks()) { for (BlockNode block : mth.getBasicBlocks()) {
block.updateCleanSuccessors(); block.updateCleanSuccessors();
fixSplitterBlock(block); fixSplitterBlock(mth, block);
} }
mth.finishBasicBlocks(); mth.finishBasicBlocks();
...@@ -36,7 +36,7 @@ public class BlockFinish extends AbstractVisitor { ...@@ -36,7 +36,7 @@ public class BlockFinish extends AbstractVisitor {
* For evey exception handler must be only one splitter block, * For evey exception handler must be only one splitter block,
* select correct one and remove others if necessary. * select correct one and remove others if necessary.
*/ */
private static void fixSplitterBlock(BlockNode block) { private static void fixSplitterBlock(MethodNode mth, BlockNode block) {
ExcHandlerAttr excHandlerAttr = block.get(AType.EXC_HANDLER); ExcHandlerAttr excHandlerAttr = block.get(AType.EXC_HANDLER);
if (excHandlerAttr == null) { if (excHandlerAttr == null) {
return; return;
...@@ -58,7 +58,7 @@ public class BlockFinish extends AbstractVisitor { ...@@ -58,7 +58,7 @@ public class BlockFinish extends AbstractVisitor {
} }
BlockNode topSplitter = BlockUtils.getTopBlock(splitters.keySet()); BlockNode topSplitter = BlockUtils.getTopBlock(splitters.keySet());
if (topSplitter == null) { if (topSplitter == null) {
LOG.warn("Unknown top splitter block from list: {}", splitters); mth.addWarn("Unknown top exception splitter block from list: " + splitters);
return; return;
} }
for (Map.Entry<BlockNode, SplitterBlockAttr> entry : splitters.entrySet()) { for (Map.Entry<BlockNode, SplitterBlockAttr> entry : splitters.entrySet()) {
......
...@@ -60,7 +60,7 @@ public class CheckRegions extends AbstractVisitor { ...@@ -60,7 +60,7 @@ public class CheckRegions extends AbstractVisitor {
&& !block.getInstructions().isEmpty() && !block.getInstructions().isEmpty()
&& !block.contains(AFlag.SKIP)) { && !block.contains(AFlag.SKIP)) {
String blockCode = getBlockInsnStr(mth, block); String blockCode = getBlockInsnStr(mth, block);
mth.addWarn("Missing block: " + block + ", code:" + CodeWriter.NL + blockCode); mth.addWarn("Missing block: " + block + ", code skipped:" + CodeWriter.NL + blockCode);
} }
} }
} }
......
...@@ -27,7 +27,6 @@ import jadx.core.dex.trycatch.TryCatchBlock; ...@@ -27,7 +27,6 @@ import jadx.core.dex.trycatch.TryCatchBlock;
import jadx.core.utils.BlockUtils; import jadx.core.utils.BlockUtils;
import jadx.core.utils.ErrorsCounter; import jadx.core.utils.ErrorsCounter;
import jadx.core.utils.RegionUtils; import jadx.core.utils.RegionUtils;
import jadx.core.utils.exceptions.JadxRuntimeException;
/** /**
* Extract blocks to separate try/catch region * Extract blocks to separate try/catch region
...@@ -67,34 +66,39 @@ public class ProcessTryCatchRegions extends AbstractRegionVisitor { ...@@ -67,34 +66,39 @@ public class ProcessTryCatchRegions extends AbstractRegionVisitor {
// for each try block search nearest dominator block // for each try block search nearest dominator block
for (TryCatchBlock tb : tryBlocks) { for (TryCatchBlock tb : tryBlocks) {
if (tb.getHandlersCount() == 0) { if (tb.getHandlersCount() == 0) {
LOG.warn("No exception handlers in catch block, method: {}", mth); mth.addWarn("No exception handlers in catch block: " + tb);
continue; continue;
} }
BitSet bs = new BitSet(mth.getBasicBlocks().size()); processTryCatchBlock(mth, tb, tryBlocksMap);
for (ExceptionHandler excHandler : tb.getHandlers()) { }
BlockNode handlerBlock = excHandler.getHandlerBlock(); }
if (handlerBlock != null) {
SplitterBlockAttr splitter = handlerBlock.get(AType.SPLITTER_BLOCK); private static void processTryCatchBlock(MethodNode mth, TryCatchBlock tb, Map<BlockNode, TryCatchBlock> tryBlocksMap) {
if (splitter != null) { BitSet bs = new BitSet(mth.getBasicBlocks().size());
BlockNode block = splitter.getBlock(); for (ExceptionHandler excHandler : tb.getHandlers()) {
bs.set(block.getId()); BlockNode handlerBlock = excHandler.getHandlerBlock();
} if (handlerBlock != null) {
} SplitterBlockAttr splitter = handlerBlock.get(AType.SPLITTER_BLOCK);
} if (splitter != null) {
List<BlockNode> domBlocks = BlockUtils.bitSetToBlocks(mth, bs); BlockNode block = splitter.getBlock();
BlockNode domBlock; bs.set(block.getId());
if (domBlocks.size() != 1) {
domBlock = BlockUtils.getTopBlock(domBlocks);
if (domBlock == null) {
throw new JadxRuntimeException("Exception block dominator not found, method:" + mth + ", dom blocks: " + domBlocks);
} }
} else {
domBlock = domBlocks.get(0);
} }
TryCatchBlock prevTB = tryBlocksMap.put(domBlock, tb); }
if (prevTB != null) { List<BlockNode> domBlocks = BlockUtils.bitSetToBlocks(mth, bs);
ErrorsCounter.methodWarn(mth, "Failed to process nested try/catch"); BlockNode domBlock;
if (domBlocks.size() != 1) {
domBlock = BlockUtils.getTopBlock(domBlocks);
if (domBlock == null) {
mth.addWarn("Exception block dominator not found, dom blocks: " + domBlocks);
return;
} }
} else {
domBlock = domBlocks.get(0);
}
TryCatchBlock prevTB = tryBlocksMap.put(domBlock, tb);
if (prevTB != null) {
mth.addWarn("Failed to process nested try/catch");
} }
} }
...@@ -105,7 +109,7 @@ public class ProcessTryCatchRegions extends AbstractRegionVisitor { ...@@ -105,7 +109,7 @@ public class ProcessTryCatchRegions extends AbstractRegionVisitor {
if (region.getSubBlocks().contains(dominator)) { if (region.getSubBlocks().contains(dominator)) {
TryCatchBlock tb = tryBlocksMap.get(dominator); TryCatchBlock tb = tryBlocksMap.get(dominator);
if (!wrapBlocks(region, tb, dominator)) { if (!wrapBlocks(region, tb, dominator)) {
ErrorsCounter.methodWarn(mth, "Can't wrap try/catch for " + region); ErrorsCounter.methodWarn(mth, "Can't wrap try/catch for region: " + region);
} }
tryBlocksMap.remove(dominator); tryBlocksMap.remove(dominator);
return true; return true;
......
...@@ -929,7 +929,7 @@ public class RegionMaker { ...@@ -929,7 +929,7 @@ public class RegionMaker {
} }
} }
for (ExceptionHandler handler : tc.getHandlers()) { for (ExceptionHandler handler : tc.getHandlers()) {
processExcHandler(handler, exits); processExcHandler(mth, handler, exits);
} }
} }
return processHandlersOutBlocks(mth, tcs); return processHandlersOutBlocks(mth, tcs);
...@@ -968,12 +968,12 @@ public class RegionMaker { ...@@ -968,12 +968,12 @@ public class RegionMaker {
return excOutRegion; return excOutRegion;
} }
private void processExcHandler(ExceptionHandler handler, Set<BlockNode> exits) { private void processExcHandler(MethodNode mth, ExceptionHandler handler, Set<BlockNode> exits) {
BlockNode start = handler.getHandlerBlock(); BlockNode start = handler.getHandlerBlock();
if (start == null) { if (start == null) {
return; return;
} }
RegionStack stack = new RegionStack(mth); RegionStack stack = new RegionStack(this.mth);
BlockNode dom; BlockNode dom;
if (handler.isFinally()) { if (handler.isFinally()) {
SplitterBlockAttr splitterAttr = start.get(AType.SPLITTER_BLOCK); SplitterBlockAttr splitterAttr = start.get(AType.SPLITTER_BLOCK);
...@@ -986,11 +986,11 @@ public class RegionMaker { ...@@ -986,11 +986,11 @@ public class RegionMaker {
stack.addExits(exits); stack.addExits(exits);
} }
BitSet domFrontier = dom.getDomFrontier(); BitSet domFrontier = dom.getDomFrontier();
List<BlockNode> handlerExits = BlockUtils.bitSetToBlocks(mth, domFrontier); List<BlockNode> handlerExits = BlockUtils.bitSetToBlocks(this.mth, domFrontier);
boolean inLoop = mth.getLoopForBlock(start) != null; boolean inLoop = this.mth.getLoopForBlock(start) != null;
for (BlockNode exit : handlerExits) { for (BlockNode exit : handlerExits) {
if ((!inLoop || BlockUtils.isPathExists(start, exit)) if ((!inLoop || BlockUtils.isPathExists(start, exit))
&& RegionUtils.isRegionContainsBlock(mth.getRegion(), exit)) { && RegionUtils.isRegionContainsBlock(this.mth.getRegion(), exit)) {
stack.addExit(exit); stack.addExit(exit);
} }
} }
...@@ -998,7 +998,7 @@ public class RegionMaker { ...@@ -998,7 +998,7 @@ public class RegionMaker {
ExcHandlerAttr excHandlerAttr = start.get(AType.EXC_HANDLER); ExcHandlerAttr excHandlerAttr = start.get(AType.EXC_HANDLER);
if (excHandlerAttr == null) { if (excHandlerAttr == null) {
LOG.warn("Missing exception handler attribute for start block"); mth.addWarn("Missing exception handler attribute for start block: " + start);
} else { } else {
handler.getHandlerRegion().addAttr(excHandlerAttr); handler.getHandlerRegion().addAttr(excHandlerAttr);
} }
......
...@@ -127,23 +127,36 @@ public abstract class BaseExternalTest extends IntegrationTest { ...@@ -127,23 +127,36 @@ public abstract class BaseExternalTest extends IntegrationTest {
String[] lines = code.split(CodeWriter.NL); String[] lines = code.split(CodeWriter.NL);
for (MethodNode mth : classNode.getMethods()) { for (MethodNode mth : classNode.getMethods()) {
if (mthPattern.matcher(mth.getName()).matches()) { if (mthPattern.matcher(mth.getName()).matches()) {
int decompiledLine = mth.getDecompiledLine(); int decompiledLine = mth.getDecompiledLine() - 1;
StringBuilder mthCode = new StringBuilder(); StringBuilder mthCode = new StringBuilder();
int startLine = getCommentLinesCount(lines, decompiledLine);
int brackets = 0; int brackets = 0;
for (int i = decompiledLine - 1; i > 0 && i < lines.length; i++) { for (int i = startLine; i > 0 && i < lines.length; i++) {
String line = lines[i]; String line = lines[i];
mthCode.append(line).append(CodeWriter.NL); mthCode.append(line).append(CodeWriter.NL);
brackets += StringUtils.countMatches(line, '{'); if (i >= decompiledLine) {
brackets -= StringUtils.countMatches(line, '}'); brackets += StringUtils.countMatches(line, '{');
if (brackets <= 0) { brackets -= StringUtils.countMatches(line, '}');
break; if (brackets <= 0) {
break;
}
} }
} }
LOG.info("\n{}", mthCode); LOG.info("Print method: {}\n{}", mth.getMethodInfo().getShortId(), mthCode);
} }
} }
} }
protected int getCommentLinesCount(String[] lines, int line) {
for (int i = line - 1; i > 0 && i < lines.length; i--) {
String str = lines[i];
if (str.isEmpty() || str.equals(CodeWriter.NL)) {
return i + 1;
}
}
return 0;
}
private void printErrorReport(JadxDecompiler jadx) { private void printErrorReport(JadxDecompiler jadx) {
jadx.printErrorsReport(); jadx.printErrorsReport();
assertThat(jadx.getErrorsCount(), is(0)); assertThat(jadx.getErrorsCount(), is(0));
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment