core: fix 'finally' extract (fix #53 and #54)

jadx-core/src/main/java/jadx/core/Jadx.java

@@ -4,7 +4,7 @@ import jadx.api.IJadxArgs;
 import jadx.core.codegen.CodeGen;
 import jadx.core.dex.visitors.ClassModifier;
 import jadx.core.dex.visitors.CodeShrinker;
-import jadx.core.dex.visitors.ConstInlinerVisitor;
+import jadx.core.dex.visitors.ConstInlineVisitor;
 import jadx.core.dex.visitors.DebugInfoVisitor;
 import jadx.core.dex.visitors.DotGraphVisitor;
 import jadx.core.dex.visitors.EnumVisitor;
@@ -73,7 +73,7 @@ public class Jadx {
 				passes.add(DotGraphVisitor.dumpRaw(outDir));
 			}
 
-			passes.add(new ConstInlinerVisitor());
+			passes.add(new ConstInlineVisitor());
 			passes.add(new FinishTypeInference());
 			passes.add(new EliminatePhiNodes());
 

jadx-core/src/main/java/jadx/core/codegen/InsnGen.java

@@ -442,11 +442,6 @@ public class InsnGen {
 				addArg(code, insn.getArg(0));
 				break;
 
-			case PHI:
-				assert isFallback();
-				code.add("PHI(").add(String.valueOf(insn.getArgsCount())).add(")");
-				break;
-
 			/* fallback mode instructions */
 			case IF:
 				assert isFallback() : "if insn in not fallback mode";

jadx-core/src/main/java/jadx/core/dex/instructions/PhiInsn.java

@@ -4,35 +4,93 @@ import jadx.core.dex.attributes.AFlag;
 import jadx.core.dex.instructions.args.ArgType;
 import jadx.core.dex.instructions.args.InsnArg;
 import jadx.core.dex.instructions.args.RegisterArg;
+import jadx.core.dex.nodes.BlockNode;
 import jadx.core.dex.nodes.InsnNode;
+import jadx.core.utils.InstructionRemover;
 import jadx.core.utils.Utils;
+import jadx.core.utils.exceptions.JadxRuntimeException;
 
-public class PhiInsn extends InsnNode {
+import java.util.IdentityHashMap;
+import java.util.Map;
+
+import org.jetbrains.annotations.NotNull;
+
+public final class PhiInsn extends InsnNode {
+
+	private final Map<RegisterArg, BlockNode> blockBinds;
 
 	public PhiInsn(int regNum, int predecessors) {
 		super(InsnType.PHI, predecessors);
+		this.blockBinds = new IdentityHashMap<RegisterArg, BlockNode>(predecessors);
 		setResult(InsnArg.reg(regNum, ArgType.UNKNOWN));
-		for (int i = 0; i < predecessors; i++) {
-			addReg(regNum, ArgType.UNKNOWN);
-		}
 		add(AFlag.DONT_INLINE);
 	}
 
+	public RegisterArg bindArg(BlockNode pred) {
+		RegisterArg arg = InsnArg.reg(getResult().getRegNum(), getResult().getType());
+		bindArg(arg, pred);
+		return arg;
+	}
+
+	public void bindArg(RegisterArg arg, BlockNode pred) {
+		if (blockBinds.containsValue(pred)) {
+			throw new JadxRuntimeException("Duplicate predecessors in PHI insn: " + pred + ", " + this);
+		}
+		addArg(arg);
+		blockBinds.put(arg, pred);
+	}
+
+	public BlockNode getBlockByArg(RegisterArg arg) {
+		return blockBinds.get(arg);
+	}
+
+	public Map<RegisterArg, BlockNode> getBlockBinds() {
+		return blockBinds;
+	}
+
 	@Override
+	@NotNull
 	public RegisterArg getArg(int n) {
 		return (RegisterArg) super.getArg(n);
 	}
 
-	public boolean removeArg(RegisterArg arg) {
-		boolean isRemoved = super.removeArg(arg);
-		if (isRemoved) {
-			arg.getSVar().setUsedInPhi(null);
+	@Override
+	public boolean removeArg(InsnArg arg) {
+		if (!(arg instanceof RegisterArg)) {
+			return false;
+		}
+		RegisterArg reg = (RegisterArg) arg;
+		if (super.removeArg(reg)) {
+			blockBinds.remove(reg);
+			InstructionRemover.fixUsedInPhiFlag(reg);
+			return true;
+		}
+		return false;
+	}
+
+	@Override
+	public boolean replaceArg(InsnArg from, InsnArg to) {
+		if (!(from instanceof RegisterArg) || !(to instanceof RegisterArg)) {
+			return false;
+		}
+		BlockNode pred = getBlockByArg((RegisterArg) from);
+		if (pred == null) {
+			throw new JadxRuntimeException("Unknown predecessor block by arg " + from + " in PHI: " + this);
+		}
+		if (removeArg(from)) {
+			bindArg((RegisterArg) to, pred);
 		}
-		return isRemoved;
+		return true;
+	}
+
+	@Override
+	public void setArg(int n, InsnArg arg) {
+		throw new JadxRuntimeException("Unsupported operation for PHI node");
 	}
 
 	@Override
 	public String toString() {
-		return "PHI: " + getResult() + " = " + Utils.listToString(getArguments());
+		return "PHI: " + getResult() + " = " + Utils.listToString(getArguments())
+				+ " binds: " + blockBinds;
 	}
 }

jadx-core/src/main/java/jadx/core/dex/instructions/args/RegisterArg.java

@@ -12,6 +12,7 @@ import jadx.core.dex.nodes.FieldNode;
 import jadx.core.dex.nodes.InsnNode;
 import jadx.core.dex.nodes.parser.FieldValueAttr;
 
+import org.jetbrains.annotations.NotNull;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
@@ -44,7 +45,7 @@ public class RegisterArg extends InsnArg implements Named {
 		return sVar;
 	}
 
-	void setSVar(SSAVar sVar) {
+	void setSVar(@NotNull SSAVar sVar) {
 		this.sVar = sVar;
 	}
 
@@ -162,7 +163,7 @@ public class RegisterArg extends InsnArg implements Named {
 
 	@Override
 	public int hashCode() {
-		return (regNum * 31 + type.hashCode()) * 31 + (sVar != null ? sVar.hashCode() : 0);
+		return regNum * 31 + type.hashCode();
 	}
 
 	@Override

jadx-core/src/main/java/jadx/core/dex/nodes/InsnNode.java

@@ -111,6 +111,10 @@ public class InsnNode extends LineAttrNode {
 		for (int i = 0; i < count; i++) {
 			if (arg == arguments.get(i)) {
 				arguments.remove(i);
+				if (arg instanceof RegisterArg) {
+					RegisterArg reg = (RegisterArg) arg;
+					reg.getSVar().removeUse(reg);
+				}
 				return true;
 			}
 		}

jadx-core/src/main/java/jadx/core/dex/trycatch/TryCatchBlock.java

@@ -64,11 +64,26 @@ public class TryCatchBlock {
 	private void unbindHandler(ExceptionHandler handler) {
 		for (BlockNode block : handler.getBlocks()) {
 			block.add(AFlag.SKIP);
+			ExcHandlerAttr excHandlerAttr = block.get(AType.EXC_HANDLER);
+			if (excHandlerAttr != null) {
+				if (excHandlerAttr.getHandler().equals(handler)) {
+					block.remove(AType.EXC_HANDLER);
+				}
+			}
+			SplitterBlockAttr splitter = handler.getHandlerBlock().get(AType.SPLITTER_BLOCK);
+			if (splitter != null) {
+				splitter.getBlock().remove(AType.SPLITTER_BLOCK);
+			}
 		}
 	}
 
 	private void removeWholeBlock(MethodNode mth) {
 		// self destruction
+		for (Iterator<ExceptionHandler> it = handlers.iterator(); it.hasNext(); ) {
+			ExceptionHandler h = it.next();
+			unbindHandler(h);
+			it.remove();
+		}
 		for (InsnNode insn : insns) {
 			insn.removeAttr(attr);
 		}
@@ -83,9 +98,22 @@ public class TryCatchBlock {
 		insn.addAttr(attr);
 	}
 
-	public void removeInsn(InsnNode insn) {
+	public void removeInsn(MethodNode mth, InsnNode insn) {
 		insns.remove(insn);
 		insn.remove(AType.CATCH_BLOCK);
+		if (insns.isEmpty()) {
+			removeWholeBlock(mth);
+		}
+	}
+
+	public void removeBlock(MethodNode mth, BlockNode block) {
+		for (InsnNode insn : block.getInstructions()) {
+			insns.remove(insn);
+			insn.remove(AType.CATCH_BLOCK);
+		}
+		if (insns.isEmpty()) {
+			removeWholeBlock(mth);
+		}
 	}
 
 	public Iterable<InsnNode> getInsns() {

jadx-core/src/main/java/jadx/core/dex/visitors/ConstInlinerVisitor.java → jadx-core/src/main/java/jadx/core/dex/visitors/ConstInlineVisitor.java

@@ -23,7 +23,7 @@ import jadx.core.utils.exceptions.JadxException;
 import java.util.ArrayList;
 import java.util.List;
 
-public class ConstInlinerVisitor extends AbstractVisitor {
+public class ConstInlineVisitor extends AbstractVisitor {
 
 	@Override
 	public void visit(MethodNode mth) throws JadxException {
@@ -38,14 +38,12 @@ public class ConstInlinerVisitor extends AbstractVisitor {
 					toRemove.add(insn);
 				}
 			}
-			if (!toRemove.isEmpty()) {
-				InstructionRemover.removeAll(mth, block, toRemove);
-			}
+			InstructionRemover.removeAll(mth, block, toRemove);
 		}
 	}
 
 	private static boolean checkInsn(MethodNode mth, InsnNode insn) {
-		if (insn.getType() != InsnType.CONST) {
+		if (insn.getType() != InsnType.CONST || insn.contains(AFlag.DONT_INLINE)) {
 			return false;
 		}
 		InsnArg arg = insn.getArg(0);

jadx-core/src/main/java/jadx/core/dex/visitors/FallbackModeVisitor.java

@@ -33,7 +33,7 @@ public class FallbackModeVisitor extends AbstractVisitor {
 					case CONST_CLASS:
 					case CMP_L:
 					case CMP_G:
-						catchAttr.getTryBlock().removeInsn(insn);
+						catchAttr.getTryBlock().removeInsn(mth, insn);
 						break;
 
 					default:

jadx-core/src/main/java/jadx/core/dex/visitors/blocksmaker/BlockProcessor.java

@@ -10,6 +10,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.CatchAttr;
 import jadx.core.dex.visitors.AbstractVisitor;
 import jadx.core.utils.BlockUtils;
 import jadx.core.utils.exceptions.JadxRuntimeException;
@@ -396,6 +397,10 @@ public class BlockProcessor extends AbstractVisitor {
 						|| !block.getSuccessors().isEmpty()) {
 					LOG.error("Block {} not deleted, method: {}", block, mth);
 				} else {
+					CatchAttr catchAttr = block.get(AType.CATCH_BLOCK);
+					if (catchAttr != null) {
+						catchAttr.getTryBlock().removeBlock(mth, block);
+					}
 					it.remove();
 				}
 			}

jadx-core/src/main/java/jadx/core/dex/visitors/blocksmaker/helpers/BlocksRemoveInfo.java

@@ -14,9 +14,14 @@ public final class BlocksRemoveInfo {
 	private final Set<BlocksPair> processed = new HashSet<BlocksPair>();
 	private final Set<BlocksPair> outs = new HashSet<BlocksPair>();
 	private final Map<RegisterArg, RegisterArg> regMap = new HashMap<RegisterArg, RegisterArg>();
-	private final BlocksPair start;
+
+	private BlocksPair start;
+	private BlocksPair end;
 
 	private int startSplitIndex;
+	private int endSplitIndex;
+
+	private BlockNode startPredecessor;
 
 	public BlocksRemoveInfo(BlocksPair start) {
 		this.start = start;
@@ -34,6 +39,18 @@ public final class BlocksRemoveInfo {
 		return start;
 	}
 
+	public void setStart(BlocksPair start) {
+		this.start = start;
+	}
+
+	public BlocksPair getEnd() {
+		return end;
+	}
+
+	public void setEnd(BlocksPair end) {
+		this.end = end;
+	}
+
 	public int getStartSplitIndex() {
 		return startSplitIndex;
 	}
@@ -42,6 +59,22 @@ public final class BlocksRemoveInfo {
 		this.startSplitIndex = startSplitIndex;
 	}
 
+	public int getEndSplitIndex() {
+		return endSplitIndex;
+	}
+
+	public void setEndSplitIndex(int endSplitIndex) {
+		this.endSplitIndex = endSplitIndex;
+	}
+
+	public void setStartPredecessor(BlockNode startPredecessor) {
+		this.startPredecessor = startPredecessor;
+	}
+
+	public BlockNode getStartPredecessor() {
+		return startPredecessor;
+	}
+
 	public Map<RegisterArg, RegisterArg> getRegMap() {
 		return regMap;
 	}
@@ -69,6 +102,7 @@ public final class BlocksRemoveInfo {
 	@Override
 	public String toString() {
 		return "BRI start: " + start
+				+ ", end: " + end
 				+ ", list: " + processed
 				+ ", outs: " + outs
 				+ ", regMap: " + regMap

jadx-core/src/main/java/jadx/core/dex/visitors/regions/RegionMaker.java

@@ -878,7 +878,6 @@ public class RegionMaker {
 		}
 	}
 
-	// TODO add blocks common for several handlers to some region
 	private void processExcHandler(ExceptionHandler handler, Set<BlockNode> exits) {
 		BlockNode start = handler.getHandlerBlock();
 		if (start == null) {

jadx-core/src/main/java/jadx/core/dex/visitors/typeinference/TypeInference.java

@@ -71,7 +71,10 @@ public class TypeInference extends AbstractVisitor {
 		for (int i = 0; i < phi.getArgsCount(); i++) {
 			RegisterArg arg = phi.getArg(i);
 			arg.setType(type);
-			arg.getSVar().setName(phi.getResult().getName());
+			SSAVar sVar = arg.getSVar();
+			if (sVar != null) {
+				sVar.setName(phi.getResult().getName());
+			}
 		}
 	}
 

jadx-core/src/main/java/jadx/core/utils/InsnList.java

@@ -29,12 +29,11 @@ public final class InsnList implements Iterable<InsnNode> {
 	}
 
 	public static int getIndex(List<InsnNode> list, InsnNode insn) {
-		int i = 0;
-		for (InsnNode curObj : list) {
-			if (curObj == insn) {
+		int size = list.size();
+		for (int i = 0; i < size; i++) {
+			if (list.get(i) == insn) {
 				return i;
 			}
-			i++;
 		}
 		return -1;
 	}

jadx-core/src/main/java/jadx/core/utils/InstructionRemover.java

 package jadx.core.utils;
 
 import jadx.core.dex.attributes.AFlag;
+import jadx.core.dex.instructions.InsnType;
+import jadx.core.dex.instructions.PhiInsn;
 import jadx.core.dex.instructions.args.InsnArg;
 import jadx.core.dex.instructions.args.InsnWrapArg;
 import jadx.core.dex.instructions.args.RegisterArg;
@@ -63,16 +65,40 @@ public class InstructionRemover {
 	}
 
 	public static void unbindInsn(MethodNode mth, InsnNode insn) {
-		RegisterArg r = insn.getResult();
-		if (r != null && r.getSVar() != null) {
-			mth.removeSVar(r.getSVar());
-		}
+		unbindResult(mth, insn);
 		for (InsnArg arg : insn.getArguments()) {
 			unbindArgUsage(mth, arg);
 		}
+		if (insn.getType() == InsnType.PHI) {
+			for (InsnArg arg : insn.getArguments()) {
+				if (arg instanceof RegisterArg) {
+					fixUsedInPhiFlag((RegisterArg) arg);
+				}
+			}
+		}
 		insn.add(AFlag.INCONSISTENT_CODE);
 	}
 
+	public static void fixUsedInPhiFlag(RegisterArg useReg) {
+		PhiInsn usedIn = null;
+		for (RegisterArg reg : useReg.getSVar().getUseList()) {
+			InsnNode parentInsn = reg.getParentInsn();
+			if (parentInsn != null
+					&& parentInsn.getType() == InsnType.PHI
+					&& parentInsn.containsArg(useReg)) {
+				usedIn = (PhiInsn) parentInsn;
+			}
+		}
+		useReg.getSVar().setUsedInPhi(usedIn);
+	}
+
+	public static void unbindResult(MethodNode mth, InsnNode insn) {
+		RegisterArg r = insn.getResult();
+		if (r != null && r.getSVar() != null) {
+			mth.removeSVar(r.getSVar());
+		}
+	}
+
 	public static void unbindArgUsage(MethodNode mth, InsnArg arg) {
 		if (arg instanceof RegisterArg) {
 			RegisterArg reg = (RegisterArg) arg;
@@ -122,6 +148,9 @@ public class InstructionRemover {
 	}
 
 	public static void removeAll(MethodNode mth, BlockNode block, List<InsnNode> insns) {
+		if (insns.isEmpty()) {
+			return;
+		}
 		removeAll(mth, block.getInstructions(), insns);
 	}
 

jadx-core/src/main/java/jadx/core/xmlgen/ManifestAttributes.java

@@ -154,6 +154,6 @@ public class ManifestAttributes {
 				return sb.deleteCharAt(sb.length() - 1).toString();
 			}
 		}
-		return "UNKNOWN_DATA_" + Integer.toHexString(value);
+		return "UNKNOWN_DATA_0x" + Integer.toHexString(value);
 	}
 }

jadx-core/src/test/java/jadx/tests/integration/TestArgInline.java

@@ -13,7 +13,7 @@ public class TestArgInline extends IntegrationTest {
 
 	public static class TestCls {
 
-		public void method(int a) {
+		public void test(int a) {
 			while (a < 10) {
 				int b = a + 1;
 				a = b;

jadx-core/src/test/java/jadx/tests/integration/loops/TestContinueInLoop2.java

@@ -52,7 +52,7 @@ public class TestContinueInLoop2 extends IntegrationTest {
 								TryCatchBlock catchBlock = catchAttr.getTryBlock();
 								if (handlerBlock != catchBlock) {
 									handlerBlock.merge(mth, catchBlock);
-									catchBlock.removeInsn(insn);
+									catchBlock.removeInsn(mth, insn);
 								}
 							}
 						}

jadx-core/src/test/java/jadx/tests/integration/trycatch/TestFinally.java

+package jadx.tests.integration.trycatch;
+
+import jadx.core.dex.nodes.ClassNode;
+import jadx.tests.api.IntegrationTest;
+
+import org.junit.Test;
+
+import static jadx.tests.api.utils.JadxMatchers.containsOne;
+import static org.hamcrest.CoreMatchers.not;
+import static org.junit.Assert.assertThat;
+
+public class TestFinally extends IntegrationTest {
+
+	public static class TestCls {
+		private static final String DISPLAY_NAME = "name";
+
+		String test(Context context, Object uri) {
+			Cursor cursor = null;
+			try {
+				String[] projection = {DISPLAY_NAME};
+				cursor = context.query(uri, projection);
+				int columnIndex = cursor.getColumnIndexOrThrow(DISPLAY_NAME);
+				cursor.moveToFirst();
+				return cursor.getString(columnIndex);
+			} finally {
+				if (cursor != null) {
+					cursor.close();
+				}
+			}
+		}
+
+		private class Context {
+			public Cursor query(Object o, String[] s) {
+				return null;
+			}
+		}
+
+		private class Cursor {
+			public void close() {
+			}
+
+			public void moveToFirst() {
+			}
+
+			public int getColumnIndexOrThrow(String s) {
+				return 0;
+			}
+
+			public String getString(int i) {
+				return null;
+			}
+		}
+	}
+
+	@Test
+	public void test() {
+		ClassNode cls = getClassNode(TestCls.class);
+		String code = cls.getCode().toString();
+
+		assertThat(code, containsOne("cursor.getString(columnIndex);"));
+		assertThat(code, not(containsOne("String str = true;")));
+	}
+}

jadx-core/src/test/java/jadx/tests/integration/trycatch/TestFinally2.java

+package jadx.tests.integration.trycatch;
+
+import jadx.core.dex.nodes.ClassNode;
+import jadx.tests.api.IntegrationTest;
+
+import java.io.ByteArrayInputStream;
+import java.io.IOException;
+import java.io.InputStream;
+
+import org.junit.Test;
+
+import static jadx.tests.api.utils.JadxMatchers.containsOne;
+import static org.junit.Assert.assertThat;
+
+public class TestFinally2 extends IntegrationTest {
+
+	public static class TestCls {
+
+		public Result test(byte[] data) throws IOException {
+			InputStream inputStream = null;
+			try {
+				inputStream = getInputStream(data);
+				decode(inputStream);
+				return new Result(400);
+			} finally {
+				closeQuietly(inputStream);
+			}
+		}
+
+		public static final class Result {
+			private final int mCode;
+
+			public Result(int code) {
+				mCode = code;
+			}
+
+			public int getCode() {
+				return mCode;
+			}
+		}
+
+		private InputStream getInputStream(byte[] data) throws IOException {
+			return new ByteArrayInputStream(data);
+		}
+
+		private int decode(InputStream inputStream) throws IOException {
+			return inputStream.available();
+		}
+
+		private void closeQuietly(InputStream is) {
+		}
+	}
+
+	@Test
+	public void test() {
+		ClassNode cls = getClassNode(TestCls.class);
+		String code = cls.getCode().toString();
+
+		assertThat(code, containsOne("decode(inputStream);"));
+		// TODO
+		// assertThat(code, not(containsOne("result =")));
+	}
+}