fix: safe SSA variables replacement in filled new array instruction (#399)

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

@@ -695,7 +695,7 @@ public class InsnGen {
 	private boolean processOverloadedArg(CodeWriter code, MethodNode callMth, InsnArg arg, int origPos) {
 		ArgType origType;
 		List<RegisterArg> arguments = callMth.getArguments(false);
-		if (arguments.isEmpty()) {
+		if (arguments == null || arguments.isEmpty()) {
 			mth.addComment("JADX WARN: used method not loaded: " + callMth + ", types can be incorrect");
 			origType = callMth.getMethodInfo().getArgumentsTypes().get(origPos);
 		} else {

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

@@ -153,4 +153,8 @@ public abstract class InsnArg extends Typed {
 	public boolean isThis() {
 		return contains(AFlag.THIS);
 	}
+
+	public InsnArg duplicate() {
+		return this;
+	}
 }

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

@@ -24,4 +24,19 @@ public class TypeImmutableArg extends RegisterArg {
 			throw new JadxRuntimeException("Can't change arg with immutable type");
 		}
 	}
+
+	@Override
+	public RegisterArg duplicate() {
+		return duplicate(getRegNum(), getSVar());
+	}
+
+	@Override
+	public RegisterArg duplicate(int regNum, SSAVar sVar) {
+		RegisterArg dup = new TypeImmutableArg(regNum, getInitType());
+		if (sVar != null) {
+			dup.setSVar(sVar);
+		}
+		dup.copyAttributesFrom(this);
+		return dup;
+	}
 }

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

@@ -8,6 +8,7 @@ import java.util.Objects;
 
 import com.android.dx.io.instructions.DecodedInstruction;
 import com.rits.cloning.Cloner;
+import org.jetbrains.annotations.Nullable;
 
 import jadx.core.dex.attributes.nodes.LineAttrNode;
 import jadx.core.dex.instructions.InsnType;
@@ -54,16 +55,27 @@ public class InsnNode extends LineAttrNode {
 		return insn;
 	}
 
-	public void setResult(RegisterArg res) {
+	public void setResult(@Nullable RegisterArg res) {
 		if (res != null) {
 			res.setParentInsn(this);
+			SSAVar ssaVar = res.getSVar();
+			if (ssaVar != null) {
+				ssaVar.setAssign(res);
+			}
 		}
 		this.result = res;
 	}
 
 	public void addArg(InsnArg arg) {
-		arg.setParentInsn(this);
 		arguments.add(arg);
+		arg.setParentInsn(this);
+		if (arg.isRegister()) {
+			RegisterArg reg = (RegisterArg) arg;
+			SSAVar ssaVar = reg.getSVar();
+			if (ssaVar != null) {
+				ssaVar.use(reg);
+			}
+		}
 	}
 
 	public InsnType getType() {

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

@@ -97,16 +97,21 @@ public class ReSugarCode extends AbstractVisitor {
 				|| instructions.get(i + len).getType() != InsnType.APUT) {
 			return null;
 		}
-		ArgType arrType = newArrayInsn.getArrayType();
-		InsnNode filledArr = new FilledNewArrayNode(arrType.getArrayElement(), len);
-		filledArr.setResult(newArrayInsn.getResult());
 		for (int j = 0; j < len; j++) {
 			InsnNode put = instructions.get(i + 1 + j);
 			if (put.getType() != InsnType.APUT) {
 				LOG.debug("Not a APUT in expected new filled array: {}, method: {}", put, mth);
 				return null;
 			}
-			filledArr.addArg(put.getArg(2));
+		}
+
+		// checks complete, apply
+		ArgType arrType = newArrayInsn.getArrayType();
+		InsnNode filledArr = new FilledNewArrayNode(arrType.getArrayElement(), len);
+		filledArr.setResult(newArrayInsn.getResult().duplicate());
+		for (int j = 0; j < len; j++) {
+			InsnNode put = instructions.get(i + 1 + j);
+			filledArr.addArg(put.getArg(2).duplicate());
 			remover.add(put);
 		}
 		return filledArr;

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

@@ -53,14 +53,7 @@ public class InstructionRemover {
 		toRemove.clear();
 	}
 
-	public static void unbindInsnList(MethodNode mth, List<InsnNode> unbind) {
-		for (InsnNode rem : unbind) {
-			unbindInsn(mth, rem);
-		}
-	}
-
 	public static void unbindInsn(MethodNode mth, InsnNode insn) {
-		unbindResult(mth, insn);
 		for (InsnArg arg : insn.getArguments()) {
 			unbindArgUsage(mth, arg);
 		}
@@ -71,6 +64,7 @@ public class InstructionRemover {
 				}
 			}
 		}
+		unbindResult(mth, insn);
 		insn.add(AFlag.INCONSISTENT_CODE);
 	}
 
@@ -90,7 +84,10 @@ public class InstructionRemover {
 	public static void unbindResult(MethodNode mth, InsnNode insn) {
 		RegisterArg r = insn.getResult();
 		if (r != null && r.getSVar() != null && mth != null) {
-			mth.removeSVar(r.getSVar());
+			SSAVar ssaVar = r.getSVar();
+			if (ssaVar.getUseCount() == 0) {
+				mth.removeSVar(ssaVar);
+			}
 		}
 	}
 
@@ -110,12 +107,15 @@ public class InstructionRemover {
 	// Don't use 'instrList.removeAll(toRemove)' because it will remove instructions by content
 	// and here can be several instructions with same content
 	private static void removeAll(MethodNode mth, List<InsnNode> insns, List<InsnNode> toRemove) {
+		if (toRemove == null || toRemove.isEmpty()) {
+			return;
+		}
 		for (InsnNode rem : toRemove) {
-			unbindInsn(mth, rem);
 			int insnsCount = insns.size();
 			for (int i = 0; i < insnsCount; i++) {
 				if (insns.get(i) == rem) {
 					insns.remove(i);
+					unbindInsn(mth, rem);
 					break;
 				}
 			}
@@ -143,9 +143,6 @@ 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/test/java/jadx/tests/integration/variables/TestVariables6.java

+package jadx.tests.integration.variables;
+
+import org.junit.Test;
+
+import jadx.core.dex.nodes.ClassNode;
+import jadx.tests.api.SmaliTest;
+
+import static org.hamcrest.Matchers.containsString;
+import static org.hamcrest.Matchers.not;
+import static org.junit.Assert.assertThat;
+
+public class TestVariables6 extends SmaliTest {
+
+	@Test
+	public void test() {
+		disableCompilation();
+		ClassNode cls = getClassNodeFromSmaliWithPath("variables", "TestVariables6");
+		String code = cls.getCode().toString();
+
+		assertThat(code, not(containsString("r4")));
+		assertThat(code, not(containsString("r1v1")));
+		assertThat(code, containsString("DateStringParser dateStringParser"));
+		assertThat(code, containsString("FinancialInstrumentMetadataAttribute startYear = this.mFinancialInstrumentMetadataDefinition.getStartYear();"));
+	}
+}

jadx-core/src/test/smali/variables/TestVariables6.smali

+.class public LTestVariables6;
+.super Lcom/paypal/android/p2pmobile/wallet/banksandcards/fragments/BasePaymentFragment;
+.source "SourceFile"
+
+# interfaces
+.implements Landroid/support/v13/app/FragmentCompat$OnRequestPermissionsResultCallback;
+.implements Landroid/widget/TextView$OnEditorActionListener;
+.implements Lcom/paypal/android/p2pmobile/common/utils/ISafeClickVerifierListener;
+.implements Lcom/paypal/android/p2pmobile/common/widgets/CSCTextWatcher$ICSCTextWatcherListener;
+
+
+# annotations
+.annotation system Ldalvik/annotation/MemberClasses;
+    value = {
+        Lcom/paypal/android/p2pmobile/wallet/banksandcards/fragments/EnterCardFragment$IEnterCardFragmentListener;
+    }
+.end annotation
+
+
+.field private static final DATE_SEPARATOR:C = '/'
+
+.field mDateFormatOrder:Lcom/paypal/android/p2pmobile/common/utils/ValidatedDateFormatOrder;
+
+
+# direct methods
+.method static constructor <clinit>()V
+    .locals 0
+
+    return-void
+.end method
+
+.method public constructor <init>()V
+    .locals 1
+
+    return-void
+.end method
+
+.method private bindStartDateToMutableCredebitCard(Lcom/paypal/android/foundation/wallet/model/MutableCredebitCard;)Z
+    .locals 10
+    .param p1    # Lcom/paypal/android/foundation/wallet/model/MutableCredebitCard;
+        .annotation build Landroid/support/annotation/NonNull;
+        .end annotation
+    .end param
+
+    .line 1024
+    iget-object v0, p0, LTestVariables6;->mFinancialInstrumentMetadataDefinition:Lcom/paypal/android/foundation/wallet/model/FinancialInstrumentMetadataDefinition;
+
+    invoke-virtual {v0}, Lcom/paypal/android/foundation/wallet/model/FinancialInstrumentMetadataDefinition;->getStartMonth()Lcom/paypal/android/foundation/wallet/model/FinancialInstrumentMetadataAttribute;
+
+    move-result-object v0
+
+    .line 1025
+    iget-object v1, p0, LTestVariables6;->mFinancialInstrumentMetadataDefinition:Lcom/paypal/android/foundation/wallet/model/FinancialInstrumentMetadataDefinition;
+
+    invoke-virtual {v1}, Lcom/paypal/android/foundation/wallet/model/FinancialInstrumentMetadataDefinition;->getStartYear()Lcom/paypal/android/foundation/wallet/model/FinancialInstrumentMetadataAttribute;
+
+    move-result-object v1
+
+    const/4 v2, 0x2
+
+    .line 1026
+    new-array v2, v2, [Lcom/paypal/android/foundation/wallet/model/FinancialInstrumentMetadataAttribute;
+
+    const/4 v3, 0x0
+
+    aput-object v0, v2, v3
+
+    const/4 v0, 0x1
+
+    aput-object v1, v2, v0
+
+    invoke-static {v2}, Lcom/paypal/android/p2pmobile/wallet/banksandcards/utils/EnterCardFragmentUtils;->attributesAreRequired([Lcom/paypal/android/foundation/wallet/model/FinancialInstrumentMetadataAttribute;)Z
+
+    move-result v2
+
+    if-nez v2, :cond_0
+
+    return v0
+
+    .line 1030
+    :cond_0
+    invoke-virtual {p0}, LTestVariables6;->getView()Landroid/view/View;
+
+    move-result-object v2
+
+    if-nez v2, :cond_1
+
+    return v3
+
+    .line 1035
+    :cond_1
+    invoke-virtual {v1}, Lcom/paypal/android/foundation/wallet/model/FinancialInstrumentMetadataAttribute;->getMaximumLength()I
+
+    move-result v6
+
+    .line 1036
+    sget v1, Lcom/paypal/android/p2pmobile/wallet/R$id;->enter_card_start_date:I
+
+    invoke-virtual {v2, v1}, Landroid/view/View;->findViewById(I)Landroid/view/View;
+
+    move-result-object v1
+
+    check-cast v1, Landroid/widget/TextView;
+
+    .line 1038
+    new-instance v2, Lcom/paypal/android/p2pmobile/common/utils/DateStringParser;
+
+    invoke-virtual {v1}, Landroid/widget/TextView;->getText()Ljava/lang/CharSequence;
+
+    move-result-object v1
+
+    invoke-interface {v1}, Ljava/lang/CharSequence;->toString()Ljava/lang/String;
+
+    move-result-object v5
+
+    iget-object v7, p0, LTestVariables6;->mDateFormatOrder:Lcom/paypal/android/p2pmobile/common/utils/ValidatedDateFormatOrder;
+
+    const/16 v8, 0x2f
+
+    const/4 v9, 0x0
+
+    move-object v4, v2
+
+    invoke-direct/range {v4 .. v9}, Lcom/paypal/android/p2pmobile/common/utils/DateStringParser;-><init>(Ljava/lang/String;ILcom/paypal/android/p2pmobile/common/utils/ValidatedDateFormatOrder;CZ)V
+
+    .line 1039
+    invoke-virtual {v2}, Lcom/paypal/android/p2pmobile/common/utils/DateStringParser;->isError()Z
+
+    move-result v1
+
+    if-nez v1, :cond_2
+
+    .line 1040
+    invoke-virtual {v2}, Lcom/paypal/android/p2pmobile/common/utils/DateStringParser;->getDate()Ljava/util/Date;
+
+    move-result-object v1
+
+    invoke-virtual {p1, v1}, Lcom/paypal/android/foundation/wallet/model/MutableCredebitCard;->setIssueDate(Ljava/util/Date;)V
+
+    return v0
+
+    :cond_2
+    return v3
+.end method