From c9906125dea32a522d0eb8ff0b9f054f2897f1a1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Marjam=C3=A4ki?= Date: Wed, 10 Jul 2019 16:59:05 +0200 Subject: [PATCH] Safe functions: Check more possible function argument values --- cli/cmdlineparser.cpp | 5 +++ lib/settings.cpp | 1 + lib/settings.h | 3 ++ lib/symboldatabase.cpp | 5 +++ lib/symboldatabase.h | 2 + lib/valueflow.cpp | 93 ++++++++++++++++++++++++++++++++++++++++++ test/testvalueflow.cpp | 21 +++++++++- 7 files changed, 128 insertions(+), 2 deletions(-) diff --git a/cli/cmdlineparser.cpp b/cli/cmdlineparser.cpp index 2b4bf8587..6ebf60a0c 100644 --- a/cli/cmdlineparser.cpp +++ b/cli/cmdlineparser.cpp @@ -181,6 +181,11 @@ bool CmdLineParser::parseFromArgs(int argc, const char* const argv[]) else if (std::strcmp(argv[i], "--inconclusive") == 0) mSettings->inconclusive = true; + // Experimental: this flag says that all functions are "safe". + // todo: add proper configuration options + else if (std::strcmp(argv[i], "--all-functions-are-safe") == 0) + mSettings->allFunctionsAreSafe = 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/settings.cpp b/lib/settings.cpp index 8833dbf3f..1e72d9c87 100644 --- a/lib/settings.cpp +++ b/lib/settings.cpp @@ -39,6 +39,7 @@ Settings::Settings() experimental(false), force(false), inconclusive(false), + allFunctionsAreSafe(false), inlineSuppressions(false), jobs(1), jointSuppressionReport(false), diff --git a/lib/settings.h b/lib/settings.h index 316ffad05..8bbd3a93b 100644 --- a/lib/settings.h +++ b/lib/settings.h @@ -146,6 +146,9 @@ public: /** @brief Inconclusive checks */ bool inconclusive; + /** @brief Experimental flag that says all functions are "safe" */ + bool allFunctionsAreSafe; + /** @brief Is --inline-suppr given? */ bool inlineSuppressions; diff --git a/lib/symboldatabase.cpp b/lib/symboldatabase.cpp index 111902478..29ccc8a76 100644 --- a/lib/symboldatabase.cpp +++ b/lib/symboldatabase.cpp @@ -2079,6 +2079,11 @@ const Token * Function::constructorMemberInitialization() const return nullptr; } +bool Function::isSafe(const Settings *settings) const +{ + return settings->allFunctionsAreSafe; +} + Function* SymbolDatabase::addGlobalFunction(Scope*& scope, const Token*& tok, const Token *argStart, const Token* funcStart) { Function* function = nullptr; diff --git a/lib/symboldatabase.h b/lib/symboldatabase.h index aea127ec1..d86c25574 100644 --- a/lib/symboldatabase.h +++ b/lib/symboldatabase.h @@ -839,6 +839,8 @@ public: setFlag(fHasBody, state); } + bool isSafe(const Settings *settings) const; + const Token *tokenDef; ///< function name token in class definition const Token *argDef; ///< function argument start '(' in class definition const Token *token; ///< function name token in implementation diff --git a/lib/valueflow.cpp b/lib/valueflow.cpp index 3a01d7957..f3c017cce 100644 --- a/lib/valueflow.cpp +++ b/lib/valueflow.cpp @@ -5338,6 +5338,98 @@ static void valueFlowDynamicBufferSize(TokenList *tokenlist, SymbolDatabase *sym } } +static bool getMinMaxValues(const ValueType *vt, const cppcheck::Platform &platform, int64_t *minValue, int64_t *maxValue) +{ + if (!vt || !vt->isIntegral() || vt->pointer) + return false; + + int bits; + switch (vt->type) { + case ValueType::Type::BOOL: + bits = 1; + break; + case ValueType::Type::CHAR: + bits = platform.char_bit; + break; + case ValueType::Type::SHORT: + bits = platform.short_bit; + break; + case ValueType::Type::INT: + bits = platform.int_bit; + break; + case ValueType::Type::LONG: + bits = platform.long_bit; + break; + case ValueType::Type::LONGLONG: + bits = platform.long_long_bit; + break; + default: + return false; + }; + + if (bits == 1) { + *minValue = 0; + *maxValue = 1; + } else if (bits < 62) { + if (vt->sign == ValueType::Sign::UNSIGNED) { + *minValue = 0; + *maxValue = (1LL << bits) - 1; + } else { + *minValue = -(1LL << (bits - 1)); + *maxValue = (1LL << (bits - 1)) - 1; + } + } else if (bits == 64) { + if (vt->sign == ValueType::Sign::UNSIGNED) { + *minValue = 0; + *maxValue = LLONG_MAX; // todo max unsigned value + } else { + *minValue = LLONG_MIN; + *maxValue = LLONG_MAX; + } + } else { + return false; + } + + return true; +} + +static void valueFlowSafeFunctions(TokenList *tokenlist, SymbolDatabase *symboldatabase, ErrorLogger *errorLogger, const Settings *settings) +{ + if (settings->platformType == cppcheck::Platform::PlatformType::Unspecified) + return; + for (const Scope *functionScope : symboldatabase->functionScopes) { + if (!functionScope->bodyStart) + continue; + const Function *function = functionScope->function; + if (!function) + continue; + + if (!function->isSafe(settings)) + continue; + + for (const Variable &arg : function->argumentList) { + int64_t minValue, maxValue; + if (!getMinMaxValues(arg.valueType(), *settings, &minValue, &maxValue)) + continue; + + std::list argValues; + argValues.emplace_back(minValue); + argValues.emplace_back(maxValue); + + valueFlowForward(const_cast(functionScope->bodyStart->next()), + functionScope->bodyEnd, + &arg, + arg.declarationId(), + argValues, + false, + false, + tokenlist, + errorLogger, + settings); + } + } +} + ValueFlow::Value::Value(const Token *c, long long val) : valueType(INT), intvalue(val), @@ -5436,6 +5528,7 @@ void ValueFlow::setValues(TokenList *tokenlist, SymbolDatabase* symboldatabase, valueFlowContainerSize(tokenlist, symboldatabase, errorLogger, settings); valueFlowContainerAfterCondition(tokenlist, symboldatabase, errorLogger, settings); } + valueFlowSafeFunctions(tokenlist, symboldatabase, errorLogger, settings); } valueFlowDynamicBufferSize(tokenlist, symboldatabase, errorLogger, settings); diff --git a/test/testvalueflow.cpp b/test/testvalueflow.cpp index 737a69b09..e3742296b 100644 --- a/test/testvalueflow.cpp +++ b/test/testvalueflow.cpp @@ -119,6 +119,8 @@ private: TEST_CASE(valueFlowContainerSize); TEST_CASE(valueFlowDynamicBufferSize); + + TEST_CASE(valueFlowSafeFunctions); } static bool isNotTokValue(const ValueFlow::Value &val) { @@ -295,8 +297,8 @@ private: settings.debugwarnings = false; } - std::list tokenValues(const char code[], const char tokstr[]) { - Tokenizer tokenizer(&settings, this); + std::list tokenValues(const char code[], const char tokstr[], const Settings *s = nullptr) { + Tokenizer tokenizer(s ? s : &settings, this); std::istringstream istr(code); errout.str(""); tokenizer.tokenize(istr, "test.cpp"); @@ -3902,6 +3904,21 @@ private: "}"; ASSERT_EQUALS(true, testValueOfX(code, 4U, 100, ValueFlow::Value::BUFFER_SIZE)); } + + void valueFlowSafeFunctions() { + const char *code; + std::list values; + Settings s; + s.allFunctionsAreSafe = true; + + code = "void f(short x) {\n" + " return x + 0;\n" + "}"; + values = tokenValues(code, "+", &s); + ASSERT_EQUALS(2, values.size()); + ASSERT_EQUALS(-0x8000, values.front().intvalue); + ASSERT_EQUALS(0x7fff, values.back().intvalue); + } }; REGISTER_TEST(TestValueFlow)