Skip to content

Commit

Permalink
Revisit loop fixes
Browse files Browse the repository at this point in the history 
* Fix cases where a needed break was not inserted after a loop
* Detect cases where a continue needs to be added after a loop
* Fix cases where the body of an if statement was incorrectly extracted
* Re-enable the construction of do while loops
  • Loading branch information
@JDLogic
JDLogic committed Oct 4, 2018
1 parent 5d5806c commit cb91553
Showing 1 changed file with 118 additions and 85 deletions.
203 changes: 118 additions & 85 deletions FernFlower-Patches/0009-LVT-Fixes-and-Support-for-Enhanced-For-loop-detectio.patch
View file
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
From ddfa385e35cc8b30dd97ae5243059a3be8b842ce Mon Sep 17 00:00:00 2001
From 06eb2cabbc331b5e216fac3835d461f36c52d5d0 Mon Sep 17 00:00:00 2001
From: LexManos <LexManos@gmail.com>
Date: Tue, 11 Apr 2017 22:54:20 -0700
Subject: [PATCH] LVT Fixes and Support for Enhanced For loop detection.
Expand Down Expand Up @@ -44,7 +44,7 @@ index 5b84ab3..57a2897 100644
}
}
diff --git a/src/org/jetbrains/java/decompiler/main/rels/MethodProcessorRunnable.java b/src/org/jetbrains/java/decompiler/main/rels/MethodProcessorRunnable.java
index 9f047d4..8ac1d92 100644
index 9f047d4..5a93ffe 100644
--- a/src/org/jetbrains/java/decompiler/main/rels/MethodProcessorRunnable.java
+++ b/src/org/jetbrains/java/decompiler/main/rels/MethodProcessorRunnable.java
@@ -11,7 +11,13 @@ import org.jetbrains.java.decompiler.main.extern.IFernflowerPreferences;
Expand Down Expand Up @@ -84,25 +84,31 @@ index 9f047d4..8ac1d92 100644
break;
}
}
@@ -135,12 +141,16 @@ public class MethodProcessorRunnable implements Runnable {
@@ -135,6 +141,10 @@ public class MethodProcessorRunnable implements Runnable {
LabelHelper.cleanUpEdges(root);

while (true) {
- MergeHelper.enhanceLoops(root);
+ if (EliminateLoopsHelper.eliminateLoops(root, cl)) {
+ continue;
+ }
+
MergeHelper.enhanceLoops(root);

if (LoopExtractHelper.extractLoops(root)) {
continue;
}
@@ -163,6 +173,12 @@ public class MethodProcessorRunnable implements Runnable {
continue;
}

+ MergeHelper.enhanceLoops(root);
+ // this has to be done last so it does not screw up the formation of for loops
+ if (MergeHelper.makeDoWhileLoops(root)) {
+ LabelHelper.cleanUpEdges(root);
+ LabelHelper.identifyLabels(root);
+ }
+
if (!IfHelper.mergeAllIfs(root)) {
break;
}
@@ -178,6 +188,8 @@ public class MethodProcessorRunnable implements Runnable {
// initializer may have at most one return point, so no transformation of method exits permitted
if (isInitializer || !ExitHelper.condenseExits(root)) {
break;
@@ -178,6 +194,8 @@ public class MethodProcessorRunnable implements Runnable {

SecondaryFunctionsHelper.identifySecondaryFunctions(root, varProc);

Expand All @@ -111,7 +117,7 @@ index 9f047d4..8ac1d92 100644
varProc.setVarDefinitions(root);

// must be the last invocation, because it makes the statement structure inconsistent
@@ -198,4 +210,30 @@ public class MethodProcessorRunnable implements Runnable {
@@ -198,4 +216,30 @@ public class MethodProcessorRunnable implements Runnable {
public boolean isFinished() {
return finished;
}
Expand Down Expand Up @@ -707,7 +713,7 @@ index dd431f1..8c9b5b6 100644
}
else {
diff --git a/src/org/jetbrains/java/decompiler/modules/decompiler/LoopExtractHelper.java b/src/org/jetbrains/java/decompiler/modules/decompiler/LoopExtractHelper.java
index 8229127..545f687 100644
index 8229127..5279618 100644
--- a/src/org/jetbrains/java/decompiler/modules/decompiler/LoopExtractHelper.java
+++ b/src/org/jetbrains/java/decompiler/modules/decompiler/LoopExtractHelper.java
@@ -8,6 +8,7 @@ import org.jetbrains.java.decompiler.modules.decompiler.stats.Statement;
Expand Down Expand Up @@ -754,10 +760,18 @@ index 8229127..545f687 100644

// search for an if condition at the end of the loop
Statement last = stat.getFirst();
@@ -93,6 +111,11 @@ public class LoopExtractHelper {
@@ -93,6 +111,19 @@ public class LoopExtractHelper {

if (set.isEmpty()) { // no direct continues in a do{}while loop
if (isExternStatement(stat, ifstat, ifstat)) {
+ Statement first = stat.getFirst();
+ while (first.type == Statement.TYPE_SEQUENCE) {
+ first = first.getFirst();
+ }
+ if (first.type == Statement.TYPE_DO && ((DoStatement)first).getLooptype() == DoStatement.LOOP_DO) {
+ return false;
+ }
+
+ for (Statement s : stats) {
+ if (!ifstat.containsStatement(s)) {
+ return false;
Expand All @@ -766,7 +780,7 @@ index 8229127..545f687 100644
extractIfBlock(stat, lastif);
return true;
}
@@ -103,7 +126,7 @@ public class LoopExtractHelper {
@@ -103,7 +134,7 @@ public class LoopExtractHelper {
return false;
}

Expand All @@ -775,7 +789,7 @@ index 8229127..545f687 100644

// search for an if condition at the entrance of the loop
Statement first = stat.getFirst();
@@ -121,6 +144,11 @@ public class LoopExtractHelper {
@@ -121,6 +152,11 @@ public class LoopExtractHelper {
Statement ifstat = firstif.getIfstat();

if (isExternStatement(stat, ifstat, ifstat)) {
Expand All @@ -787,7 +801,7 @@ index 8229127..545f687 100644
extractIfBlock(stat, firstif);
return true;
}
@@ -181,5 +209,10 @@ public class LoopExtractHelper {
@@ -181,5 +217,10 @@ public class LoopExtractHelper {
loop.addPredecessor(edge);
}
}
Expand Down Expand Up @@ -1000,7 +1014,7 @@ index 0000000..c62c74d
+}
\ No newline at end of file
diff --git a/src/org/jetbrains/java/decompiler/modules/decompiler/MergeHelper.java b/src/org/jetbrains/java/decompiler/modules/decompiler/MergeHelper.java
index efc1891..49862b5 100644
index efc1891..29df03d 100644
--- a/src/org/jetbrains/java/decompiler/modules/decompiler/MergeHelper.java
+++ b/src/org/jetbrains/java/decompiler/modules/decompiler/MergeHelper.java
@@ -4,8 +4,13 @@ package org.jetbrains.java.decompiler.modules.decompiler;
Expand Down Expand Up @@ -1042,67 +1056,35 @@ index efc1891..49862b5 100644
}

return (stat.getLooptype() != oldloop);
@@ -142,13 +150,45 @@ public class MergeHelper {
@@ -142,7 +150,8 @@ public class MergeHelper {
if (firstif.iftype == IfStatement.IFTYPE_IF) {
if (firstif.getIfstat() == null) {
StatEdge ifedge = firstif.getIfEdge();
- if (isDirectPath(stat, ifedge.getDestination())) {
+ boolean infinite = false;
+
+ // there are some special cases where we can fix this
+ // FIXME do these result from an error somewhere else?
+ if (!isDirectPath(stat, ifedge.getDestination())) {
+ // bad infinite loop needs special handling
+ infinite = ifedge.getType() == StatEdge.TYPE_CONTINUE && stat.getFirst().equals(firstif) && stat.equals(ifedge.getDestination());
+
+ // inside a switch or loop and we need to add a break after the current loop
+ Statement parent = stat.getParent();
+ if (!infinite && parent != null) {
+ if (parent.type != Statement.TYPE_SEQUENCE || parent.getStats().getLast().equals(stat)) {
+ Statement outer = parent.getParent();
+ while (outer != null && outer.type != Statement.TYPE_SWITCH && outer.type != Statement.TYPE_DO) {
+ outer = outer.getParent();
+ }
+
+ if (outer != null) {
+ List<StatEdge> edges = outer.getAllSuccessorEdges();
+ if (!edges.isEmpty()) {
+ StatEdge edge = edges.get(0);
+ if (edge.getDestination().equals(ifedge.getDestination())) {
+ stat.addSuccessor(new StatEdge(StatEdge.TYPE_BREAK, stat, ifedge.getDestination(), outer));
+ }
+ }
+ }
+ }
+ }
+ }
+
+ if (infinite || isDirectPath(stat, ifedge.getDestination())) {
+ if (isDirectPath(stat, ifedge.getDestination()) || addContinueOrBreak(stat, ifedge)) {
// exit condition identified
stat.setLooptype(DoStatement.LOOP_WHILE);

// negate condition (while header)
IfExprent ifexpr = (IfExprent)firstif.getHeadexprent().copy();
- ifexpr.negateIf();
+ if (!infinite) {
+ ifexpr.negateIf();
+ }

if (stat.getConditionExprent() != null) {
ifexpr.getCondition().addBytecodeOffsets(stat.getConditionExprent().bytecode);
@@ -159,6 +199,11 @@ public class MergeHelper {

// remove edges
firstif.getFirst().removeSuccessor(ifedge);
+
+ if (infinite) {
+ ifedge = firstif.getAllSuccessorEdges().get(0);
+ }
+
firstif.removeSuccessor(firstif.getAllSuccessorEdges().get(0));
@@ -186,7 +195,7 @@ public class MergeHelper {
return true;
}
}
- else {
+ //else { // fix infinite loops
StatEdge elseedge = firstif.getAllSuccessorEdges().get(0);
if (isDirectPath(stat, elseedge.getDestination())) {
// exit condition identified
@@ -240,7 +249,7 @@ public class MergeHelper {

if (stat.getAllSuccessorEdges().isEmpty()) {
@@ -325,6 +370,8 @@ public class MergeHelper {
return true;
}
- }
+ //}
}
}
}
@@ -325,6 +334,8 @@ public class MergeHelper {
}
else {
preData = current.getNeighbours(StatEdge.TYPE_REGULAR, Statement.DIRECTION_BACKWARD).get(0);
Expand All @@ -1111,7 +1093,7 @@ index efc1891..49862b5 100644
preData = getLastDirectData(preData);
if (preData != null && !preData.getExprents().isEmpty()) {
initDoExprent = preData.getExprents().get(preData.getExprents().size() - 1);
@@ -363,12 +410,16 @@ public class MergeHelper {
@@ -363,12 +374,16 @@ public class MergeHelper {
stat.setIncExprent(exp);
}

Expand All @@ -1132,7 +1114,7 @@ index efc1891..49862b5 100644
}
}

@@ -401,15 +452,296 @@ public class MergeHelper {
@@ -401,15 +416,347 @@ public class MergeHelper {
return stat;
}

Expand Down Expand Up @@ -1182,9 +1164,9 @@ index efc1891..49862b5 100644
+ }
+ }
+ }
}
+ }
+ break;
}
+ }
+ }
+ else {
+ break;
Expand Down Expand Up @@ -1254,7 +1236,8 @@ index efc1891..49862b5 100644
+ holder.getInstance().type == Exprent.EXPRENT_VAR) {
+ VarExprent copy = (VarExprent)initExprents[1].getLeft();
+ VarExprent inc = (VarExprent)holder.getInstance();
+ if (copy.getIndex() == inc.getIndex() && copy.getVersion() == inc.getVersion() && !inc.isVarReferenced(stat.getTopParent(), copy)) {
+ if (copy.getIndex() == inc.getIndex() && copy.getVersion() == inc.getVersion() &&
+ !inc.isVarReferenced(stat.getTopParent(), copy) && !isNextCall(initExprents[1].getRight())) {
+ preData.getExprents().remove(initExprents[1]);
+ initExprents[1].getBytecodeRange(initExprents[1].getRight().bytecode);
+ stat.getIncExprent().getBytecodeRange(initExprents[1].getRight().bytecode);
Expand Down Expand Up @@ -1326,8 +1309,8 @@ index efc1891..49862b5 100644
+ initExprents[2].getRight().addBytecodeOffsets(initExprents[2].bytecode);
+ initExprents[2].getRight().addBytecodeOffsets(stat.getIncExprent().bytecode);
+ stat.setIncExprent(initExprents[2].getRight());
+ }
+ }
}
}
+
+ return true;
+ }
Expand Down Expand Up @@ -1369,9 +1352,11 @@ index efc1891..49862b5 100644
+ if (ret != null) {
+ return ret;
+ }
+ }
+ return null;
+ }
}
return null;
}
-}
\ No newline at end of file
+
+ private static Exprent getUncast(Exprent exp) {
+ if (exp.type == Exprent.EXPRENT_FUNCTION) {
Expand All @@ -1387,11 +1372,9 @@ index efc1891..49862b5 100644
+ exp = getUncast(exp);
+ if (exp.type == Exprent.EXPRENT_INVOCATION) {
+ return (InvocationExprent) exp;
}
return null;
}
-}
\ No newline at end of file
+ }
+ return null;
+ }
+
+ private static boolean isIteratorCall(Exprent exp) {
+ final InvocationExprent iexp = asInvocationExprent(exp);
Expand Down Expand Up @@ -1436,6 +1419,56 @@ index efc1891..49862b5 100644
+ InvocationExprent inv = (InvocationExprent)exp;
+ return inv.isUnboxingCall() && isNextCall(inv.getInstance());
+ }
+
+ public static boolean makeDoWhileLoops(RootStatement root) {
+ if (makeDoWhileRec(root)) {
+ SequenceHelper.condenseSequences(root);
+ return true;
+ }
+ return false;
+ }
+
+ private static boolean makeDoWhileRec(Statement stat) {
+ boolean ret = false;
+
+ for (Statement st : stat.getStats()) {
+ ret |= makeDoWhileRec(st);
+ }
+
+ if (stat.type == Statement.TYPE_DO) {
+ DoStatement dostat = (DoStatement)stat;
+ if (dostat.getLooptype() == DoStatement.LOOP_DO) {
+ matchDoWhile(dostat);
+ ret |= dostat.getLooptype() != DoStatement.LOOP_DO;
+ }
+ }
+
+ return ret;
+ }
+
+ private static boolean addContinueOrBreak(DoStatement stat, StatEdge ifedge) {
+ Statement outer = stat.getParent();
+ while (outer != null && outer.type != Statement.TYPE_SWITCH && outer.type != Statement.TYPE_DO) {
+ outer = outer.getParent();
+ }
+
+ if (outer != null && (outer.type == Statement.TYPE_SWITCH || ((DoStatement)outer).getLooptype() != DoStatement.LOOP_DO)) {
+ Statement parent = stat.getParent();
+ if (parent.type != Statement.TYPE_SEQUENCE || parent.getStats().getLast().equals(stat)) {
+ // need to insert a break or continue after the loop
+ if (ifedge.getDestination().equals(outer)) {
+ stat.addSuccessor(new StatEdge(StatEdge.TYPE_CONTINUE, stat, ifedge.getDestination(), outer));
+ return true;
+ }
+ else if (MergeHelper.isDirectPath(outer, ifedge.getDestination())) {
+ stat.addSuccessor(new StatEdge(StatEdge.TYPE_BREAK, stat, ifedge.getDestination(), outer));
+ return true;
+ }
+ }
+ }
+
+ return false;
+ }
+}
diff --git a/src/org/jetbrains/java/decompiler/modules/decompiler/PPandMMHelper.java b/src/org/jetbrains/java/decompiler/modules/decompiler/PPandMMHelper.java
index d73535f..94dc0a3 100644
Expand Down

0 comments on commit cb91553

Please sign in to comment.