Skip to content

Commit 2359b9f

Browse files
danmardanmar
committed
valueflow: limit valueflow in functions that have many ifs
1 parent eb0998d commit 2359b9f

4 files changed

Lines changed: 71 additions & 14 deletions

File tree

‎.selfcheck_suppressions‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,3 +23,5 @@ autoNoType
2323
bailoutUninitVar
2424

2525
*:externals/*/*
26+
27+
performanceValueflowMaxIfCountExceeded

‎cli/cmdlineparser.cpp‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -606,6 +606,9 @@ bool CmdLineParser::parseFromArgs(int argc, const char* const argv[])
606606
else if (std::strncmp(argv[i], "--performance-valueflow-max-time=", 33) == 0)
607607
mSettings.performanceValueFlowMaxTime = std::atoi(argv[i] + 33);
608608

609+
else if (std::strncmp(argv[i], "--performance-valueflow-max-if-count=", 37) == 0)
610+
mSettings.performanceValueFlowMaxIfCount = std::atoi(argv[i] + 37);
611+
609612
// Specify platform
610613
else if (std::strncmp(argv[i], "--platform=", 11) == 0) {
611614
const std::string platform(11+argv[i]);
@@ -1212,6 +1215,13 @@ void CmdLineParser::printHelp()
12121215
" is 2. A larger value will mean more errors can be found\n"
12131216
" but also means the analysis will be slower.\n"
12141217
" --output-file=<file> Write results to file, rather than standard error.\n"
1218+
" --performance-valueflow-max-if-count=<limit>\n"
1219+
" If you have many conditional scopes in a function then\n"
1220+
" the number of possible control flow paths through that\n"
1221+
" function explodes and that can lead to very long analysis\n"
1222+
" time. Valueflow is limited in functions that have more\n"
1223+
" than <limit> conditional scopes. The default limit is\n"
1224+
" 100, which few functions will reach.\n"
12151225
" --platform=<type>, --platform=<file>\n"
12161226
" Specifies platform specific types and sizes. The\n"
12171227
" available builtin platforms are:\n"

‎lib/settings.h‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -254,6 +254,9 @@ class CPPCHECKLIB Settings {
254254
/** @brief Experimental: --performance-valueflow-max-time=T */
255255
int performanceValueFlowMaxTime;
256256

257+
/** @brief --performance-valueflow-max-if-count=C */
258+
int performanceValueFlowMaxIfCount = 100;
259+
257260
/** @brief plist output (--plist-output=&lt;dir&gt;) */
258261
std::string plistOutput;
259262

‎lib/valueflow.cpp‎

Lines changed: 56 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -5747,9 +5747,15 @@ static bool intersects(const C1& c1, const C2& c2)
57475747
return false;
57485748
}
57495749

