Skip to content

Commit 325b3ac

Browse files
skylotskylot
committed
fix: use correct args copy/replace in wrapped insns (#2835)
1 parent 9a8a116 commit 325b3ac

10 files changed

Lines changed: 144 additions & 37 deletions

File tree

‎jadx-core/src/main/java/jadx/core/dex/instructions/ConstStringNode.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,6 @@ public boolean isSame(InsnNode obj) {
3535

3636
@Override
3737
public String toString() {
38-
return super.toString() + ' ' + StringUtils.getInstance().unescapeString(str);
38+
return super.baseString() + StringUtils.getInstance().unescapeString(str) + super.attributesString();
3939
}
4040
}

‎jadx-core/src/main/java/jadx/core/dex/instructions/args/InsnWrapArg.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -84,14 +84,14 @@ public String toShortString() {
8484
if (wrappedInsn.getType() == InsnType.CONST_STR) {
8585
return "(\"" + ((ConstStringNode) wrappedInsn).getString() + "\")";
8686
}
87-
return "(wrap:" + type + ":" + wrappedInsn.getType() + ')';
87+
return "(wrap " + type + ":" + wrappedInsn.getType() + ')';
8888
}
8989

9090
@Override
9191
public String toString() {
9292
if (wrappedInsn.getType() == InsnType.CONST_STR) {
9393
return "(\"" + ((ConstStringNode) wrappedInsn).getString() + "\")";
9494
}
95-
return "(wrap:" + type + ":" + wrappedInsn + ')';
95+
return "(wrap " + type + ":" + wrappedInsn + ')';
9696
}
9797
}

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

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -406,6 +406,14 @@ public boolean isDeepEquals(InsnNode other) {
406406
&& Objects.equals(arguments, other.arguments);
407407
}
408408

