danmar#6981 crash in checkvaarg.cpp (with possible fix). Avoid segfault. Add SymbolDatabase::validate() to allow validating smyboldatabase

amai2012 · amai2012 · commit ba0859e838ae · 2015-12-05T20:55:26.000+01:00
diff --git a/lib/checkvaarg.cpp b/lib/checkvaarg.cpp
@@ -40,6 +40,8 @@ void CheckVaarg::va_start_argument()
     for (std::size_t i = 0; i < functions; ++i) {
         const Scope* scope = symbolDatabase->functionScopes[i];
         const Function* function = scope->function;
+        if (!function)
+            continue;
         for (const Token* tok = scope->classStart->next(); tok != scope->classEnd; tok = tok->next()) {
             if (!tok->scope()->isExecutable())
                 tok = tok->scope()->classEnd;
diff --git a/lib/symboldatabase.cpp b/lib/symboldatabase.cpp
@@ -1425,6 +1425,25 @@ bool SymbolDatabase::isFunction(const Token *tok, const Scope* outerScope, const
     return false;
 }
 
+void SymbolDatabase::validate() const
+{
+    const std::size_t functions = functionScopes.size();
+    for (std::size_t i = 0; i < functions; ++i) {
+        const Scope* scope = functionScopes[i];
+        if (scope->isExecutable()) {
+            const Function* function = scope->function;
+            if (!function) {
+                cppcheckError(nullptr);
+            }
+        }
+    }
+}
+
+void SymbolDatabase::cppcheckError(const Token *tok) const
+{
+    throw InternalError(tok, "Analysis failed. If the code is valid then please report this failure.", InternalError::INTERNAL);
+}
+
 bool Variable::isPointerArray() const
 {
     return isArray() && nameToken() && nameToken()->previous() && (nameToken()->previous()->str() == "*");
diff --git a/lib/symboldatabase.h b/lib/symboldatabase.h
@@ -1007,6 +1007,11 @@ class CPPCHECKLIB SymbolDatabase {
 
     bool isCPP() const;
 
+    /*
+     * @brief Do a sanity check
+     */
+    void validate() const;
+
     /** Set valuetype in provided tokenlist */
     static void setValueTypeInTokenList(Token *tokens);
 
@@ -1022,6 +1027,11 @@ class CPPCHECKLIB SymbolDatabase {
     const Type *findTypeInNested(const Token *tok, const Scope *startScope) const;
     const Scope *findNamespace(const Token * tok, const Scope * scope) const;
     Function *findFunctionInScope(const Token *func, const Scope *ns);
+    /**
+     * Send error message to error logger about internal bug.
+     * @param tok the token that this bug concerns.
+     */
+    void cppcheckError(const Token *tok) const __attribute__((noreturn));
 
     /** Whether iName is a keyword as defined in http://en.cppreference.com/w/c/keyword and http://en.cppreference.com/w/cpp/keyword*/
     bool isReservedName(const std::string& iName) const;
diff --git a/lib/tokenize.cpp b/lib/tokenize.cpp
@@ -9840,6 +9840,8 @@ void Tokenizer::createSymbolDatabase()
 {
     if (!_symbolDatabase)
         _symbolDatabase = new SymbolDatabase(this, _settings, _errorLogger);
+    if (_settings->debug)
+        _symbolDatabase->validate();
 }
 
 void Tokenizer::deleteSymbolDatabase()
diff --git a/lib/valueflow.cpp b/lib/valueflow.cpp
@@ -1982,7 +1982,7 @@ static void valueFlowForLoopSimplify(Token * const bodyStart, const unsigned int
                     break;
                 if (parent->str() == "?") {
                     if (parent->astOperand2() != p)
-                        parent = NULL;
+                        parent = nullptr;
                     break;
                 }
             }
diff --git a/test/testautovariables.cpp b/test/testautovariables.cpp
@@ -446,7 +446,7 @@ class TestAutoVariables : public TestFixture {
 
     void testautovar_return2() {
         check("class Fred {\n"
-              "    int* func1()\n"
+              "    int* func1();\n"
               "}\n"
               "int* Fred::func1()\n"
               "{\n"
@@ -876,7 +876,6 @@ class TestAutoVariables : public TestFixture {
               "    return foo();\n"
               "}");
         ASSERT_EQUALS("", errout.str());
-
         // Don't crash with function in unknown scope (#4076)
         check("X& a::Bar() {}"
               "X& foo() {"
diff --git a/test/testvaarg.cpp b/test/testvaarg.cpp
@@ -50,6 +50,7 @@ class TestVaarg : public TestFixture {
         TEST_CASE(va_end_missing);
         TEST_CASE(va_list_usedBeforeStarted);
         TEST_CASE(va_start_subsequentCalls);
+        TEST_CASE(unknownFunctionScope);
     }
 
     void wrongParameterTo_va_start() {
@@ -227,6 +228,15 @@ class TestVaarg : public TestFixture {
               "}");
         ASSERT_EQUALS("", errout.str());
     }
+
+    void unknownFunctionScope() {
+        check("void BG_TString::Format() {\n"
+              "  BG_TChar * f;\n"
+              "  va_start(args,f);\n"
+              "  BG_TString result(f);\n"
+              "}");
+        ASSERT_EQUALS("", errout.str());
+    }
 };
 
 REGISTER_TEST(TestVaarg)

Original file line number	Diff line number	Diff line change
`@@ -9840,6 +9840,8 @@ void Tokenizer::createSymbolDatabase()`
`9840`	`9840`	`{`
`9841`	`9841`	`if (!_symbolDatabase)`
`9842`	`9842`	`_symbolDatabase = new SymbolDatabase(this, _settings, _errorLogger);`
	`9843`	`+ if (_settings->debug)`
	`9844`	`+ _symbolDatabase->validate();`
`9843`	`9845`	`}`
`9844`	`9846`
`9845`	`9847`	`void Tokenizer::deleteSymbolDatabase()`
Original file line number	Diff line number	Diff line change
`@@ -1982,7 +1982,7 @@ static void valueFlowForLoopSimplify(Token * const bodyStart, const unsigned int`
`1982`	`1982`	`break;`
`1983`	`1983`	`if (parent->str() == "?") {`
`1984`	`1984`	`if (parent->astOperand2() != p)`
`1985`		`- parent = NULL;`
	`1985`	`+ parent = nullptr;`
`1986`	`1986`	`break;`
`1987`	`1987`	`}`
`1988`	`1988`	`}`