diff --git a/lib/analyzer.h b/lib/analyzer.h index 224a1c877..dd4fecdb1 100644 --- a/lib/analyzer.h +++ b/lib/analyzer.h @@ -20,6 +20,7 @@ #define analyzerH #include "config.h" +#include "mathlib.h" #include #include #include @@ -155,8 +156,9 @@ struct Analyzer { /// Update the state of the value virtual void update(Token* tok, Action a, Direction d) = 0; /// Try to evaluate the value of a token(most likely a condition) - virtual std::vector evaluate(Evaluate e, const Token* tok, const Token* ctx = nullptr) const = 0; - std::vector evaluate(const Token* tok, const Token* ctx = nullptr) const { + virtual std::vector evaluate(Evaluate e, const Token* tok, const Token* ctx = nullptr) const = 0; + std::vector evaluate(const Token* tok, const Token* ctx = nullptr) const + { return evaluate(Evaluate::Integral, tok, ctx); } /// Lower any values to possible diff --git a/lib/forwardanalyzer.cpp b/lib/forwardanalyzer.cpp index 7a717e879..46575479d 100644 --- a/lib/forwardanalyzer.cpp +++ b/lib/forwardanalyzer.cpp @@ -74,7 +74,7 @@ struct ForwardTraversal { std::pair evalCond(const Token* tok, const Token* ctx = nullptr) const { if (!tok) return std::make_pair(false, false); - std::vector result = analyzer->evaluate(tok, ctx); + std::vector result = analyzer->evaluate(tok, ctx); // TODO: We should convert to bool bool checkThen = std::any_of(result.begin(), result.end(), [](int x) { return x != 0; @@ -600,7 +600,8 @@ struct ForwardTraversal { if (conTok && updateRecursive(conTok) == Progress::Break) return Break(); bool isEmpty = false; - std::vector result = analyzer->evaluate(Analyzer::Evaluate::ContainerEmpty, conTok); + std::vector result = + analyzer->evaluate(Analyzer::Evaluate::ContainerEmpty, conTok); if (result.empty()) analyzer->assume(conTok, false, Analyzer::Assume::ContainerEmpty); else diff --git a/lib/reverseanalyzer.cpp b/lib/reverseanalyzer.cpp index 1ecedc8e4..56927ee11 100644 --- a/lib/reverseanalyzer.cpp +++ b/lib/reverseanalyzer.cpp @@ -18,7 +18,7 @@ struct ReverseTraversal { const Settings* settings; std::pair evalCond(const Token* tok) { - std::vector result = analyzer->evaluate(tok); + std::vector result = analyzer->evaluate(tok); // TODO: We should convert to bool bool checkThen = std::any_of(result.begin(), result.end(), [](int x) { return x == 1; diff --git a/lib/valueflow.cpp b/lib/valueflow.cpp index 3b80b934f..35d8d532d 100644 --- a/lib/valueflow.cpp +++ b/lib/valueflow.cpp @@ -803,11 +803,11 @@ static void setTokenValue(Token* tok, ValueFlow::Value value, const Settings* se // increment else if (parent->str() == "++") { for (const ValueFlow::Value &val : tok->values()) { - if (!val.isIntValue() && !val.isFloatValue()) + if (!val.isIntValue() && !val.isFloatValue() && !val.isSymbolicValue()) continue; ValueFlow::Value v(val); if (parent == tok->previous()) { - if (v.isIntValue()) + if (v.isIntValue() || v.isSymbolicValue()) v.intvalue = v.intvalue + 1; else v.floatValue = v.floatValue + 1.0; @@ -819,11 +819,11 @@ static void setTokenValue(Token* tok, ValueFlow::Value value, const Settings* se // decrement else if (parent->str() == "--") { for (const ValueFlow::Value &val : tok->values()) { - if (!val.isIntValue() && !val.isFloatValue()) + if (!val.isIntValue() && !val.isFloatValue() && !val.isSymbolicValue()) continue; ValueFlow::Value v(val); if (parent == tok->previous()) { - if (v.isIntValue()) + if (v.isIntValue() || v.isSymbolicValue()) v.intvalue = v.intvalue - 1; else v.floatValue = v.floatValue - 1.0; @@ -2080,7 +2080,7 @@ struct ValueFlowAnalyzer : Analyzer { // Check if its assigned to the same value if (value && !value->isImpossible() && Token::simpleMatch(tok->astParent(), "=") && astIsLHS(tok) && astIsIntegral(tok->astParent()->astOperand2(), false)) { - std::vector result = evaluate(Evaluate::Integral, tok->astParent()->astOperand2()); + std::vector result = evaluate(Evaluate::Integral, tok->astParent()->astOperand2()); if (!result.empty() && value->equalTo(result.front())) return Action::Idempotent; } @@ -2188,22 +2188,48 @@ struct ValueFlowAnalyzer : Analyzer { return true; } - bool isSameSymbolicValue(const Token* tok, ErrorPath* errorPath = nullptr) const + const Token* findMatch(const Token* tok) const + { + return findAstNode(tok, [&](const Token* child) { + return match(child); + }); + } + + bool isSameSymbolicValue(const Token* tok, ValueFlow::Value* value = nullptr) const { if (!useSymbolicValues()) return false; if (Token::Match(tok, "%assign%")) return false; + const ValueFlow::Value* currValue = getValue(tok); + if (!currValue) + return false; + const bool exact = !currValue->isIntValue() || currValue->isImpossible(); for (const ValueFlow::Value& v : tok->values()) { if (!v.isSymbolicValue()) continue; - if (!v.isKnown()) + const bool toImpossible = v.isImpossible() && currValue->isKnown(); + if (!v.isKnown() && !toImpossible) continue; - if (v.intvalue != 0) + if (exact && v.intvalue != 0) continue; + std::vector r; + ValueFlow::Value::Bound bound = currValue->bound; if (match(v.tokvalue)) { - if (errorPath) - errorPath->insert(errorPath->end(), v.errorPath.begin(), v.errorPath.end()); + r = {currValue->intvalue}; + } else if (!exact && findMatch(v.tokvalue)) { + r = evaluate(Evaluate::Integral, v.tokvalue, tok); + if (bound == ValueFlow::Value::Bound::Point) + bound = v.bound; + } + if (!r.empty()) { + if (value) { + value->errorPath.insert(value->errorPath.end(), v.errorPath.begin(), v.errorPath.end()); + value->intvalue = r.front() + v.intvalue; + if (toImpossible) + value->setImpossible(); + value->bound = bound; + } return true; } } @@ -2302,11 +2328,12 @@ struct ValueFlowAnalyzer : Analyzer { return Action::None; } - virtual std::vector evaluate(Evaluate e, const Token* tok, const Token* ctx = nullptr) const OVERRIDE { + virtual std::vector evaluate(Evaluate e, const Token* tok, const Token* ctx = nullptr) const OVERRIDE + { if (e == Evaluate::Integral) { if (tok->hasKnownIntValue()) return {static_cast(tok->values().front().intvalue)}; - std::vector result; + std::vector result; ProgramMemory pm = pms.get(tok, ctx, getProgramState()); if (Token::Match(tok, "&&|%oror%")) { if (conditionIsTrue(tok, pm)) @@ -2384,7 +2411,7 @@ struct ValueFlowAnalyzer : Analyzer { // Make a copy of the value to modify it localValue = *value; value = &localValue; - isSameSymbolicValue(tok, &value->errorPath); + isSameSymbolicValue(tok, &localValue); } // Read first when moving forward if (d == Direction::Forward && a.isRead()) diff --git a/test/testcondition.cpp b/test/testcondition.cpp index 4c1843f79..9281961cf 100644 --- a/test/testcondition.cpp +++ b/test/testcondition.cpp @@ -3826,6 +3826,16 @@ private: "}\n"); ASSERT_EQUALS("[test.cpp:2] -> [test.cpp:4]: (style) Condition 'z<1' is always false\n", errout.str()); + check("bool f(int &index, const int s, const double * const array, double & x) {\n" + " if (index >= s)\n" + " return false;\n" + " else {\n" + " x = array[index];\n" + " return (index++) >= s;\n" + " }\n" + "}\n"); + ASSERT_EQUALS("[test.cpp:2] -> [test.cpp:6]: (style) Condition '(index++)>=s' is always false\n", errout.str()); + check("struct a {\n" " a *b() const;\n" "} c;\n" diff --git a/test/testvalueflow.cpp b/test/testvalueflow.cpp index edd3577cd..ebffa5cc2 100644 --- a/test/testvalueflow.cpp +++ b/test/testvalueflow.cpp @@ -6357,6 +6357,77 @@ private: "}"; ASSERT_EQUALS(true, testValueOfX(code, 3U, "y", 0)); ASSERT_EQUALS(true, testValueOfXImpossible(code, 3U, "y", -1)); + + code = "void f(int y) {\n" + " int x = y - 1;\n" + " if (y == 1)\n" + " int a = x;\n" + "}\n"; + ASSERT_EQUALS(true, testValueOfXKnown(code, 4U, 0)); + + code = "void f(int y) {\n" + " int x = y * y;\n" + " if (y == 2)\n" + " int a = x;\n" + "}\n"; + ASSERT_EQUALS(true, testValueOfXKnown(code, 4U, 4)); + + code = "void f(int x, int y) {\n" + " if (x == y*y)\n" + " if (y == 2)\n" + " int a = x;\n" + "}\n"; + ASSERT_EQUALS(true, testValueOfXKnown(code, 4U, 4)); + + code = "void f(int x, int y) {\n" + " if (x > y*y)\n" + " if (y == 2)\n" + " int a = x;\n" + "}\n"; + ASSERT_EQUALS(true, testValueOfXImpossible(code, 4U, 4)); + + code = "void f(int x, int y) {\n" + " if (x != y*y)\n" + " if (y == 2)\n" + " int a = x;\n" + "}\n"; + ASSERT_EQUALS(true, testValueOfXImpossible(code, 4U, 4)); + + code = "void f(int x, int y) {\n" + " if (x >= y*y)\n" + " if (y == 2)\n" + " int a = x;\n" + "}\n"; + ASSERT_EQUALS(true, testValueOfXImpossible(code, 4U, 3)); + + code = "void f(int x, int y) {\n" + " if (x == y*y)\n" + " if (y != 2)\n" + " int a = x;\n" + "}\n"; + TODO_ASSERT_EQUALS(true, false, testValueOfXImpossible(code, 4U, 4)); + + code = "void f(int x, int y) {\n" + " if (x == y*y)\n" + " if (y > 2)\n" + " int a = x;\n" + "}\n"; + ASSERT_EQUALS(true, testValueOfX(code, 4U, 9)); + + code = "struct A {\n" + " A* b();\n" + " int c() const;\n" + "};\n" + "void f(A *d) {\n" + " if (!d || d->c() != 1)\n" + " return;\n" + " A * y = d;\n" + " d = d->b();\n" + " A * x = d;\n" + " A* z = x;\n" + "}\n"; + ASSERT_EQUALS(true, testValueOfX(code, 11U, "d", 0)); + ASSERT_EQUALS(false, testValueOfXImpossible(code, 11U, 0)); } void valueFlowSmartPointer()