Check for functions calls in sizeof calculations (#1111)

* Check for functions calls in sizeof calculations

* Use seperate message and id for sizeofFunction

* Check for overloads

* Using decltype with a function should not be an error

* Fix warning

* Fix false positives when running pass the close paren

* Fix test error

* Try to fix more false positives

* Traverse using astOperand2

* Only check first argument

* Update fixes from feedback from PR

lib/checksizeof.cpp

@@ -317,6 +317,46 @@ void CheckSizeof::sizeofCalculationError(const Token *tok, bool inconclusive)
                 "sizeofCalculation", "Found calculation inside sizeof().", CWE682, inconclusive);
 }
 
+//-----------------------------------------------------------------------------
+
+void CheckSizeof::sizeofFunction()
+{
+    if (!_settings->isEnabled(Settings::WARNING))
+        return;
+
+    for (const Token *tok = _tokenizer->tokens(); tok; tok = tok->next()) {
+        if (Token::simpleMatch(tok, "sizeof (")) {
+
+            // ignore if the `sizeof` result is cast to void inside a macro, i.e. the calculation is
+            // expected to be parsed but skipped, such as in a disabled custom ASSERT() macro
+            if (tok->isExpandedMacro() && tok->previous()) {
+                const Token *cast_end = (tok->previous()->str() == "(") ? tok->previous() : tok;
+                if (Token::simpleMatch(cast_end->tokAt(-3), "( void )") ||
+                    Token::simpleMatch(cast_end->previous(), "static_cast<void>")) {
+                    continue;
+                }
+            }
+
+            if (const Token *argument = tok->next()->astOperand2()) {
+                const Token *checkToken = argument->previous();
+                if (checkToken->tokType() == Token::eName)
+                    break;
+                const Function * fun = checkToken->function();
+                // Dont report error if the function is overloaded
+                if (fun && fun->nestedIn->functionMap.count(checkToken->str()) == 1) {
+                    sizeofFunctionError(tok);
+                }
+            }
+        }
+    }
+}
+
+void CheckSizeof::sizeofFunctionError(const Token *tok)
+{
+    reportError(tok, Severity::warning,
+                "sizeofFunctionCall", "Found function call inside sizeof().", CWE682, false);
+}
+
 //-----------------------------------------------------------------------------
 // Check for code like sizeof()*sizeof() or sizeof(ptr)/value
 //-----------------------------------------------------------------------------

lib/checksizeof.h

@@ -56,6 +56,7 @@ public:
         // Checks
         checkSizeof.sizeofsizeof();
         checkSizeof.sizeofCalculation();
+        checkSizeof.sizeofFunction();
         checkSizeof.suspiciousSizeofCalculation();
         checkSizeof.checkSizeofForArrayParameter();
         checkSizeof.checkSizeofForPointerSize();
@@ -73,6 +74,9 @@ public:
     /** @brief %Check for calculations inside sizeof */
     void sizeofCalculation();
 
+    /** @brief %Check for function call inside sizeof */
+    void sizeofFunction();
+
     /** @brief %Check for suspicious calculations with sizeof results */
     void suspiciousSizeofCalculation();
 
@@ -92,6 +96,7 @@ private:
     // Error messages..
     void sizeofsizeofError(const Token* tok);
     void sizeofCalculationError(const Token* tok, bool inconclusive);
+    void sizeofFunctionError(const Token* tok);
     void multiplySizeofError(const Token* tok);
     void divideSizeofError(const Token* tok);
     void sizeofForArrayParameterError(const Token* tok);
@@ -111,6 +116,7 @@ private:
         c.sizeofForNumericParameterError(nullptr);
         c.sizeofsizeofError(nullptr);
         c.sizeofCalculationError(nullptr, false);
+        c.sizeofFunctionError(nullptr);
         c.multiplySizeofError(nullptr);
         c.divideSizeofError(nullptr);
         c.sizeofVoidError(nullptr);
@@ -129,6 +135,7 @@ private:
                "- using sizeof(pointer) instead of the size of pointed data\n"
                "- look for 'sizeof sizeof ..'\n"
                "- look for calculations inside sizeof()\n"
+               "- look for function calls inside sizeof()\n"
                "- look for suspicious calculations with sizeof()\n"
                "- using 'sizeof(void)' which is undefined\n";
     }

test/testsizeof.cpp

@@ -40,6 +40,7 @@ private:
 
         TEST_CASE(sizeofsizeof);
         TEST_CASE(sizeofCalculation);
+        TEST_CASE(sizeofFunction);
         TEST_CASE(checkPointerSizeof);
         TEST_CASE(checkPointerSizeofStruct);
         TEST_CASE(sizeofDivisionMemset);
@@ -162,6 +163,67 @@ private:
                       "[test.cpp:5]: (warning, inconclusive) Found calculation inside sizeof().\n", errout.str());
     }
 
+    void sizeofFunction() {
+        check("class Foo\n"
+              "{\n"
+              "    int bar() { return 1; };\n"
+              "}\n"
+              "Foo f;int a=sizeof(f.bar());");
+        ASSERT_EQUALS("[test.cpp:5]: (warning) Found function call inside sizeof().\n", errout.str());
+
+        check("class Foo\n"
+              "{\n"
+              "    int bar() { return 1; };\n"
+              "    int bar() const { return 1; };\n"
+              "}\n"
+              "Foo f;int a=sizeof(f.bar());");
+        ASSERT_EQUALS("", errout.str());
+
+        check("class Foo\n"
+              "{\n"
+              "    int bar() { return 1; };\n"
+              "}\n"
+              "Foo * fp;int a=sizeof(fp->bar());");
+        ASSERT_EQUALS("[test.cpp:5]: (warning) Found function call inside sizeof().\n", errout.str());
+
+        check("int a=sizeof(foo());");
+        ASSERT_EQUALS("", errout.str());
+
+        check("int foo() { return 1; }; int a=sizeof(foo());");
+        ASSERT_EQUALS("[test.cpp:1]: (warning) Found function call inside sizeof().\n", errout.str());
+
+        check("int foo() { return 1; }; sizeof(decltype(foo()));");
+        ASSERT_EQUALS("", errout.str());
+
+        check("int foo(int) { return 1; }; int a=sizeof(foo(0))");
+        ASSERT_EQUALS("[test.cpp:1]: (warning) Found function call inside sizeof().\n", errout.str());
+
+        check("char * buf; int a=sizeof(*buf);");
+        ASSERT_EQUALS("", errout.str());
+
+        check("int a=sizeof(foo())");
+        ASSERT_EQUALS("", errout.str());
+        
+        check("int foo(int) { return 1; }; char buf[1024]; int a=sizeof(buf), foo(0)");
+        ASSERT_EQUALS("", errout.str());
+
+        check("template<class T>\n"
+              "struct A\n"
+              "{\n"
+              "    static B f(const B &);\n"
+              "    static A f(const A &);\n"
+              "    static A &g();\n"
+              "    static T &h();\n"
+              "\n"
+              "    enum {\n"
+              "        X = sizeof(f(g() >> h())) == sizeof(A),\n"
+              "        Y = sizeof(f(g() << h())) == sizeof(A),\n"
+              "        Z = X & Y\n"
+              "    };\n"
+              "};\n");
+        ASSERT_EQUALS("", errout.str());
+    }
+
     void sizeofForArrayParameter() {
         check("void f() {\n"
               "    int a[10];\n"