Skip to content

Commit

Permalink
SONARJAVA-1538 - CFG SE : nested statements in try catch end up with …
Browse files Browse the repository at this point in the history 
…wrong CFG
  • Loading branch information
@DidierBesset @Wohops
DidierBesset authored and Wohops committed Mar 16, 2016
1 parent a5684b9 commit a32b023
Show file tree
Hide file tree
Showing 5 changed files with 139 additions and 18 deletions.
5 changes: 1 addition & 4 deletions its/ruling/src/test/resources/jdk6/squid-S2259.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@
361,
],
'jdk6:java/net/SocksSocketImpl.java':[
398,
714,
],
'jdk6:java/net/URI.java':[
Expand All @@ -64,10 +65,6 @@
'jdk6:java/security/CodeSource.java':[
531,
],
'jdk6:java/security/Signature.java':[
976,
1015,
],
'jdk6:java/security/UnresolvedPermission.java':[
558,
],
Expand Down
4 changes: 0 additions & 4 deletions its/ruling/src/test/resources/jdk6/squid-S2583.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,10 +30,6 @@
'jdk6:java/math/BigDecimal.java':[
3660,
],
'jdk6:java/net/ServerSocket.java':[
470,
475,
],
'jdk6:java/nio/ByteBuffer.java':[
1104,
1131,
Expand Down
27 changes: 17 additions & 10 deletions java-frontend/src/main/java/org/sonar/java/cfg/CFG.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,7 @@

import javax.annotation.CheckForNull;
import javax.annotation.Nullable;

import java.util.ArrayList;
import java.util.Deque;
import java.util.LinkedHashSet;
Expand Down Expand Up @@ -254,6 +255,10 @@ private void prune() {
block.id = id;
id += 1;
}
inactiveBlocks.removeAll(blocks);
if (!inactiveBlocks.isEmpty()) {
prune();
}
}
}

Expand Down Expand Up @@ -765,21 +770,13 @@ private void buildTryStatement(TryStatementTree tryStatementTree) {
}
currentBlock = beforeFinally;
build(tryStatementTree.block());
if(currentBlock.exitBlock != null && currentBlock.exitBlock.isFinallyBlock) {
for (Block catchBlock : catches) {
currentBlock.exitBlock.addSuccessor(catchBlock);
}
} else {
for (Block catchBlock : catches) {
currentBlock.addSuccessor(catchBlock);
}
}
linkToCatchBlocks(beforeFinally, catches);
build((List<? extends Tree>) tryStatementTree.resources());
currentBlock = createBlock(currentBlock);
currentBlock.elements.add(tryStatementTree);
if (finallyBlockTree != null) {
exitBlocks.pop();
if(catches.isEmpty()) {
if (catches.isEmpty()) {
currentBlock.addExitSuccessor(finallyOrEndBlock);
}
}
Expand All @@ -788,6 +785,16 @@ private void buildTryStatement(TryStatementTree tryStatementTree) {
}
}

protected void linkToCatchBlocks(Block block, List<Block> catches) {
Block blockForSuccessor = block;
if (block.exitBlock != null && block.exitBlock.isFinallyBlock) {
blockForSuccessor = block.exitBlock;
}
for (Block catchBlock : catches) {
blockForSuccessor.addSuccessor(catchBlock);
}
}

private void buildThrowStatement(ThrowStatementTree throwStatementTree) {
// FIXME this won't work if it is intended to be caught by a try statement.
currentBlock = createUnconditionalJump(throwStatementTree, exitBlock());
Expand Down
38 changes: 38 additions & 0 deletions java-frontend/src/test/files/se/ConditionAlwaysTrueOrFalseCheck.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1567,3 +1567,41 @@ void testAfterAddAssignment(int y) {
}
}
}

