fix: handle xor on boolean (#921)

skylot · Sep 13, 2020 · 1bbcac2 · 1bbcac2
1 parent 60b2353
commit 1bbcac2
Show file tree

Hide file tree

Showing 3 changed files with 105 additions and 9 deletions.
diff --git a/jadx-core/src/main/java/jadx/core/dex/visitors/typeinference/TypeInferenceVisitor.java b/jadx-core/src/main/java/jadx/core/dex/visitors/typeinference/TypeInferenceVisitor.java
@@ -20,6 +20,8 @@
 import jadx.core.dex.attributes.AType;
 import jadx.core.dex.attributes.nodes.PhiListAttr;
 import jadx.core.dex.info.ClassInfo;
+import jadx.core.dex.instructions.ArithNode;
+import jadx.core.dex.instructions.ArithOp;
 import jadx.core.dex.instructions.BaseInvokeNode;
 import jadx.core.dex.instructions.IndexInsnNode;
 import jadx.core.dex.instructions.InsnType;
@@ -759,15 +761,16 @@ && fixBooleanUsage(mth, bound)) {
 
 	private boolean fixBooleanUsage(MethodNode mth, ITypeBound bound) {
 		ArgType boundType = bound.getType();
-		if (!boundType.isPrimitive() || boundType == ArgType.BOOLEAN) {
+		if (boundType == ArgType.BOOLEAN
+				|| (boundType.isTypeKnown() && !boundType.isPrimitive())) {
 			return false;
 		}
 		RegisterArg boundArg = bound.getArg();
 		if (boundArg == null) {
 			return false;
 		}
 		InsnNode insn = boundArg.getParentInsn();
-		if (insn == null) {
+		if (insn == null || insn.getType() == InsnType.IF) {
 			return false;
 		}
 		BlockNode blockNode = BlockUtils.getBlockByInsn(mth, insn);
@@ -779,21 +782,47 @@ private boolean fixBooleanUsage(MethodNode mth, ITypeBound bound) {
 		if (insnIndex == -1) {
 			return false;
 		}
-		if (insn.getType() == InsnType.CAST) {
+		InsnType insnType = insn.getType();
+		if (insnType == InsnType.CAST) {
 			// replace cast
 			ArgType type = (ArgType) ((IndexInsnNode) insn).getIndex();
 			TernaryInsn convertInsn = prepareBooleanConvertInsn(insn.getResult(), boundArg, type);
 			BlockUtils.replaceInsn(mth, blockNode, insnIndex, convertInsn);
-		} else {
-			// insert before insn
-			RegisterArg resultArg = boundArg.duplicateWithNewSSAVar(mth);
-			TernaryInsn convertInsn = prepareBooleanConvertInsn(resultArg, boundArg, boundType);
-			insnList.add(insnIndex, convertInsn);
-			insn.replaceArg(bound.getArg(), convertInsn.getResult().duplicate());
+			return true;
+		}
+		if (insnType == InsnType.ARITH) {
+			ArithNode arithInsn = (ArithNode) insn;
+			if (arithInsn.getOp() == ArithOp.XOR && arithInsn.getArgsCount() == 2) {
+				// replace (boolean ^ 1) with (!boolean)
+				InsnArg secondArg = arithInsn.getArg(1);
+				if (secondArg.isLiteral() && ((LiteralArg) secondArg).getLiteral() == 1) {
+					InsnNode convertInsn = notBooleanToInt(arithInsn, boundArg);
+					BlockUtils.replaceInsn(mth, blockNode, insnIndex, convertInsn);
+					return true;
+				}
+			}
 		}
+
+		// insert before insn
+		RegisterArg resultArg = boundArg.duplicateWithNewSSAVar(mth);
+		TernaryInsn convertInsn = prepareBooleanConvertInsn(resultArg, boundArg, boundType);
+		insnList.add(insnIndex, convertInsn);
+		insn.replaceArg(boundArg, convertInsn.getResult().duplicate());
 		return true;
 	}
 
+	private InsnNode notBooleanToInt(ArithNode insn, RegisterArg boundArg) {
+		InsnNode notInsn = new InsnNode(InsnType.NOT, 1);
+		notInsn.addArg(boundArg.duplicate());
+		notInsn.add(AFlag.SYNTHETIC);
+
+		InsnArg notArg = InsnArg.wrapArg(notInsn);
+		notArg.setType(ArgType.BOOLEAN);
+		TernaryInsn convertInsn = ModVisitor.makeBooleanConvertInsn(insn.getResult(), notArg, ArgType.INT);
+		convertInsn.add(AFlag.SYNTHETIC);
+		return convertInsn;
+	}
+
 	private TernaryInsn prepareBooleanConvertInsn(RegisterArg resultArg, RegisterArg boundArg, ArgType useType) {
 		RegisterArg useArg = boundArg.getSVar().getAssign().duplicate();
 		TernaryInsn convertInsn = ModVisitor.makeBooleanConvertInsn(resultArg, useArg, useType);

diff --git a/jadx-core/src/test/java/jadx/tests/integration/types/TestTypeResolver15.java b/jadx-core/src/test/java/jadx/tests/integration/types/TestTypeResolver15.java
@@ -0,0 +1,40 @@
+package jadx.tests.integration.types;
+
+import org.junit.jupiter.api.Test;
+
+import jadx.tests.api.SmaliTest;
+
+import static jadx.tests.api.utils.assertj.JadxAssertions.assertThat;
+
+/**
+ * Issue 921 (second case)
+ */
+public class TestTypeResolver15 extends SmaliTest {
+
+	public static class TestCls {
+		private void test(boolean z) {
+			useInt(z ? 0 : 8);
+			useInt(!z ? 1 : 0); // replaced with xor in smali test
+		}
+
+		private void useInt(int i) {
+		}
+	}
+
+	@Test
+	public void test() {
+		noDebugInfo();
+		assertThat(getClassNode(TestCls.class))
+				.code()
+				// .containsOne("useInt(!z ? 1 : 0);") // TODO: convert to ternary
+				.containsOne("useInt(z ? 0 : 8);");
+	}
+
+	@Test
+	public void testSmali() {
+		assertThat(getClassNodeFromSmali())
+				.code()
+				.containsOne("useInt(z ? 0 : 8);")
+				.containsOne("useInt(!z ? 1 : 0);");
+	}
+}
diff --git a/jadx-core/src/test/smali/types/TestTypeResolver15.smali b/jadx-core/src/test/smali/types/TestTypeResolver15.smali
@@ -0,0 +1,27 @@
+.class public Ltypes/TestTypeResolver15;
+.super Ljava/lang/Object;
+
+.method private test(Z)V
+    .locals 2
+
+    if-eqz p1, :cond_0
+
+	const/4 v1, 0x0
+	goto :goto_0
+
+	:cond_0
+	const/16 v1, 0x8
+
+	:goto_0
+	invoke-virtual {p0, v1}, Ltypes/TestTypeResolver15;->useInt(I)V
+
+	xor-int/lit8 p1, p1, 0x1
+	invoke-virtual {p0, p1}, Ltypes/TestTypeResolver15;->useInt(I)V
+
+	return-void
+.end method
+
+.method private useInt(I)V
+    .registers 2
+    return-void
+.end method