diff --git a/lib/checkbool.cpp b/lib/checkbool.cpp index 72f521fd2..3bf3ca017 100644 --- a/lib/checkbool.cpp +++ b/lib/checkbool.cpp @@ -27,6 +27,7 @@ #include "symboldatabase.h" #include "token.h" #include "tokenize.h" +#include "valueflow.h" #include #include @@ -454,3 +455,31 @@ void CheckBool::assignBoolToFloatError(const Token *tok) reportError(tok, Severity::style, "assignBoolToFloat", "Boolean value assigned to floating point variable.", CWE704, false); } + +void CheckBool::returnValueOfFunctionReturningBool(void) +{ + if (!mSettings->isEnabled(Settings::STYLE)) + return; + + const SymbolDatabase * const symbolDatabase = mTokenizer->getSymbolDatabase(); + + for (const Scope * scope : symbolDatabase->functionScopes) { + if (!(scope->function && Token::Match(scope->function->retDef, "bool|_Bool"))) + continue; + + for (const Token* tok = scope->bodyStart->next(); tok && (tok != scope->bodyEnd); tok = tok->next()) { + // Skip lambdas + const Token* tok2 = findLambdaEndToken(tok); + if (tok2) + tok = tok2; + else if (Token::simpleMatch(tok, "return") && tok->astOperand1() && + (tok->astOperand1()->getValueGE(2, mSettings) || tok->astOperand1()->getValueLE(-1, mSettings))) + returnValueBoolError(tok); + } + } +} + +void CheckBool::returnValueBoolError(const Token *tok) +{ + reportError(tok, Severity::style, "returnNonBoolInBooleanFunction", "Non-boolean value returned from function returning bool"); +} diff --git a/lib/checkbool.h b/lib/checkbool.h index b147e88f4..d8263fddf 100644 --- a/lib/checkbool.h +++ b/lib/checkbool.h @@ -58,6 +58,7 @@ public: checkBool.checkComparisonOfBoolWithInt(); checkBool.checkAssignBoolToFloat(); checkBool.pointerArithBool(); + checkBool.returnValueOfFunctionReturningBool(); } /** @brief Run checks against the simplified token list */ @@ -100,6 +101,9 @@ public: void pointerArithBool(); void pointerArithBoolCond(const Token *tok); + /** @brief %Check if a function returning bool returns an integer other than 0 or 1 */ + void returnValueOfFunctionReturningBool(); + private: // Error messages.. void comparisonOfFuncReturningBoolError(const Token *tok, const std::string &expression); @@ -112,6 +116,7 @@ private: void bitwiseOnBooleanError(const Token *tok, const std::string &varname, const std::string &op); void comparisonOfBoolExpressionWithIntError(const Token *tok, bool n0o1); void pointerArithBoolError(const Token *tok); + void returnValueBoolError(const Token *tok); void getErrorMessages(ErrorLogger *errorLogger, const Settings *settings) const override { CheckBool c(nullptr, settings, errorLogger); @@ -126,6 +131,7 @@ private: c.comparisonOfBoolExpressionWithIntError(nullptr, true); c.pointerArithBoolError(nullptr); c.comparisonOfBoolWithInvalidComparator(nullptr, "expression"); + c.returnValueBoolError(nullptr); } static std::string myName() { @@ -140,7 +146,8 @@ private: "- comparison of a boolean value with boolean value using relational operator\n" "- using bool in bitwise expression\n" "- pointer addition in condition (either dereference is forgot or pointer overflow is required to make the condition false)\n" - "- Assigning bool value to pointer or float\n"; + "- Assigning bool value to pointer or float\n" + "- Returning an integer other than 0 or 1 from a function with boolean return value\n"; } }; /// @} diff --git a/test/testbool.cpp b/test/testbool.cpp index 35308ca03..1be8a2d46 100644 --- a/test/testbool.cpp +++ b/test/testbool.cpp @@ -64,6 +64,8 @@ private: // Converting pointer addition result to bool TEST_CASE(pointerArithBool1); + + TEST_CASE(returnNonBool); } void check(const char code[], bool experimental = false, const char filename[] = "test.cpp") { @@ -1008,6 +1010,103 @@ private: "}"); ASSERT_EQUALS("[test.cpp:2]: (error) Converting pointer arithmetic result to bool. The bool is always true unless there is undefined behaviour.\n", errout.str()); } + + void returnNonBool() { + check("bool f(void) {\n" + " return 0;\n" + "}"); + ASSERT_EQUALS("", errout.str()); + + check("bool f(void) {\n" + " return 1;\n" + "}"); + ASSERT_EQUALS("", errout.str()); + + check("bool f(void) {\n" + " return 2;\n" + "}"); + ASSERT_EQUALS("[test.cpp:2]: (style) Non-boolean value returned from function returning bool\n", errout.str()); + + check("bool f(void) {\n" + " return -1;\n" + "}"); + ASSERT_EQUALS("[test.cpp:2]: (style) Non-boolean value returned from function returning bool\n", errout.str()); + + check("bool f(void) {\n" + " return 1 + 1;\n" + "}"); + ASSERT_EQUALS("[test.cpp:2]: (style) Non-boolean value returned from function returning bool\n", errout.str()); + + check("bool f(void) {\n" + " int x = 0;\n" + " return x;\n" + "}"); + ASSERT_EQUALS("", errout.str()); + + check("bool f(void) {\n" + " int x = 10;\n" + " return x;\n" + "}"); + ASSERT_EQUALS("[test.cpp:3]: (style) Non-boolean value returned from function returning bool\n", errout.str()); + + check("bool f(void) {\n" + " return 2 < 1;\n" + "}"); + ASSERT_EQUALS("", errout.str()); + + check("bool f(void) {\n" + " int ret = 0;\n" + " if (a)\n" + " ret = 1;\n" + " return ret;\n" + "}"); + ASSERT_EQUALS("", errout.str()); + + check("bool f(void) {\n" + " int ret = 0;\n" + " if (a)\n" + " ret = 3;\n" + " return ret;\n" + "}"); + ASSERT_EQUALS("[test.cpp:5]: (style) Non-boolean value returned from function returning bool\n", errout.str()); + + check("bool f(void) {\n" + " if (a)\n" + " return 3;\n" + " return 4;\n" + "}"); + ASSERT_EQUALS("[test.cpp:3]: (style) Non-boolean value returned from function returning bool\n" + "[test.cpp:4]: (style) Non-boolean value returned from function returning bool\n", errout.str()); + + check("bool f(void) {\n" + " return;\n" + "}"); + ASSERT_EQUALS("", errout.str()); + + check("bool f(void) {\n" + "auto x = [](void) { return -1; };\n" + "return false;\n" + "}\n"); + ASSERT_EQUALS("", errout.str()); + + check("bool f(void) {\n" + "auto x = [](void) { return -1; };\n" + "return 2;\n" + "}\n"); + ASSERT_EQUALS("[test.cpp:3]: (style) Non-boolean value returned from function returning bool\n", errout.str()); + + check("bool f(void) {\n" + "auto x = [](void) -> int { return -1; };\n" + "return false;\n" + "}\n"); + ASSERT_EQUALS("", errout.str()); + + check("bool f(void) {\n" + "auto x = [](void) -> int { return -1; };\n" + "return 2;\n" + "}\n"); + ASSERT_EQUALS("[test.cpp:3]: (style) Non-boolean value returned from function returning bool\n", errout.str()); + } }; REGISTER_TEST(TestBool)