public class TryCatchCFG {

private Object monitor;
private boolean shutdown;

private void doSomething() {
}

void fun(boolean abort) {
while (!abort) {
try {
synchronized (monitor) {
long delay = 1000L;
while (!shutdown && delay > 0) {
long now = System.currentTimeMillis();
monitor.wait(delay);
delay -= (System.currentTimeMillis() - now);
}
if (shutdown) {
abort = true;
}
doSomething(); // may throw an exception
}

} catch (RuntimeException e) {
if (abort) {
System.out.println("Abort");
} else {
System.out.println("Retry");
}
} catch (InterruptedException e) {
Thread.currentThread().interrupt();
}
}
}
}

83 changes: 83 additions & 0 deletions java-frontend/src/test/java/org/sonar/java/cfg/CFGTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1302,4 +1302,87 @@ public void method_reference() throws Exception {
).successors(0));
cfgChecker.check(cfg);
}

@Test
public void try_statement_with_CFG_blocks() {
CFG cfg = buildCFG(
" private void f(boolean action) {\n" +
" try {\n" +
" if (action) {" +
" performAction();" +
" }" +
" doSomething();" +
"} catch(Exception e) { foo();} bar(); }");
CFGChecker cfgChecker = checker(
block(
element(Tree.Kind.TRY_STATEMENT)).successors(3, 5),
block(
element(Tree.Kind.IDENTIFIER, "action")).terminator(Kind.IF_STATEMENT).successors(2, 4),
block(
element(Tree.Kind.IDENTIFIER, "performAction"),
element(Kind.METHOD_INVOCATION)).successors(2),
block(
element(Tree.Kind.IDENTIFIER, "foo"),
element(Kind.METHOD_INVOCATION)).successors(1),
block(
element(Tree.Kind.IDENTIFIER, "doSomething"),
element(Kind.METHOD_INVOCATION)).successors(1, 3),
block(
element(Tree.Kind.IDENTIFIER, "bar"),
element(Kind.METHOD_INVOCATION)).successors(0));
cfgChecker.check(cfg);
cfg = buildCFG(
" private void f(boolean action) {\n" +
" try {\n" +
" doSomething();" +
" if (action) {" +
" performAction();" +
" }" +
"} catch(Exception e) { foo();} bar(); }");
cfgChecker = checker(
block(
element(Tree.Kind.TRY_STATEMENT)).successors(3, 5),
block(
element(Tree.Kind.IDENTIFIER, "doSomething"),
element(Kind.METHOD_INVOCATION),
element(Tree.Kind.IDENTIFIER, "action")).terminator(Kind.IF_STATEMENT).successors(2, 4),
block(
element(Tree.Kind.IDENTIFIER, "performAction"),
element(Kind.METHOD_INVOCATION)).successors(2),
block(
element(Tree.Kind.IDENTIFIER, "foo"),
element(Kind.METHOD_INVOCATION)).successors(1),
new BlockChecker(1, 3),
block(
element(Tree.Kind.IDENTIFIER, "bar"),
element(Kind.METHOD_INVOCATION)).successors(0));
cfgChecker.check(cfg);
cfg = buildCFG(
" private void f(boolean action) {\n" +
" try {\n" +
" if (action) {" +
" performAction();" +
" }" +
" doSomething();" +
"} finally { foo();} bar(); }");
cfgChecker = checker(
block(
element(Tree.Kind.TRY_STATEMENT)).successors(2, 5),
block(
element(Tree.Kind.IDENTIFIER, "action")).terminator(Kind.IF_STATEMENT).successors(3, 4),
block(
element(Tree.Kind.IDENTIFIER, "performAction"),
element(Kind.METHOD_INVOCATION)).successors(3),
block(
element(Tree.Kind.IDENTIFIER, "doSomething"),
element(Kind.METHOD_INVOCATION)).successors(2),
block(
element(Tree.Kind.IDENTIFIER, "foo"),
element(Kind.METHOD_INVOCATION)).successors(0, 1),
block(
element(Tree.Kind.IDENTIFIER, "bar"),
element(Kind.METHOD_INVOCATION)).successors(0));
cfgChecker.check(cfg);
}

}

0 comments on commit a32b023

Please sign in to comment.