SONARJAVA-1538 - CFG SE : nested statements in try catch end up with wrong CFG

Author: DidierBesset

its/ruling/src/test/resources/jdk6/squid-S2259.json

@@ -52,6 +52,7 @@
 361,
 ],
 'jdk6:java/net/SocksSocketImpl.java':[
+398,
 714,
 ],
 'jdk6:java/net/URI.java':[
@@ -64,10 +65,6 @@
 'jdk6:java/security/CodeSource.java':[
 531,
 ],
-'jdk6:java/security/Signature.java':[
-976,
-1015,
-],
 'jdk6:java/security/UnresolvedPermission.java':[
 558,
 ],

its/ruling/src/test/resources/jdk6/squid-S2583.json

@@ -30,10 +30,6 @@
 'jdk6:java/math/BigDecimal.java':[
 3660,
 ],
-'jdk6:java/net/ServerSocket.java':[
-470,
-475,
-],
 'jdk6:java/nio/ByteBuffer.java':[
 1104,
 1131,

java-frontend/src/main/java/org/sonar/java/cfg/CFG.java

@@ -64,6 +64,7 @@
 
 import javax.annotation.CheckForNull;
 import javax.annotation.Nullable;
+
 import java.util.ArrayList;
 import java.util.Deque;
 import java.util.LinkedHashSet;
@@ -254,6 +255,10 @@ private void prune() {
         block.id = id;
         id += 1;
       }
+      inactiveBlocks.removeAll(blocks);
+      if (!inactiveBlocks.isEmpty()) {
+        prune();
+      }
     }
   }
 
@@ -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);
       }
     }
@@ -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());

java-frontend/src/test/files/se/ConditionAlwaysTrueOrFalseCheck.java

@@ -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();
+      }
+    }
+  }
+}
+

java-frontend/src/test/java/org/sonar/java/cfg/CFGTest.java

@@ -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);
+  }
+
 }