Fixed #5462 (non-void function does not return a value)

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 <cmath> functions like atan(0, x);
 //---------------------------------------------------------------------------

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"

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)