Skip to content

Commit a6698b7

Browse files
Kroppebjaskarth
authored andcommitted
Fix incorrect ppmm inlining (#451)
* Add bad test for ppmm inlining * Update test * Fix ppmm bug * More bad case :( * Fix array assign ppmm bug
1 parent 7108bed commit a6698b7

File tree

5 files changed

+446
-29
lines changed

5 files changed

+446
-29
lines changed

Diff for: src/org/jetbrains/java/decompiler/modules/decompiler/SimplifyExprentsHelper.java

+22-24
Original file line numberDiff line numberDiff line change
@@ -683,20 +683,30 @@ private static boolean inlinePPIAndMMI(Exprent expr, Exprent next) {
683683
// Try to find the first valid usage of a variable for PPMM inlining.
684684
// Returns Pair{parent exprent, variable exprent to replace}
685685
private static Pair<Exprent, VarExprent> findFirstValidUsage(VarExprent match, Exprent next) {
686-
Deque<Exprent> stack = new LinkedList<>();
686+
List<Exprent> stack = new ArrayList<>();
687+
List<Exprent> parent = new ArrayList<>();
687688
stack.add(next);
689+
parent.add(null);
688690

689691
while (!stack.isEmpty()) {
690-
Exprent expr = stack.removeLast();
692+
Exprent expr = stack.remove(stack.size() - 1);
693+
Exprent parentExpr = parent.remove(parent.size() - 1);
691694

692695
List<Exprent> exprents = expr.getAllExprents();
693696

697+
if (parentExpr != null &&
698+
expr instanceof VarExprent ve &&
699+
ve.getIndex() == match.getIndex() &&
700+
ve.getVersion() == match.getVersion()) {
701+
return Pair.of(parentExpr, ve);
702+
}
703+
694704
if (expr instanceof FunctionExprent) {
695705
FunctionExprent func = (FunctionExprent) expr;
696706

697707
// Don't inline ppmm into more ppmm
698708
if (isPPMM(func)) {
699-
continue;
709+
return null;
700710
}
701711

702712
// Don't consider || or &&
@@ -718,30 +728,18 @@ private static Pair<Exprent, VarExprent> findFirstValidUsage(VarExprent match, E
718728
// Reverse iteration to ensure DFS
719729
for (int i = exprents.size() - 1; i >= 0; i--) {
720730
Exprent ex = exprents.get(i);
721-
boolean add = true;
722-
723-
// Skip LHS of assignment as it is invalid
724-
if (expr instanceof AssignmentExprent) {
725-
add = ex != ((AssignmentExprent) expr).getLeft();
726-
}
727731

728-
// Check var if we find
729-
if (add && ex instanceof VarExprent) {
730-
VarExprent ve = (VarExprent) ex;
731-
732-
if (ve.getIndex() == match.getIndex() && ve.getVersion() == match.getVersion()) {
733-
return Pair.of(expr, ve);
734-
}
735-
}
736-
737-
// Ignore ++/-- exprents as they aren't valid usages to replace
738-
if (ex instanceof FunctionExprent) {
739-
add = !isPPMM((FunctionExprent) ex);
732+
// Avoid making something like `++a = 5`. It shouldn't happen but better be safe than sorry.
733+
if (expr instanceof AssignmentExprent asExpr &&
734+
ex == asExpr.getLeft() &&
735+
ex instanceof VarExprent innerEx &&
736+
innerEx.getIndex() == match.getIndex()
737+
) {
738+
continue;
740739
}
741740

742-
if (add) {
743-
stack.add(ex);
744-
}
741+
stack.add(ex);
742+
parent.add(expr);
745743
}
746744
}
747745

Diff for: testData/results/pkg/TestArrayPPMM.dec

+49-3
Original file line numberDiff line numberDiff line change
@@ -37,8 +37,20 @@ public class TestArrayPPMM {
3737
this.accept(array[i++], array[++i]);// 39
3838
}// 40
3939

40-
private void accept(int i, int j) {
40+
public void test9(int[] array, int i) {
41+
array[++i] = i;// 43
4142
}// 44
43+
44+
public void test10(int i) {
45+
this.getArray()[++i] = i;// 47
46+
}// 48
47+
48+
private void accept(int i, int j) {
49+
}// 52
50+
51+
private int[] getArray() {
52+
return new int[10];// 55
53+
}
4254
}
4355

4456
class 'pkg/TestArrayPPMM' {
@@ -190,8 +202,37 @@ class 'pkg/TestArrayPPMM' {
190202
10 37
191203
}
192204

193-
method 'accept (II)V' {
205+
method 'test9 ([II)V' {
194206
0 40
207+
1 40
208+
2 40
209+
3 40
210+
5 40
211+
6 40
212+
7 41
213+
}
214+
215+
method 'test10 (I)V' {
216+
0 44
217+
1 44
218+
2 44
219+
3 44
220+
4 44
221+
5 44
222+
6 44
223+
8 44
224+
9 44
225+
a 45
226+
}
227+
228+
method 'accept (II)V' {
229+
0 48
230+
}
231+
232+
method 'getArray ()[I' {
233+
0 51
234+
1 51
235+
4 51
195236
}
196237
}
197238

@@ -214,4 +255,9 @@ Lines mapping:
214255
36 <-> 34
215256
39 <-> 37
216257
40 <-> 38
217-
44 <-> 41
258+
43 <-> 41
259+
44 <-> 42
260+
47 <-> 45
261+
48 <-> 46
262+
52 <-> 49
263+
55 <-> 52

0 commit comments

Comments
 (0)