fix: merge new-array and fill-array-data with move between (#462)

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

@@ -50,6 +50,7 @@ import jadx.core.dex.nodes.FieldNode;
 import jadx.core.dex.nodes.InsnNode;
 import jadx.core.dex.nodes.MethodNode;
 import jadx.core.dex.nodes.RootNode;
+import jadx.core.utils.DebugUtils;
 import jadx.core.utils.RegionUtils;
 import jadx.core.utils.exceptions.CodegenException;
 import jadx.core.utils.exceptions.JadxRuntimeException;
@@ -211,12 +212,14 @@ public class InsnGen {
 		makeInsn(insn, code, null);
 	}
 
+	private static final Set<Flags> EMPTY_FLAGS = EnumSet.noneOf(Flags.class);
+	private static final Set<Flags> BODY_ONLY_FLAG = EnumSet.of(Flags.BODY_ONLY);
+	private static final Set<Flags> BODY_ONLY_NOWRAP_FLAGS = EnumSet.of(Flags.BODY_ONLY_NOWRAP);
+
 	protected void makeInsn(InsnNode insn, CodeWriter code, Flags flag) throws CodegenException {
 		try {
-			Set<Flags> state = EnumSet.noneOf(Flags.class);
 			if (flag == Flags.BODY_ONLY || flag == Flags.BODY_ONLY_NOWRAP) {
-				state.add(flag);
-				makeInsnBody(code, insn, state);
+				makeInsnBody(code, insn, flag == Flags.BODY_ONLY ? BODY_ONLY_FLAG : BODY_ONLY_NOWRAP_FLAGS);
 			} else {
 				if (flag != Flags.INLINE) {
 					code.startLineWithNum(insn.getSourceLine());
@@ -229,7 +232,7 @@ public class InsnGen {
 						code.add(" = ");
 					}
 				}
-				makeInsnBody(code, insn, state);
+				makeInsnBody(code, insn, EMPTY_FLAGS);
 				if (flag != Flags.INLINE) {
 					code.add(';');
 				}
@@ -373,6 +376,17 @@ public class InsnGen {
 				filledNewArray((FilledNewArrayNode) insn, code);
 				break;
 
+			case FILL_ARRAY:
+				FillArrayNode arrayNode = (FillArrayNode) insn;
+				if (fallback) {
+					String arrStr = arrayNode.dataToString();
+					addArg(code, insn.getArg(0));
+					code.add(" = {").add(arrStr.substring(1, arrStr.length() - 1)).add("} // fill-array");
+				} else {
+					fillArray(code, arrayNode);
+				}
+				break;
+
 			case AGET:
 				addArg(code, insn.getArg(0));
 				code.add('[');
@@ -491,13 +505,6 @@ public class InsnGen {
 				code.startLine('}');
 				break;
 
-			case FILL_ARRAY:
-				fallbackOnlyInsn(insn);
-				FillArrayNode arrayNode = (FillArrayNode) insn;
-				String arrStr = arrayNode.dataToString();
-				code.add('{').add(arrStr.substring(1, arrStr.length() - 1)).add('}');
-				break;
-
 			case NEW_INSTANCE:
 				// only fallback - make new instance in constructor invoke
 				fallbackOnlyInsn(insn);
@@ -519,6 +526,26 @@ public class InsnGen {
 		}
 	}
 
+	/**
+	 * In most cases must be combined with new array instructions.
+	 * Use one by one array fill (can be replaced with System.arrayCopy)
+	 */
+	private void fillArray(CodeWriter code, FillArrayNode arrayNode) throws CodegenException {
+		code.add("// fill-array-data instruction");
+		code.startLine();
+		List<LiteralArg> args = arrayNode.getLiteralArgs(arrayNode.getElementType());
+		InsnArg arrArg = arrayNode.getArg(0);
+		int len = args.size();
+		for (int i = 0; i < len; i++) {
+			if (i != 0) {
+				code.add(';');
+				code.startLine();
+			}
+			addArg(code, arrArg);
+			code.add('[').add(Integer.toString(i)).add("] = ").add(lit(args.get(i)));
+		}
+	}
+
 	private void oneArgInsn(CodeWriter code, InsnNode insn, Set<Flags> state, char op) throws CodegenException {
 		boolean wrap = state.contains(Flags.BODY_ONLY);
 		if (wrap) {
@@ -533,6 +560,7 @@ public class InsnGen {
 
 	private void fallbackOnlyInsn(InsnNode insn) throws CodegenException {
 		if (!fallback) {
+			DebugUtils.dump(mth, "fallback");
 			throw new CodegenException(insn.getType() + " can be used only in fallback mode");
 		}
 	}

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

@@ -8,9 +8,7 @@ import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
 import jadx.core.dex.attributes.AFlag;
-import jadx.core.dex.nodes.DexNode;
 import jadx.core.dex.nodes.InsnNode;
-import jadx.core.utils.InsnUtils;
 import jadx.core.utils.exceptions.JadxRuntimeException;
 
 public class RegisterArg extends InsnArg implements Named {
@@ -121,17 +119,6 @@ public class RegisterArg extends InsnArg implements Named {
 		return dup;
 	}
 
-	/**
-	 * Return constant value from register assign or null if not constant
-	 */
-	public Object getConstValue(DexNode dex) {
-		InsnNode parInsn = getAssignInsn();
-		if (parInsn == null) {
-			return null;
-		}
-		return InsnUtils.getConstValueByInsn(dex, parInsn);
-	}
-
 	@Nullable
 	public InsnNode getAssignInsn() {
 		if (sVar == null) {

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

@@ -5,6 +5,7 @@ import java.util.LinkedHashMap;
 import java.util.List;
 import java.util.Map;
 
+import org.jetbrains.annotations.Nullable;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
@@ -116,15 +117,14 @@ public class ModVisitor extends AbstractVisitor {
 
 					case NEW_ARRAY:
 						// replace with filled array if 'fill-array' is next instruction
-						int next = i + 1;
-						if (next < size) {
-							InsnNode ni = block.getInstructions().get(next);
-							if (ni.getType() == InsnType.FILL_ARRAY) {
-								InsnNode filledArr = makeFilledArrayInsn(mth, (NewArrayNode) insn, (FillArrayNode) ni);
-								if (filledArr != null) {
-									replaceInsn(block, i, filledArr);
-									remover.addAndUnbind(ni);
-								}
+						NewArrayNode newArrInsn = (NewArrayNode) insn;
+						InsnNode nextInsn = getFirstUseSkipMove(insn.getResult());
+						if (nextInsn != null && nextInsn.getType() == InsnType.FILL_ARRAY) {
+							FillArrayNode fillArrInsn = (FillArrayNode) nextInsn;
+							if (checkArrSizes(mth, newArrInsn, fillArrInsn)) {
+								InsnNode filledArr = makeFilledArrayInsn(mth, newArrInsn, fillArrInsn);
+								replaceInsn(block, i, filledArr);
+								remover.addAndUnbind(nextInsn);
 							}
 						}
 						break;
@@ -168,7 +168,8 @@ public class ModVisitor extends AbstractVisitor {
 								IfCondition condition = IfCondition.fromIfNode(ifNode);
 								InsnArg zero = new LiteralArg(0, type);
 								InsnArg one = new LiteralArg(
-										type == ArgType.DOUBLE ? Double.doubleToLongBits(1)
+										type == ArgType.DOUBLE
+												? Double.doubleToLongBits(1)
 												: type == ArgType.FLOAT ? Float.floatToIntBits(1) : 1,
 										type);
 								TernaryInsn ternary = new TernaryInsn(condition, insn.getResult(), one, zero);
@@ -185,6 +186,17 @@ public class ModVisitor extends AbstractVisitor {
 		}
 	}
 
+	private static boolean checkArrSizes(MethodNode mth, NewArrayNode newArrInsn, FillArrayNode fillArrInsn) {
+		int dataSize = fillArrInsn.getSize();
+		InsnArg arrSizeArg = newArrInsn.getArg(0);
+		Object value = InsnUtils.getConstValueByArg(mth.dex(), arrSizeArg);
+		if (value instanceof LiteralArg) {
+			long literal = ((LiteralArg) value).getLiteral();
+			return dataSize == (int) literal;
+		}
+		return false;
+	}
+
 	private static boolean isCastDuplicate(IndexInsnNode castInsn) {
 		InsnArg arg = castInsn.getArg(0);
 		if (arg.isRegister()) {
@@ -314,6 +326,28 @@ public class ModVisitor extends AbstractVisitor {
 		return parentInsn;
 	}
 
+	/**
+	 * Return first usage instruction for arg.
+	 * If used only once try to follow move chain
+	 */
+	@Nullable
+	private static InsnNode getFirstUseSkipMove(RegisterArg arg) {
+		SSAVar sVar = arg.getSVar();
+		int useCount = sVar.getUseCount();
+		if (useCount == 0) {
+			return null;
+		}
+		RegisterArg useArg = sVar.getUseList().get(0);
+		InsnNode parentInsn = useArg.getParentInsn();
+		if (parentInsn == null) {
+			return null;
+		}
+		if (useCount == 1 && parentInsn.getType() == InsnType.MOVE) {
+			return getFirstUseSkipMove(parentInsn.getResult());
+		}
+		return parentInsn;
+	}
+
 	private static InsnNode makeFilledArrayInsn(MethodNode mth, NewArrayNode newArrayNode, FillArrayNode insn) {
 		ArgType insnArrayType = newArrayNode.getArrayType();
 		ArgType insnElementType = insnArrayType.getArrayElement();

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

@@ -155,27 +155,12 @@ public class ReSugarCode extends AbstractVisitor {
 			return false;
 		}
 		InsnArg indexArg = insn.getArg(1);
-		int index = -1;
-		if (indexArg.isLiteral()) {
-			index = (int) ((LiteralArg) indexArg).getLiteral();
-		} else if (indexArg.isRegister()) {
-			RegisterArg reg = (RegisterArg) indexArg;
-			index = getIntConst(reg.getConstValue(mth.dex()));
-		} else if (indexArg.isInsnWrap()) {
-			InsnNode constInsn = ((InsnWrapArg) indexArg).getWrapInsn();
-			index = getIntConst(InsnUtils.getConstValueByInsn(mth.dex(), constInsn));
-		}
-		return index == putIndex;
-	}
-
-	private static int getIntConst(Object value) {
-		if (value instanceof Integer) {
-			return (Integer) value;
-		}
-		if (value instanceof Long) {
-			return ((Long) value).intValue();
+		Object value = InsnUtils.getConstValueByArg(mth.dex(), indexArg);
+		if (value instanceof LiteralArg) {
+			int index = (int) ((LiteralArg) value).getLiteral();
+			return index == putIndex;
 		}
-		return -1;
+		return false;
 	}
 
 	private static void processEnumSwitch(MethodNode mth, SwitchNode insn) {

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

@@ -12,6 +12,9 @@ import jadx.core.dex.instructions.ConstClassNode;
 import jadx.core.dex.instructions.ConstStringNode;
 import jadx.core.dex.instructions.IndexInsnNode;
 import jadx.core.dex.instructions.InsnType;
+import jadx.core.dex.instructions.args.InsnArg;
+import jadx.core.dex.instructions.args.InsnWrapArg;
+import jadx.core.dex.instructions.args.RegisterArg;
 import jadx.core.dex.nodes.DexNode;
 import jadx.core.dex.nodes.FieldNode;
 import jadx.core.dex.nodes.InsnNode;
@@ -64,6 +67,33 @@ public class InsnUtils {
 	}
 
 	/**
+	 * Search constant assigned to provided arg.
+	 *
+	 * @return LiteralArg, String, ArgType or null
+	 */
+	public static Object getConstValueByArg(DexNode dex, InsnArg arg) {
+		if (arg.isLiteral()) {
+			return arg;
+		}
+		if (arg.isRegister()) {
+			RegisterArg reg = (RegisterArg) arg;
+			InsnNode parInsn = reg.getAssignInsn();
+			if (parInsn == null) {
+				return null;
+			}
+			if (parInsn.getType() == InsnType.MOVE) {
+				return getConstValueByArg(dex, parInsn.getArg(0));
+			}
+			return getConstValueByInsn(dex, parInsn);
+		}
+		if (arg.isInsnWrap()) {
+			InsnNode insn = ((InsnWrapArg) arg).getWrapInsn();
+			return getConstValueByInsn(dex, insn);
+		}
+		return null;
+	}
+
+	/**
 	 * Return constant value from insn or null if not constant.
 	 *
 	 * @return LiteralArg, String, ArgType or null

jadx-core/src/test/java/jadx/tests/integration/arrays/TestArrayFillWithMove.java

+package jadx.tests.integration.arrays;
+
+import org.junit.jupiter.api.Test;
+
+import jadx.core.dex.nodes.ClassNode;
+import jadx.tests.api.SmaliTest;
+
+import static org.hamcrest.CoreMatchers.containsString;
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.Matchers.not;
+
+public class TestArrayFillWithMove extends SmaliTest {
+
+	@Test
+	public void test() {
+		ClassNode cls = getClassNodeFromSmaliFiles("TestCls");
+		String code = cls.getCode().toString();
+
+		assertThat(code, not(containsString("// fill-array-data instruction")));
+		assertThat(code, not(containsString("arr[0] = 0;")));
+
+		assertThat(code, containsString("return new long[]{0, 1}"));
+	}
+}

jadx-core/src/test/java/jadx/tests/integration/arrays/TestFillArrayData.java

+package jadx.tests.integration.arrays;
+
+import org.junit.jupiter.api.Test;
+
+import jadx.core.dex.nodes.ClassNode;
+import jadx.tests.api.SmaliTest;
+
+import static org.hamcrest.CoreMatchers.containsString;
+import static org.hamcrest.MatcherAssert.assertThat;
+
+public class TestFillArrayData extends SmaliTest {
+
+	@Test
+	public void test() {
+		ClassNode cls = getClassNodeFromSmaliFiles("TestCls");
+		String code = cls.getCode().toString();
+
+		assertThat(code, containsString("jArr[0] = 1;"));
+		assertThat(code, containsString("jArr[1] = 2;"));
+	}
+}

jadx-core/src/test/smali/arrays/TestArrayFillWithMove/TestCls.smali

+.class public Larrays/TestCls;
+.super Ljava/lang/Object;
+
+.method public test()[J
+    .registers 4
+
+    const/16 v3, 0x2
+
+    move/from16 v0, v3
+
+    new-array v0, v0, [J
+
+    move-object/from16 v1, v0
+
+    fill-array-data v1, :array_0
+
+    .local v1, "arr":[J
+    return v1
+
+    :array_0
+    .array-data 8
+        0x0
+        0x1
+    .end array-data
+.end method

jadx-core/src/test/smali/arrays/TestFillArrayData/TestCls.smali

+.class public Larrays/TestCls;
+.super Ljava/lang/Object;
+
+.method public test([J)V
+    .registers 2
+
+    fill-array-data p1, :array_0
+
+    return-void
+
+    :array_0
+    .array-data 8
+        0x1
+        0x2
+    .end array-data
+.end method