Skip to content

Commit 42fa3df

Browse files
author
emmanue1
committed
Improve 'for' statement reconstruction
1 parent 210935d commit 42fa3df

File tree

7 files changed

+131
-58
lines changed

7 files changed

+131
-58
lines changed

‎src/main/java/org/jd/core/v1/service/converter/classfiletojavasyntax/util/ControlFlowGraphReducer.java‎

Lines changed: 72 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -1031,14 +1031,26 @@ protected static boolean reduceJsr(BitSet visited, BasicBlock basicBlock, BitSet
10311031
}
10321032

10331033
protected static boolean reduceLoop(BitSet visited, BasicBlock basicBlock, BitSet jsrTargets) {
1034+
Object clone = visited.clone();
10341035
boolean reduced = reduce(visited, basicBlock.getSub1(), jsrTargets);
10351036

10361037
if (reduced == false) {
10371038
BitSet visitedMembers = new BitSet();
1038-
createContinueLoop(visitedMembers, basicBlock.getSub1());
1039-
visited.andNot(visitedMembers);
1039+
BasicBlock updateBasicBlock = searchUpdateBlockAndCreateContinueLoop(visitedMembers, basicBlock.getSub1());
1040+
1041+
visited = (BitSet)((BitSet)clone).clone();
10401042
reduced = reduce(visited, basicBlock.getSub1(), jsrTargets);
10411043

1044+
if (updateBasicBlock != null) {
1045+
BasicBlock ifBasicBlock = basicBlock.getControlFlowGraph().newBasicBlock(TYPE_IF, basicBlock.getSub1().getFromOffset(), basicBlock.getToOffset());
1046+
1047+
ifBasicBlock.setCondition(END);
1048+
ifBasicBlock.setSub1(basicBlock.getSub1());
1049+
ifBasicBlock.setNext(updateBasicBlock);
1050+
updateBasicBlock.getPredecessors().add(ifBasicBlock);
1051+
basicBlock.setSub1(ifBasicBlock);
1052+
}
1053+
10421054
if (reduced == false) {
10431055
visitedMembers.clear();
10441056

@@ -1209,7 +1221,9 @@ protected static void replaceLoopStartWithSwitchBreak(BitSet visited, BasicBlock
12091221
}
12101222
}
12111223

1212-
protected static void createContinueLoop(BitSet visited, BasicBlock basicBlock) {
1224+
protected static BasicBlock searchUpdateBlockAndCreateContinueLoop(BitSet visited, BasicBlock basicBlock) {
1225+
BasicBlock updateBasicBlock = null;
1226+
12131227
if (!basicBlock.matchType(GROUP_END) && (visited.get(basicBlock.getIndex()) == false)) {
12141228
visited.set(basicBlock.getIndex());
12151229

@@ -1218,92 +1232,103 @@ protected static void createContinueLoop(BitSet visited, BasicBlock basicBlock)
12181232
case TYPE_JSR:
12191233
case TYPE_CONDITION:
12201234
case TYPE_CONDITION_TERNARY_OPERATOR:
1221-
createContinueLoop(visited, basicBlock, basicBlock.getBranch());
1235+
updateBasicBlock = searchUpdateBlockAndCreateContinueLoop(visited, basicBlock, basicBlock.getBranch());
12221236
case TYPE_START:
12231237
case TYPE_STATEMENTS:
12241238
case TYPE_GOTO:
12251239
case TYPE_GOTO_IN_TERNARY_OPERATOR:
12261240
case TYPE_LOOP:
1227-
createContinueLoop(visited, basicBlock, basicBlock.getNext());
1241+
if (updateBasicBlock == null) {
1242+
updateBasicBlock = searchUpdateBlockAndCreateContinueLoop(visited, basicBlock, basicBlock.getNext());
1243+
}
12281244
break;
12291245
case TYPE_TRY:
12301246
case TYPE_TRY_JSR:
12311247
case TYPE_TRY_ECLIPSE:
1232-
createContinueLoop(visited, basicBlock, basicBlock.getSub1());
1248+
updateBasicBlock = searchUpdateBlockAndCreateContinueLoop(visited, basicBlock, basicBlock.getSub1());
12331249
case TYPE_TRY_DECLARATION:
12341250
for (BasicBlock.ExceptionHandler exceptionHandler : basicBlock.getExceptionHandlers()) {
1235-
createContinueLoop(visited, basicBlock, exceptionHandler.getBasicBlock());
1251+
if (updateBasicBlock == null) {
1252+
updateBasicBlock = searchUpdateBlockAndCreateContinueLoop(visited, basicBlock, exceptionHandler.getBasicBlock());
1253+
}
1254+
}
1255+
if (updateBasicBlock == null) {
1256+
updateBasicBlock = searchUpdateBlockAndCreateContinueLoop(visited, basicBlock, basicBlock.getNext());
12361257
}
1237-
createContinueLoop(visited, basicBlock, basicBlock.getNext());
12381258
break;
12391259
case TYPE_IF_ELSE:
12401260
case TYPE_TERNARY_OPERATOR:
1241-
createContinueLoop(visited, basicBlock, basicBlock.getSub2());
1261+
updateBasicBlock = searchUpdateBlockAndCreateContinueLoop(visited, basicBlock, basicBlock.getSub2());
12421262
case TYPE_IF:
1243-
createContinueLoop(visited, basicBlock, basicBlock.getSub1());
1244-
createContinueLoop(visited, basicBlock, basicBlock.getNext());
1263+
if (updateBasicBlock == null) {
1264+
updateBasicBlock = searchUpdateBlockAndCreateContinueLoop(visited, basicBlock, basicBlock.getSub1());
1265+
}
1266+
if (updateBasicBlock == null) {
1267+
updateBasicBlock = searchUpdateBlockAndCreateContinueLoop(visited, basicBlock, basicBlock.getNext());
1268+
}
12451269
break;
12461270
case TYPE_CONDITION_OR:
12471271
case TYPE_CONDITION_AND:
1248-
createContinueLoop(visited, basicBlock, basicBlock.getSub1());
1249-
createContinueLoop(visited, basicBlock, basicBlock.getSub2());
1272+
updateBasicBlock = searchUpdateBlockAndCreateContinueLoop(visited, basicBlock, basicBlock.getSub1());
1273+
if (updateBasicBlock == null) {
1274+
updateBasicBlock = searchUpdateBlockAndCreateContinueLoop(visited, basicBlock, basicBlock.getSub2());
1275+
}
12501276
break;
12511277
case TYPE_SWITCH:
1252-
createContinueLoop(visited, basicBlock, basicBlock.getNext());
1278+
updateBasicBlock = searchUpdateBlockAndCreateContinueLoop(visited, basicBlock, basicBlock.getNext());
12531279
case TYPE_SWITCH_DECLARATION:
12541280
for (SwitchCase switchCase : basicBlock.getSwitchCases()) {
1255-
createContinueLoop(visited, basicBlock, switchCase.getBasicBlock());
1281+
if (updateBasicBlock == null) {
1282+
updateBasicBlock = searchUpdateBlockAndCreateContinueLoop(visited, basicBlock, switchCase.getBasicBlock());
1283+
}
12561284
}
12571285
break;
12581286
}
12591287
}
1288+
1289+
return updateBasicBlock;
12601290
}
12611291

1262-
protected static void createContinueLoop(BitSet visited, BasicBlock basicBlock, BasicBlock subBasicBlock) {
1292+
protected static BasicBlock searchUpdateBlockAndCreateContinueLoop(BitSet visited, BasicBlock basicBlock, BasicBlock subBasicBlock) {
12631293
if (subBasicBlock != null) {
1264-
if ((subBasicBlock.getPredecessors().size() > 1) && (basicBlock.getFromOffset() < subBasicBlock.getFromOffset())) {
1265-
boolean condition;
1294+
if (basicBlock.getFromOffset() < subBasicBlock.getFromOffset()) {
12661295

12671296
if (basicBlock.getFirstLineNumber() == Expression.UNKNOWN_LINE_NUMBER) {
1268-
condition = subBasicBlock.matchType(GROUP_SINGLE_SUCCESSOR) && (subBasicBlock.getNext().getType() == TYPE_LOOP_START);
1269-
} else {
1270-
condition = (basicBlock.getFirstLineNumber() > subBasicBlock.getFirstLineNumber());
1271-
}
1272-
1273-
if (condition) {
1274-
Set<BasicBlock> predecessors = subBasicBlock.getPredecessors();
1275-
Iterator<BasicBlock> iterator = predecessors.iterator();
1276-
BasicBlock lastPredecessor = iterator.next();
1297+
if (subBasicBlock.matchType(GROUP_SINGLE_SUCCESSOR) && (subBasicBlock.getNext().getType() == TYPE_LOOP_START)) {
1298+
int stackDepth = ByteCodeParser.evalStackDepth(subBasicBlock);
12771299

1278-
if (lastPredecessor.getType() != TYPE_GOTO_IN_TERNARY_OPERATOR) {
1279-
while (iterator.hasNext()) {
1280-
BasicBlock predecessor = iterator.next();
1281-
if (predecessor.getType() == TYPE_GOTO_IN_TERNARY_OPERATOR) {
1282-
lastPredecessor = null;
1300+
while (stackDepth != 0) {
1301+
Set<BasicBlock> predecessors = basicBlock.getPredecessors();
1302+
if (predecessors.size() != 1) {
12831303
break;
12841304
}
1285-
if (lastPredecessor.getFromOffset() < predecessor.getFromOffset()) {
1286-
lastPredecessor = predecessor;
1287-
}
1305+
stackDepth += ByteCodeParser.evalStackDepth(subBasicBlock = predecessors.iterator().next());
12881306
}
12891307

1290-
if (lastPredecessor != null) {
1291-
iterator = predecessors.iterator();
1292-
1293-
while (iterator.hasNext()) {
1294-
BasicBlock predecessor = iterator.next();
1295-
if (predecessor != lastPredecessor) {
1296-
iterator.remove();
1297-
predecessor.replace(subBasicBlock, LOOP_CONTINUE);
1298-
}
1299-
}
1300-
}
1308+
removePredecessors(subBasicBlock);
1309+
return subBasicBlock;
13011310
}
1311+
} else if (basicBlock.getFirstLineNumber() > subBasicBlock.getFirstLineNumber()) {
1312+
removePredecessors(subBasicBlock);
1313+
return subBasicBlock;
13021314
}
13031315
}
13041316

1305-
createContinueLoop(visited, subBasicBlock);
1317+
return searchUpdateBlockAndCreateContinueLoop(visited, subBasicBlock);
13061318
}
1319+
1320+
return null;
1321+
}
1322+
1323+
protected static void removePredecessors(BasicBlock basicBlock) {
1324+
Set<BasicBlock> predecessors = basicBlock.getPredecessors();
1325+
Iterator<BasicBlock> iterator = predecessors.iterator();
1326+
1327+
while (iterator.hasNext()) {
1328+
iterator.next().replace(basicBlock, LOOP_CONTINUE);
1329+
}
1330+
1331+
predecessors.clear();
13071332
}
13081333

13091334
protected static void changeEndLoopToJump(BitSet visited, BasicBlock target, BasicBlock basicBlock) {

‎src/main/java/org/jd/core/v1/service/converter/classfiletojavasyntax/util/LoopStatementMaker.java‎

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,7 @@ protected static Statement makeLoop(LocalVariableMaker localVariableMaker, Basic
5555
return statement;
5656
}
5757

58-
int lineNumber = condition.getLineNumber();
58+
int lineNumber = (condition == null) ? Expression.UNKNOWN_LINE_NUMBER : condition.getLineNumber();
5959
int subStatementsSize = subStatements.size();
6060

6161
switch (subStatementsSize) {
@@ -279,8 +279,9 @@ protected static Expressions<Expression> extractUpdate(Statements statements, in
279279
update.add(expression);
280280
}
281281

282-
if (update.size() > 1)
282+
if (update.size() > 1) {
283283
Collections.reverse(update);
284+
}
284285

285286
return update;
286287
}
@@ -337,6 +338,10 @@ protected static Statement createForStatementWithoutLineNumber(BasicBlock basicB
337338
}
338339

339340
protected static Statement makeForEachArray(LocalVariableMaker localVariableMaker, Statements<Statement> statements, Expression condition, Statements<Statement> subStatements) {
341+
if (condition == null) {
342+
return null;
343+
}
344+
340345
int statementsSize = statements.size();
341346

342347
if ((statementsSize < 3) || (subStatements.size() < 2)) {
@@ -502,6 +507,10 @@ protected static Statement makeForEachArray(LocalVariableMaker localVariableMake
502507
}
503508

504509
protected static Statement makeForEachList(LocalVariableMaker localVariableMaker, Statements<Statement> statements, Expression condition, Statements<Statement> subStatements) {
510+
if (condition == null) {
511+
return null;
512+
}
513+
505514
if ((statements.size() < 1) || (subStatements.size() < 1)) {
506515
return null;
507516
}

‎src/main/java/org/jd/core/v1/service/converter/classfiletojavasyntax/util/StatementMaker.java‎

Lines changed: 22 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -33,10 +33,7 @@
3333
import org.jd.core.v1.util.DefaultList;
3434
import org.jd.core.v1.util.DefaultStack;
3535

36-
import java.util.BitSet;
37-
import java.util.Comparator;
38-
import java.util.Iterator;
39-
import java.util.List;
36+
import java.util.*;
4037

4138
import static org.jd.core.v1.service.converter.classfiletojavasyntax.model.cfg.BasicBlock.*;
4239

@@ -263,6 +260,16 @@ protected void makeStatements(WatchDog watchdog, BasicBlock basicBlock, Statemen
263260
}
264261
}
265262

263+
protected Statements makeSubStatements(WatchDog watchdog, BasicBlock basicBlock, Statements<Statement> statements, Statements jumps, Statements<Statement> updateStatements) {
264+
Statements<Statement> subStatements = makeSubStatements(watchdog, basicBlock, statements, jumps);
265+
266+
if (updateStatements != null) {
267+
subStatements.addAll(updateStatements);
268+
}
269+
270+
return subStatements;
271+
}
272+
266273
protected Statements makeSubStatements(WatchDog watchdog, BasicBlock basicBlock, Statements<Statement> statements, Statements jumps) {
267274
Statements<Statement> subStatements = new Statements<>();
268275

@@ -598,14 +605,20 @@ protected void parseIf(WatchDog watchdog, BasicBlock basicBlock, Statements stat
598605
@SuppressWarnings("unchecked")
599606
protected void parseLoop(WatchDog watchdog, BasicBlock basicBlock, Statements statements, Statements jumps) {
600607
BasicBlock sub1 = basicBlock.getSub1();
608+
Statements<Statement> updateStatements = null;
609+
610+
if ((sub1.getType() == TYPE_IF) && (sub1.getCondition() == END)) {
611+
updateStatements = makeSubStatements(watchdog, sub1.getNext(), statements, jumps);
612+
sub1 = sub1.getSub1();
613+
}
601614

602615
if (sub1.getType() == TYPE_IF) {
603616
BasicBlock ifBB = sub1;
604617

605618
if (ifBB.getNext() == LOOP_END) {
606619
// 'while' or 'for' loop
607620
makeStatements(watchdog, ifBB.getCondition(), statements, jumps);
608-
statements.add(LoopStatementMaker.makeLoop(localVariableMaker, basicBlock, statements, stack.pop(), makeSubStatements(watchdog, ifBB.getSub1(), statements, jumps), jumps));
621+
statements.add(LoopStatementMaker.makeLoop(localVariableMaker, basicBlock, statements, stack.pop(), makeSubStatements(watchdog, ifBB.getSub1(), statements, jumps, updateStatements), jumps));
609622
makeStatements(watchdog, basicBlock.getNext(), statements, jumps);
610623
return;
611624
}
@@ -624,7 +637,7 @@ protected void parseLoop(WatchDog watchdog, BasicBlock basicBlock, Statements st
624637
// 'while' or 'for' loop
625638
ifBB.getCondition().inverseCondition();
626639
makeStatements(watchdog, ifBB.getCondition(), statements, jumps);
627-
statements.add(LoopStatementMaker.makeLoop(localVariableMaker, basicBlock, statements, stack.pop(), makeSubStatements(watchdog, ifBB.getNext(), statements, jumps), jumps));
640+
statements.add(LoopStatementMaker.makeLoop(localVariableMaker, basicBlock, statements, stack.pop(), makeSubStatements(watchdog, ifBB.getNext(), statements, jumps, updateStatements), jumps));
628641
}
629642

630643
makeStatements(watchdog, basicBlock.getNext(), statements, jumps);
@@ -649,21 +662,21 @@ protected void parseLoop(WatchDog watchdog, BasicBlock basicBlock, Statements st
649662

650663
if ((sub1.getType() == TYPE_LOOP) && (sub1.getNext() == last) && (countStartLoop(sub1.getSub1()) == 0)) {
651664
changeEndLoopToStartLoop(new BitSet(), sub1.getSub1());
652-
subStatements = makeSubStatements(watchdog, sub1.getSub1(), statements, jumps);
665+
subStatements = makeSubStatements(watchdog, sub1.getSub1(), statements, jumps, updateStatements);
653666

654667
assert subStatements.getLast() == ContinueStatement.CONTINUE;
655668

656669
subStatements.removeLast();
657670
} else {
658671
createDoWhileContinue(last);
659-
subStatements = makeSubStatements(watchdog, sub1, statements, jumps);
672+
subStatements = makeSubStatements(watchdog, sub1, statements, jumps, updateStatements);
660673
}
661674

662675
makeStatements(watchdog, last.getCondition(), subStatements, jumps);
663676
statements.add(LoopStatementMaker.makeDoWhileLoop(basicBlock, last, stack.pop(), subStatements, jumps));
664677
} else {
665678
// Infinite loop
666-
statements.add(LoopStatementMaker.makeLoop(basicBlock, statements, makeSubStatements(watchdog, sub1, statements, jumps), jumps));
679+
statements.add(LoopStatementMaker.makeLoop(basicBlock, statements, makeSubStatements(watchdog, sub1, statements, jumps, updateStatements), jumps));
667680
}
668681

669682
makeStatements(watchdog, basicBlock.getNext(), statements, jumps);

‎src/test/java/org/jd/core/v1/ControlFlowGraphTest.java‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -768,6 +768,11 @@ public void testJdk170ForMultipleVariables2() throws Exception {
768768
checkCFGReduction(searchMethod(getResource("zip/data-java-jdk-1.7.0.zip"), "org/jd/core/test/For", "forMultipleVariables2"));
769769
}
770770

771+
@Test
772+
public void testJdk170ForBreak() throws Exception {
773+
checkCFGReduction(searchMethod(getResource("zip/data-java-jdk-1.7.0.zip"), "org/jd/core/test/For", "forBreak"));
774+
}
775+
771776

772777
// --- Test 'break' and 'continue' ------------------------------------------------------------------------------ //
773778
@Test

‎src/test/resources/java/org/jd/core/test/For.java‎

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -490,4 +490,25 @@ public void forIterator(Map map) {
490490
}
491491
}
492492
}
493+
494+
public void forBreak(Object[] array) {
495+
System.out.println("start");
496+
497+
for (int i=0; i<array.length; i++) {
498+
Object o = array[i];
499+
500+
if (o == null) {
501+
System.out.println("array[" + i + "] = null");
502+
if (i > 0) {
503+
array[i] = "null";
504+
continue;
505+
}
506+
}
507+
508+
System.out.println("array[" + i + "] = " + o);
509+
break;
510+
}
511+
512+
System.out.println("end");
513+
}
493514
}

‎src/test/resources/zip/data-java-jdk-1.7.0-no-debug-info.zip‎

206 Bytes
Binary file not shown.

‎src/test/resources/zip/data-java-jdk-1.7.0.zip‎

283 Bytes
Binary file not shown.

0 commit comments

Comments
 (0)