Rule 15.4: Improve query

 * Accurately determine which loops each `break` and `goto` statement
   breaks out of.
    - `break` breaks out of the nearest enclosing breakable, which
       could be a switch.
    - `goto` depends on the target, and which breakables are shared
      between both the goto and target.
 * Add a placeholder location for the break or goto (so that it can be
   more easily verified).
 * Include more tests.

Author: lcartey

c/misra/src/rules/RULE-15-4/LoopIterationCondition.ql

@@ -15,15 +15,41 @@
 import cpp
 import codingstandards.c.misra
 
-from Loop loop
+/**
+ * A breaking statement.
+ */
+class BreakOrGotoStmt extends JumpStmt {
+  BreakOrGotoStmt() {
+    this instanceof BreakStmt or
+    this instanceof GotoStmt
+  }
+
+  /**
+   * Gets a loop this breaks out of, if any.
+   *
+   * - This can produce no results if this is a `break` and the enclosing breakable is a switch statement.
+   * - This can produce no result if this is a `goto`, and the target is within the same nearest enclosing loop.
+   * - This can produce multiple results if this is a `goto`, and the target is outside multiple enclosing loops.
+   */
+  Loop getABrokenLoop() {
+    result = this.(BreakStmt).getBreakable()
+    or
+    exists(GotoStmt goto |
+      goto = this and
+      // Find any loop that encloses this goto
+      result.getChildStmt*() = goto and
+      // But does not enclose the target of the goto i.e. the goto breaks out of it
+      not result.getChildStmt*() = goto.getTarget()
+    )
+  }
+}
+
+from Loop loop, BreakOrGotoStmt breakOrGoto
 where
   not isExcluded(loop, Statements2Package::loopIterationConditionQuery()) and
-  count(Stmt terminationStmt |
-    loop.getChildStmt*() = terminationStmt and
-    (
-      terminationStmt instanceof BreakStmt
-      or
-      terminationStmt instanceof GotoStmt
-    )
-  ) > 1
-select loop, "$@ statement contains more than one break or goto statement", loop, "Iteration"
+  // More than one break or goto statement in the loop
+  count(BreakOrGotoStmt terminationStmt | terminationStmt.getABrokenLoop() = loop) > 1 and
+  // Report a break or goto statement
+  breakOrGoto.getABrokenLoop() = loop
+select loop, "Iteration statement contains more than one $@.", breakOrGoto,
+  "break or goto statement"

c/misra/test/rules/RULE-15-4/LoopIterationCondition.expected

@@ -1,2 +1,7 @@
-| test.c:24:3:32:3 | for(...;...;...) ... | $@ statement contains more than one break or goto statement | test.c:24:3:32:3 | for(...;...;...) ... | Iteration |
-| test.c:38:3:45:3 | while (...) ... | $@ statement contains more than one break or goto statement | test.c:38:3:45:3 | while (...) ... | Iteration |
+| test.c:24:3:32:3 | for(...;...;...) ... | Iteration statement contains more than one $@. | test.c:26:7:26:12 | break; | break or goto statement |
+| test.c:24:3:32:3 | for(...;...;...) ... | Iteration statement contains more than one $@. | test.c:29:7:29:12 | break; | break or goto statement |
+| test.c:24:3:32:3 | for(...;...;...) ... | Iteration statement contains more than one $@. | test.c:31:5:31:12 | goto ... | break or goto statement |
+| test.c:38:3:45:3 | while (...) ... | Iteration statement contains more than one $@. | test.c:40:7:40:12 | break; | break or goto statement |
+| test.c:38:3:45:3 | while (...) ... | Iteration statement contains more than one $@. | test.c:43:7:43:14 | goto ... | break or goto statement |
+| test.c:61:3:72:3 | while (...) ... | Iteration statement contains more than one $@. | test.c:64:7:64:12 | break; | break or goto statement |
+| test.c:61:3:72:3 | while (...) ... | Iteration statement contains more than one $@. | test.c:68:7:68:14 | goto ... | break or goto statement |

c/misra/test/rules/RULE-15-4/test.c

@@ -43,4 +43,49 @@ L1:;
       goto L1;
     }
   }
+
+  while (k < 10) { // COMPLIANT - the nested goto
+                   // only applies to the nested loop
+    if (k > 5) {
+      break;
+    }
+    while (k < 3) { // COMPLIANT
+      break;
+    }
+  }
+}
+
+void f3(int k) {
+L3:
+  k++;
+  while (k < 10) { // NON_COMPLIANT - the nested goto
+                   // only applies to the switch
+    if (k > 5) {
+      break;
+    }
+    switch (k) {
+    case 1:
+      goto L3;
+    case 2:
+      break;
+    }
+  }
+}
+
+void f4(int k) {
+  k++;
+  while (k < 10) { // COMPLIANT
+    if (k > 5) {
+      break;
+    }
+    switch (k) {
+    case 1:
+      goto L4;
+    case 2:
+      k += 1;
+    L4:
+      k += 2;
+      break;
+    }
+  }
 }