Skip to content

Commit eb0db32

Browse files
danmardanmar
committed
Fixed cppcheck-opensource#6560 (ValueFlow: handling ternary operator better in valueFlowSubFunction)
1 parent 7f15873 commit eb0db32

5 files changed

Lines changed: 67 additions & 14 deletions

File tree

‎lib/token.cpp‎

Lines changed: 10 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1135,18 +1135,21 @@ bool Token::isCalculation() const
11351135
return true;
11361136
}
11371137

1138-
static bool isUnaryPreOp(const Token *op)
1138+
bool Token::isUnaryPreOp() const
11391139
{
1140-
if (!op->astOperand1() || op->astOperand2())
1140+
if (!astOperand1() || astOperand2())
11411141
return false;
1142-
if (!Token::Match(op, "++|--"))
1142+
if (!Token::Match(this, "++|--"))
11431143
return true;
1144-
const Token *tok = op->astOperand1();
1144+
const Token *tokbefore = _previous;
1145+
const Token *tokafter = _next;
11451146
for (int distance = 1; distance < 10; distance++) {
1146-
if (tok == op->tokAt(-distance))
1147+
if (tokbefore == _astOperand1)
11471148
return false;
1148-
if (tok == op->tokAt(distance))
1149+
if (tokafter == _astOperand1)
11491150
return true;
1151+
tokbefore = tokbefore->_previous;
1152+
tokafter = tokafter->_previous;
11501153
}
11511154
return false; // <- guess
11521155
}
@@ -1158,7 +1161,7 @@ std::string Token::expressionString() const
11581161
while (start->astOperand1() && start->astOperand2())
11591162
start = start->astOperand1();
11601163
const Token *end = top;
1161-
while (end->astOperand1() && (end->astOperand2() || isUnaryPreOp(end))) {
1164+
while (end->astOperand1() && (end->astOperand2() || end->isUnaryPreOp())) {
11621165
if (Token::Match(end,"(|[")) {
11631166
end = end->link();
11641167
break;

‎lib/token.h‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -276,6 +276,7 @@ class CPPCHECKLIB Token {
276276
bool isBoolean() const {
277277
return _type == eBoolean;
278278
}
279+
bool isUnaryPreOp() const;
279280

280281
unsigned int flags() const {
281282
return _flags;

‎lib/valueflow.cpp‎

Lines changed: 45 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -147,16 +147,20 @@ static std::map<unsigned int, MathLib::bigint> getProgramMemory(const Token *tok
147147
const std::map<unsigned int, MathLib::bigint> programMemory1(programMemory);
148148
int indentlevel = 0;
149149
for (const Token *tok2 = tok; tok2; tok2 = tok2->previous()) {
150-
if (Token::Match(tok2, "[;{}] %var% = %num% ;")) {
151-
const Token *vartok = tok2->next();
152-
const Token *numtok = tok2->tokAt(3);
153-
if (programMemory.find(vartok->varId()) == programMemory.end())
154-
programMemory[vartok->varId()] = MathLib::toLongNumber(numtok->str());
155-
}
156150
if (Token::Match(tok2, "[;{}] %varid% = %var% ;", varid)) {
157151
const Token *vartok = tok2->tokAt(3);
158152
programMemory[vartok->varId()] = value.intvalue;
159153
}
154+
if (Token::Match(tok2, "[;{}] %var% =")) {
155+
const Token *vartok = tok2->next();
156+
if (programMemory.find(vartok->varId()) == programMemory.end()) {
157+
MathLib::bigint result = 0;
158+
bool error = false;
159+
execute(tok2->tokAt(2)->astOperand2(), &programMemory, &result, &error);
160+
if (!error)
161+
programMemory[vartok->varId()] = result;
162+
}
163+
}
160164
if (tok2->str() == "{") {
161165
if (indentlevel <= 0)
162166
break;
@@ -761,6 +765,16 @@ static void removeValues(std::list<ValueFlow::Value> &values, const std::list<Va
761765
}
762766
}
763767

768+
static void valueFlowAST(Token *tok, unsigned int varid, const ValueFlow::Value &value)
769+
{
770+
if (!tok)
771+
return;
772+
if (tok->varId() == varid)
773+
setTokenValue(tok, value);
774+
valueFlowAST(const_cast<Token*>(tok->astOperand1()), varid, value);
775+
valueFlowAST(const_cast<Token*>(tok->astOperand2()), varid, value);
776+
}
777+
764778
static bool valueFlowForward(Token * const startToken,
765779
const Token * const endToken,
766780
const Variable * const var,
@@ -1026,6 +1040,31 @@ static bool valueFlowForward(Token * const startToken,
10261040
else if (returnStatement && tok2->str() == ";")
10271041
return false;
10281042

1043+
// If a ? is seen and it's known that the condition is true/false..
1044+
else if (tok2->str() == "?") {
1045+
const Token *condition = tok2->astOperand1();
1046+
std::list<ValueFlow::Value>::const_iterator it;
1047+
for (it = values.begin(); it != values.end(); ++it) {
1048+
const std::map<unsigned int, MathLib::bigint> programMemory(getProgramMemory(tok2, varid, *it));
1049+
if (conditionIsTrue(condition, programMemory))
1050+
valueFlowAST(const_cast<Token*>(tok2->astOperand2()->astOperand1()), varid, *it);
1051+
else if (conditionIsFalse(condition, programMemory))
1052+
valueFlowAST(const_cast<Token*>(tok2->astOperand2()->astOperand2()), varid, *it);
1053+
else
1054+
valueFlowAST(const_cast<Token*>(tok2->astOperand2()), varid, *it);
1055+
}
1056+
// Skip conditional expressions..
1057+
while (tok2->astOperand1() || tok2->astOperand2()) {
1058+
if (tok2->astOperand2())
1059+
tok2 = const_cast<Token*>(tok2->astOperand2());
1060+
else if (tok2->isUnaryPreOp())
1061+
tok2 = const_cast<Token*>(tok2->astOperand1());
1062+
else
1063+
break;
1064+
}
1065+
tok2 = tok2->next();
1066+
}
1067+
10291068
if (tok2->varId() == varid) {
10301069
// bailout: assignment
10311070
if (Token::Match(tok2->previous(), "!!* %name% %op%") && tok2->next()->isAssignmentOp()) {

‎test/testnullpointer.cpp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2373,7 +2373,7 @@ class TestNullPointer : public TestFixture {
23732373
check("void f(int *p = 0) {\n"
23742374
" std::cout << p ? *p : 0;\n" // Due to operator precedence, this is equivalent to: (std::cout << p) ? *p : 0;
23752375
"}");
2376-
TODO_ASSERT_EQUALS("[test.cpp:2]: (warning) Possible null pointer dereference if the default parameter value is used: p\n", "", errout.str()); // Check the first branch of ternary
2376+
ASSERT_EQUALS("[test.cpp:2]: (warning) Possible null pointer dereference if the default parameter value is used: p\n", errout.str()); // Check the first branch of ternary
23772377

23782378
check("void f(char *p = 0) {\n"
23792379
" std::cout << p ? *p : 0;\n" // Due to operator precedence, this is equivalent to: (std::cout << p) ? *p : 0;

‎test/testvalueflow.cpp‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1449,6 +1449,16 @@ class TestValueFlow : public TestFixture {
14491449
" leaveNotifyEvent(0);\n"
14501450
"}";
14511451
testValueOfX(code, 2U, 2); // No complaint about Token::Match called with varid 0. (#6443)
1452+
1453+
// #6560 - multivariables
1454+
code = "void f1(int x) {\n"
1455+
" int a = x && y;\n"
1456+
" int b = a ? x : 0;\n"
1457+
"}\n"
1458+
"void f2() {\n"
1459+
" f1(0);\n"
1460+
"}";
1461+
ASSERT_EQUALS(false, testValueOfX(code, 3U, 0));
14521462
}
14531463

14541464
void valueFlowFunctionReturn() {

0 commit comments

Comments
 (0)