Added several new va_arg related checks:

- Wrong parameter passed to va_start() (#3850)
- Reference passed to va_start() (#3849)
- Missing va_end() (#3295)
- Using va_list before it is opened (#3295)
- Subsequent calls to va_start/va_copy()

lib/checkvaarg.cpp

@@ -0,0 +1,142 @@
+/*
+ * Cppcheck - A tool for static C/C++ code analysis
+ * Copyright (C) 2007-2014 Daniel Marjamäki and Cppcheck team.
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation, either version 3 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#include "checkvaarg.h"
+#include "symboldatabase.h"
+
+//---------------------------------------------------------------------------
+
+// Register this check class (by creating a static instance of it)
+namespace {
+    CheckVaarg instance;
+}
+
+
+//---------------------------------------------------------------------------
+// Ensure that correct parameter is passed to va_start()
+//---------------------------------------------------------------------------
+
+void CheckVaarg::va_start_argument()
+{
+    const SymbolDatabase* const symbolDatabase = _tokenizer->getSymbolDatabase();
+    const std::size_t functions = symbolDatabase->functionScopes.size();
+    for (std::size_t i = 0; i < functions; ++i) {
+        const Scope* scope = symbolDatabase->functionScopes[i];
+        const Function* function = scope->function;
+        for (const Token* tok = scope->classStart->next(); tok != scope->classEnd; tok = tok->next()) {
+            if (Token::simpleMatch(tok, "va_start (")) {
+                const Token* param2 = tok->tokAt(2)->nextArgument();
+                const Variable* var = param2->variable();
+                if (var && var->isReference())
+                    referenceAs_va_start_error(param2, var->name());
+                if (var->index() + 2 < function->argCount() && _settings->isEnabled("warning")) {
+                    std::list<Variable>::const_reverse_iterator it = function->argumentList.rbegin();
+                    it++;
+                    wrongParameterTo_va_start_error(tok, var->name(), it->name());
+                }
+                tok = tok->linkAt(1);
+            }
+        }
+    }
+}
+
+void CheckVaarg::wrongParameterTo_va_start_error(const Token *tok, const std::string& paramIsName, const std::string& paramShouldName)
+{
+    reportError(tok, Severity::warning,
+                "va_start_wrongParameter", "'" + paramIsName + "' given to va_start() is not last named argument of the function. Did you intend to pass '" + paramShouldName + "'?");
+}
+
+void CheckVaarg::referenceAs_va_start_error(const Token *tok, const std::string& paramName)
+{
+    reportError(tok, Severity::error,
+                "va_start_referencePassed", "Using reference '" + paramName + "' as parameter for va_start() results in undefined behaviour.");
+}
+
+//---------------------------------------------------------------------------
+// Detect missing va_end() if va_start() was used
+// Detect va_list usage after va_end()
+//---------------------------------------------------------------------------
+
+void CheckVaarg::va_list_usage()
+{
+    const SymbolDatabase* const symbolDatabase = _tokenizer->getSymbolDatabase();
+    for (unsigned int varid = 1; varid < symbolDatabase->getVariableListSize(); varid++) {
+        const Variable* var = symbolDatabase->getVariableFromVarId(varid);
+        if (!var || var->isPointer() || var->isReference() || var->isArray() || !var->scope() || var->typeStartToken()->str() != "va_list")
+            continue;
+        if (!var->isLocal() && !var->isArgument()) // Check only local variables and arguments
+            continue;
+
+        bool open = var->isArgument(); // va_list passed as argument are opened
+        bool exitOnEndOfStatement = false;
+
+        const Token* tok = var->nameToken()->next();
+        for (const Token* tok = var->nameToken()->next(); tok != var->scope()->classEnd; tok = tok->next()) {
+            if (Token::Match(tok, "va_start ( %varid% ,", var->declarationId())) {
+                if (open)
+                    va_start_subsequentCallsError(tok, var->name());
+                open = true;
+                tok = tok->linkAt(1);
+            } else if (Token::Match(tok, "va_end ( %varid% )", var->declarationId())) {
+                if (!open)
+                    va_list_usedBeforeStartedError(tok, var->name());
+                open = false;
+                tok = tok->linkAt(1);
+            } else if (Token::simpleMatch(tok, "va_copy (")) {
+                bool nopen = open;
+                if (tok->linkAt(1)->previous()->varId() == var->declarationId()) { // Source
+                    if (!open)
+                        va_list_usedBeforeStartedError(tok, var->name());
+                    nopen = false;
+                }
+                if (tok->tokAt(2)->varId() == var->declarationId()) { // Destination
+                    if (open)
+                        va_start_subsequentCallsError(tok, var->name());
+                    nopen = true;
+                }
+                open = nopen;
+                tok = tok->linkAt(1);
+            } else if (Token::Match(tok, "throw|return"))
+                exitOnEndOfStatement = true;
+            else if (!open && tok->varId() == var->declarationId())
+                va_list_usedBeforeStartedError(tok, var->name());
+            else if (exitOnEndOfStatement && tok->str() == ";")
+                break;
+        }
+        if (open && !var->isArgument())
+            va_end_missingError(tok, var->name());
+    }
+}
+
+void CheckVaarg::va_end_missingError(const Token *tok, const std::string& varname)
+{
+    reportError(tok, Severity::error,
+                "va_end_missing", "va_list '" + varname + "' was opened but not closed by va_end().");
+}
+
+void CheckVaarg::va_list_usedBeforeStartedError(const Token *tok, const std::string& varname)
+{
+    reportError(tok, Severity::error,
+                "va_list_usedBeforeStarted", "va_list '" + varname + "' used before va_start() was called.");
+}
+
+void CheckVaarg::va_start_subsequentCallsError(const Token *tok, const std::string& varname)
+{
+    reportError(tok, Severity::error,
+                "va_start_subsequentCalls", "va_start() or va_copy() called subsequently on '" + varname + "' without va_end() inbetween.");
+}

lib/checkvaarg.h

@@ -0,0 +1,86 @@
+/*
+ * Cppcheck - A tool for static C/C++ code analysis
+ * Copyright (C) 2007-2014 Daniel Marjamäki and Cppcheck team.
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation, either version 3 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+
+
+//---------------------------------------------------------------------------
+#ifndef checkvaargtH
+#define checkvaargtH
+//---------------------------------------------------------------------------
+
+#include "config.h"
+#include "check.h"
+
+/// @addtogroup Checks
+/// @{
+
+/**
+ * @brief Checking for misusage of variable argument lists
+ */
+
+class CPPCHECKLIB CheckVaarg : public Check {
+public:
+    CheckVaarg() : Check(myName()) {
+    }
+
+    CheckVaarg(const Tokenizer *tokenizer, const Settings *settings, ErrorLogger *errorLogger)
+        : Check(myName(), tokenizer, settings, errorLogger) {
+    }
+
+    virtual void runSimplifiedChecks(const Tokenizer *tokenizer, const Settings *settings, ErrorLogger *errorLogger) {
+        CheckVaarg check(tokenizer, settings, errorLogger);
+        check.va_start_argument();
+        check.va_list_usage();
+    }
+
+    void va_start_argument();
+    void va_list_usage();
+
+private:
+    void wrongParameterTo_va_start_error(const Token *tok, const std::string& paramIsName, const std::string& paramShouldName);
+    void referenceAs_va_start_error(const Token *tok, const std::string& paramName);
+    void va_end_missingError(const Token *tok, const std::string& varname);
+    void va_list_usedBeforeStartedError(const Token *tok, const std::string& varname);
+    void va_start_subsequentCallsError(const Token *tok, const std::string& varname);
+
+    void getErrorMessages(ErrorLogger *errorLogger, const Settings *settings) const {
+        CheckVaarg c(0, settings, errorLogger);
+        c.wrongParameterTo_va_start_error(0, "arg1", "arg2");
+        c.referenceAs_va_start_error(0, "arg1");
+        c.va_end_missingError(0, "vl");
+        c.va_list_usedBeforeStartedError(0, "vl");
+        c.va_start_subsequentCallsError(0, "vl");
+    }
+
+    static std::string myName() {
+        return "CheckVaarg";
+    }
+
+    std::string classInfo() const {
+        return "Check for misusage of variable argument lists:\n"
+               "* Wrong parameter passed to va_start()\n"
+               "* Reference passed to va_start()\n"
+               "* Missing va_end()\n"
+               "* Using va_list before it is opened\n"
+               "* Subsequent calls to va_start/va_copy()\n";
+    }
+};
+
+/// @}
+
+//---------------------------------------------------------------------------
+#endif // checkvaargtH

lib/cppcheck.vcxproj

@@ -60,6 +60,7 @@
     <ClCompile Include="checkuninitvar.cpp" />
     <ClCompile Include="checkunusedfunctions.cpp" />
     <ClCompile Include="checkunusedvar.cpp" />
+    <ClCompile Include="checkvaarg.cpp" />
     <ClCompile Include="cppcheck.cpp" />
     <ClCompile Include="errorlogger.cpp" />
     <ClCompile Include="executionpath.cpp" />
@@ -103,6 +104,7 @@
     <ClInclude Include="checkuninitvar.h" />
     <ClInclude Include="checkunusedfunctions.h" />
     <ClInclude Include="checkunusedvar.h" />
+    <ClInclude Include="checkvaarg.h" />
     <ClInclude Include="config.h" />
     <ClInclude Include="cppcheck.h" />
     <ClInclude Include="errorlogger.h" />

lib/cppcheck.vcxproj.filters

@@ -134,6 +134,9 @@
     <ClCompile Include="check.cpp">
       <Filter>Source Files</Filter>
     </ClCompile>
+    <ClCompile Include="checkvaarg.cpp">
+      <Filter>Source Files</Filter>
+    </ClCompile>
   </ItemGroup>
   <ItemGroup>
     <ClInclude Include="checkbufferoverrun.h">
@@ -265,6 +268,9 @@
     <ClInclude Include="valueflow.h">
       <Filter>Header Files</Filter>
     </ClInclude>
+    <ClInclude Include="checkvaarg.h">
+      <Filter>Header Files</Filter>
+    </ClInclude>
   </ItemGroup>
   <ItemGroup>
     <ResourceCompile Include="version.rc" />

test/testrunner.vcxproj

@@ -83,6 +83,7 @@
     <ClCompile Include="testunusedfunctions.cpp" />
     <ClCompile Include="testunusedprivfunc.cpp" />
     <ClCompile Include="testunusedvar.cpp" />
+    <ClCompile Include="testvaarg.cpp" />
     <ClCompile Include="testvalueflow.cpp" />
   </ItemGroup>
   <ItemGroup>

test/testrunner.vcxproj.filters

@@ -184,6 +184,9 @@
     <ClCompile Include="testsamples.cpp">
       <Filter>Source Files</Filter>
     </ClCompile>
+    <ClCompile Include="testvaarg.cpp">
+      <Filter>Source Files</Filter>
+    </ClCompile>
   </ItemGroup>
   <ItemGroup>
     <ClInclude Include="options.h">

test/testvaarg.cpp

@@ -0,0 +1,184 @@
+/*
+ * Cppcheck - A tool for static C/C++ code analysis
+ * Copyright (C) 2007-2014 Daniel Marjamäki and Cppcheck team.
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation, either version 3 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#include "testsuite.h"
+#include "checkvaarg.h"
+#include "tokenize.h"
+#include <sstream>
+
+extern std::ostringstream errout;
+
+class TestVaarg : public TestFixture {
+public:
+    TestVaarg() : TestFixture("TestVaarg") {}
+
+private:
+    void check(const char code[]) {
+        // Clear the error buffer..
+        errout.str("");
+
+        Settings settings;
+        settings.addEnabled("warning");
+
+        // Tokenize..
+        Tokenizer tokenizer(&settings, this);
+        std::istringstream istr(code);
+        tokenizer.tokenize(istr, "test.cpp");
+
+        // Check..
+        CheckVaarg checkVaarg(&tokenizer, &settings, this);
+        checkVaarg.runSimplifiedChecks(&tokenizer, &settings, this);
+    }
+
+    void run() {
+        TEST_CASE(wrongParameterTo_va_start);
+        TEST_CASE(referenceAs_va_start);
+        TEST_CASE(va_end_missing);
+        TEST_CASE(va_list_usedBeforeStarted);
+        TEST_CASE(va_start_subsequentCalls);
+    }
+
+    void wrongParameterTo_va_start() {
+        check("void Format(char* szFormat, char* szBuffer, size_t nSize, ...) {\n"
+              "    va_list arg_ptr;\n"
+              "    va_start(arg_ptr, szFormat);\n"
+              "    va_end(arg_ptr);\n"
+              "}");
+        ASSERT_EQUALS("[test.cpp:3]: (warning) 'szFormat' given to va_start() is not last named argument of the function. Did you intend to pass 'nSize'?\n", errout.str());
+
+        check("void Format(char* szFormat, char* szBuffer, size_t nSize, ...) {\n"
+              "    va_list arg_ptr;\n"
+              "    va_start(arg_ptr, szBuffer);\n"
+              "    va_end(arg_ptr);\n"
+              "}");
+        ASSERT_EQUALS("[test.cpp:3]: (warning) 'szBuffer' given to va_start() is not last named argument of the function. Did you intend to pass 'nSize'?\n", errout.str());
+
+        check("void Format(char* szFormat, char* szBuffer, size_t nSize, ...) {\n"
+              "    va_list arg_ptr;\n"
+              "    va_start(arg_ptr, nSize);\n"
+              "    va_end(arg_ptr);\n"
+              "}");
+        ASSERT_EQUALS("", errout.str());
+    }
+
+    void referenceAs_va_start() {
+        check("void Format(char* szFormat, char (&szBuffer)[_Size], ...) {\n"
+              "    va_list arg_ptr;\n"
+              "    va_start(arg_ptr, szBuffer);\n"
+              "    va_end(arg_ptr);\n"
+              "}");
+        ASSERT_EQUALS("[test.cpp:3]: (error) Using reference 'szBuffer' as parameter for va_start() results in undefined behaviour.\n", errout.str());
+
+        check("void Format(char* szFormat, char (*szBuffer)[_Size], ...) {\n"
+              "    va_list arg_ptr;\n"
+              "    va_start(arg_ptr, szBuffer);\n"
+              "    va_end(arg_ptr);\n"
+              "}");
+        ASSERT_EQUALS("", errout.str());
+    }
+
+    void va_end_missing() {
+        check("void Format(char* szFormat, char (*szBuffer)[_Size], ...) {\n"
+              "    va_list arg_ptr;\n"
+              "    va_start(arg_ptr, szBuffer);\n"
+              "    va_end(arg_ptr);\n"
+              "}");
+        ASSERT_EQUALS("", errout.str());
+
+        check("void Format(char* szFormat, char (*szBuffer)[_Size], ...) {\n"
+              "    va_list arg_ptr;\n"
+              "    va_start(arg_ptr, szBuffer);\n"
+              "}");
+        ASSERT_EQUALS("[test.cpp:2]: (error) va_list 'arg_ptr' was opened but not closed by va_end().\n", errout.str());
+
+        check("void Format(char* szFormat, char (*szBuffer)[_Size], ...) {\n"
+              "    va_list arg_ptr;\n"
+              "    va_start(arg_ptr, szBuffer);\n"
+              "    if(sth) return;\n"
+              "    va_end(arg_ptr);\n"
+              "}");
+        ASSERT_EQUALS("[test.cpp:2]: (error) va_list 'arg_ptr' was opened but not closed by va_end().\n", errout.str());
+    }
+
+    void va_list_usedBeforeStarted() {
+        check("void Format(char* szFormat, char (*szBuffer)[_Size], ...) {\n"
+              "    va_list arg_ptr;\n"
+              "    return va_arg(arg_ptr, float);\n"
+              "}");
+        ASSERT_EQUALS("[test.cpp:3]: (error) va_list 'arg_ptr' used before va_start() was called.\n", errout.str());
+
+        check("void Format(char* szFormat, char (*szBuffer)[_Size], ...) {\n"
+              "    va_list arg_ptr;\n"
+              "    foo(arg_ptr);\n"
+              "}");
+        ASSERT_EQUALS("[test.cpp:3]: (error) va_list 'arg_ptr' used before va_start() was called.\n", errout.str());
+
+        check("void Format(char* szFormat, char (*szBuffer)[_Size], ...) {\n"
+              "    va_list arg_ptr;\n"
+              "    va_copy(f, arg_ptr);\n"
+              "}");
+        ASSERT_EQUALS("[test.cpp:3]: (error) va_list 'arg_ptr' used before va_start() was called.\n", errout.str());
+
+        check("void Format(char* szFormat, char (*szBuffer)[_Size], ...) {\n"
+              "    va_list arg_ptr;\n"
+              "    va_start(arg_ptr, szBuffer);\n"
+              "    va_end(arg_ptr);\n"
+              "    return va_arg(arg_ptr, float);\n"
+              "}");
+        ASSERT_EQUALS("[test.cpp:5]: (error) va_list 'arg_ptr' used before va_start() was called.\n", errout.str());
+
+        check("void Format(char* szFormat, char (*szBuffer)[_Size], ...) {\n"
+              "    va_list arg_ptr;\n"
+              "    va_start(arg_ptr, szBuffer);\n"
+              "    va_end(arg_ptr);\n"
+              "    va_start(arg_ptr, szBuffer);\n"
+              "    foo(va_arg(arg_ptr, float));\n"
+              "    va_end(arg_ptr);\n"
+              "}");
+        ASSERT_EQUALS("", errout.str());
+    }
+
+    void va_start_subsequentCalls() {
+        check("void Format(char* szFormat, char (*szBuffer)[_Size], ...) {\n"
+              "    va_list arg_ptr;\n"
+              "    va_start(arg_ptr, szBuffer);\n"
+              "    va_start(arg_ptr, szBuffer);\n"
+              "    va_end(arg_ptr);\n"
+              "}");
+        ASSERT_EQUALS("[test.cpp:4]: (error) va_start() or va_copy() called subsequently on 'arg_ptr' without va_end() inbetween.\n", errout.str());
+
+        check("void Format(char* szFormat, char (*szBuffer)[_Size], ...) {\n"
+              "    va_list vl1;\n"
+              "    va_start(vl1, szBuffer);\n"
+              "    va_copy(vl1, vl1);\n"
+              "    va_end(vl1);\n"
+              "}");
+        ASSERT_EQUALS("[test.cpp:4]: (error) va_start() or va_copy() called subsequently on 'vl1' without va_end() inbetween.\n", errout.str());
+
+        check("void Format(char* szFormat, char (*szBuffer)[_Size], ...) {\n"
+              "    va_list arg_ptr;\n"
+              "    va_start(arg_ptr, szBuffer);\n"
+              "    va_end(arg_ptr);\n"
+              "    va_start(arg_ptr, szBuffer);\n"
+              "    va_end(arg_ptr);\n"
+              "}");
+        ASSERT_EQUALS("", errout.str());
+    }
+};
+
+REGISTER_TEST(TestVaarg)