diff --git a/lib/checkfunctions.cpp b/lib/checkfunctions.cpp index 7a410f1b0..2e3e0dbfb 100644 --- a/lib/checkfunctions.cpp +++ b/lib/checkfunctions.cpp @@ -245,6 +245,76 @@ void CheckFunctions::ignoredReturnErrorCode(const Token* tok, const std::string& "$symbol:" + function + "\nError code from the return value of function $symbol() is not used.", CWE252, Certainty::normal); } +//--------------------------------------------------------------------------- +// Check for ignored return values. +//--------------------------------------------------------------------------- +static const Token *checkMissingReturnScope(const Token *tok); + +void CheckFunctions::checkMissingReturn() +{ + const SymbolDatabase *symbolDatabase = mTokenizer->getSymbolDatabase(); + for (const Scope *scope : symbolDatabase->functionScopes) { + const Function *function = scope->function; + if (!function || !function->hasBody()) + continue; + if (function->type != Function::Type::eFunction && function->type != Function::Type::eOperatorEqual) + continue; + if (Token::Match(function->retDef, "void %name%")) + continue; + const Token *errorToken = checkMissingReturnScope(scope->bodyEnd); + if (errorToken) + missingReturnError(errorToken); + } +} + +static const Token *checkMissingReturnScope(const Token *tok) +{ + tok = tok->previous(); + while (tok) { + if (tok->str() == "{") + return tok->next(); + if (tok->str() == "}") { + if (tok->scope()->type == Scope::ScopeType::eSwitch) { + // find break/default + bool hasDefault = false; + for (const Token *switchToken = tok->link(); switchToken != tok; switchToken = switchToken->next()) { + if (Token::simpleMatch(switchToken, "break ;")) + return switchToken; + if (Token::simpleMatch(switchToken, "default :")) + hasDefault = true; + else if (switchToken->str() == "{" && switchToken->scope()->isLoopScope()) + switchToken = switchToken->link(); + } + if (!hasDefault) + return tok->link(); + } else if (tok->scope()->type == Scope::ScopeType::eIf) { + return tok; + } else if (tok->scope()->type == Scope::ScopeType::eElse) { + const Token *errorToken = checkMissingReturnScope(tok); + if (errorToken) + return errorToken; + tok = tok->link(); + if (Token::simpleMatch(tok->tokAt(-2), "} else {")) + return checkMissingReturnScope(tok->tokAt(-2)); + return tok; + } + // FIXME + return nullptr; + } + if (tok->isKeyword() && Token::Match(tok, "return|throw")) + return nullptr; + if (Token::Match(tok, "; !!}")) + return tok->next(); + tok = tok->previous(); + } + return nullptr; +} + +void CheckFunctions::missingReturnError(const Token* tok) +{ + reportError(tok, Severity::error, "missingReturn", + "Found a exit path from function with non-void return type that has missing return statement", CWE758, Certainty::normal); +} //--------------------------------------------------------------------------- // Detect passing wrong values to functions like atan(0, x); //--------------------------------------------------------------------------- diff --git a/lib/checkfunctions.h b/lib/checkfunctions.h index f3b3766d6..fbba09696 100644 --- a/lib/checkfunctions.h +++ b/lib/checkfunctions.h @@ -63,8 +63,8 @@ public: void runChecks(const Tokenizer *tokenizer, const Settings *settings, ErrorLogger *errorLogger) OVERRIDE { CheckFunctions checkFunctions(tokenizer, settings, errorLogger); - // Checks checkFunctions.checkIgnoredReturnValue(); + checkFunctions.checkMissingReturn(); // Missing "return" in exit path // --check-library : functions with nonmatching configuration checkFunctions.checkLibraryMatchFunctions(); @@ -105,6 +105,9 @@ public: void checkLibraryMatchFunctions(); private: + /** @brief %Check for missing "return" */ + void checkMissingReturn(); + void invalidFunctionArgError(const Token *tok, const std::string &functionName, int argnr, const ValueFlow::Value *invalidValue, const std::string &validstr); void invalidFunctionArgBoolError(const Token *tok, const std::string &functionName, int argnr); void invalidFunctionArgStrError(const Token *tok, const std::string &functionName, nonneg int argnr); @@ -115,6 +118,7 @@ private: void memsetZeroBytesError(const Token *tok); void memsetFloatError(const Token *tok, const std::string &var_value); void memsetValueOutOfRangeError(const Token *tok, const std::string &value); + void missingReturnError(const Token *tok); void getErrorMessages(ErrorLogger *errorLogger, const Settings *settings) const OVERRIDE { CheckFunctions c(nullptr, settings, errorLogger); @@ -132,6 +136,7 @@ private: c.memsetZeroBytesError(nullptr); c.memsetFloatError(nullptr, "varname"); c.memsetValueOutOfRangeError(nullptr, "varname"); + c.missingReturnError(nullptr); } static std::string myName() { @@ -140,6 +145,7 @@ private: std::string classInfo() const OVERRIDE { return "Check function usage:\n" + "- missing 'return' in non-void function\n" "- return value of certain functions not used\n" "- invalid input values for functions\n" "- Warn if a function is called whose usage is discouraged\n" diff --git a/test/testfunctions.cpp b/test/testfunctions.cpp index 9f1a075eb..06b5f8a32 100644 --- a/test/testfunctions.cpp +++ b/test/testfunctions.cpp @@ -81,6 +81,9 @@ private: // memset.. TEST_CASE(memsetZeroBytes); TEST_CASE(memsetInvalid2ndParam); + + // missing "return" + TEST_CASE(checkMissingReturn); } void check(const char code[], const char filename[]="test.cpp", const Settings* settings_=nullptr) { @@ -1356,6 +1359,45 @@ private: "}"); ASSERT_EQUALS("[test.cpp:4]: (portability) The 2nd memset() argument '1.0f+i' is a float, its representation is implementation defined.\n", errout.str()); } + + void checkMissingReturn() { + check("int f() {}"); + ASSERT_EQUALS("[test.cpp:1]: (error) Found a exit path from function with non-void return type that has missing return statement\n", errout.str()); + + // switch + check("int f() {\n" + " switch (x) {\n" + " case 1: break;\n" // <- error + " case 2: return 1;\n" + " }\n" + "}"); + ASSERT_EQUALS("[test.cpp:3]: (error) Found a exit path from function with non-void return type that has missing return statement\n", errout.str()); + + // if/else + check("int f(int x) {\n" + " if (x) {\n" + " return 1;\n" + " }\n" // <- error (missing else) + "}"); + ASSERT_EQUALS("[test.cpp:4]: (error) Found a exit path from function with non-void return type that has missing return statement\n", errout.str()); + + check("int f(int x) {\n" + " if (x) {\n" + " ;\n" // <- error + " } else {\n" + " return 1;\n" + " }\n" + "}"); + ASSERT_EQUALS("[test.cpp:3]: (error) Found a exit path from function with non-void return type that has missing return statement\n", errout.str()); + + // loop + check("int f(int x) {\n" + " while (1) {\n" + " dostuff();\n" + " }\n" + "}"); + ASSERT_EQUALS("", errout.str()); + } }; REGISTER_TEST(TestFunctions)