From 05d35b063d8a828204512b88b4a9dc30ee504fd5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Marjam=C3=A4ki?= Date: Wed, 10 Jul 2019 20:00:21 +0200 Subject: [PATCH] Function return: Extra check of safe function return values --- cfg/cppcheck-cfg.rng | 7 +++++++ cfg/std.cfg | 2 +- cli/cmdlineparser.cpp | 5 +++++ lib/library.cpp | 18 +++++++++++++++++ lib/library.h | 6 ++++++ lib/settings.cpp | 1 + lib/settings.h | 3 +++ lib/valueflow.cpp | 45 ++++++++++++++++++++++++++++++++++++++++++ test/testvalueflow.cpp | 14 +++++++++++++ 9 files changed, 100 insertions(+), 1 deletion(-) diff --git a/cfg/cppcheck-cfg.rng b/cfg/cppcheck-cfg.rng index 6aaeaba8d..884951554 100644 --- a/cfg/cppcheck-cfg.rng +++ b/cfg/cppcheck-cfg.rng @@ -132,6 +132,13 @@ + + + + all|([0-9]*:[0-9]*) + + + diff --git a/cfg/std.cfg b/cfg/std.cfg index 84079a959..3f46d3e89 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 6ebf60a0c..c0634cb38 100644 --- a/cli/cmdlineparser.cpp +++ b/cli/cmdlineparser.cpp @@ -186,6 +186,11 @@ 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 393088179..d637d85f0 100644 --- a/lib/library.cpp +++ b/lib/library.cpp @@ -618,6 +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; + } } else if (functionnodename == "arg") { const char* argNrString = functionnode->Attribute("nr"); if (!argNrString) @@ -1195,6 +1200,19 @@ int Library::returnValueContainer(const Token *ftok) const return it != mReturnValueContainer.end() ? it->second : -1; } +bool Library::returnValueSafeValues(const Token *ftok, int64_t *v1, int64_t *v2) 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; + +} + bool Library::hasminsize(const Token *ftok) const { if (isNotLibraryFunction(ftok)) diff --git a/lib/library.h b/lib/library.h index 2dc4449fd..019871d8c 100644 --- a/lib/library.h +++ b/lib/library.h @@ -181,6 +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, int64_t *v1, int64_t *v2) const; bool isnoreturn(const Token *ftok) const; bool isnotnoreturn(const Token *ftok) const; @@ -541,6 +542,11 @@ private: std::map mReturnValue; std::map mReturnValueType; std::map mReturnValueContainer; + struct SafeValues { + int64_t minValue; + int64_t maxValue; + }; + std::map mReturnSafeValues; 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 1e72d9c87..46657eadc 100644 --- a/lib/settings.cpp +++ b/lib/settings.cpp @@ -40,6 +40,7 @@ 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 8bbd3a93b..902a11dc8 100644 --- a/lib/settings.h +++ b/lib/settings.h @@ -149,6 +149,9 @@ public: /** @brief Experimental flag that says all functions are "safe" */ bool allFunctionsAreSafe; + /** @brief Experimental flag to safety check function output */ + bool checkAllSafeFunctionReturnValues; + /** @brief Is --inline-suppr given? */ bool inlineSuppressions; diff --git a/lib/valueflow.cpp b/lib/valueflow.cpp index f3c017cce..0ad3411da 100644 --- a/lib/valueflow.cpp +++ b/lib/valueflow.cpp @@ -5393,6 +5393,18 @@ static bool getMinMaxValues(const ValueType *vt, const cppcheck::Platform &platf return true; } +static bool getMinMaxValues(const std::string &typestr, const Settings *settings, int64_t *minvalue, int64_t *maxvalue) +{ + TokenList typeTokens(settings); + std::istringstream istr(typestr+";"); + if (!typeTokens.createTokens(istr)) + return false; + typeTokens.simplifyPlatformTypes(); + typeTokens.simplifyStdType(); + const ValueType &vt = ValueType::parseDecl(typeTokens.front(), settings); + return getMinMaxValues(&vt, *settings, minvalue, maxvalue); +} + static void valueFlowSafeFunctions(TokenList *tokenlist, SymbolDatabase *symboldatabase, ErrorLogger *errorLogger, const Settings *settings) { if (settings->platformType == cppcheck::Platform::PlatformType::Unspecified) @@ -5430,6 +5442,38 @@ static void valueFlowSafeFunctions(TokenList *tokenlist, SymbolDatabase *symbold } } +static void valueFlowSafeFunctionReturn(TokenList *tokenlist, const Settings *settings) +{ + if (!settings->checkAllSafeFunctionReturnValues) + return; + for (Token *tok = tokenlist->front(); tok; tok = tok->next()) { + if (!tok->astParent() || tok->str() != "(") + continue; + if (!Token::Match(tok->previous(), "%name%")) + continue; + int64_t v1,v2; + if (!settings->library.returnValueSafeValues(tok->previous(), &v1, &v2)) + continue; + + // Get min/max values for return type + const std::string typestr = settings->library.returnValueType(tok->previous()); + int64_t minvalue, maxvalue; + if (!getMinMaxValues(typestr, settings, &minvalue, &maxvalue)) + continue; + + auto value = [](int64_t v, int64_t minvalue, int64_t 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); + } +} + ValueFlow::Value::Value(const Token *c, long long val) : valueType(INT), intvalue(val), @@ -5496,6 +5540,7 @@ void ValueFlow::setValues(TokenList *tokenlist, SymbolDatabase* symboldatabase, valueFlowNumber(tokenlist); valueFlowString(tokenlist); valueFlowArray(tokenlist); + valueFlowSafeFunctionReturn(tokenlist, settings); valueFlowGlobalConstVar(tokenlist, settings); valueFlowGlobalStaticVar(tokenlist, settings); valueFlowPointerAlias(tokenlist); diff --git a/test/testvalueflow.cpp b/test/testvalueflow.cpp index e3742296b..a99cd77e4 100644 --- a/test/testvalueflow.cpp +++ b/test/testvalueflow.cpp @@ -3919,6 +3919,20 @@ private: ASSERT_EQUALS(-0x8000, values.front().intvalue); ASSERT_EQUALS(0x7fff, values.back().intvalue); } + + + void valueFlowSafeFunctionReturn() { + const char *code; + std::list values; + Settings s; + s.checkAllSafeFunctionReturnValues = true; + + code = "x = rand();"; + values = tokenValues(code, "(", &s); + ASSERT_EQUALS(2, values.size()); + ASSERT_EQUALS(INT_MIN, values.front().intvalue); + ASSERT_EQUALS(INT_MAX, values.back().intvalue); + } }; REGISTER_TEST(TestValueFlow)