Fix 9948 and 10234: false negative: knownConditionTrueFalse and stlOutOfBounds (#3372)

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 Token* exprTok,
-        const std::list<ValueFlow::Value>& values,
+                                         const std::list<ValueFlow::Value>& values,
-        TokenList* const tokenlist,
+                                         TokenList* const tokenlist,
-        const Settings* settings)
+                                         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) {
+                         std::all_of(values.begin(), values.end(), [](const ValueFlow::Value& v) {
-                    return v.isIntValue() || v.isFloatValue();
+                             return v.isIntValue() || v.isFloatValue();
-                    })))
+                         })))
-                    values.remove_if([&](const ValueFlow::Value& v) {
+                        values.remove_if([&](const ValueFlow::Value& v) { return v.isImpossible(); });
-                        return v.isImpossible();
+                    for(Token* start:nextExprs) {
-                    });
+                        Analyzer::Result r = forward(start, cond.vartok, values, tokenlist, settings);
-                    Analyzer::Result r = forward(parent->astOperand2(), cond.vartok, values, tokenlist, settings);
+                        if (r.terminate != Analyzer::Terminate::None)
-                    if (r.terminate != Analyzer::Terminate::None)
+                            return;
-                        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 Token* exprTok,
-        const ValueFlow::Value& value,
+                                                           const ValueFlow::Value& value,
-        TokenList* tokenlist)
+                                                           TokenList* tokenlist)
+// *INDENT-ON*
 {
     ContainerExpressionAnalyzer a(exprTok, value, tokenlist);
     return valueFlowGenericForward(top, a, tokenlist->getSettings());

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() {

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() {