5750-
static void valueFlowAfterAssign(TokenList *tokenlist, SymbolDatabase* symboldatabase, ErrorLogger *errorLogger, const Settings *settings)
5750+
static void valueFlowAfterAssign(TokenList *tokenlist,
5751+
SymbolDatabase* symboldatabase,
5752+
ErrorLogger *errorLogger,
5753+
const Settings *settings,
5754+
const std::set<const Scope*>& skippedFunctions)
57515755
{
57525756
for (const Scope * scope : symboldatabase->functionScopes) {
5757+
if (skippedFunctions.count(scope))
5758+
continue;
57535759
std::unordered_map<nonneg int, std::unordered_set<nonneg int>> backAssigns;
57545760
for (Token* tok = const_cast<Token*>(scope->bodyStart); tok != scope->bodyEnd; tok = tok->next()) {
57555761
// Assignment
@@ -6051,9 +6057,12 @@ struct ConditionHandler {
60516057
void traverseCondition(TokenList* tokenlist,
60526058
SymbolDatabase* symboldatabase,
60536059
const Settings* settings,
6060+
const std::set<const Scope*>& skippedFunctions,
60546061
const std::function<void(const Condition& cond, Token* tok, const Scope* scope)>& f) const
60556062
{
60566063
for (const Scope *scope : symboldatabase->functionScopes) {
6064+
if (skippedFunctions.count(scope))
6065+
continue;
60576066
for (Token *tok = const_cast<Token *>(scope->bodyStart); tok != scope->bodyEnd; tok = tok->next()) {
60586067
if (Token::Match(tok, "if|while|for ("))
60596068
continue;
@@ -6086,8 +6095,9 @@ struct ConditionHandler {
60866095
void beforeCondition(TokenList* tokenlist,
60876096
SymbolDatabase* symboldatabase,
60886097
ErrorLogger* errorLogger,
6089-
const Settings* settings) const {
6090-
traverseCondition(tokenlist, symboldatabase, settings, [&](const Condition& cond, Token* tok, const Scope*) {
6098+
const Settings* settings,
6099+
const std::set<const Scope*>& skippedFunctions) const {
6100+
traverseCondition(tokenlist, symboldatabase, settings, skippedFunctions, [&](const Condition& cond, Token* tok, const Scope*) {
60916101
if (cond.vartok->exprId() == 0)
60926102
return;
60936103

@@ -6233,8 +6243,9 @@ struct ConditionHandler {
62336243
void afterCondition(TokenList* tokenlist,
62346244
SymbolDatabase* symboldatabase,
62356245
ErrorLogger* errorLogger,
6236-
const Settings* settings) const {
6237-
traverseCondition(tokenlist, symboldatabase, settings, [&](const Condition& cond, Token* condTok, const Scope* scope) {
6246+
const Settings* settings,
6247+
const std::set<const Scope*>& skippedFunctions) const {
6248+
traverseCondition(tokenlist, symboldatabase, settings, skippedFunctions, [&](const Condition& cond, Token* condTok, const Scope* scope) {
62386249
const Token* top = condTok->astTop();
62396250

62406251
const MathLib::bigint path = cond.getPath();
@@ -6564,10 +6575,11 @@ static void valueFlowCondition(const ValuePtr<ConditionHandler>& handler,
65646575
TokenList* tokenlist,
65656576
SymbolDatabase* symboldatabase,
65666577
ErrorLogger* errorLogger,
6567-
const Settings* settings)
6578+
const Settings* settings,
6579+
const std::set<const Scope*>& skippedFunctions)
65686580
{
6569-
handler->beforeCondition(tokenlist, symboldatabase, errorLogger, settings);
6570-
handler->afterCondition(tokenlist, symboldatabase, errorLogger, settings);
6581+
handler->beforeCondition(tokenlist, symboldatabase, errorLogger, settings, skippedFunctions);
6582+
handler->afterCondition(tokenlist, symboldatabase, errorLogger, settings, skippedFunctions);
65716583
}
65726584

65736585
struct SimpleConditionHandler : ConditionHandler {
@@ -8966,6 +8978,36 @@ void ValueFlow::setValues(TokenList *tokenlist, SymbolDatabase* symboldatabase,
89668978

89678979
const std::uint64_t stopTime = getValueFlowStopTime(settings);
89688980

8981+
std::set<const Scope*> skippedFunctions;
8982+
if (settings->performanceValueFlowMaxIfCount < 1000) {
8983+
for (const Scope* functionScope: symboldatabase->functionScopes) {
8984+
int countIfScopes = 0;
8985+
std::vector<const Scope*> scopes{functionScope};
8986+
while (!scopes.empty()) {
8987+
const Scope* s = scopes.back();
8988+
scopes.pop_back();
8989+
for (const Scope* s2: s->nestedList) {
8990+
scopes.emplace_back(s2);
8991+
if (s2->type == Scope::ScopeType::eIf)
8992+
++countIfScopes;
8993+
}
8994+
}
8995+
if (countIfScopes > settings->performanceValueFlowMaxIfCount) {
8996+
skippedFunctions.emplace(functionScope);
8997+
8998+
const std::string& functionName = functionScope->className;
8999+
const std::list<ErrorMessage::FileLocation> callstack(1, ErrorMessage::FileLocation(functionScope->bodyStart, tokenlist));
9000+
const ErrorMessage errmsg(callstack, tokenlist->getSourceFilePath(), Severity::information,
9001+
"ValueFlow analysis is limited in " + functionName + " because if-count in function " +
9002+
std::to_string(countIfScopes) + " exceeds limit " +
9003+
std::to_string(settings->performanceValueFlowMaxIfCount) + ". The limit can be adjusted with "
9004+
"--performance-valueflow-max-if-count. Increasing the if-count limit will likely increase the "
9005+
"analysis time.", "performanceValueflowMaxIfCountExceeded", Certainty::normal);
9006+
errorLogger->reportErr(errmsg);
9007+
}
9008+
}
9009+
}
9010+
89699011
std::size_t values = 0;
89709012
std::size_t n = settings->valueFlowMaxIterations;
89719013
while (n > 0 && values != getTotalValues(tokenlist)) {
@@ -8976,7 +9018,7 @@ void ValueFlow::setValues(TokenList *tokenlist, SymbolDatabase* symboldatabase,
89769018
if (std::time(nullptr) < stopTime)
89779019
valueFlowSymbolicOperators(symboldatabase, settings);
89789020
if (std::time(nullptr) < stopTime)
8979-
valueFlowCondition(SymbolicConditionHandler{}, tokenlist, symboldatabase, errorLogger, settings);
9021+
valueFlowCondition(SymbolicConditionHandler{}, tokenlist, symboldatabase, errorLogger, settings, skippedFunctions);
89809022
if (std::time(nullptr) < stopTime)
89819023
valueFlowSymbolicInfer(symboldatabase, settings);
89829024
if (std::time(nullptr) < stopTime)
@@ -8986,11 +9028,11 @@ void ValueFlow::setValues(TokenList *tokenlist, SymbolDatabase* symboldatabase,
89869028
if (std::time(nullptr) < stopTime)
89879029
valueFlowRightShift(tokenlist, settings);
89889030
if (std::time(nullptr) < stopTime)
8989-
valueFlowAfterAssign(tokenlist, symboldatabase, errorLogger, settings);
9031+
valueFlowAfterAssign(tokenlist, symboldatabase, errorLogger, settings, skippedFunctions);
89909032
if (std::time(nullptr) < stopTime)
89919033
valueFlowAfterSwap(tokenlist, symboldatabase, errorLogger, settings);
89929034
if (std::time(nullptr) < stopTime)
8993-
valueFlowCondition(SimpleConditionHandler{}, tokenlist, symboldatabase, errorLogger, settings);
9035+
valueFlowCondition(SimpleConditionHandler{}, tokenlist, symboldatabase, errorLogger, settings, skippedFunctions);
89949036
if (std::time(nullptr) < stopTime)
89959037
valueFlowInferCondition(tokenlist, settings);
89969038
if (std::time(nullptr) < stopTime)
@@ -9016,13 +9058,13 @@ void ValueFlow::setValues(TokenList *tokenlist, SymbolDatabase* symboldatabase,
90169058
if (std::time(nullptr) < stopTime)
90179059
valueFlowIterators(tokenlist, settings);
90189060
if (std::time(nullptr) < stopTime)
9019-
valueFlowCondition(IteratorConditionHandler{}, tokenlist, symboldatabase, errorLogger, settings);
9061+
valueFlowCondition(IteratorConditionHandler{}, tokenlist, symboldatabase, errorLogger, settings, skippedFunctions);
90209062
if (std::time(nullptr) < stopTime)
90219063
valueFlowIteratorInfer(tokenlist, settings);
9022-
if (std::time(nullptr) < stopTime)
9064+
if (std::time(nullptr) < stopTime && skippedFunctions.empty())
90239065
valueFlowContainerSize(tokenlist, symboldatabase, errorLogger, settings);
90249066
if (std::time(nullptr) < stopTime)
9025-
valueFlowCondition(ContainerConditionHandler{}, tokenlist, symboldatabase, errorLogger, settings);
9067+
valueFlowCondition(ContainerConditionHandler{}, tokenlist, symboldatabase, errorLogger, settings, skippedFunctions);
90269068
}
90279069
if (std::time(nullptr) < stopTime)
90289070
valueFlowSafeFunctions(tokenlist, symboldatabase, settings);

0 commit comments

Comments
 (0)