core: improve 'finally' extraction, refactor instructions

jadx-core/src/main/java/jadx/core/clsp/ConvertToClsSet.java

@@ -52,7 +52,8 @@ public class ConvertToClsSet {
 		LOG.info("done");
 	}
 
-	private static void addFilesFromDirectory(File dir, List<InputFile> inputFiles) throws IOException, DecodeException {
+	private static void addFilesFromDirectory(File dir,
+			List<InputFile> inputFiles) throws IOException, DecodeException {
 		File[] files = dir.listFiles();
 		if (files == null) {
 			return;

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

@@ -61,20 +61,15 @@ public class ArithNode extends InsnNode {
 	}
 
 	@Override
-	public boolean equals(Object obj) {
+	public boolean isSame(InsnNode obj) {
 		if (this == obj) {
 			return true;
 		}
-		if (!(obj instanceof ArithNode) || !super.equals(obj)) {
+		if (!(obj instanceof ArithNode) || !super.isSame(obj)) {
 			return false;
 		}
-		ArithNode that = (ArithNode) obj;
-		return op == that.op;
-	}
-
-	@Override
-	public int hashCode() {
-		return 31 * super.hashCode() + op.hashCode();
+		ArithNode other = (ArithNode) obj;
+		return op == other.op;
 	}
 
 	@Override

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

@@ -17,20 +17,15 @@ public final class ConstClassNode extends InsnNode {
 	}
 
 	@Override
-	public boolean equals(Object obj) {
+	public boolean isSame(InsnNode obj) {
 		if (this == obj) {
 			return true;
 		}
-		if (!(obj instanceof ConstClassNode) || !super.equals(obj)) {
+		if (!(obj instanceof ConstClassNode) || !super.isSame(obj)) {
 			return false;
 		}
-		ConstClassNode that = (ConstClassNode) obj;
-		return clsType.equals(that.clsType);
-	}
-
-	@Override
-	public int hashCode() {
-		return 31 * super.hashCode() + clsType.hashCode();
+		ConstClassNode other = (ConstClassNode) obj;
+		return clsType.equals(other.clsType);
 	}
 
 	@Override

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

@@ -16,20 +16,15 @@ public final class ConstStringNode extends InsnNode {
 	}
 
 	@Override
-	public boolean equals(Object obj) {
+	public boolean isSame(InsnNode obj) {
 		if (this == obj) {
 			return true;
 		}
-		if (!(obj instanceof ConstStringNode) || !super.equals(obj)) {
+		if (!(obj instanceof ConstStringNode) || !super.isSame(obj)) {
 			return false;
 		}
-		ConstStringNode that = (ConstStringNode) obj;
-		return str.equals(that.str);
-	}
-
-	@Override
-	public int hashCode() {
-		return 31 * super.hashCode() + str.hashCode();
+		ConstStringNode other = (ConstStringNode) obj;
+		return str.equals(other.str);
 	}
 
 	@Override

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

@@ -55,19 +55,14 @@ public final class FillArrayNode extends InsnNode {
 	}
 
 	@Override
-	public boolean equals(Object obj) {
+	public boolean isSame(InsnNode obj) {
 		if (this == obj) {
 			return true;
 		}
-		if (!(obj instanceof FillArrayNode) || !super.equals(obj)) {
+		if (!(obj instanceof FillArrayNode) || !super.isSame(obj)) {
 			return false;
 		}
-		FillArrayNode that = (FillArrayNode) obj;
-		return elemType.equals(that.elemType) && data == that.data;
-	}
-
-	@Override
-	public int hashCode() {
-		return 31 * super.hashCode() + elemType.hashCode() + data.hashCode();
+		FillArrayNode other = (FillArrayNode) obj;
+		return elemType.equals(other.elemType) && data == other.data;
 	}
 }

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

@@ -21,23 +21,6 @@ public class GotoNode extends InsnNode {
 	}
 
 	@Override
-	public boolean equals(Object obj) {
-		if (this == obj) {
-			return true;
-		}
-		if (!(obj instanceof GotoNode) || !super.equals(obj)) {
-			return false;
-		}
-		GotoNode gotoNode = (GotoNode) obj;
-		return target == gotoNode.target;
-	}
-
-	@Override
-	public int hashCode() {
-		return 31 * super.hashCode() + target;
-	}
-
-	@Override
 	public String toString() {
 		return super.toString() + "-> " + InsnUtils.formatOffset(target);
 	}

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

@@ -4,6 +4,7 @@ import jadx.core.dex.instructions.args.ArgType;
 import jadx.core.dex.instructions.args.InsnArg;
 import jadx.core.dex.instructions.args.PrimitiveType;
 import jadx.core.dex.nodes.BlockNode;
+import jadx.core.dex.nodes.InsnNode;
 import jadx.core.utils.InsnUtils;
 
 import com.android.dx.io.instructions.DecodedInstruction;
@@ -71,20 +72,15 @@ public class IfNode extends GotoNode {
 	}
 
 	@Override
-	public boolean equals(Object obj) {
+	public boolean isSame(InsnNode obj) {
 		if (this == obj) {
 			return true;
 		}
-		if (!(obj instanceof IfNode) || !super.equals(obj)) {
+		if (!(obj instanceof IfNode) || !super.isSame(obj)) {
 			return false;
 		}
-		IfNode ifNode = (IfNode) obj;
-		return op == ifNode.op;
-	}
-
-	@Override
-	public int hashCode() {
-		return 31 * super.hashCode() + op.hashCode();
+		IfNode other = (IfNode) obj;
+		return op == other.op;
 	}
 
 	@Override

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

@@ -17,20 +17,15 @@ public class IndexInsnNode extends InsnNode {
 	}
 
 	@Override
-	public boolean equals(Object obj) {
+	public boolean isSame(InsnNode obj) {
 		if (this == obj) {
 			return true;
 		}
-		if (!(obj instanceof IndexInsnNode) || !super.equals(obj)) {
+		if (!(obj instanceof IndexInsnNode) || !super.isSame(obj)) {
 			return false;
 		}
-		IndexInsnNode that = (IndexInsnNode) obj;
-		return index == null ? that.index == null : index.equals(that.index);
-	}
-
-	@Override
-	public int hashCode() {
-		return 31 * super.hashCode() + (index != null ? index.hashCode() : 0);
+		IndexInsnNode other = (IndexInsnNode) obj;
+		return index == null ? other.index == null : index.equals(other.index);
 	}
 
 	@Override

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

@@ -56,7 +56,6 @@ public class InsnDecoder {
 				InsnNode insn = decode(rawInsn, i);
 				if (insn != null) {
 					insn.setOffset(i);
-					insn.setInsnHashCode(calcHashCode(rawInsn));
 				}
 				instructions[i] = insn;
 			} else {
@@ -67,21 +66,6 @@ public class InsnDecoder {
 		return instructions;
 	}
 
-	private int calcHashCode(DecodedInstruction insn) {
-		int hash = insn.getOpcode();
-		hash = hash * 31 + insn.getClass().getName().hashCode();
-		hash = hash * 31 + insn.getFormat().ordinal();
-		hash = hash * 31 + insn.getRegisterCount();
-		hash = hash * 31 + insn.getIndex();
-		hash = hash * 31 + insn.getTarget();
-		hash = hash * 31 + insn.getA();
-		hash = hash * 31 + insn.getB();
-		hash = hash * 31 + insn.getC();
-		hash = hash * 31 + insn.getD();
-		hash = hash * 31 + insn.getE();
-		return hash;
-	}
-
 	private InsnNode decode(DecodedInstruction insn, int offset) throws DecodeException {
 		switch (insn.getOpcode()) {
 			case Opcodes.NOP:

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

@@ -45,23 +45,15 @@ public class InvokeNode extends InsnNode {
 	}
 
 	@Override
-	public boolean equals(Object obj) {
+	public boolean isSame(InsnNode obj) {
 		if (this == obj) {
 			return true;
 		}
-		if (!(obj instanceof InvokeNode) || !super.equals(obj)) {
+		if (!(obj instanceof InvokeNode) || !super.isSame(obj)) {
 			return false;
 		}
-		InvokeNode that = (InvokeNode) obj;
-		return type == that.type && mth.equals(that.mth);
-	}
-
-	@Override
-	public int hashCode() {
-		int result = super.hashCode();
-		result = 31 * result + type.hashCode();
-		result = 31 * result + mth.hashCode();
-		return result;
+		InvokeNode other = (InvokeNode) obj;
+		return type == other.type && mth.equals(other.mth);
 	}
 
 	@Override

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

@@ -37,26 +37,17 @@ public class SwitchNode extends InsnNode {
 	}
 
 	@Override
-	public boolean equals(Object obj) {
+	public boolean isSame(InsnNode obj) {
 		if (this == obj) {
 			return true;
 		}
-		if (!(obj instanceof SwitchNode) || !super.equals(obj)) {
+		if (!(obj instanceof SwitchNode) || !super.isSame(obj)) {
 			return false;
 		}
-		SwitchNode that = (SwitchNode) obj;
-		return def == that.def
-				&& Arrays.equals(keys, that.keys)
-				&& Arrays.equals(targets, that.targets);
-	}
-
-	@Override
-	public int hashCode() {
-		int result = super.hashCode();
-		result = 31 * result + Arrays.hashCode(keys);
-		result = 31 * result + Arrays.hashCode(targets);
-		result = 31 * result + def;
-		return result;
+		SwitchNode other = (SwitchNode) obj;
+		return def == other.def
+				&& Arrays.equals(keys, other.keys)
+				&& Arrays.equals(targets, other.targets);
 	}
 
 	@Override

jadx-core/src/main/java/jadx/core/dex/instructions/mods/ConstructorInsn.java

@@ -92,26 +92,16 @@ public class ConstructorInsn extends InsnNode {
 	}
 
 	@Override
-	public boolean equals(Object o) {
-		if (this == o) {
+	public boolean isSame(InsnNode obj) {
+		if (this == obj) {
 			return true;
 		}
-		if (!(o instanceof ConstructorInsn) || !super.equals(o)) {
+		if (!(obj instanceof ConstructorInsn) || !super.isSame(obj)) {
 			return false;
 		}
-		ConstructorInsn that = (ConstructorInsn) o;
-		return callMth.equals(that.callMth)
-				&& callType == that.callType
-				&& instanceArg.equals(that.instanceArg);
-	}
-
-	@Override
-	public int hashCode() {
-		int result = super.hashCode();
-		result = 31 * result + callMth.hashCode();
-		result = 31 * result + callType.hashCode();
-		result = 31 * result + instanceArg.hashCode();
-		return result;
+		ConstructorInsn other = (ConstructorInsn) obj;
+		return callMth.equals(other.callMth)
+				&& callType == other.callType;
 	}
 
 	@Override

jadx-core/src/main/java/jadx/core/dex/instructions/mods/TernaryInsn.java

@@ -60,11 +60,11 @@ public final class TernaryInsn extends InsnNode {
 	}
 
 	@Override
-	public boolean equals(Object obj) {
+	public boolean isSame(InsnNode obj) {
 		if (this == obj) {
 			return true;
 		}
-		if (!(obj instanceof TernaryInsn) || !super.equals(obj)) {
+		if (!(obj instanceof TernaryInsn) || !super.isSame(obj)) {
 			return false;
 		}
 		TernaryInsn that = (TernaryInsn) obj;
@@ -72,11 +72,6 @@ public final class TernaryInsn extends InsnNode {
 	}
 
 	@Override
-	public int hashCode() {
-		return 31 * super.hashCode() + condition.hashCode();
-	}
-
-	@Override
 	public String toString() {
 		return InsnUtils.formatOffset(offset) + ": TERNARY"
 				+ getResult() + " = "

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

@@ -22,7 +22,6 @@ public class InsnNode extends LineAttrNode {
 	private RegisterArg result;
 	private final List<InsnArg> arguments;
 	protected int offset;
-	protected int insnHashCode = super.hashCode();
 
 	public InsnNode(InsnType type, int argsCount) {
 		this.insnType = type;
@@ -197,39 +196,31 @@ public class InsnNode extends LineAttrNode {
 				+ Utils.listToString(arguments);
 	}
 
-	public void setInsnHashCode(int insnHashCode) {
-		this.insnHashCode = insnHashCode;
+	/**
+	 * Compare instruction only by identity.
+	 */
+	@Override
+	public final int hashCode() {
+		return super.hashCode();
 	}
 
+	/**
+	 * Compare instruction only by identity.
+	 */
 	@Override
-	public int hashCode() {
-		return insnHashCode;
+	public final boolean equals(Object obj) {
+		return super.equals(obj);
 	}
 
-	@Override
-	public boolean equals(Object obj) {
-		if (this == obj) {
+	/**
+	 * 'Soft' equals, don't compare arguments, only instruction specific parameters.
+	 */
+	public boolean isSame(InsnNode other) {
+		if (this == other) {
 			return true;
 		}
-		if (obj == null) {
-			return false;
-		}
-		if (hashCode() != obj.hashCode()) {
-			return false;
-		}
-		if (!(obj instanceof InsnNode)) {
-			return false;
-		}
-		InsnNode other = (InsnNode) obj;
-		if (insnType != other.insnType) {
-			return false;
-		}
-		if (arguments.size() != other.arguments.size()) {
-			return false;
-		}
-
-		// TODO !!! finish equals
-		return true;
+		return insnType == other.insnType
+				&& arguments.size() == other.arguments.size();
 	}
 
 }

jadx-core/src/main/java/jadx/core/dex/regions/conditions/IfInfo.java

@@ -28,7 +28,7 @@ public final class IfInfo {
 	}
 
 	private IfInfo(IfCondition condition, BlockNode thenBlock, BlockNode elseBlock,
-	               Set<BlockNode> mergedBlocks, Set<BlockNode> skipBlocks) {
+			Set<BlockNode> mergedBlocks, Set<BlockNode> skipBlocks) {
 		this.condition = condition;
 		this.thenBlock = thenBlock;
 		this.elseBlock = elseBlock;

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

@@ -66,7 +66,8 @@ public class ReSugarCode extends AbstractVisitor {
 	/**
 	 * Replace new array and sequence of array-put to new filled-array instruction.
 	 */
-	private static InsnNode processNewArray(MethodNode mth, List<InsnNode> instructions, int i, InstructionRemover remover) {
+	private static InsnNode processNewArray(MethodNode mth, List<InsnNode> instructions, int i,
+			InstructionRemover remover) {
 		InsnNode insn = instructions.get(i);
 		InsnArg arg = insn.getArg(0);
 		if (!arg.isLiteral()) {

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

@@ -293,10 +293,11 @@ public class BlockFinallyExtract extends AbstractVisitor {
 	}
 
 	private static boolean sameInsns(InsnNode remInsn, InsnNode fInsn, BlocksRemoveInfo removeInfo) {
-		if (remInsn.getType() != fInsn.getType()
-				|| remInsn.getArgsCount() != fInsn.getArgsCount()) {
+		if (!remInsn.isSame(fInsn)) {
 			return false;
 		}
+		// TODO: check instance arg in ConstructorInsn
+		// TODO: compare literals
 		for (int i = 0; i < remInsn.getArgsCount(); i++) {
 			InsnArg remArg = remInsn.getArg(i);
 			InsnArg fArg = fInsn.getArg(i);
@@ -328,7 +329,7 @@ public class BlockFinallyExtract extends AbstractVisitor {
 			return true;
 		}
 		if (remBlock.getPredecessors().size() != 1) {
-			LOG.warn("Finally extract failed: remBlock pred: {}, {}", remBlock, remBlock.getPredecessors());
+			LOG.warn("Finally extract failed: remBlock pred: {}, {}, method: {}", remBlock, remBlock.getPredecessors(), mth);
 			return false;
 		}
 		BlockNode remBlockPred = remBlock.getPredecessors().get(0);

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

@@ -51,7 +51,7 @@ public class CheckRegions extends AbstractVisitor {
 					// TODO
 					// mth.add(AFlag.INCONSISTENT_CODE);
 					LOG.debug(" Duplicated block: {} in {}", block, mth);
-					printRegionsWithBlock(mth, block);
+					// printRegionsWithBlock(mth, block);
 				}
 			}
 		});

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

@@ -61,7 +61,8 @@ public class DepthRegionTraversal {
 		return false;
 	}
 
-	private static boolean traverseIterativeInternal(MethodNode mth, IRegionIterativeVisitor visitor, IContainer container) {
+	private static boolean traverseIterativeInternal(MethodNode mth, IRegionIterativeVisitor visitor,
+			IContainer container) {
 		if (container instanceof IRegion) {
 			IRegion region = (IRegion) container;
 			if (visitor.visitRegion(mth, region)) {

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

@@ -124,7 +124,8 @@ public class LoopRegionVisitor extends AbstractVisitor implements IRegionVisitor
 		return true;
 	}
 
-	private static LoopType checkArrayForEach(MethodNode mth, InsnNode initInsn, InsnNode incrInsn, IfCondition condition) {
+	private static LoopType checkArrayForEach(MethodNode mth, InsnNode initInsn, InsnNode incrInsn,
+			IfCondition condition) {
 		if (!(incrInsn instanceof ArithNode)) {
 			return null;
 		}

jadx-core/src/test/java/jadx/tests/api/compiler/ClassFileManager.java

@@ -22,7 +22,7 @@ public class ClassFileManager extends ForwardingJavaFileManager<StandardJavaFile
 
 	@Override
 	public JavaFileObject getJavaFileForOutput(Location location, String className,
-	                                           Kind kind, FileObject sibling) throws IOException {
+			Kind kind, FileObject sibling) throws IOException {
 		JavaClassObject clsObject = new JavaClassObject(className, kind);
 		classLoader.getClsMap().put(className, clsObject);
 		return clsObject;

jadx-core/src/test/java/jadx/tests/api/compiler/StaticCompiler.java

@@ -54,7 +54,8 @@ public class StaticCompiler {
 		}
 
 		@Override
-		public JavaFileObject getJavaFileForOutput(Location location, String className, JavaFileObject.Kind kind, FileObject sibling) throws IOException {
+		public JavaFileObject getJavaFileForOutput(Location location, String className, JavaFileObject.Kind kind,
+				FileObject sibling) throws IOException {
 			if (kind == JavaFileObject.Kind.CLASS) {
 				File file = new File(outDir, className.replace('.', '/') + ".class");
 				files.add(file);

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

+package jadx.tests.integration.trycatch;
+
+import jadx.core.dex.nodes.ClassNode;
+import jadx.core.dex.visitors.DepthTraversal;
+import jadx.core.dex.visitors.IDexTreeVisitor;
+import jadx.tests.api.IntegrationTest;
+
+import java.util.List;
+
+import org.junit.Test;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import static jadx.tests.api.utils.JadxMatchers.containsOne;
+import static org.junit.Assert.assertThat;
+
+public class TestTryCatchFinally3 extends IntegrationTest {
+
+	public static class TestCls {
+		private static final Logger LOG = LoggerFactory.getLogger(TestCls.class);
+
+		public static void process(ClassNode cls, List<IDexTreeVisitor> passes) {
+			try {
+				cls.load();
+				for (IDexTreeVisitor visitor : passes) {
+					DepthTraversal.visit(visitor, cls);
+				}
+			} catch (Exception e) {
+				LOG.error("Class process exception: {}", cls, e);
+			} finally {
+				cls.unload();
+			}
+		}
+	}
+
+	@Test
+	public void test() {
+		ClassNode cls = getClassNode(TestCls.class);
+		String code = cls.getCode().toString();
+
+		assertThat(code, containsOne("for (IDexTreeVisitor visitor : passes) {"));
+
+		assertThat(code, containsOne("} catch (Exception e) {"));
+		assertThat(code, containsOne("LOG.error(\"Class process exception: {}\", cls, e);"));
+
+		assertThat(code, containsOne("} finally {"));
+		assertThat(code, containsOne("cls.unload();"));
+	}
+}