Skip to content

Commit 9990975

Browse files
firewavefirewave
authored
consolidated ValueFlow tuning options into ValueFlowOptions (#6332)
1 parent 9d4d118 commit 9990975

10 files changed

Lines changed: 97 additions & 47 deletions

‎cli/cmdlineparser.cpp‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -894,12 +894,12 @@ CmdLineParser::Result CmdLineParser::parseFromArgs(int argc, const char* const a
894894
// Experimental: limit execution time for extended valueflow analysis. basic valueflow analysis
895895
// is always executed.
896896
else if (std::strncmp(argv[i], "--performance-valueflow-max-time=", 33) == 0) {
897-
if (!parseNumberArg(argv[i], 33, mSettings.performanceValueFlowMaxTime, true))
897+
if (!parseNumberArg(argv[i], 33, mSettings.vfOptions.maxTime, true))
898898
return Result::Fail;
899899
}
900900

901901
else if (std::strncmp(argv[i], "--performance-valueflow-max-if-count=", 37) == 0) {
902-
if (!parseNumberArg(argv[i], 37, mSettings.performanceValueFlowMaxIfCount, true))
902+
if (!parseNumberArg(argv[i], 37, mSettings.vfOptions.maxIfCount, true))
903903
return Result::Fail;
904904
}
905905

@@ -1323,7 +1323,7 @@ CmdLineParser::Result CmdLineParser::parseFromArgs(int argc, const char* const a
13231323
}
13241324

13251325
else if (std::strncmp(argv[i], "--valueflow-max-iterations=", 27) == 0) {
1326-
if (!parseNumberArg(argv[i], 27, mSettings.valueFlowMaxIterations))
1326+
if (!parseNumberArg(argv[i], 27, mSettings.vfOptions.maxIterations))
13271327
return Result::Fail;
13281328
}
13291329

‎lib/forwardanalyzer.cpp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -648,7 +648,7 @@ namespace {
648648
}
649649
} else if (tok->isControlFlowKeyword() && Token::Match(tok, "if|while|for (") &&
650650
Token::simpleMatch(tok->next()->link(), ") {")) {
651-
if (settings.checkLevel == Settings::CheckLevel::normal && ++branchCount > 4) {
651+
if ((settings.vfOptions.maxForwardBranches > 0) && (++branchCount > settings.vfOptions.maxForwardBranches)) {
652652
// TODO: should be logged on function-level instead of file-level
653653
reportError(Severity::information, "normalCheckLevelMaxBranches", "Limiting analysis of branches. Use --check-level=exhaustive to analyze all branches.");
654654
return Break(Analyzer::Terminate::Bail);

‎lib/settings.cpp‎

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -290,14 +290,18 @@ void Settings::setCheckLevel(CheckLevel level)
290290
if (level == CheckLevel::normal) {
291291
// Checking should finish in reasonable time.
292292
checkLevel = level;
293-
performanceValueFlowMaxSubFunctionArgs = 8;
294-
performanceValueFlowMaxIfCount = 100;
293+
vfOptions.maxSubFunctionArgs = 8;
294+
vfOptions.maxIfCount = 100;
295+
vfOptions.doConditionExpressionAnalysis = false;
296+
vfOptions.maxForwardBranches = 4;
295297
}
296298
else if (level == CheckLevel::exhaustive) {
297299
// Checking can take a little while. ~ 10 times slower than normal analysis is OK.
298300
checkLevel = CheckLevel::exhaustive;
299-
performanceValueFlowMaxIfCount = -1;
300-
performanceValueFlowMaxSubFunctionArgs = 256;
301+
vfOptions.maxIfCount = -1;
302+
vfOptions.maxSubFunctionArgs = 256;
303+
vfOptions.doConditionExpressionAnalysis = true;
304+
vfOptions.maxForwardBranches = -1;
301305
}
302306
}
303307

‎lib/settings.h‎

Lines changed: 36 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -254,15 +254,6 @@ class CPPCHECKLIB WARN_UNUSED Settings {
254254

255255
Platform platform;
256256

257-
/** @brief Experimental: --performance-valueflow-max-time=T */
258-
int performanceValueFlowMaxTime = -1;
259-
260-
/** @brief --performance-valueflow-max-if-count=C */
261-
int performanceValueFlowMaxIfCount = -1;
262-
263-
/** @brief max number of sets of arguments to pass to subfuncions in valueflow */
264-
int performanceValueFlowMaxSubFunctionArgs = 256;
265-
266257
/** @brief pid of cppcheck. Intention is that this is set in the main process. */
267258
int pid;
268259

@@ -391,8 +382,42 @@ class CPPCHECKLIB WARN_UNUSED Settings {
391382
/** @brief forced includes given by the user */
392383
std::list<std::string> userIncludes;
393384

394-
/** @brief the maximum iterations of valueflow (--valueflow-max-iterations=T) */
395-
std::size_t valueFlowMaxIterations = 4;
385+
// TODO: adjust all options so 0 means "disabled" and -1 "means "unlimited"
386+
struct ValueFlowOptions
387+
{
388+
/** @brief the maximum iterations to execute */
389+
std::size_t maxIterations = 4;
390+
391+
/** @brief maximum numer if-branches */
392+
int maxIfCount = -1;
393+
394+
/** @brief maximum number of sets of arguments to pass to subfuncions */
395+
int maxSubFunctionArgs = 256;
396+
397+
/** @brief Experimental: maximum execution time */
398+
int maxTime = -1;
399+
400+
/** @brief Control if condition expression analysis is performed */
401+
bool doConditionExpressionAnalysis = true;
402+
403+
/** @brief Maximum performed for-loop count */
404+
int maxForLoopCount = 10000;
405+
406+
/** @brief Maximum performed forward branches */
407+
int maxForwardBranches = -1;
408+
409+
/** @brief Maximum performed alignof recursion */
410+
int maxAlignOfRecursion = 100;
411+
412+
/** @brief Maximum performed sizeof recursion */
413+
int maxSizeOfRecursion = 100;
414+
415+
/** @brief Maximum expression varid depth */
416+
int maxExprVarIdDepth = 4;
417+
};
418+
419+
/** @brief The ValueFlow options */
420+
ValueFlowOptions vfOptions;
396421

397422
/** @brief Is --verbose given? */
398423
bool verbose{};

‎lib/symboldatabase.cpp‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3713,6 +3713,8 @@ bool Variable::arrayDimensions(const Settings& settings, bool& isContainer)
37133713
// check for empty array dimension []
37143714
if (dim->next()->str() != "]") {
37153715
dimension_.tok = dim->astOperand2();
3716+
// TODO: only perform when ValueFlow is enabled
3717+
// TODO: collect timing information for this call?
37163718
ValueFlow::valueFlowConstantFoldAST(const_cast<Token *>(dimension_.tok), settings);
37173719
if (dimension_.tok && dimension_.tok->hasKnownIntValue()) {
37183720
dimension_.num = dimension_.tok->getKnownIntValue();

‎lib/tokenize.cpp‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3414,6 +3414,7 @@ bool Tokenizer::simplifyTokens1(const std::string &configuration)
34143414
if (!mSettings.buildDir.empty())
34153415
Summaries::create(*this, configuration);
34163416

3417+
// TODO: apply this through Settings::ValueFlowOptions
34173418
// TODO: do not run valueflow if no checks are being performed at all - e.g. unusedFunctions only
34183419
const char* disableValueflowEnv = std::getenv("DISABLE_VALUEFLOW");
34193420
const bool doValueFlow = !disableValueflowEnv || (std::strcmp(disableValueflowEnv, "1") != 0);

‎lib/valueflow.cpp‎

Lines changed: 24 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1161,8 +1161,10 @@ static size_t bitCeil(size_t x)
11611161

11621162
static size_t getAlignOf(const ValueType& vt, const Settings& settings, int maxRecursion = 0)
11631163
{
1164-
if (maxRecursion == 100)
1164+
if (maxRecursion == settings.vfOptions.maxAlignOfRecursion) {
1165+
// TODO: add bailout message
11651166
return 0;
1167+
}
11661168
if (vt.pointer || vt.reference != Reference::None || vt.isPrimitive()) {
11671169
auto align = ValueFlow::getSizeOf(vt, settings);
11681170
return align == 0 ? 0 : bitCeil(align);
@@ -1196,8 +1198,10 @@ static nonneg int getSizeOfType(const Token *typeTok, const Settings &settings)
11961198

11971199
size_t ValueFlow::getSizeOf(const ValueType &vt, const Settings &settings, int maxRecursion)
11981200
{
1199-
if (maxRecursion == 100)
1201+
if (maxRecursion == settings.vfOptions.maxSizeOfRecursion) {
1202+
// TODO: add bailout message
12001203
return 0;
1204+
}
12011205
if (vt.pointer || vt.reference != Reference::None)
12021206
return settings.platform.sizeof_pointer;
12031207
if (vt.type == ValueType::Type::BOOL || vt.type == ValueType::Type::CHAR)
@@ -3294,9 +3298,10 @@ struct ExpressionAnalyzer : SingleValueFlowAnalyzer {
32943298
}
32953299

32963300
void setupExprVarIds(const Token* start, int depth = 0) {
3297-
constexpr int maxDepth = 4;
3298-
if (depth > maxDepth)
3301+
if (depth > settings.vfOptions.maxExprVarIdDepth) {
3302+
// TODO: add bailout message
32993303
return;
3304+
}
33003305
visitAstNodes(start, [&](const Token* tok) {
33013306
const bool top = depth == 0 && tok == start;
33023307
const bool ispointer = astIsPointer(tok) || astIsSmartPointer(tok) || astIsIterator(tok);
@@ -5399,7 +5404,7 @@ static const Scope* getLoopScope(const Token* tok)
53995404
//
54005405
static void valueFlowConditionExpressions(const TokenList &tokenlist, const SymbolDatabase& symboldatabase, ErrorLogger &errorLogger, const Settings &settings)
54015406
{
5402-
if (!settings.daca && (settings.checkLevel == Settings::CheckLevel::normal))
5407+
if (!settings.daca && !settings.vfOptions.doConditionExpressionAnalysis)
54035408
{
54045409
if (settings.debugwarnings) {
54055410
ErrorMessage::FileLocation loc(tokenlist.getSourceFilePath(), 0, 0);
@@ -5416,7 +5421,7 @@ static void valueFlowConditionExpressions(const TokenList &tokenlist, const Symb
54165421
continue;
54175422
}
54185423

5419-
if (settings.daca && (settings.checkLevel == Settings::CheckLevel::normal))
5424+
if (settings.daca && !settings.vfOptions.doConditionExpressionAnalysis)
54205425
continue;
54215426

54225427
for (Token* tok = const_cast<Token*>(scope->bodyStart); tok != scope->bodyEnd; tok = tok->next()) {
@@ -7195,13 +7200,14 @@ static bool valueFlowForLoop2(const Token *tok,
71957200
ProgramMemory startMemory(programMemory);
71967201
ProgramMemory endMemory;
71977202

7198-
int maxcount = 10000;
7203+
int maxcount = settings.vfOptions.maxForLoopCount;
71997204
while (result != 0 && !error && --maxcount > 0) {
72007205
endMemory = programMemory;
72017206
execute(thirdExpression, programMemory, &result, &error, settings);
72027207
if (!error)
72037208
execute(secondExpression, programMemory, &result, &error, settings);
72047209
}
7210+
// TODO: add bailout message
72057211

72067212
if (memory1)
72077213
memory1->swap(startMemory);
@@ -7580,7 +7586,7 @@ static bool productParams(const Settings& settings, const std::unordered_map<Key
75807586
args.back()[p.first] = p.second.front();
75817587
}
75827588
bool bail = false;
7583-
int max = settings.performanceValueFlowMaxSubFunctionArgs;
7589+
int max = settings.vfOptions.maxSubFunctionArgs;
75847590
for (const auto& p:vars) {
75857591
if (args.size() > max) {
75867592
bail = true;
@@ -7611,6 +7617,7 @@ static bool productParams(const Settings& settings, const std::unordered_map<Key
76117617
if (args.size() > max) {
76127618
bail = true;
76137619
args.resize(max);
7620+
// TODO: add bailout message
76147621
}
76157622

76167623
for (const auto& arg:args) {
@@ -9474,7 +9481,7 @@ struct ValueFlowPassRunner {
94749481
bool run(std::initializer_list<ValuePtr<ValueFlowPass>> passes) const
94759482
{
94769483
std::size_t values = 0;
9477-
std::size_t n = state.settings.valueFlowMaxIterations;
9484+
std::size_t n = state.settings.vfOptions.maxIterations;
94789485
while (n > 0 && values != getTotalValues()) {
94799486
values = getTotalValues();
94809487
if (std::any_of(passes.begin(), passes.end(), [&](const ValuePtr<ValueFlowPass>& pass) {
@@ -9501,8 +9508,10 @@ struct ValueFlowPassRunner {
95019508
bool run(const ValuePtr<ValueFlowPass>& pass) const
95029509
{
95039510
auto start = Clock::now();
9504-
if (start > stop)
9511+
if (start > stop) {
9512+
// TODO: add bailout message
95059513
return true;
9514+
}
95069515
if (!state.tokenlist.isCPP() && pass->cpp())
95079516
return false;
95089517
if (timerResults) {
@@ -9524,7 +9533,7 @@ struct ValueFlowPassRunner {
95249533

95259534
void setSkippedFunctions()
95269535
{
9527-
if (state.settings.performanceValueFlowMaxIfCount > 0) {
9536+
if (state.settings.vfOptions.maxIfCount > 0) {
95289537
for (const Scope* functionScope : state.symboldatabase.functionScopes) {
95299538
int countIfScopes = 0;
95309539
std::vector<const Scope*> scopes{functionScope};
@@ -9537,7 +9546,7 @@ struct ValueFlowPassRunner {
95379546
++countIfScopes;
95389547
}
95399548
}
9540-
if (countIfScopes > state.settings.performanceValueFlowMaxIfCount) {
9549+
if (countIfScopes > state.settings.vfOptions.maxIfCount) {
95419550
state.skippedFunctions.emplace(functionScope);
95429551

95439552
if (state.settings.severity.isEnabled(Severity::information)) {
@@ -9550,7 +9559,7 @@ struct ValueFlowPassRunner {
95509559
Severity::information,
95519560
"Limiting ValueFlow analysis in function '" + functionName + "' since it is too complex. "
95529561
"Please specify --check-level=exhaustive to perform full analysis.",
9553-
"checkLevelNormal",
9562+
"checkLevelNormal", // TODO: use more specific ID
95549563
Certainty::normal);
95559564
state.errorLogger.reportErr(errmsg);
95569565
}
@@ -9561,8 +9570,8 @@ struct ValueFlowPassRunner {
95619570

95629571
void setStopTime()
95639572
{
9564-
if (state.settings.performanceValueFlowMaxTime >= 0)
9565-
stop = Clock::now() + std::chrono::seconds{state.settings.performanceValueFlowMaxTime};
9573+
if (state.settings.vfOptions.maxTime >= 0)
9574+
stop = Clock::now() + std::chrono::seconds{state.settings.vfOptions.maxTime};
95669575
}
95679576

95689577
ValueFlowState state;

‎test/testcmdlineparser.cpp‎

Lines changed: 17 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1906,14 +1906,14 @@ class TestCmdlineParser : public TestFixture {
19061906
REDIRECT;
19071907
const char * const argv[] = {"cppcheck", "--valueflow-max-iterations=0", "file.cpp"};
19081908
ASSERT_EQUALS_ENUM(CmdLineParser::Result::Success, parser->parseFromArgs(3, argv));
1909-
ASSERT_EQUALS(0, settings->valueFlowMaxIterations);
1909+
ASSERT_EQUALS(0, settings->vfOptions.maxIterations);
19101910
}
19111911

19121912
void valueFlowMaxIterations2() {
19131913
REDIRECT;
19141914
const char * const argv[] = {"cppcheck", "--valueflow-max-iterations=11", "file.cpp"};
19151915
ASSERT_EQUALS_ENUM(CmdLineParser::Result::Success, parser->parseFromArgs(3, argv));
1916-
ASSERT_EQUALS(11, settings->valueFlowMaxIterations);
1916+
ASSERT_EQUALS(11, settings->vfOptions.maxIterations);
19171917
}
19181918

19191919
void valueFlowMaxIterationsInvalid() {
@@ -2006,7 +2006,7 @@ class TestCmdlineParser : public TestFixture {
20062006
REDIRECT;
20072007
const char * const argv[] = {"cppcheck", "--performance-valueflow-max-time=12", "file.cpp"};
20082008
ASSERT_EQUALS_ENUM(CmdLineParser::Result::Success, parser->parseFromArgs(3, argv));
2009-
ASSERT_EQUALS(12, settings->performanceValueFlowMaxTime);
2009+
ASSERT_EQUALS(12, settings->vfOptions.maxTime);
20102010
}
20112011

20122012
void performanceValueflowMaxTimeInvalid() {
@@ -2020,7 +2020,7 @@ class TestCmdlineParser : public TestFixture {
20202020
REDIRECT;
20212021
const char * const argv[] = {"cppcheck", "--performance-valueflow-max-if-count=12", "file.cpp"};
20222022
ASSERT_EQUALS_ENUM(CmdLineParser::Result::Success, parser->parseFromArgs(3, argv));
2023-
ASSERT_EQUALS(12, settings->performanceValueFlowMaxIfCount);
2023+
ASSERT_EQUALS(12, settings->vfOptions.maxIfCount);
20242024
}
20252025

20262026
void performanceValueFlowMaxIfCountInvalid() {
@@ -2569,31 +2569,38 @@ class TestCmdlineParser : public TestFixture {
25692569
}
25702570
#endif
25712571

2572+
// the CLI default to --check-level=normal
25722573
void checkLevelDefault() {
25732574
REDIRECT;
25742575
const char * const argv[] = {"cppcheck", "file.cpp"};
25752576
ASSERT_EQUALS_ENUM(CmdLineParser::Result::Success, parser->parseFromArgs(2, argv));
25762577
ASSERT_EQUALS_ENUM(Settings::CheckLevel::normal, settings->checkLevel);
2577-
ASSERT_EQUALS(100, settings->performanceValueFlowMaxIfCount);
2578-
ASSERT_EQUALS(8, settings->performanceValueFlowMaxSubFunctionArgs);
2578+
ASSERT_EQUALS(100, settings->vfOptions.maxIfCount);
2579+
ASSERT_EQUALS(8, settings->vfOptions.maxSubFunctionArgs);
2580+
ASSERT_EQUALS(false, settings->vfOptions.doConditionExpressionAnalysis);
2581+
ASSERT_EQUALS(4, settings->vfOptions.maxForwardBranches);
25792582
}
25802583

25812584
void checkLevelNormal() {
25822585
REDIRECT;
25832586
const char * const argv[] = {"cppcheck", "--check-level=normal", "file.cpp"};
25842587
ASSERT_EQUALS_ENUM(CmdLineParser::Result::Success, parser->parseFromArgs(3, argv));
25852588
ASSERT_EQUALS_ENUM(Settings::CheckLevel::normal, settings->checkLevel);
2586-
ASSERT_EQUALS(100, settings->performanceValueFlowMaxIfCount);
2587-
ASSERT_EQUALS(8, settings->performanceValueFlowMaxSubFunctionArgs);
2589+
ASSERT_EQUALS(100, settings->vfOptions.maxIfCount);
2590+
ASSERT_EQUALS(8, settings->vfOptions.maxSubFunctionArgs);
2591+
ASSERT_EQUALS(false, settings->vfOptions.doConditionExpressionAnalysis);
2592+
ASSERT_EQUALS(4, settings->vfOptions.maxForwardBranches);
25882593
}
25892594

25902595
void checkLevelExhaustive() {
25912596
REDIRECT;
25922597
const char * const argv[] = {"cppcheck", "--check-level=exhaustive", "file.cpp"};
25932598
ASSERT_EQUALS_ENUM(CmdLineParser::Result::Success, parser->parseFromArgs(3, argv));
25942599
ASSERT_EQUALS_ENUM(Settings::CheckLevel::exhaustive, settings->checkLevel);
2595-
ASSERT_EQUALS(-1, settings->performanceValueFlowMaxIfCount);
2596-
ASSERT_EQUALS(256, settings->performanceValueFlowMaxSubFunctionArgs);
2600+
ASSERT_EQUALS(-1, settings->vfOptions.maxIfCount);
2601+
ASSERT_EQUALS(256, settings->vfOptions.maxSubFunctionArgs);
2602+
ASSERT_EQUALS(true, settings->vfOptions.doConditionExpressionAnalysis);
2603+
ASSERT_EQUALS(-1, settings->vfOptions.maxForwardBranches);
25972604
}
25982605

25992606
void checkLevelUnknown() {

‎test/testsettings.cpp‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -272,8 +272,10 @@ class TestSettings : public TestFixture {
272272
{
273273
Settings s;
274274
ASSERT_EQUALS_ENUM(s.checkLevel, Settings::CheckLevel::exhaustive);
275-
ASSERT_EQUALS(s.performanceValueFlowMaxIfCount, -1);
276-
ASSERT_EQUALS(s.performanceValueFlowMaxSubFunctionArgs, 256);
275+
ASSERT_EQUALS(s.vfOptions.maxIfCount, -1);
276+
ASSERT_EQUALS(s.vfOptions.maxSubFunctionArgs, 256);
277+
ASSERT_EQUALS(true, s.vfOptions.doConditionExpressionAnalysis);
278+
ASSERT_EQUALS(-1, s.vfOptions.maxForwardBranches);
277279
}
278280
};
279281

‎test/testvalueflow.cpp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8537,7 +8537,7 @@ class TestValueFlow : public TestFixture {
85378537

85388538
void performanceIfCount() {
85398539
/*const*/ Settings s(settings);
8540-
s.performanceValueFlowMaxIfCount = 1;
8540+
s.vfOptions.maxIfCount = 1;
85418541

85428542
const char *code;
85438543

0 commit comments

Comments
 (0)