Skip to content

Commit 2209cc3

Browse files
Fix #8192: FN condition always false in for loop condition
When a for loop's condition is impossible given the initial value (e.g. `for (int i = 0; i > 10; i++)`), cppcheck was not emitting a knownConditionTrueFalse warning. Fix by populating memory1, memory2 and memoryAfter with the init state when the condition is immediately false (and no error occured). We can then set the value for the condition token and thus emit a knownConditionTrueFalse warning. Signed-off-by: Francois Berder <fberder@outlook.fr>
1 parent 950567e commit 2209cc3

2 files changed

Lines changed: 43 additions & 19 deletions

File tree

lib/valueflow.cpp

Lines changed: 42 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -5175,8 +5175,21 @@ static bool valueFlowForLoop2(const Token *tok,
51755175
if (error)
51765176
return false;
51775177
execute(secondExpression, programMemory, &result, &error, settings);
5178-
if (result == 0) // 2nd expression is false => no looping
5178+
if (result == 0) {
5179+
if (!error) { // 2nd expression is definitely false => no looping
5180+
ProgramMemory startMemory(programMemory);
5181+
ProgramMemory endMemory(programMemory);
5182+
5183+
if (memory1)
5184+
memory1->swap(startMemory);
5185+
if (memory2)
5186+
memory2->swap(endMemory);
5187+
if (memoryAfter)
5188+
memoryAfter->swap(programMemory);
5189+
return true;
5190+
}
51795191
return false;
5192+
}
51805193
if (error) {
51815194
// If a variable is reassigned in second expression, return false
51825195
bool reassign = false;
@@ -5385,23 +5398,34 @@ static void valueFlowForLoop(const TokenList &tokenlist, const SymbolDatabase& s
53855398
} else {
53865399
ProgramMemory mem1, mem2, memAfter;
53875400
if (valueFlowForLoop2(tok, &mem1, &mem2, &memAfter, settings)) {
5388-
for (const auto& p : mem1) {
5389-
if (!p.second.isIntValue())
5390-
continue;
5391-
if (p.second.isImpossible())
5392-
continue;
5393-
if (p.first.tok->varId() == 0)
5394-
continue;
5395-
valueFlowForLoopSimplify(bodyStart, p.first.tok, false, p.second.intvalue, tokenlist, errorLogger, settings);
5396-
}
5397-
for (const auto& p : mem2) {
5398-
if (!p.second.isIntValue())
5399-
continue;
5400-
if (p.second.isImpossible())
5401-
continue;
5402-
if (p.first.tok->varId() == 0)
5403-
continue;
5404-
valueFlowForLoopSimplify(bodyStart, p.first.tok, false, p.second.intvalue, tokenlist, errorLogger, settings);
5401+
if (mem1 == memAfter) { // #8192 check if loop never runs
5402+
Token* condTok = getCondTok(tok);
5403+
if (condTok && !condTok->hasKnownIntValue()) {
5404+
ValueFlow::Value v(0);
5405+
v.setKnown();
5406+
ValueFlow::setTokenValue(condTok, std::move(v), settings);
5407+
}
5408+
} else {
5409+
for (const auto& p : mem1) {
5410+
if (!p.second.isIntValue())
5411+
continue;
5412+
if (p.second.isImpossible())
5413+
continue;
5414+
if (p.first.tok->varId() == 0)
5415+
continue;
5416+
5417+
valueFlowForLoopSimplify(bodyStart, p.first.tok, false, p.second.intvalue, tokenlist, errorLogger, settings);
5418+
}
5419+
for (const auto& p : mem2) {
5420+
if (!p.second.isIntValue())
5421+
continue;
5422+
if (p.second.isImpossible())
5423+
continue;
5424+
if (p.first.tok->varId() == 0)
5425+
continue;
5426+
5427+
valueFlowForLoopSimplify(bodyStart, p.first.tok, false, p.second.intvalue, tokenlist, errorLogger, settings);
5428+
}
54055429
}
54065430
for (const auto& p : memAfter) {
54075431
if (!p.second.isIntValue())

test/testcondition.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5639,7 +5639,7 @@ class TestCondition : public TestFixture {
56395639
check("void f() {\n" // #8192
56405640
" for (int i = 0; i > 10; ++i) {}\n"
56415641
"}\n");
5642-
TODO_ASSERT_EQUALS("[test.cpp:2]: (style) Condition 'i>10' is always false\n", "", errout_str());
5642+
ASSERT_EQUALS("[test.cpp:2:23]: (style) Condition 'i>10' is always false [knownConditionTrueFalse]\n", errout_str());
56435643

56445644
check("void f() {\n"
56455645
" for (int i = 1000; i < 20; ++i) {}\n"

0 commit comments

Comments
 (0)