From 0832830a959b9463b70834bd52eccd710e787a1b Mon Sep 17 00:00:00 2001 From: Paul Fultz II Date: Sat, 23 May 2020 00:15:13 -0500 Subject: [PATCH] Fix issue 9721: ValueFlow: Comparison is always false, but ValueFlow says it is always true (#2658) --- lib/valueflow.cpp | 26 ++++++++++++++++++-------- test/testcondition.cpp | 9 +++++++++ 2 files changed, 27 insertions(+), 8 deletions(-) diff --git a/lib/valueflow.cpp b/lib/valueflow.cpp index cf784ddfc..29f38d1ee 100644 --- a/lib/valueflow.cpp +++ b/lib/valueflow.cpp @@ -4321,9 +4321,19 @@ static void valueFlowInferCondition(TokenList* tokenlist, } else if (tok->isComparisonOp()) { MathLib::bigint val = 0; const Token* varTok = nullptr; + std::string op = tok->str(); if (tok->astOperand1()->hasKnownIntValue()) { val = tok->astOperand1()->values().front().intvalue; varTok = tok->astOperand2(); + // Flip the operator + if (op == ">") + op = "<"; + else if (op == "<") + op = ">"; + else if (op == ">=") + op = "<="; + else if (op == "<=") + op = ">="; } else if (tok->astOperand2()->hasKnownIntValue()) { val = tok->astOperand2()->values().front().intvalue; varTok = tok->astOperand1(); @@ -4336,22 +4346,22 @@ static void valueFlowInferCondition(TokenList* tokenlist, continue; const ValueFlow::Value* result = nullptr; bool known = false; - if (Token::Match(tok, "==|!=")) { + if (op == "==" || op == "!=") { result = proveNotEqual(varTok->values(), val); - known = tok->str() == "!="; - } else if (Token::Match(tok, "<|>=")) { + known = op == "!="; + } else if (op == "<" || op == ">=") { result = proveLessThan(varTok->values(), val); - known = tok->str() == "<"; + known = op == "<"; if (!result && !isSaturated(val)) { result = proveGreaterThan(varTok->values(), val - 1); - known = tok->str() == ">="; + known = op == ">="; } - } else if (Token::Match(tok, ">|<=")) { + } else if (op == ">" || op == "<=") { result = proveGreaterThan(varTok->values(), val); - known = tok->str() == ">"; + known = op == ">"; if (!result && !isSaturated(val)) { result = proveLessThan(varTok->values(), val + 1); - known = tok->str() == "<="; + known = op == "<="; } } if (!result) diff --git a/test/testcondition.cpp b/test/testcondition.cpp index e7531a812..d02abd8ad 100644 --- a/test/testcondition.cpp +++ b/test/testcondition.cpp @@ -3507,6 +3507,15 @@ private: " return pos;\n" "}\n"); ASSERT_EQUALS("[test.cpp:2] -> [test.cpp:5]: (style) Condition 'pos>0' is always true\n", errout.str()); + + // #9721 + check("void f(int x) {\n" + " if (x > 127) {\n" + " if ( (x>255) || (-128>x) )\n" + " return;\n" + " }\n" + "}\n"); + ASSERT_EQUALS("[test.cpp:2] -> [test.cpp:3]: (style) Condition '-128>x' is always false\n", errout.str()); } void alwaysTrueContainer() {