From eb0db322ebeda9211a267586f2f064e5fa84cbc8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Marjam=C3=A4ki?= Date: Sun, 24 May 2015 17:02:00 +0200 Subject: [PATCH] Fixed #6560 (ValueFlow: handling ternary operator better in valueFlowSubFunction) --- lib/token.cpp | 17 ++++++++------ lib/token.h | 1 + lib/valueflow.cpp | 51 +++++++++++++++++++++++++++++++++++----- test/testnullpointer.cpp | 2 +- test/testvalueflow.cpp | 10 ++++++++ 5 files changed, 67 insertions(+), 14 deletions(-) diff --git a/lib/token.cpp b/lib/token.cpp index 77523d197..c0ddb9952 100644 --- a/lib/token.cpp +++ b/lib/token.cpp @@ -1135,18 +1135,21 @@ bool Token::isCalculation() const return true; } -static bool isUnaryPreOp(const Token *op) +bool Token::isUnaryPreOp() const { - if (!op->astOperand1() || op->astOperand2()) + if (!astOperand1() || astOperand2()) return false; - if (!Token::Match(op, "++|--")) + if (!Token::Match(this, "++|--")) return true; - const Token *tok = op->astOperand1(); + const Token *tokbefore = _previous; + const Token *tokafter = _next; for (int distance = 1; distance < 10; distance++) { - if (tok == op->tokAt(-distance)) + if (tokbefore == _astOperand1) return false; - if (tok == op->tokAt(distance)) + if (tokafter == _astOperand1) return true; + tokbefore = tokbefore->_previous; + tokafter = tokafter->_previous; } return false; // <- guess } @@ -1158,7 +1161,7 @@ std::string Token::expressionString() const while (start->astOperand1() && start->astOperand2()) start = start->astOperand1(); const Token *end = top; - while (end->astOperand1() && (end->astOperand2() || isUnaryPreOp(end))) { + while (end->astOperand1() && (end->astOperand2() || end->isUnaryPreOp())) { if (Token::Match(end,"(|[")) { end = end->link(); break; diff --git a/lib/token.h b/lib/token.h index 9e3cb5e48..b0321a420 100644 --- a/lib/token.h +++ b/lib/token.h @@ -276,6 +276,7 @@ public: bool isBoolean() const { return _type == eBoolean; } + bool isUnaryPreOp() const; unsigned int flags() const { return _flags; diff --git a/lib/valueflow.cpp b/lib/valueflow.cpp index 1549e2d31..7e8ce2ef4 100644 --- a/lib/valueflow.cpp +++ b/lib/valueflow.cpp @@ -147,16 +147,20 @@ static std::map getProgramMemory(const Token *tok const std::map programMemory1(programMemory); int indentlevel = 0; for (const Token *tok2 = tok; tok2; tok2 = tok2->previous()) { - if (Token::Match(tok2, "[;{}] %var% = %num% ;")) { - const Token *vartok = tok2->next(); - const Token *numtok = tok2->tokAt(3); - if (programMemory.find(vartok->varId()) == programMemory.end()) - programMemory[vartok->varId()] = MathLib::toLongNumber(numtok->str()); - } if (Token::Match(tok2, "[;{}] %varid% = %var% ;", varid)) { const Token *vartok = tok2->tokAt(3); programMemory[vartok->varId()] = value.intvalue; } + if (Token::Match(tok2, "[;{}] %var% =")) { + const Token *vartok = tok2->next(); + if (programMemory.find(vartok->varId()) == programMemory.end()) { + MathLib::bigint result = 0; + bool error = false; + execute(tok2->tokAt(2)->astOperand2(), &programMemory, &result, &error); + if (!error) + programMemory[vartok->varId()] = result; + } + } if (tok2->str() == "{") { if (indentlevel <= 0) break; @@ -761,6 +765,16 @@ static void removeValues(std::list &values, const std::listvarId() == varid) + setTokenValue(tok, value); + valueFlowAST(const_cast(tok->astOperand1()), varid, value); + valueFlowAST(const_cast(tok->astOperand2()), varid, value); +} + static bool valueFlowForward(Token * const startToken, const Token * const endToken, const Variable * const var, @@ -1026,6 +1040,31 @@ static bool valueFlowForward(Token * const startToken, else if (returnStatement && tok2->str() == ";") return false; + // If a ? is seen and it's known that the condition is true/false.. + else if (tok2->str() == "?") { + const Token *condition = tok2->astOperand1(); + std::list::const_iterator it; + for (it = values.begin(); it != values.end(); ++it) { + const std::map programMemory(getProgramMemory(tok2, varid, *it)); + if (conditionIsTrue(condition, programMemory)) + valueFlowAST(const_cast(tok2->astOperand2()->astOperand1()), varid, *it); + else if (conditionIsFalse(condition, programMemory)) + valueFlowAST(const_cast(tok2->astOperand2()->astOperand2()), varid, *it); + else + valueFlowAST(const_cast(tok2->astOperand2()), varid, *it); + } + // Skip conditional expressions.. + while (tok2->astOperand1() || tok2->astOperand2()) { + if (tok2->astOperand2()) + tok2 = const_cast(tok2->astOperand2()); + else if (tok2->isUnaryPreOp()) + tok2 = const_cast(tok2->astOperand1()); + else + break; + } + tok2 = tok2->next(); + } + if (tok2->varId() == varid) { // bailout: assignment if (Token::Match(tok2->previous(), "!!* %name% %op%") && tok2->next()->isAssignmentOp()) { diff --git a/test/testnullpointer.cpp b/test/testnullpointer.cpp index d3856fc99..539f9152b 100644 --- a/test/testnullpointer.cpp +++ b/test/testnullpointer.cpp @@ -2373,7 +2373,7 @@ private: check("void f(int *p = 0) {\n" " std::cout << p ? *p : 0;\n" // Due to operator precedence, this is equivalent to: (std::cout << p) ? *p : 0; "}"); - 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 + 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 check("void f(char *p = 0) {\n" " std::cout << p ? *p : 0;\n" // Due to operator precedence, this is equivalent to: (std::cout << p) ? *p : 0; diff --git a/test/testvalueflow.cpp b/test/testvalueflow.cpp index c9ec34653..b70580dc1 100644 --- a/test/testvalueflow.cpp +++ b/test/testvalueflow.cpp @@ -1449,6 +1449,16 @@ private: " leaveNotifyEvent(0);\n" "}"; testValueOfX(code, 2U, 2); // No complaint about Token::Match called with varid 0. (#6443) + + // #6560 - multivariables + code = "void f1(int x) {\n" + " int a = x && y;\n" + " int b = a ? x : 0;\n" + "}\n" + "void f2() {\n" + " f1(0);\n" + "}"; + ASSERT_EQUALS(false, testValueOfX(code, 3U, 0)); } void valueFlowFunctionReturn() {