409+
@SuppressWarnings("unchecked")
410+
public static <T extends InsnArg> @Nullable T duplicateArg(@Nullable T arg) {
411+
if (arg == null) {
412+
return null;
413+
}
414+
return (T) arg.duplicate();
415+
}
416+
409417
protected final <T extends InsnNode> T copyCommonParams(T copy) {
410418
if (copy.getArgsCount() == 0) {
411419
for (InsnArg arg : this.getArguments()) {

‎jadx-core/src/main/java/jadx/core/dex/visitors/SimplifyVisitor.java‎

Lines changed: 24 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -85,7 +85,6 @@ private boolean simplifyBlock(MethodNode mth, BlockNode block) {
8585
int insnCount = list.size();
8686
InsnNode modInsn = simplifyInsn(mth, insn, null);
8787
if (modInsn != null) {
88-
modInsn.rebindArgs();
8988
if (i < list.size() && list.get(i) == insn) {
9089
list.set(i, modInsn);
9190
} else {
@@ -95,6 +94,8 @@ private boolean simplifyBlock(MethodNode mth, BlockNode block) {
9594
}
9695
list.set(idx, modInsn);
9796
}
97+
InsnRemover.unbindInsn(mth, insn);
98+
modInsn.rebindArgs();
9899
if (list.size() < insnCount) {
99100
// some insns removed => restart block processing
100101
simplifyBlock(mth, block);
@@ -239,8 +240,8 @@ private static InsnNode processCast(MethodNode mth, IndexInsnNode castInsn, @Nul
239240
|| shadowedByOuterCast(mth.root(), castToType, parentInsn)) {
240241
InsnNode insnNode = new InsnNode(InsnType.MOVE, 1);
241242
insnNode.setOffset(castInsn.getOffset());
242-
insnNode.setResult(castInsn.getResult());
243-
insnNode.addArg(castArg);
243+
insnNode.setResult(InsnNode.duplicateArg(castInsn.getResult()));
244+
insnNode.addArg(castArg.duplicate());
244245
return insnNode;
245246
}
246247
return null;
@@ -576,7 +577,11 @@ private static InsnNode simplifyArith(ArithNode arith) {
576577
if (litArg.isNegative()) {
577578
LiteralArg negLitArg = litArg.negate();
578579
if (negLitArg != null) {
579-
return new ArithNode(ArithOp.SUB, arith.getResult(), arith.getArg(0), negLitArg);
580+
RegisterArg resArg = InsnNode.duplicateArg(arith.getResult());
581+
ArithNode newInsn = new ArithNode(ArithOp.SUB, resArg, arith.getArg(0).duplicate(), negLitArg);
582+
newInsn.copyAttributesFrom(arith);
583+
newInsn.setOffset(arith.getOffset());
584+
return newInsn;
580585
}
581586
}
582587
break;
@@ -586,10 +591,12 @@ private static InsnNode simplifyArith(ArithNode arith) {
586591
InsnArg firstArg = arith.getArg(0);
587592
long lit = litArg.getLiteral();
588593
if (firstArg.getType() == ArgType.BOOLEAN && (lit == 0 || lit == 1)) {
589-
InsnNode node = new InsnNode(lit == 0 ? InsnType.MOVE : InsnType.NOT, 1);
590-
node.setResult(arith.getResult());
591-
node.addArg(firstArg);
592-
return node;
594+
InsnNode newInsn = new InsnNode(lit == 0 ? InsnType.MOVE : InsnType.NOT, 1);
595+
newInsn.setResult(InsnNode.duplicateArg(arith.getResult()));
596+
newInsn.addArg(firstArg.duplicate());
597+
newInsn.copyAttributesFrom(arith);
598+
newInsn.setOffset(arith.getOffset());
599+
return newInsn;
593600
}
594601
break;
595602
}
@@ -637,16 +644,22 @@ private static ArithNode convertFieldArith(MethodNode mth, InsnNode insn) {
637644
}
638645
if (wrapType == InsnType.ARITH) {
639646
ArithNode ar = (ArithNode) wrap;
640-
return ArithNode.oneArgOp(ar.getOp(), fArg, ar.getArg(1));
647+
ArithNode newInsn = ArithNode.oneArgOp(ar.getOp(), fArg, ar.getArg(1).duplicate());
648+
newInsn.copyAttributesFrom(insn);
649+
newInsn.setOffset(insn.getOffset());
650+
return newInsn;
641651
}
642652
int argsCount = wrap.getArgsCount();
643653
InsnNode concat = new InsnNode(InsnType.STR_CONCAT, argsCount - 1);
644654
for (int i = 1; i < argsCount; i++) {
645-
concat.addArg(wrap.getArg(i));
655+
concat.addArg(wrap.getArg(i).duplicate());
646656
}
647657
InsnArg concatArg = InsnArg.wrapArg(concat);
648658
concatArg.setType(ArgType.STRING);
649-
return ArithNode.oneArgOp(ArithOp.ADD, fArg, concatArg);
659+
ArithNode newInsn = ArithNode.oneArgOp(ArithOp.ADD, fArg, concatArg);
660+
newInsn.copyAttributesFrom(wrap);
661+
newInsn.setOffset(wrap.getOffset());
662+
return newInsn;
650663
} catch (Exception e) {
651664
LOG.debug("Can't convert field arith insn: {}, mth: {}", insn, mth, e);
652665
}

‎jadx-core/src/main/java/jadx/core/dex/visitors/regions/TernaryMod.java‎

Lines changed: 11 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -111,23 +111,26 @@ private static boolean makeTernaryInsn(MethodNode mth, IfRegion ifRegion) {
111111
RegisterArg resArg;
112112
if (thenPhi.getArgsCount() == 2) {
113113
resArg = thenPhi.getResult();
114-
InsnRemover.unbindResult(mth, thenInsn);
115114
} else {
116115
resArg = thenResArg;
117116
thenPhi.removeArg(elseResArg);
118117
}
119-
InsnArg thenArg = InsnArg.wrapInsnIntoArg(thenInsn);
120-
InsnArg elseArg = InsnArg.wrapInsnIntoArg(elseInsn);
121-
TernaryInsn ternInsn = new TernaryInsn(ifRegion.getCondition(), resArg, thenArg, elseArg);
118+
InsnArg thenArg = InsnArg.wrapInsnIntoArg(thenInsn.copyWithoutResult());
119+
InsnArg elseArg = InsnArg.wrapInsnIntoArg(elseInsn.copyWithoutResult());
120+
TernaryInsn ternInsn = new TernaryInsn(ifRegion.getCondition(), resArg.duplicate(), thenArg, elseArg);
122121
int branchLine = Math.max(thenInsn.getSourceLine(), elseInsn.getSourceLine());
123122
ternInsn.setSourceLine(Math.max(ifRegion.getSourceLine(), branchLine));
124123

125-
thenInsn.setResult(null); // unset without unbind, SSA var still in use
126-
InsnRemover.unbindResult(mth, elseInsn);
124+
InsnRemover.unbindInsn(mth, thenInsn);
125+
InsnRemover.unbindInsn(mth, elseInsn);
126+
ternInsn.rebindArgs();
127+
if (thenPhi.getArgsCount() == 0) {
128+
InsnRemover.unbindResult(mth, thenPhi);
129+
InsnRemover.delistPhi(mth, thenPhi);
130+
}
127131

128132
// remove 'if' instruction
129133
header.getInstructions().clear();
130-
ternInsn.rebindArgs();
131134
header.getInstructions().add(ternInsn);
132135

133136
clearConditionBlocks(conditionBlocks, header);
@@ -321,11 +324,11 @@ private static void replaceWithTernary(MethodNode mth, IfRegion ifRegion, BlockN
321324
InsnArg elseArg;
322325
if (elseAssign != null && elseAssign.isConstInsn()) {
323326
// inline constant
327+
elseArg = InsnArg.wrapInsnIntoArg(elseAssign.copyWithoutResult());
324328
SSAVar elseVar = elseAssign.getResult().getSVar();
325329
if (elseVar.getUseCount() == 1 && elseVar.getOnlyOneUseInPhi() == phiInsn) {
326330
InsnRemover.remove(mth, elseAssign);
327331
}
328-
elseArg = InsnArg.wrapInsnIntoArg(elseAssign);
329332
} else {
330333
elseArg = otherArg.duplicate();
331334
}

‎jadx-core/src/main/java/jadx/core/utils/DebugChecks.java‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -78,8 +78,13 @@ private static void checkInsn(MethodNode mth, BlockNode block, InsnNode insn) {
7878
for (InsnArg arg : insn.getArguments()) {
7979
if (arg instanceof RegisterArg) {
8080
checkVar(mth, insn, (RegisterArg) arg);
81-
} else if (arg.isInsnWrap()) {
81+
} else if (arg instanceof InsnWrapArg) {
8282
InsnNode wrapInsn = ((InsnWrapArg) arg).getWrapInsn();
83+
if (wrapInsn.contains(AFlag.DONT_GENERATE)
84+
&& !insn.contains(AFlag.DONT_GENERATE)
85+
&& !mth.contains(AFlag.DONT_GENERATE)) {
86+
throw new JadxRuntimeException("Not generated wrapped insn: \n " + wrapInsn + ",\nouter insn:\n " + insn);
87+
}
8388
checkInsn(mth, block, wrapInsn);
8489
}
8590
}

‎jadx-core/src/test/java/jadx/tests/integration/arith/TestFieldIncrement.java‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,10 +3,12 @@
33
import org.junit.jupiter.api.Test;
44

55
import jadx.tests.api.IntegrationTest;
6-
import jadx.tests.api.utils.assertj.JadxAssertions;
6+
7+
import static jadx.tests.api.utils.assertj.JadxAssertions.assertThat;
78

89
public class TestFieldIncrement extends IntegrationTest {
910

11+
@SuppressWarnings("unused")
1012
public static class TestCls {
1113
public int instanceField = 1;
1214
public static int staticField = 1;
@@ -27,7 +29,7 @@ public void method3(String s) {
2729

2830
@Test
2931
public void test() {
30-
JadxAssertions.assertThat(getClassNode(TestCls.class))
32+
assertThat(getClassNode(TestCls.class))
3133
.code()
3234
.contains("instanceField++;")
3335
.contains("staticField--;")

‎jadx-core/src/test/java/jadx/tests/integration/arith/TestFieldIncrement3.java‎

Lines changed: 14 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -5,10 +5,12 @@
55
import org.junit.jupiter.api.Test;
66

77
import jadx.tests.api.IntegrationTest;
8-
import jadx.tests.api.utils.assertj.JadxAssertions;
8+
9+
import static jadx.tests.api.utils.assertj.JadxAssertions.assertThat;
910

1011
public class TestFieldIncrement3 extends IntegrationTest {
1112

13+
@SuppressWarnings("SpellCheckingInspection")
1214
public static class TestCls {
1315
static int tileX;
1416
static int tileY;
@@ -21,28 +23,28 @@ public static void test() {
2123
int direction = rd.nextInt(7);
2224
switch (direction) {
2325
case 0:
24-
targetPos.x = ((tileX + 1) * 55) + 55;
25-
targetPos.y = ((tileY + 1) * 35) + 35;
26+
targetPos.x = (tileX + 1) * 55 + 55;
27+
targetPos.y = (tileY + 1) * 35 + 35;
2628
break;
2729
case 2:
28-
targetPos.x = ((tileX + 1) * 55) + 55;
29-
targetPos.y = ((tileY - 1) * 35) + 35;
30+
targetPos.x = (tileX + 1) * 55 + 55;
31+
targetPos.y = (tileY - 1) * 35 + 35;
3032
break;
3133
case 4:
32-
targetPos.x = ((tileX - 1) * 55) + 55;
33-
targetPos.y = ((tileY - 1) * 35) + 35;
34+
targetPos.x = (tileX - 1) * 55 + 55;
35+
targetPos.y = (tileY - 1) * 35 + 35;
3436
break;
3537
case 6:
36-
targetPos.x = ((tileX - 1) * 55) + 55;
37-
targetPos.y = ((tileY + 1) * 35) + 35;
38+
targetPos.x = (tileX - 1) * 55 + 55;
39+
targetPos.y = (tileY + 1) * 35 + 35;
3840
break;
3941
default:
4042
break;
4143
}
4244
directVect.x = targetPos.x - newPos.x;
4345
directVect.y = targetPos.y - newPos.y;
4446

45-
float hPos = (float) Math.sqrt((directVect.x * directVect.x) + (directVect.y * directVect.y));
47+
float hPos = (float) Math.sqrt(directVect.x * directVect.x + directVect.y * directVect.y);
4648
directVect.x /= hPos;
4749
directVect.y /= hPos;
4850
}
@@ -57,14 +59,14 @@ public Vector2() {
5759
}
5860

5961
public boolean equals(Vector2 other) {
60-
return (this.x == other.x && this.y == other.y);
62+
return this.x == other.x && this.y == other.y;
6163
}
6264
}
6365
}
6466

6567
@Test
6668
public void test() {
67-
JadxAssertions.assertThat(getClassNode(TestCls.class))
69+
assertThat(getClassNode(TestCls.class))
6870
.code()
6971
.contains("directVect.x = targetPos.x - newPos.x;");
7072
}

‎jadx-core/src/test/java/jadx/tests/integration/variables/TestVariables7.java‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
package jadx.tests.integration.variables;
2+
3+
import org.junit.jupiter.api.Test;
4+
5+
import jadx.tests.api.SmaliTest;
6+
7+
import static jadx.tests.api.utils.assertj.JadxAssertions.assertThat;
8+
9+
public class TestVariables7 extends SmaliTest {
10+
@Test
11+
public void testNoDebug() {
12+
getArgs().setDebugInfo(false);
13+
assertThat(getClassNodeFromSmali())
14+
.code()
15+
.doesNotContain("r0");
16+
}
17+
}

‎jadx-core/src/test/smali/variables/TestVariables7.smali‎

Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,57 @@
1+
.class Lvariables/TestVariables7;
2+
.super Ljava/lang/Object;
3+
4+
.method test([BII)I
5+
.registers 12
6+
.param p1, "a" # [B
7+
.param p2, "b" # I
8+
.param p3, "c" # I
9+
10+
.prologue
11+
.line 290
12+
invoke-static {p3}, Ljava/lang/Integer;->toOctalString(I)Ljava/lang/String;
13+
move-result-object v3
14+
15+
.line 291
16+
.local v3, "oct":Ljava/lang/String;
17+
invoke-virtual {v3}, Ljava/lang/String;->length()I
18+
move-result v2
19+
20+
.line 292
21+
.local v2, "len":I
22+
sub-int v4, p2, v2
23+
24+
.line 293
25+
.local v4, "off":I
26+
const/4 v5, 0x0
27+
28+
.line 294
29+
.local v5, "sum":I
30+
const/4 v1, 0x0
31+
.local v1, "j":I
32+
:goto_c
33+
if-lt v1, v2, :cond_f
34+
35+
.line 299
36+
return v5
37+
38+
.line 295
39+
:cond_f
40+
invoke-virtual {v3, v1}, Ljava/lang/String;->charAt(I)C
41+
move-result v0
42+
43+
.line 296
44+
.local v0, "ch":C
45+
and-int/lit16 v6, v0, 0xff
46+
add-int/lit8 v6, v6, -0x30
47+
add-int/2addr v5, v6
48+
49+
.line 297
50+
add-int v6, v4, v1
51+
int-to-byte v7, v0
52+
aput-byte v7, p1, v6
53+
54+
.line 294
55+
add-int/lit8 v1, v1, 0x1
56+
goto :goto_c
57+
.end method

0 commit comments

Comments
 (0)