misra_14_2 various fixes (#4324)

addons/misra.py

@@ -711,6 +711,7 @@ def getForLoopCounterVariables(forToken):
     if not tn or tn.str != '(':
         return None
     vars_defined = set()
+    vars_initialized = set()
     vars_exit = set()
     vars_modified = set()
     cur_clause = 1
@@ -722,14 +723,16 @@ def getForLoopCounterVariables(forToken):
             elif cur_clause == 2:
                 vars_exit.add(tn.variable)
             elif cur_clause == 3:
-                if tn.next and hasSideEffectsRecursive(tn.next):
+                if tn.next and countSideEffectsRecursive(tn.next) > 0:
                     vars_modified.add(tn.variable)
                 elif tn.previous and tn.previous.str in ('++', '--'):
                     vars_modified.add(tn.variable)
+        if cur_clause == 1 and tn.isAssignmentOp and tn.astOperand1.variable:
+            vars_initialized.add(tn.astOperand1.variable)
         if tn.str == ';':
             cur_clause += 1
         tn = tn.next
-    return vars_defined & vars_exit & vars_modified
+    return vars_defined | vars_initialized, vars_exit & vars_modified
 
 
 def findCounterTokens(cond):
@@ -779,23 +782,23 @@ def isFloatCounterInWhileLoop(whileToken):
     return False
 
 
-def hasSideEffectsRecursive(expr):
+def countSideEffectsRecursive(expr):
     if not expr or expr.str == ';':
-        return False
+        return 0
     if expr.str == '=' and expr.astOperand1 and expr.astOperand1.str == '[':
         prev = expr.astOperand1.previous
         if prev and (prev.str == '{' or prev.str == '{'):
-            return hasSideEffectsRecursive(expr.astOperand2)
+            return countSideEffectsRecursive(expr.astOperand2)
     if expr.str == '=' and expr.astOperand1 and expr.astOperand1.str == '.':
         e = expr.astOperand1
         while e and e.str == '.' and e.astOperand2:
             e = e.astOperand1
         if e and e.str == '.':
-            return False
+            return 0
     if expr.isAssignmentOp or expr.str in {'++', '--'}:
-        return True
+        return 1
     # Todo: Check function calls
-    return hasSideEffectsRecursive(expr.astOperand1) or hasSideEffectsRecursive(expr.astOperand2)
+    return countSideEffectsRecursive(expr.astOperand1) + countSideEffectsRecursive(expr.astOperand2)
 
 
 def isBoolExpression(expr):
@@ -2694,12 +2697,12 @@ class MisraChecker:
 
     def misra_13_5(self, data):
         for token in data.tokenlist:
-            if token.isLogicalOp and hasSideEffectsRecursive(token.astOperand2):
+            if token.isLogicalOp and countSideEffectsRecursive(token.astOperand2) > 0:
                 self.reportError(token, 13, 5)
 
     def misra_13_6(self, data):
         for token in data.tokenlist:
-            if token.str == 'sizeof' and hasSideEffectsRecursive(token.next):
+            if token.str == 'sizeof' and countSideEffectsRecursive(token.next) > 0:
                 self.reportError(token, 13, 6)
 
     def misra_14_1(self, data):
@@ -2717,32 +2720,43 @@ class MisraChecker:
 
     def misra_14_2(self, data):
         for token in data.tokenlist:
+            if token.str == 'for':
                 expressions = getForLoopExpressions(token)
                 if not expressions:
                     continue
                 if expressions[0] and not expressions[0].isAssignmentOp:
                     self.reportError(token, 14, 2)
-            elif hasSideEffectsRecursive(expressions[1]):
+                if countSideEffectsRecursive(expressions[1]) > 0:
+                    self.reportError(token, 14, 2)
+                if countSideEffectsRecursive(expressions[2]) > 1:
+                    self.reportError(token, 14, 2)
+
+                counter_vars_first_clause, counter_vars_exit_modified = getForLoopCounterVariables(token)
+                if len(counter_vars_exit_modified) == 0:
+                    # if it's not possible to identify a loop counter, all 3 clauses must be empty
+                    for idx in range(len(expressions)):
+                        if expressions[idx]:
+                            self.reportError(token, 14, 2)
+                            break
+                elif len(counter_vars_exit_modified) > 1:
+                    # there shall be a single loop counter
+                    self.reportError(token, 14, 2)
+                else: # len(counter_vars_exit_modified) == 1:
+                    loop_counter = counter_vars_exit_modified.pop()
+                    # if the first clause is not empty, then it shall (declare and) initialize the loop counter
+                    if expressions[0] is not None and loop_counter not in counter_vars_first_clause:
                         self.reportError(token, 14, 2)
 
                     # Inspect modification of loop counter in loop body
-            counter_vars = getForLoopCounterVariables(token)
+                    body_scope = token.next.link.next.scope
-            outer_scope = token.scope
-            body_scope = None
-            tn = token.next
-            while tn and tn.next != outer_scope.bodyEnd:
-                if tn.scope and tn.scope.nestedIn == outer_scope:
-                    body_scope = tn.scope
-                    break
-                tn = tn.next
                     if not body_scope:
                         continue
                     tn = body_scope.bodyStart
                     while tn and tn != body_scope.bodyEnd:
-                if tn.variable and tn.variable in counter_vars:
+                        if tn.variable == loop_counter:
                             if tn.next:
                                 # TODO: Check modifications in function calls
-                        if hasSideEffectsRecursive(tn.next):
+                                if countSideEffectsRecursive(tn.next) > 0:
                                     self.reportError(tn, 14, 2)
                         tn = tn.next
 

addons/test/misra/misra-test.c

@@ -894,7 +894,8 @@ void misra_12_3(int a, int b, int c) {
 
   f((1,2),3); // TODO
 
-  for (i=0; i<10; i++, j++){} // 12.3
+  // third clause: 2 persistent side effects instead of 1 (14.2)
+  for (i=0; i<10; i++, j++){} // 12.3 14.2
   for (int i = 0, p = &a1;  // 12.3 14.2
           i < 42;
           ++i, ++p ) // 12.3
@@ -1157,7 +1158,7 @@ static void misra_14_2_init_value(int32_t *var) {
 }
 static void misra_14_2_fn1(bool b) {
   for (;i++<10;) {} // 14.2
-  for (;i<10;dostuff()) {} // TODO
+  for (;i<10;dostuff()) {} // 14.2
   int32_t g = 0;
   int g_arr[42];
   g += 2; // no-warning
@@ -1175,7 +1176,12 @@ static void misra_14_2_fn1(bool b) {
   for (misra_14_2_init_value(&i); i < 10; ++i) {} // no-warning FIXME: False positive for 14.2 Trac #9491
 
   bool abort = false;
-  for (i = 0; (i < 10) && !abort; ++i) { // no-warning
+  for (i = 0; (i < 10) && !abort; ++i) { // 14.2 as 'i' is not a variable
+      if (b) {
+        abort = true;
+      }
+  }
+  for (int i = 0; (i < 10) && !abort; ++i) { // no warning
       if (b) {
         abort = true;
       }
@@ -1186,6 +1192,18 @@ static void misra_14_2_fn1(bool b) {
   for (int i = x; i < 42; i++) {
       x++; // no warning
   }
+  // 1st clause item 2 + loop counter modification
+  for(x = 0; x < 10; x++) {
+    x++; // 14.2
+  }
+  // third clause: 2 persistent side effects instead of 1 (14.2)
+  for (int i = 0; i < 10; i++, x++) { // 12.3 14.2
+  }
+
+  // 2 loop counters, there shall be only 1
+  for(int i=0, j=0; (i<10) && (j<10); i++, j++) { // 12.3 14.2
+  }
+
   for (int i = (x - 3); i < 42; i++) {
       x ^= 3; // no warning
   }
@@ -1218,16 +1236,18 @@ static void misra_14_2_fn2(void)
     for (int i = 0, j = 19; y < 10, --j > 10; y++, j--) { // 14.2 12.3
         i++; // no warning
     }
-    for (int i = 0; y < 10; y++) { // TODO: 14.2
+    // 1st clause is not empty, but is not used in 2nd and 3rd clause
+    for (int i = 0; y < 10; y++) { // 14.2
         i++; // no warning
     }
-    for (int i = 0; i < 10; y++) { // TODO: 14.2
+    for (; y < 10; y++) {} // without 1st clause, no error
+    for (int i = 0; i < 10; y++) { // 14.2
         i++; // no warning
     }
-    for (int i = 0; y < 10; i++) { // TODO: 14.2
+    for (int i = 0; y < 10; i++) { // 14.2
         i++; // no warning
     }
-    for (int i = 0; i < 10; (y+=i)) {
+    for (int i = 0; i < 10; (y+=i)) { // 14.2
         i++; // no warning
     }
 
@@ -1323,7 +1343,7 @@ static void misra_15_4(void) {
       if (y==2) {
         break;
       }
-      for (z = 0; y < 42; ++z) {
+      for (z = 0; y < 42; ++z) { // 14.2
         if (z==1) {
           break;
         }