From 45d1ca6f7c781354ec2ff5ad41d4170e3e80f4ff Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Marjam=C3=A4ki?= Date: Thu, 25 Jul 2019 17:19:51 +0200 Subject: [PATCH] Safe checks: Clarify a warning message --- cli/cmdlineparser.cpp | 8 ++++++++ lib/check.cpp | 9 +++++++++ lib/check.h | 2 ++ lib/checktype.cpp | 5 ++++- lib/valueflow.cpp | 19 +++++++++++++++---- lib/valueflow.h | 4 ++++ 6 files changed, 42 insertions(+), 5 deletions(-) diff --git a/cli/cmdlineparser.cpp b/cli/cmdlineparser.cpp index 2b4bf8587..64012af97 100644 --- a/cli/cmdlineparser.cpp +++ b/cli/cmdlineparser.cpp @@ -181,6 +181,14 @@ bool CmdLineParser::parseFromArgs(int argc, const char* const argv[]) else if (std::strcmp(argv[i], "--inconclusive") == 0) mSettings->inconclusive = true; + // Experimental: Safe checking + else if (std::strcmp(argv[i], "--safe-classes") == 0) + mSettings->safeChecks.classes = true; + + // Experimental: Safe checking + else if (std::strcmp(argv[i], "--safe-functions") == 0) + mSettings->safeChecks.externalFunctions = mSettings->safeChecks.internalFunctions = 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/check.cpp b/lib/check.cpp index c173b3518..4ae15e2c7 100644 --- a/lib/check.cpp +++ b/lib/check.cpp @@ -66,3 +66,12 @@ std::list &Check::instances() return _instances; #endif } + +std::string Check::getMessageId(const ValueFlow::Value &value, const char id[]) +{ + if (value.condition != nullptr) + return id + std::string("Cond"); + if (value.safe) + return std::string("safe") + (char)std::toupper(id[0]) + (id + 1); + return id; +} diff --git a/lib/check.h b/lib/check.h index 82ad4c5e1..0c20ebf1b 100644 --- a/lib/check.h +++ b/lib/check.h @@ -118,6 +118,8 @@ public: return false; } + static std::string getMessageId(const ValueFlow::Value &value, const char id[]); + protected: const Tokenizer * const mTokenizer; const Settings * const mSettings; diff --git a/lib/checktype.cpp b/lib/checktype.cpp index e18aeceba..9d09f7d79 100644 --- a/lib/checktype.cpp +++ b/lib/checktype.cpp @@ -200,9 +200,12 @@ void CheckType::integerOverflowError(const Token *tok, const ValueFlow::Value &v else msg = "Signed integer overflow for expression '" + expr + "'."; + if (value.safe) + msg = "Safe checks: " + msg; + reportError(getErrorPath(tok, &value, "Integer overflow"), value.errorSeverity() ? Severity::error : Severity::warning, - (value.condition == nullptr) ? "integerOverflow" : "integerOverflowCond", + getMessageId(value, "integerOverflow").c_str(), msg, CWE190, value.isInconclusive()); diff --git a/lib/valueflow.cpp b/lib/valueflow.cpp index 1d6a5fcef..2d2c5caca 100644 --- a/lib/valueflow.cpp +++ b/lib/valueflow.cpp @@ -374,6 +374,7 @@ static void combineValueProperties(const ValueFlow::Value &value1, const ValueFl result->varId = (value1.varId != 0U) ? value1.varId : value2.varId; result->varvalue = (result->varId == value1.varId) ? value1.varvalue : value2.varvalue; result->errorPath = (value1.errorPath.empty() ? value2 : value1).errorPath; + result->safe = value1.safe || value2.safe; } @@ -5462,9 +5463,11 @@ static void valueFlowSafeFunctions(TokenList *tokenlist, SymbolDatabase *symbold argValues.emplace_back(0); argValues.back().valueType = ValueFlow::Value::ValueType::CONTAINER_SIZE; argValues.back().errorPath.emplace_back(arg.nameToken(), "Assuming " + arg.name() + " is empty"); + argValues.back().safe = true; argValues.emplace_back(1000000); argValues.back().valueType = ValueFlow::Value::ValueType::CONTAINER_SIZE; argValues.back().errorPath.emplace_back(arg.nameToken(), "Assuming " + arg.name() + " size is 1000000"); + argValues.back().safe = true; for (const ValueFlow::Value &value : argValues) valueFlowContainerForward(const_cast(functionScope->bodyStart), arg.declarationId(), value, settings, tokenlist->isCPP()); continue; @@ -5477,6 +5480,9 @@ static void valueFlowSafeFunctions(TokenList *tokenlist, SymbolDatabase *symbold if (!isLow && !isHigh && !all) continue; + const bool safeLow = !isLow; + const bool safeHigh = !isHigh; + if ((!isLow || !isHigh) && all) { MathLib::bigint minValue, maxValue; if (getMinMaxValues(arg.valueType(), *settings, &minValue, &maxValue)) { @@ -5490,11 +5496,13 @@ static void valueFlowSafeFunctions(TokenList *tokenlist, SymbolDatabase *symbold argValues.emplace_back(0); argValues.back().valueType = ValueFlow::Value::ValueType::FLOAT; argValues.back().floatValue = isLow ? low : -1E25f; - argValues.back().errorPath.emplace_back(arg.nameToken(), "Assuming argument has value " + MathLib::toString(argValues.back().floatValue)); + argValues.back().errorPath.emplace_back(arg.nameToken(), "Safe checks: Assuming argument has value " + MathLib::toString(argValues.back().floatValue)); + argValues.back().safe = true; argValues.emplace_back(0); argValues.back().valueType = ValueFlow::Value::ValueType::FLOAT; argValues.back().floatValue = isHigh ? high : 1E25f; - argValues.back().errorPath.emplace_back(arg.nameToken(), "Assuming argument has value " + MathLib::toString(argValues.back().floatValue)); + argValues.back().errorPath.emplace_back(arg.nameToken(), "Safe checks: Assuming argument has value " + MathLib::toString(argValues.back().floatValue)); + argValues.back().safe = true; valueFlowForward(const_cast(functionScope->bodyStart->next()), functionScope->bodyEnd, &arg, @@ -5512,11 +5520,13 @@ static void valueFlowSafeFunctions(TokenList *tokenlist, SymbolDatabase *symbold std::list argValues; if (isLow) { argValues.emplace_back(low); - argValues.back().errorPath.emplace_back(arg.nameToken(), "Assuming argument has value " + MathLib::toString(low)); + argValues.back().errorPath.emplace_back(arg.nameToken(), std::string(safeLow ? "Safe checks: " : "") + "Assuming argument has value " + MathLib::toString(low)); + argValues.back().safe = safeLow; } if (isHigh) { argValues.emplace_back(high); - argValues.back().errorPath.emplace_back(arg.nameToken(), "Assuming argument has value " + MathLib::toString(high)); + argValues.back().errorPath.emplace_back(arg.nameToken(), std::string(safeHigh ? "Safe checks: " : "") + "Assuming argument has value " + MathLib::toString(high)); + argValues.back().safe = safeHigh; } if (!argValues.empty()) @@ -5574,6 +5584,7 @@ ValueFlow::Value::Value(const Token *c, long long val) varvalue(val), condition(c), varId(0U), + safe(false), conditional(false), defaultArg(false), lifetimeKind(LifetimeKind::Object), diff --git a/lib/valueflow.h b/lib/valueflow.h index 54e46b65d..62a7c7c37 100644 --- a/lib/valueflow.h +++ b/lib/valueflow.h @@ -50,6 +50,7 @@ namespace ValueFlow { varvalue(val), condition(nullptr), varId(0U), + safe(false), conditional(false), defaultArg(false), lifetimeKind(LifetimeKind::Object), @@ -161,6 +162,9 @@ namespace ValueFlow { /** For calculated values - varId that calculated value depends on */ nonneg int varId; + /** value relies on safe checking */ + bool safe; + /** Conditional value */ bool conditional;