diff --git a/cfg/cppcheck-cfg.rng b/cfg/cppcheck-cfg.rng index 884951554..7af29dfdf 100644 --- a/cfg/cppcheck-cfg.rng +++ b/cfg/cppcheck-cfg.rng @@ -133,7 +133,7 @@ - + all|([0-9]*:[0-9]*) diff --git a/cfg/std.cfg b/cfg/std.cfg index 2962a0e62..529dbb056 100644 --- a/cfg/std.cfg +++ b/cfg/std.cfg @@ -3054,7 +3054,7 @@ The obsolete function 'gets' is called. With 'gets' you'll get a buffer overrun - + false diff --git a/cli/cmdlineparser.cpp b/cli/cmdlineparser.cpp index c0634cb38..6ebf60a0c 100644 --- a/cli/cmdlineparser.cpp +++ b/cli/cmdlineparser.cpp @@ -186,11 +186,6 @@ bool CmdLineParser::parseFromArgs(int argc, const char* const argv[]) else if (std::strcmp(argv[i], "--all-functions-are-safe") == 0) mSettings->allFunctionsAreSafe = true; - // Experimental: this flag says that all "safe output values" from functions should be checked. - // todo: add proper configuration options - else if (std::strcmp(argv[i], "--check-all-safe-return-values") == 0) - mSettings->checkAllSafeFunctionReturnValues = true; - // Enforce language (--language=, -x) else if (std::strncmp(argv[i], "--language=", 11) == 0 || std::strcmp(argv[i], "-x") == 0) { std::string str; diff --git a/lib/library.cpp b/lib/library.cpp index 9cb0bec72..a72952750 100644 --- a/lib/library.cpp +++ b/lib/library.cpp @@ -618,10 +618,11 @@ Library::Error Library::loadFunction(const tinyxml2::XMLElement * const node, co mReturnValueType[name] = type; if (const char *container = functionnode->Attribute("container")) mReturnValueContainer[name] = std::atoi(container); - if (const char *safeValues = functionnode->Attribute("safeValues")) { - SafeValues sf{LLONG_MIN, LLONG_MAX}; - if (std::strcmp(safeValues, "all") == 0) - mReturnSafeValues[name] = sf; + if (const char *unknownReturnValues = functionnode->Attribute("unknownValues")) { + if (std::strcmp(unknownReturnValues, "all") == 0) { + std::vector values{LLONG_MIN, LLONG_MAX}; + mUnknownReturnValues[name] = values; + } } } else if (functionnodename == "arg") { const char* argNrString = functionnode->Attribute("nr"); @@ -1200,17 +1201,12 @@ int Library::returnValueContainer(const Token *ftok) const return it != mReturnValueContainer.end() ? it->second : -1; } -bool Library::returnValueSafeValues(const Token *ftok, MathLib::bigint *v1, MathLib::bigint *v2) const +std::vector Library::unknownReturnValues(const Token *ftok) const { if (isNotLibraryFunction(ftok)) - return false; - const std::map::const_iterator it = mReturnSafeValues.find(getFunctionName(ftok)); - if (it == mReturnSafeValues.end()) - return false; - *v1 = it->second.minValue; - *v2 = it->second.maxValue; - return true; - + return std::vector(); + const std::map>::const_iterator it = mUnknownReturnValues.find(getFunctionName(ftok)); + return (it == mUnknownReturnValues.end()) ? std::vector() : it->second; } bool Library::hasminsize(const Token *ftok) const diff --git a/lib/library.h b/lib/library.h index d1e094be9..8f7d94d4a 100644 --- a/lib/library.h +++ b/lib/library.h @@ -181,7 +181,7 @@ public: const std::string& returnValue(const Token *ftok) const; const std::string& returnValueType(const Token *ftok) const; int returnValueContainer(const Token *ftok) const; - bool returnValueSafeValues(const Token *ftok, MathLib::bigint *v1, MathLib::bigint *v2) const; + std::vector unknownReturnValues(const Token *ftok) const; bool isnoreturn(const Token *ftok) const; bool isnotnoreturn(const Token *ftok) const; @@ -542,11 +542,7 @@ private: std::map mReturnValue; std::map mReturnValueType; std::map mReturnValueContainer; - struct SafeValues { - MathLib::bigint minValue; - MathLib::bigint maxValue; - }; - std::map mReturnSafeValues; + std::map> mUnknownReturnValues; std::map mReportErrors; std::map mProcessAfterCode; std::set mMarkupExtensions; // file extensions of markup files diff --git a/lib/settings.cpp b/lib/settings.cpp index 46657eadc..1e72d9c87 100644 --- a/lib/settings.cpp +++ b/lib/settings.cpp @@ -40,7 +40,6 @@ Settings::Settings() force(false), inconclusive(false), allFunctionsAreSafe(false), - checkAllSafeFunctionReturnValues(false), inlineSuppressions(false), jobs(1), jointSuppressionReport(false), diff --git a/lib/settings.h b/lib/settings.h index 902a11dc8..c3952dc84 100644 --- a/lib/settings.h +++ b/lib/settings.h @@ -149,8 +149,8 @@ public: /** @brief Experimental flag that says all functions are "safe" */ bool allFunctionsAreSafe; - /** @brief Experimental flag to safety check function output */ - bool checkAllSafeFunctionReturnValues; + /** @brief check unknown function return values */ + std::set checkUnknownFunctionReturn; /** @brief Is --inline-suppr given? */ bool inlineSuppressions; diff --git a/lib/valueflow.cpp b/lib/valueflow.cpp index 2c6480992..6e26af491 100644 --- a/lib/valueflow.cpp +++ b/lib/valueflow.cpp @@ -5442,17 +5442,19 @@ static void valueFlowSafeFunctions(TokenList *tokenlist, SymbolDatabase *symbold } } -static void valueFlowSafeFunctionReturn(TokenList *tokenlist, const Settings *settings) +static void valueFlowUnknownFunctionReturn(TokenList *tokenlist, const Settings *settings) { - if (!settings->checkAllSafeFunctionReturnValues) + if (settings->checkUnknownFunctionReturn.empty()) return; for (Token *tok = tokenlist->front(); tok; tok = tok->next()) { if (!tok->astParent() || tok->str() != "(") continue; if (!Token::Match(tok->previous(), "%name%")) continue; - MathLib::bigint v1,v2; - if (!settings->library.returnValueSafeValues(tok->previous(), &v1, &v2)) + if (settings->checkUnknownFunctionReturn.find(tok->previous()->str()) == settings->checkUnknownFunctionReturn.end()) + continue; + std::vector unknownValues = settings->library.unknownReturnValues(tok->astOperand1()); + if (unknownValues.empty()) continue; // Get min/max values for return type @@ -5461,16 +5463,13 @@ static void valueFlowSafeFunctionReturn(TokenList *tokenlist, const Settings *se if (!getMinMaxValues(typestr, settings, &minvalue, &maxvalue)) continue; - auto value = [](MathLib::bigint v, MathLib::bigint minvalue, MathLib::bigint maxvalue) { - if (v < minvalue) - return ValueFlow::Value(minvalue); - if (v > maxvalue) - return ValueFlow::Value(maxvalue); - return ValueFlow::Value(v); - }; - - setTokenValue(const_cast(tok), value(v1, minvalue, maxvalue), settings); - setTokenValue(const_cast(tok), value(v2, minvalue, maxvalue), settings); + for (MathLib::bigint value : unknownValues) { + if (value < minvalue) + value = minvalue; + else if (value > maxvalue) + value = maxvalue; + setTokenValue(const_cast(tok), ValueFlow::Value(value), settings); + } } } @@ -5540,7 +5539,7 @@ void ValueFlow::setValues(TokenList *tokenlist, SymbolDatabase* symboldatabase, valueFlowNumber(tokenlist); valueFlowString(tokenlist); valueFlowArray(tokenlist); - valueFlowSafeFunctionReturn(tokenlist, settings); + valueFlowUnknownFunctionReturn(tokenlist, settings); valueFlowGlobalConstVar(tokenlist, settings); valueFlowGlobalStaticVar(tokenlist, settings); valueFlowPointerAlias(tokenlist); diff --git a/test/testvalueflow.cpp b/test/testvalueflow.cpp index a99cd77e4..307b3e164 100644 --- a/test/testvalueflow.cpp +++ b/test/testvalueflow.cpp @@ -120,7 +120,8 @@ private: TEST_CASE(valueFlowDynamicBufferSize); - TEST_CASE(valueFlowSafeFunctions); + TEST_CASE(valueFlowAllFunctionParameterValues); + TEST_CASE(valueFlowUnknownFunctionReturn); } static bool isNotTokValue(const ValueFlow::Value &val) { @@ -3905,7 +3906,7 @@ private: ASSERT_EQUALS(true, testValueOfX(code, 4U, 100, ValueFlow::Value::BUFFER_SIZE)); } - void valueFlowSafeFunctions() { + void valueFlowAllFunctionParameterValues() { const char *code; std::list values; Settings s; @@ -3921,11 +3922,12 @@ private: } - void valueFlowSafeFunctionReturn() { + void valueFlowUnknownFunctionReturn() { const char *code; std::list values; Settings s; - s.checkAllSafeFunctionReturnValues = true; + LOAD_LIB_2(s.library, "std.cfg"); + s.checkUnknownFunctionReturn.insert("rand"); code = "x = rand();"; values = tokenValues(code, "(", &s);