core: fixed 'this' attribute propagation for move insn (#345)

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

@@ -102,16 +102,11 @@ public class NameGen {
 		if (fallback) {
 			return getFallbackName(arg);
 		}
-		String name = arg.getName();
-		String varName;
-		if (name != null) {
-			if ("this".equals(name)) {
-				return name;
-			}
-			varName = name;
-		} else {
-			varName = guessName(arg);
+		if (arg.isThis()) {
+			return RegisterArg.THIS_ARG_NAME;
 		}
+		String name = arg.getName();
+		String varName = name != null ? name : guessName(arg);
 		if (NameMapper.isReserved(varName)) {
 			return varName + "R";
 		}

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

@@ -140,7 +140,6 @@ public abstract class InsnArg extends Typed {
 	}
 
 	public boolean isThis() {
-		// must be implemented in RegisterArg
-		return false;
+		return contains(AFlag.THIS);
 	}
 }

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

@@ -4,7 +4,6 @@ import java.util.Objects;
 
 import org.jetbrains.annotations.NotNull;
 
-import jadx.core.dex.attributes.AFlag;
 import jadx.core.dex.instructions.InsnType;
 import jadx.core.dex.instructions.PhiInsn;
 import jadx.core.dex.nodes.DexNode;
@@ -13,6 +12,8 @@ import jadx.core.utils.InsnUtils;
 
 public class RegisterArg extends InsnArg implements Named {
 
+	public static final String THIS_ARG_NAME = "this";
+
 	protected final int regNum;
 	// not null after SSATransform pass
 	private SSAVar sVar;
@@ -44,6 +45,9 @@ public class RegisterArg extends InsnArg implements Named {
 	}
 
 	public String getName() {
+		if (isThis()) {
+			return THIS_ARG_NAME;
+		}
 		if (sVar == null) {
 			return null;
 		}
@@ -117,22 +121,6 @@ public class RegisterArg extends InsnArg implements Named {
 		return InsnUtils.getConstValueByInsn(dex, parInsn);
 	}
 
-	@Override
-	public boolean isThis() {
-		if (contains(AFlag.THIS)) {
-			return true;
-		}
-		// maybe it was moved from 'this' register
-		InsnNode ai = getAssignInsn();
-		if (ai != null && ai.getType() == InsnType.MOVE) {
-			InsnArg arg = ai.getArg(0);
-			if (arg != this) {
-				return arg.isThis();
-			}
-		}
-		return false;
-	}
-
 	public InsnNode getAssignInsn() {
 		if (sVar == null) {
 			return null;
@@ -188,6 +176,9 @@ public class RegisterArg extends InsnArg implements Named {
 		}
 		sb.append(" ");
 		sb.append(type);
+		if (!isAttrStorageEmpty()) {
+			sb.append(' ').append(getAttributesString());
+		}
 		sb.append(")");
 		return sb.toString();
 	}

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

@@ -4,8 +4,6 @@ import org.jetbrains.annotations.NotNull;
 
 public class TypeImmutableArg extends RegisterArg {
 
-	public static final String THIS_ARG_NAME = "this";
-
 	public TypeImmutableArg(int rn, ArgType type) {
 		super(rn, type);
 	}
@@ -21,18 +19,7 @@ public class TypeImmutableArg extends RegisterArg {
 	}
 
 	@Override
-	public String getName() {
-		if (isThis()) {
-			return THIS_ARG_NAME;
-		}
-		return super.getName();
-	}
-
-	@Override
 	void setSVar(@NotNull SSAVar sVar) {
-		if (isThis()) {
-			sVar.setName(THIS_ARG_NAME);
-		}
 		sVar.setTypeImmutable(type);
 		super.setSVar(sVar);
 	}

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

@@ -89,8 +89,7 @@ public class CodeShrinker extends AbstractVisitor {
 		}
 
 		public WrapInfo checkInline(int assignPos, RegisterArg arg) {
-			if (!arg.isThis()
-					&& (assignPos >= inlineBorder || !canMove(assignPos, inlineBorder))) {
+			if (assignPos >= inlineBorder || !canMove(assignPos, inlineBorder)) {
 				return null;
 			}
 			inlineBorder = assignPos;
@@ -214,9 +213,9 @@ public class CodeShrinker extends AbstractVisitor {
 //					continue;
 //				}
 				SSAVar sVar = arg.getSVar();
-				// allow inline only one use arg or 'this'
+				// allow inline only one use arg
 				if (sVar == null
-						|| sVar.getVariableUseCount() != 1 && !arg.isThis()
+						|| sVar.getVariableUseCount() != 1
 						|| sVar.contains(AFlag.DONT_INLINE)) {
 					continue;
 				}

jadx-core/src/main/java/jadx/core/dex/visitors/ssa/SSATransform.java

@@ -53,7 +53,7 @@ public class SSATransform extends AbstractVisitor {
 
 		fixLastAssignInTry(mth);
 		removeBlockerInsns(mth);
-		markThisArg(mth);
+		markThisArgs(mth.getThisArg());
 
 		boolean repeatFix;
 		int k = 0;
@@ -407,10 +407,30 @@ public class SSATransform extends AbstractVisitor {
 		return true;
 	}
 
-	private static void markThisArg(MethodNode mth) {
-		RegisterArg thisArg = mth.getThisArg();
+	private static void markThisArgs(RegisterArg thisArg) {
 		if (thisArg != null) {
-			thisArg.getSVar().getUseList().forEach(arg -> arg.add(AFlag.THIS));
+			markOneArgAsThis(thisArg);
+			thisArg.getSVar().getUseList().forEach(SSATransform::markOneArgAsThis);
+		}
+	}
+
+	private static void markOneArgAsThis(RegisterArg arg) {
+		if (arg == null) {
+			return;
+		}
+		arg.add(AFlag.THIS);
+		arg.setName(RegisterArg.THIS_ARG_NAME);
+		// mark all moved 'this'
+		InsnNode parentInsn = arg.getParentInsn();
+		if (parentInsn != null
+				&& parentInsn.getType() == InsnType.MOVE
+				&& parentInsn.getArg(0) == arg) {
+			RegisterArg resArg = parentInsn.getResult();
+			if (resArg.getRegNum() != arg.getRegNum()
+					&& !resArg.getSVar().isUsedInPhi()) {
+				markThisArgs(resArg);
+				parentInsn.add(AFlag.SKIP);
+			}
 		}
 	}
 }

jadx-core/src/test/java/jadx/tests/integration/usethis/TestDontInlineThis.java

+package jadx.tests.integration.usethis;
+
+import java.util.Random;
+
+import org.junit.Test;
+
+import jadx.core.dex.nodes.ClassNode;
+import jadx.tests.api.IntegrationTest;
+
+import static jadx.tests.api.utils.JadxMatchers.containsOne;
+import static org.junit.Assert.assertThat;
+
+public class TestDontInlineThis extends IntegrationTest {
+
+	public static class TestCls {
+		public int field = new Random().nextInt();
+
+		private TestCls test() {
+			TestCls res;
+			if (field == 7) {
+				res = this;
+			} else {
+				res = new TestCls();
+			}
+			res.method();
+			return res;
+		}
+
+		private void method() {
+		}
+	}
+
+	@Test
+	public void test() {
+		ClassNode cls = getClassNode(TestCls.class);
+		String code = cls.getCode().toString();
+
+		assertThat(code, containsOne("TestDontInlineThis$TestCls res"));
+		assertThat(code, containsOne("res = this;"));
+		assertThat(code, containsOne("res = new TestDontInlineThis$TestCls();"));
+	}
+}

jadx-core/src/test/java/jadx/tests/integration/usethis/TestInlineThis2.java

+package jadx.tests.integration.usethis;
+
+import java.util.Objects;
+
+import org.junit.Test;
+
+import jadx.core.dex.nodes.ClassNode;
+import jadx.tests.api.IntegrationTest;
+
+import static jadx.tests.api.utils.JadxMatchers.containsOne;
+import static org.hamcrest.CoreMatchers.containsString;
+import static org.hamcrest.CoreMatchers.not;
+import static org.junit.Assert.assertThat;
+
+public class TestInlineThis2 extends IntegrationTest {
+
+	public static class TestCls {
+		public int field;
+
+		private void test() {
+			TestCls thisVar = this;
+			if (Objects.isNull(thisVar)) {
+				System.out.println("null");
+			}
+			thisVar.method();
+			thisVar.field = 123;
+		}
+
+		private void method() {
+		}
+	}
+
+	@Test
+	public void test() {
+		ClassNode cls = getClassNode(TestCls.class);
+		String code = cls.getCode().toString();
+
+		assertThat(code, not(containsString("thisVar")));
+		assertThat(code, not(containsString("thisVar.method()")));
+		assertThat(code, not(containsString("thisVar.field")));
+		assertThat(code, not(containsString("= this")));
+
+		assertThat(code, containsOne("if (Objects.isNull(this)) {"));
+		assertThat(code, containsOne("this.field = 123;"));
+		assertThat(code, containsOne("method();"));
+	}
+}