Fixed #2378 (Refactoring: create utility function that skips redundant if/for/while)

lib/checkbufferoverrun.cpp

@@ -222,6 +222,9 @@ static bool bailoutIfSwitch(const Token *tok, const unsigned int varid)
         else if (str1 == "if" && tok->str() == "break")
             return true;
 
+        // bailout for "return"
+        else if (tok->str() == "return")
+            return true;
 
         // bailout if varid is found
         else if (tok->varId() == varid)
@@ -636,8 +639,24 @@ void CheckBufferOverrun::checkFunctionCall(const Token &tok, unsigned int par, c
             // Check the parameter usage in the function scope..
             for (; ftok; ftok = ftok->next())
             {
-                if (ftok->str() == "if")
+                if (Token::Match(ftok, "if|for|while ("))
-                    break;
+                {
+                    // bailout if there is buffer usage..
+                    if (bailoutIfSwitch(ftok, parameterVarId))
+                    {
+                        break;
+                    }
+
+                    // no bailout is needed. skip the if-block
+                    else
+                    {
+                        // goto end of if block..
+                        ftok = ftok->next()->link()->next()->link();
+                        if (Token::simpleMatch(ftok, "} else {"))
+                            ftok = ftok->tokAt(2)->link();
+                        continue;
+                    }
+                }
 
                 if (ftok->str() == "}")
                     break;

test/testbufferoverrun.cpp

@@ -131,6 +131,7 @@ private:
         TEST_CASE(buffer_overrun_14);
         TEST_CASE(buffer_overrun_15); // ticket #1787
         TEST_CASE(buffer_overrun_16);
+        TEST_CASE(buffer_overrun_bailoutIfSwitch);  // ticket #2378 : bailoutIfSwitch
 
         // It is undefined behaviour to point out of bounds of an array
         // the address beyond the last element is in bounds
@@ -1546,7 +1547,7 @@ private:
               "  char s[3];\n"
               "  f1(s,3);\n"
               "}\n");
-        //ASSERT_EQUALS("[test.cpp:3]: (possible error) Buffer access out-of-bounds\n", errout.str());
+        TODO_ASSERT_EQUALS("[test.cpp:8] -> [test.cpp:3]: (error) Buffer access out-of-bounds\n", errout.str());
         ASSERT_EQUALS("", errout.str());
 
         check("void f1(char *s,int size)\n"
@@ -1834,6 +1835,44 @@ private:
         ASSERT_EQUALS("", errout.str());
     }
 
+    void buffer_overrun_bailoutIfSwitch()
+    {
+        // No false positive
+        check("void f1(char *s) {\n"
+              "    if (x) s[100] = 0;\n"
+              "}\n"
+              "\n"
+              "void f2() {\n"
+              "    char a[10];\n"
+              "    f1(a);"
+              "}");
+        ASSERT_EQUALS("", errout.str());
+
+        // No false positive
+        check("void f1(char *s) {\n"
+              "    if (x) return;\n"
+              "    s[100] = 0;\n"
+              "}\n"
+              "\n"
+              "void f2() {\n"
+              "    char a[10];\n"
+              "    f1(a);"
+              "}");
+        ASSERT_EQUALS("", errout.str());
+
+        // No false negative
+        check("void f1(char *s) {\n"
+              "    if (x) { }\n"
+              "    s[100] = 0;\n"
+              "}\n"
+              "\n"
+              "void f2() {\n"
+              "    char a[10];\n"
+              "    f1(a);"
+              "}");
+        ASSERT_EQUALS("[test.cpp:8] -> [test.cpp:3]: (error) Array 'a[10]' index 100 out of bounds\n", errout.str());
+    }
+
     void pointer_out_of_bounds_1()
     {
         check("void f() {\n"