Skip to content

Commit aec1a7f

Browse files
Fix #6492 valueflow: for loop condition not used for conditional analysis (#8472)
To be merged after #8471. Ignoring the debug messages since the line numbers were flaky. --------- Co-authored-by: chrchr-github <noreply@github.com>
1 parent 6fa5b6b commit aec1a7f

3 files changed

Lines changed: 27 additions & 13 deletions

File tree

lib/valueflow.cpp

Lines changed: 19 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -4961,6 +4961,24 @@ static void valueFlowCondition(const ValuePtr<ConditionHandler>& handler,
49614961
handler->afterCondition(tokenlist, symboldatabase, errorLogger, settings, skippedFunctions);
49624962
}
49634963

4964+
static const Token* getConditionVariable(const Token* tok)
4965+
{
4966+
if (tok->str() == "!")
4967+
return tok->astOperand1();
4968+
4969+
if (const Token* parent = tok->astParent()) {
4970+
if (Token::Match(parent, "%oror%|&&|?") ||
4971+
Token::Match(parent->previous(), "if|while (") ||
4972+
(parent->str() == ";" && astIsLHS(tok) && Token::simpleMatch(parent->astParent(), ";"))) { // for loop condition
4973+
if (Token::simpleMatch(tok, "="))
4974+
return tok->astOperand1();
4975+
if (!Token::Match(tok, "%comp%|%assign%"))
4976+
return tok;
4977+
}
4978+
}
4979+
return nullptr;
4980+
}
4981+
49644982
struct SimpleConditionHandler : ConditionHandler {
49654983
std::vector<Condition> parse(const Token* tok, const Settings& /*settings*/) const override {
49664984

@@ -4979,19 +4997,7 @@ struct SimpleConditionHandler : ConditionHandler {
49794997
if (!conds.empty())
49804998
return conds;
49814999

4982-
const Token* vartok = nullptr;
4983-
4984-
if (tok->str() == "!") {
4985-
vartok = tok->astOperand1();
4986-
4987-
} else if (tok->astParent() && (Token::Match(tok->astParent(), "%oror%|&&|?") ||
4988-
Token::Match(tok->astParent()->previous(), "if|while ("))) {
4989-
if (Token::simpleMatch(tok, "="))
4990-
vartok = tok->astOperand1();
4991-
else if (!Token::Match(tok, "%comp%|%assign%"))
4992-
vartok = tok;
4993-
}
4994-
5000+
const Token* vartok = getConditionVariable(tok);
49955001
if (!vartok)
49965002
return {};
49975003
Condition cond;

test/testnullpointer.cpp

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3723,6 +3723,13 @@ class TestNullPointer : public TestFixture {
37233723
ASSERT_EQUALS("[test.cpp:3:9] -> [test.cpp:2:19]: (warning) Either the condition 'p' is redundant or there is possible null pointer dereference: p. [nullPointerRedundantCheck]\n"
37243724
"[test.cpp:8:9] -> [test.cpp:7:18]: (warning) Either the condition 's' is redundant or there is possible null pointer dereference: s. [nullPointerRedundantCheck]\n",
37253725
errout_str());
3726+
3727+
check("struct S { int a; };\n" // #6492
3728+
"void h(const S* s) {\n"
3729+
" for (int i = s->a; s; ++i) {}\n"
3730+
"}\n");
3731+
ASSERT_EQUALS("[test.cpp:3:24] -> [test.cpp:3:18]: (warning) Either the condition 's' is redundant or there is possible null pointer dereference: s. [nullPointerRedundantCheck]\n",
3732+
errout_str());
37263733
}
37273734

37283735
void nullpointerDeadCode() {

test/testsimplifytokens.cpp

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2207,6 +2207,7 @@ class TestSimplifyTokens : public TestFixture {
22072207
"for ( i = 0 ; ( i < sz ) && ( sz > 3 ) ; ++ i ) { }\n"
22082208
"}";
22092209
ASSERT_EQUALS(expected, tokenizeAndStringify(code, dinit(TokenizeAndStringifyOptions, $.cpp = false)));
2210+
ignore_errout();
22102211
}
22112212

22122213
void simplifyKnownVariables49() { // #3691

0 commit comments

Comments
 (0)