fix: don't inline 'null' object to make code compilable (#964)

jadx-core/src/main/java/jadx/core/dex/attributes/AFlag.java

@@ -15,6 +15,7 @@ public enum AFlag {
 
 	DONT_WRAP,
 	DONT_INLINE,
+	DONT_INLINE_CONST,
 	DONT_GENERATE, // process as usual, but don't output to generated code
 	COMMENT_OUT, // process as usual, but comment insn in generated code
 	REMOVE, // can be completely removed

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

@@ -10,7 +10,6 @@ import jadx.core.dex.instructions.ConstStringNode;
 import jadx.core.dex.instructions.IndexInsnNode;
 import jadx.core.dex.instructions.InsnType;
 import jadx.core.dex.instructions.InvokeNode;
-import jadx.core.dex.instructions.InvokeType;
 import jadx.core.dex.instructions.args.ArgType;
 import jadx.core.dex.instructions.args.InsnArg;
 import jadx.core.dex.instructions.args.LiteralArg;
@@ -73,17 +72,8 @@ public class ConstInlineVisitor extends AbstractVisitor {
 				return;
 			}
 			long lit = ((LiteralArg) constArg).getLiteral();
-			if (lit == 0 && checkObjectInline(sVar)) {
-				if (sVar.getUseCount() == 1) {
-					InsnNode assignInsn = insn.getResult().getAssignInsn();
-					if (assignInsn != null) {
-						assignInsn.add(AFlag.DONT_INLINE);
-					}
-				}
-				return;
-			}
-			// don't inline const values in synchronized statement
-			if (checkForSynchronizeBlock(insn, sVar)) {
+			if (lit == 0 && forbidNullInlines(sVar)) {
+				// all usages forbids inlining
 				return;
 			}
 		} else if (insnType == InsnType.CONST_STR) {
@@ -117,20 +107,6 @@ public class ConstInlineVisitor extends AbstractVisitor {
 		replaceConst(mth, insn, constArg, toRemove);
 	}
 
-	private static boolean checkForSynchronizeBlock(InsnNode insn, SSAVar ssaVar) {
-		for (RegisterArg reg : ssaVar.getUseList()) {
-			InsnNode parentInsn = reg.getParentInsn();
-			if (parentInsn != null) {
-				InsnType insnType = parentInsn.getType();
-				if (insnType == InsnType.MONITOR_ENTER || insnType == InsnType.MONITOR_EXIT) {
-					insn.add(AFlag.DONT_INLINE);
-					return true;
-				}
-			}
-		}
-		return false;
-	}
-
 	private static boolean checkForFinallyBlock(SSAVar sVar) {
 		List<SSAVar> ssaVars = sVar.getCodeVar().getSsaVars();
 		if (ssaVars.size() <= 1) {
@@ -153,37 +129,60 @@ public class ConstInlineVisitor extends AbstractVisitor {
 	}
 
 	/**
-	 * Don't inline null object if:
-	 * - used as instance arg in invoke instruction
-	 * - used in 'array.length'
+	 * Don't inline null object
 	 */
-	private static boolean checkObjectInline(SSAVar sVar) {
-		for (RegisterArg useArg : sVar.getUseList()) {
+	private static boolean forbidNullInlines(SSAVar sVar) {
+		List<RegisterArg> useList = sVar.getUseList();
+		if (useList.isEmpty()) {
+			return false;
+		}
+		int k = 0;
+		for (RegisterArg useArg : useList) {
 			InsnNode insn = useArg.getParentInsn();
 			if (insn == null) {
 				continue;
 			}
-			InsnType insnType = insn.getType();
-			if (insnType == InsnType.INVOKE) {
-				InvokeNode inv = (InvokeNode) insn;
-				if (inv.getInvokeType() != InvokeType.STATIC
-						&& inv.getArg(0) == useArg) {
-					return true;
-				}
-			} else if (insnType == InsnType.ARRAY_LENGTH) {
-				if (insn.getArg(0) == useArg) {
-					return true;
-				}
+			if (!canUseNull(insn, useArg)) {
+				useArg.add(AFlag.DONT_INLINE_CONST);
+				k++;
 			}
 		}
-		return false;
+		return k == useList.size();
 	}
 
-	private static int replaceConst(MethodNode mth, InsnNode constInsn, InsnArg constArg, List<InsnNode> toRemove) {
+	private static boolean canUseNull(InsnNode insn, RegisterArg useArg) {
+		switch (insn.getType()) {
+			case INVOKE:
+				return ((InvokeNode) insn).getInstanceArg() != useArg;
+
+			case ARRAY_LENGTH:
+			case AGET:
+			case APUT:
+			case IGET:
+			case SWITCH:
+			case MONITOR_ENTER:
+			case MONITOR_EXIT:
+			case INSTANCE_OF:
+				return insn.getArg(0) != useArg;
+
+			case IPUT:
+				return insn.getArg(1) != useArg;
+		}
+		return true;
+	}
+
+	private static void replaceConst(MethodNode mth, InsnNode constInsn, InsnArg constArg, List<InsnNode> toRemove) {
 		SSAVar ssaVar = constInsn.getResult().getSVar();
+		if (ssaVar.getUseCount() == 0) {
+			toRemove.add(constInsn);
+			return;
+		}
 		List<RegisterArg> useList = new ArrayList<>(ssaVar.getUseList());
 		int replaceCount = 0;
 		for (RegisterArg arg : useList) {
+			if (arg.contains(AFlag.DONT_INLINE_CONST)) {
+				continue;
+			}
 			if (replaceArg(mth, arg, constArg, constInsn, toRemove)) {
 				replaceCount++;
 			}
@@ -191,7 +190,6 @@ public class ConstInlineVisitor extends AbstractVisitor {
 		if (replaceCount == useList.size()) {
 			toRemove.add(constInsn);
 		}
-		return replaceCount;
 	}
 
 	private static boolean replaceArg(MethodNode mth, RegisterArg arg, InsnArg constArg, InsnNode constInsn, List<InsnNode> toRemove) {

jadx-core/src/main/java/jadx/core/dex/visitors/shrink/CodeShrinkVisitor.java

@@ -93,10 +93,14 @@ public class CodeShrinkVisitor extends AbstractVisitor {
 		}
 		List<RegisterArg> useList = sVar.getUseList();
 		if (!useList.isEmpty()) {
-			InsnNode parentInsn = useList.get(0).getParentInsn();
+			RegisterArg useArg = useList.get(0);
+			InsnNode parentInsn = useArg.getParentInsn();
 			if (parentInsn != null && parentInsn.contains(AFlag.DONT_GENERATE)) {
 				return;
 			}
+			if (!assignInline && useArg.contains(AFlag.DONT_INLINE_CONST)) {
+				return;
+			}
 		}
 
 		int assignPos = insnList.getIndex(assignInsn);

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

@@ -58,7 +58,7 @@ public class Utils {
 		if (objects == null) {
 			return "";
 		}
-		return listToString(objects, joiner, Object::toString);
+		return listToString(objects, joiner, Objects::toString);
 	}
 
 	public static <T> String listToString(Iterable<T> objects, Function<T, String> toStr) {

jadx-core/src/test/java/jadx/tests/integration/others/TestWrongCode2.java

+package jadx.tests.integration.others;
+
+import org.junit.jupiter.api.Test;
+
+import jadx.tests.api.IntegrationTest;
+
+import static jadx.tests.api.utils.assertj.JadxAssertions.assertThat;
+
+public class TestWrongCode2 extends IntegrationTest {
+
+	public static class TestCls {
+		@SuppressWarnings("ConstantConditions")
+		public String test() {
+			A a = null;
+			a.str = "";
+			return a.str;
+		}
+
+		@SuppressWarnings("ConstantConditions")
+		public int test2() {
+			int[] a = null;
+			a[1] = 2;
+			return a[0];
+		}
+
+		@SuppressWarnings({ "ConstantConditions", "SynchronizationOnLocalVariableOrMethodParameter" })
+		public boolean test3() {
+			A a = null;
+			synchronized (a) {
+				return true;
+			}
+		}
+
+		public boolean test4() {
+			return null instanceof A;
+		}
+
+		// everything is 'A' :)
+		@SuppressWarnings({ "MethodName", "LocalVariableName" }) // ignore checkstyle
+		public A A() {
+			A A = A();
+			A.A = A;
+			return A;
+		}
+
+		@SuppressWarnings("MemberName")
+		public static class A {
+			public String str;
+			public A A;
+		}
+	}
+
+	@Test
+	public void test() {
+		assertThat(getClassNode(TestCls.class))
+				.code()
+				.containsOne("return a.str;");
+	}
+
+	@Test
+	public void testNoDebug() {
+		noDebugInfo();
+		getClassNode(TestCls.class);
+	}
+}