Fix 11227: ValueFlow: Known value not set in condition for ternary operator (#4653)

* Evaluate args before function call

* Fix tests

* Format

* Add test for 11227

* Format

* Fix known condition
This commit is contained in:
Paul Fultz II 2022-12-18 15:07:43 -06:00 committed by GitHub
parent d8451eda5f
commit 9da574f4a0
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
5 changed files with 43 additions and 5 deletions

View File

@ -2422,7 +2422,8 @@ bool isVariableChanged(const Token *tok, int indirect, const Settings *settings,
tok2 = tok2->astParent(); tok2 = tok2->astParent();
} }
while (Token::simpleMatch(tok2->astParent(), "?") || (Token::simpleMatch(tok2->astParent(), ":") && Token::simpleMatch(tok2->astParent()->astParent(), "?"))) while ((Token::simpleMatch(tok2, ":") && Token::simpleMatch(tok2->astParent(), "?")) ||
(Token::simpleMatch(tok2->astParent(), ":") && Token::simpleMatch(tok2->astParent()->astParent(), "?")))
tok2 = tok2->astParent(); tok2 = tok2->astParent();
if (tok2->astParent() && tok2->astParent()->tokType() == Token::eIncDecOp) if (tok2->astParent() && tok2->astParent()->tokType() == Token::eIncDecOp)

View File

@ -1643,8 +1643,8 @@ void CheckOther::constVariableError(const Variable *var, const Function *functio
ErrorPath errorPath; ErrorPath errorPath;
std::string id = "const" + vartype; std::string id = "const" + vartype;
std::string message = "$symbol:" + varname + "\n" + vartype + " '$symbol' can be declared as " + ptrRefArray; std::string message = "$symbol:" + varname + "\n" + vartype + " '$symbol' can be declared as " + ptrRefArray;
errorPath.emplace_back(var ? var->nameToken() : nullptr, message); errorPath.emplace_back(var->nameToken(), message);
if (var && var->isArgument() && function && function->functionPointerUsage) { if (var->isArgument() && function && function->functionPointerUsage) {
errorPath.emplace_front(function->functionPointerUsage, "You might need to cast the function pointer here"); errorPath.emplace_front(function->functionPointerUsage, "You might need to cast the function pointer here");
id += "Callback"; id += "Callback";
message += ". However it seems that '" + function->name() + "' is a callback function, if '$symbol' is declared with const you might also need to cast function pointer(s)."; message += ". However it seems that '" + function->name() + "' is a callback function, if '$symbol' is declared with const you might also need to cast function pointer(s).";

View File

@ -805,6 +805,18 @@ struct ForwardTraversal {
if (updateRecursive(tok->next()->astOperand2()) == Progress::Break) if (updateRecursive(tok->next()->astOperand2()) == Progress::Break)
return Break(); return Break();
return Break(); return Break();
} else if (Token* callTok = callExpr(tok)) {
// Since the call could be an unknown macro, traverse the tokens as a range instead of recursively
if (!Token::simpleMatch(callTok, "( )") &&
updateRange(callTok->next(), callTok->link(), depth - 1) == Progress::Break)
return Break();
if (updateTok(callTok) == Progress::Break)
return Break();
if (start != callTok && updateRecursive(callTok->astOperand1()) == Progress::Break)
return Break();
tok = callTok->link();
if (!tok)
return Break();
} else { } else {
if (updateTok(tok, &next) == Progress::Break) if (updateTok(tok, &next) == Progress::Break)
return Break(); return Break();
@ -812,7 +824,7 @@ struct ForwardTraversal {
if (precedes(next, end)) if (precedes(next, end))
tok = next->previous(); tok = next->previous();
else else
return Break(); return Progress::Continue;
} }
} }
// Prevent infinite recursion // Prevent infinite recursion
@ -852,6 +864,20 @@ struct ForwardTraversal {
return nullptr; return nullptr;
} }
static Token* callExpr(Token* tok)
{
while (tok->astParent() && astIsLHS(tok)) {
if (!Token::Match(tok, "%name%|::|<|."))
break;
if (Token::simpleMatch(tok, "<") && !tok->link())
break;
tok = tok->astParent();
}
if (isFunctionCall(tok))
return tok;
return nullptr;
}
static Token* skipTo(Token* tok, const Token* dest, const Token* end = nullptr) { static Token* skipTo(Token* tok, const Token* dest, const Token* end = nullptr) {
if (end && dest->index() > end->index()) if (end && dest->index() > end->index())
return nullptr; return nullptr;

View File

@ -8358,7 +8358,7 @@ static void valueFlowContainerSize(TokenList* tokenlist,
ValueFlow::Value value(tok->tokAt(2)->astOperand2()->values().front()); ValueFlow::Value value(tok->tokAt(2)->astOperand2()->values().front());
value.valueType = ValueFlow::Value::ValueType::CONTAINER_SIZE; value.valueType = ValueFlow::Value::ValueType::CONTAINER_SIZE;
value.setKnown(); value.setKnown();
valueFlowForward(tok->next(), containerTok, value, tokenlist); valueFlowForward(tok->linkAt(2), containerTok, value, tokenlist);
} }
} }
} }

View File

@ -4457,6 +4457,17 @@ private:
" return;\n" " return;\n"
"}\n"); "}\n");
ASSERT_EQUALS("", errout.str()); ASSERT_EQUALS("", errout.str());
// #11227
check("struct S {\n"
" int get();\n"
"};\n"
"void f(const S* s) {\n"
" if (!s)\n"
" return;\n"
" g(s ? s->get() : 0);\n"
"}\n");
ASSERT_EQUALS("[test.cpp:5] -> [test.cpp:7]: (style) Condition 's' is always true\n", errout.str());
} }
void alwaysTrueSymbolic() void alwaysTrueSymbolic()