From 3d19b33c3ec1a1e13f3102de11ec99e46ce0c7f8 Mon Sep 17 00:00:00 2001
From: Paul Fultz II <pfultz2@yahoo.com>
Date: Mon, 2 Aug 2021 03:51:34 -0500
Subject: [PATCH] Fix 9948 and 10234: false negative: knownConditionTrueFalse
 and stlOutOfBounds (#3372)

---
 lib/valueflow.cpp      | 43 ++++++++++++++++++++++++++----------------
 test/testcondition.cpp |  6 ++++++
 test/teststl.cpp       |  7 +++++++
 3 files changed, 40 insertions(+), 16 deletions(-)

diff --git a/lib/valueflow.cpp b/lib/valueflow.cpp
index a436afae0..5fe9eebe8 100644
--- a/lib/valueflow.cpp
+++ b/lib/valueflow.cpp
@@ -2589,11 +2589,13 @@ static Analyzer::Result valueFlowForward(Token* startToken,
     return valueFlowForwardExpression(startToken, endToken, expr, values, tokenlist, settings);
 }
 
+// *INDENT-OFF*
 static Analyzer::Result valueFlowForward(Token* top,
-        const Token* exprTok,
-        const std::list<ValueFlow::Value>& values,
-        TokenList* const tokenlist,
-        const Settings* settings)
+                                         const Token* exprTok,
+                                         const std::list<ValueFlow::Value>& values,
+                                         TokenList* const tokenlist,
+                                         const Settings* settings)
+// *INDENT-ON*
 {
     Analyzer::Result result{};
     for (const ValueFlow::Value& v : values) {
@@ -4605,6 +4607,10 @@ struct ConditionHandler {
                     parent = nullptr;
                 }
                 if (parent) {
+                    std::vector<Token*> nextExprs = {parent->astOperand2()};
+                    if (astIsLHS(parent) && parent->astParent() && parent->astParent()->str() == parent->str()) {
+                        nextExprs.push_back(parent->astParent()->astOperand2());
+                    }
                     const std::string& op(parent->str());
                     std::list<ValueFlow::Value> values;
                     if (op == "&&")
@@ -4613,17 +4619,20 @@ struct ConditionHandler {
                         values = elseValues;
                     if (Token::Match(tok, "==|!=") || (tok == cond.vartok && astIsBool(tok)))
                         changePossibleToKnown(values);
+                    // *INDENT-OFF*
                     if (astIsFloat(cond.vartok, false) ||
                         (!cond.vartok->valueType() &&
-                    std::all_of(values.begin(), values.end(), [](const ValueFlow::Value& v) {
-                    return v.isIntValue() || v.isFloatValue();
-                    })))
-                    values.remove_if([&](const ValueFlow::Value& v) {
-                        return v.isImpossible();
-                    });
-                    Analyzer::Result r = forward(parent->astOperand2(), cond.vartok, values, tokenlist, settings);
-                    if (r.terminate != Analyzer::Terminate::None)
-                        return;
+                         std::all_of(values.begin(), values.end(), [](const ValueFlow::Value& v) {
+                             return v.isIntValue() || v.isFloatValue();
+                         })))
+                        values.remove_if([&](const ValueFlow::Value& v) { return v.isImpossible(); });
+                    for(Token* start:nextExprs) {
+                        Analyzer::Result r = forward(start, cond.vartok, values, tokenlist, settings);
+                        if (r.terminate != Analyzer::Terminate::None)
+                            return;
+                    }
+                    // *INDENT-ON*
+
                 }
             }
 
@@ -6179,10 +6188,12 @@ static Analyzer::Result valueFlowContainerForward(Token* startToken,
     return valueFlowGenericForward(startToken, endToken, a, tokenlist->getSettings());
 }
 
+// *INDENT-OFF*
 static Analyzer::Result valueFlowContainerForwardRecursive(Token* top,
-        const Token* exprTok,
-        const ValueFlow::Value& value,
-        TokenList* tokenlist)
+                                                           const Token* exprTok,
+                                                           const ValueFlow::Value& value,
+                                                           TokenList* tokenlist)
+// *INDENT-ON*
 {
     ContainerExpressionAnalyzer a(exprTok, value, tokenlist);
     return valueFlowGenericForward(top, a, tokenlist->getSettings());
diff --git a/test/testcondition.cpp b/test/testcondition.cpp
index acf6d7d5e..64b4cd9b6 100644
--- a/test/testcondition.cpp
+++ b/test/testcondition.cpp
@@ -3727,6 +3727,12 @@ private:
               "    }\n"
               "}\n");
         ASSERT_EQUALS("", errout.str());
+
+        // #9948
+        check("bool f(bool a, bool b) {\n"
+              "    return a || ! b || ! a;\n"
+              "}\n");
+        ASSERT_EQUALS("[test.cpp:2] -> [test.cpp:2]: (style) Condition '!a' is always true\n", errout.str());
     }
 
     void alwaysTrueInfer() {
diff --git a/test/teststl.cpp b/test/teststl.cpp
index 2e23cd0dd..3880c79a0 100644
--- a/test/teststl.cpp
+++ b/test/teststl.cpp
@@ -589,6 +589,13 @@ private:
                     "    return 0;\n"
                     "}\n");
         ASSERT_EQUALS("", errout.str());
+
+        checkNormal("void foo(const std::vector<int> &v) {\n"
+                    "    if(v.size() >=1 && v[0] == 4 && v[1] == 2){}\n"
+                    "}\n");
+        ASSERT_EQUALS("test.cpp:2:warning:Either the condition 'v.size()>=1' is redundant or v size can be 1. Expression 'v[1]' cause access out of bounds.\n"
+                      "test.cpp:2:note:condition 'v.size()>=1'\n"
+                      "test.cpp:2:note:Access out of bounds\n", errout.str());
     }
 
     void outOfBoundsIndexExpression() {