From 065412facadbfb9d9f37f91ec0ce3163929f093a Mon Sep 17 00:00:00 2001 From: Paul Date: Mon, 13 Mar 2023 13:53:43 -0500 Subject: [PATCH 1/5] Add impossible values in condition processing --- lib/valueflow.cpp | 60 +++++++++++++++++++++++++++++------------------ 1 file changed, 37 insertions(+), 23 deletions(-) diff --git a/lib/valueflow.cpp b/lib/valueflow.cpp index 3bf26415d1d..7ba291e2efb 100644 --- a/lib/valueflow.cpp +++ b/lib/valueflow.cpp @@ -5970,6 +5970,36 @@ struct ConditionHandler { return findPath(true_values) | findPath(false_values); } + Token* getContextAndValues(Token* condTok, std::list& thenValues, std::list& elseValues, bool known=false) const + { + const MathLib::bigint path = getPath(); + const bool allowKnown = path == 0; + const bool allowImpossible = impossible && allowKnown; + + bool inverted2 = inverted; + Token* ctx = skipNotAndCasts(condTok, &inverted2); + bool then = inverted2 ? !inverted2 : true; + + if (!Token::Match(condTok, "!=|=|(|.") && condTok != vartok) { + thenValues.insert(thenValues.end(), true_values.cbegin(), true_values.cend()); + if (allowImpossible && (known || isConditionKnown(ctx, !then))) + insertImpossible(elseValues, false_values); + } + if (!Token::Match(condTok, "==|!")) { + elseValues.insert(elseValues.end(), false_values.cbegin(), false_values.cend()); + if (allowImpossible && (known || isConditionKnown(ctx, then))) { + insertImpossible(thenValues, true_values); + if (isBool()) + insertNegateKnown(thenValues, true_values); + } + } + + if (inverted2) + std::swap(thenValues, elseValues); + + return ctx; + } + Condition() : vartok(nullptr), true_values(), false_values(), inverted(false), impossible(true) {} }; @@ -6198,31 +6228,11 @@ struct ConditionHandler { const MathLib::bigint path = cond.getPath(); const bool allowKnown = path == 0; - const bool allowImpossible = cond.impossible && allowKnown; std::list thenValues; std::list elseValues; - bool inverted = cond.inverted; - Token* ctx = skipNotAndCasts(condTok, &inverted); - bool then = cond.inverted ? !inverted : true; - - if (!Token::Match(condTok, "!=|=|(|.") && condTok != cond.vartok) { - thenValues.insert(thenValues.end(), cond.true_values.cbegin(), cond.true_values.cend()); - if (allowImpossible && isConditionKnown(ctx, !then)) - insertImpossible(elseValues, cond.false_values); - } - if (!Token::Match(condTok, "==|!")) { - elseValues.insert(elseValues.end(), cond.false_values.cbegin(), cond.false_values.cend()); - if (allowImpossible && isConditionKnown(ctx, then)) { - insertImpossible(thenValues, cond.true_values); - if (cond.isBool()) - insertNegateKnown(thenValues, cond.true_values); - } - } - - if (inverted) - std::swap(thenValues, elseValues); + Token* ctx = cond.getContextAndValues(condTok, thenValues, elseValues); if (Token::Match(ctx->astParent(), "%oror%|&&")) { Token* parent = ctx->astParent(); @@ -6237,12 +6247,16 @@ struct ConditionHandler { if (astIsLHS(parent) && parent->astParent() && parent->astParent()->str() == parent->str()) { nextExprs.push_back(parent->astParent()->astOperand2()); } + std::list andValues; + std::list orValues; + cond.getContextAndValues(condTok, andValues, orValues, true); + const std::string& op(parent->str()); std::list values; if (op == "&&") - values = thenValues; + values = andValues; else if (op == "||") - values = elseValues; + values = orValues; if (allowKnown && (Token::Match(condTok, "==|!=") || cond.isBool())) changePossibleToKnown(values); if (astIsFloat(cond.vartok, false) || From ca8e12d587d0ba138cc51247e271d189df0dc2d8 Mon Sep 17 00:00:00 2001 From: Paul Date: Mon, 13 Mar 2023 13:58:53 -0500 Subject: [PATCH 2/5] Fix FP --- lib/valueflow.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/valueflow.cpp b/lib/valueflow.cpp index 7ba291e2efb..5b1bf722629 100644 --- a/lib/valueflow.cpp +++ b/lib/valueflow.cpp @@ -5978,7 +5978,7 @@ struct ConditionHandler { bool inverted2 = inverted; Token* ctx = skipNotAndCasts(condTok, &inverted2); - bool then = inverted2 ? !inverted2 : true; + bool then = inverted ? !inverted2 : true; if (!Token::Match(condTok, "!=|=|(|.") && condTok != vartok) { thenValues.insert(thenValues.end(), true_values.cbegin(), true_values.cend()); From ee1c9754cd6701ec321d6d7459b5663aa1166b48 Mon Sep 17 00:00:00 2001 From: Paul Date: Mon, 13 Mar 2023 14:16:13 -0500 Subject: [PATCH 3/5] Add test --- test/teststl.cpp | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/test/teststl.cpp b/test/teststl.cpp index 9766c0d1ac9..551d70d2950 100644 --- a/test/teststl.cpp +++ b/test/teststl.cpp @@ -4740,6 +4740,16 @@ class TestStl : public TestFixture { " return debug_valueflow(it)->second;\n" "}\n"); ASSERT_EQUALS("", errout.str()); + + // #11557 + check("bool f(const std::vector& v, std::vector::iterator it, bool b) {\n" + " if (it == v.end())\n" + " return false;\n" + " if (b && ((it + 1) == v.end() || (*(it + 1)) != nullptr))\n" + " return false;\n" + " return true;\n" + "}\n"); + ASSERT_EQUALS("", errout.str()); } void dereferenceInvalidIterator2() { From 8e103085bd7cf07e04d696ce33779dd28c3ece7c Mon Sep 17 00:00:00 2001 From: Paul Date: Mon, 13 Mar 2023 14:16:26 -0500 Subject: [PATCH 4/5] Format --- lib/valueflow.cpp | 5 ++++- test/teststl.cpp | 12 ++++++------ 2 files changed, 10 insertions(+), 7 deletions(-) diff --git a/lib/valueflow.cpp b/lib/valueflow.cpp index 5b1bf722629..2a403c7dc27 100644 --- a/lib/valueflow.cpp +++ b/lib/valueflow.cpp @@ -5970,7 +5970,10 @@ struct ConditionHandler { return findPath(true_values) | findPath(false_values); } - Token* getContextAndValues(Token* condTok, std::list& thenValues, std::list& elseValues, bool known=false) const + Token* getContextAndValues(Token* condTok, + std::list& thenValues, + std::list& elseValues, + bool known = false) const { const MathLib::bigint path = getPath(); const bool allowKnown = path == 0; diff --git a/test/teststl.cpp b/test/teststl.cpp index 551d70d2950..90f7ffa6b80 100644 --- a/test/teststl.cpp +++ b/test/teststl.cpp @@ -4743,12 +4743,12 @@ class TestStl : public TestFixture { // #11557 check("bool f(const std::vector& v, std::vector::iterator it, bool b) {\n" - " if (it == v.end())\n" - " return false;\n" - " if (b && ((it + 1) == v.end() || (*(it + 1)) != nullptr))\n" - " return false;\n" - " return true;\n" - "}\n"); + " if (it == v.end())\n" + " return false;\n" + " if (b && ((it + 1) == v.end() || (*(it + 1)) != nullptr))\n" + " return false;\n" + " return true;\n" + "}\n"); ASSERT_EQUALS("", errout.str()); } From 74eff8dc0e1923ed16862e2b0f144c70e75dbf76 Mon Sep 17 00:00:00 2001 From: Paul Date: Wed, 15 Mar 2023 21:02:22 -0500 Subject: [PATCH 5/5] Simplify boolean expression --- lib/valueflow.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/valueflow.cpp b/lib/valueflow.cpp index 2a403c7dc27..36f2bef9fd7 100644 --- a/lib/valueflow.cpp +++ b/lib/valueflow.cpp @@ -5981,7 +5981,7 @@ struct ConditionHandler { bool inverted2 = inverted; Token* ctx = skipNotAndCasts(condTok, &inverted2); - bool then = inverted ? !inverted2 : true; + bool then = !inverted || !inverted2; if (!Token::Match(condTok, "!=|=|(|.") && condTok != vartok) { thenValues.insert(thenValues.end(), true_values.cbegin(), true_values.cend());