Fix 11844: FP negativeIndex for known loop (#5282)

Author: pfultz2

lib/astutils.cpp

@@ -900,9 +900,16 @@ bool extractForLoopValues(const Token *forToken,
     if (!initExpr || !initExpr->isBinaryOp() || initExpr->str() != "=" || !Token::Match(initExpr->astOperand1(), "%var%"))
         return false;
     std::vector<MathLib::bigint> minInitValue = getMinValue(ValueFlow::makeIntegralInferModel(), initExpr->astOperand2()->values());
+    if (minInitValue.empty()) {
+        const ValueFlow::Value* v = initExpr->astOperand2()->getMinValue(true);
+        if (v)
+            minInitValue.push_back(v->intvalue);
+    }
+    if (minInitValue.empty())
+        return false;
     varid = initExpr->astOperand1()->varId();
     knownInitValue = initExpr->astOperand2()->hasKnownIntValue();
-    initValue = minInitValue.empty() ? 0 : minInitValue.front();
+    initValue = minInitValue.front();
     partialCond = Token::Match(condExpr, "%oror%|&&");
     visitAstNodes(condExpr, [varid, &condExpr](const Token *tok) {
         if (Token::Match(tok, "%oror%|&&"))

lib/token.cpp

@@ -2415,25 +2415,40 @@ const ValueFlow::Value* Token::getValue(const MathLib::bigint val) const
     return it == mImpl->mValues->end() ? nullptr : &*it;
 }
 
-const ValueFlow::Value* Token::getMaxValue(bool condition, MathLib::bigint path) const
+template<class Compare>
+static const ValueFlow::Value* getCompareValue(const std::list<ValueFlow::Value>& values,
+                                               bool condition,
+                                               MathLib::bigint path,
+                                               Compare compare)
 {
-    if (!mImpl->mValues)
-        return nullptr;
     const ValueFlow::Value* ret = nullptr;
-    for (const ValueFlow::Value& value : *mImpl->mValues) {
+    for (const ValueFlow::Value& value : values) {
         if (!value.isIntValue())
             continue;
         if (value.isImpossible())
             continue;
         if (path > -0 && value.path != 0 && value.path != path)
             continue;
-        if ((!ret || value.intvalue > ret->intvalue) &&
-            ((value.condition != nullptr) == condition))
+        if ((!ret || compare(value.intvalue, ret->intvalue)) && ((value.condition != nullptr) == condition))
             ret = &value;
     }
     return ret;
 }
 
+const ValueFlow::Value* Token::getMaxValue(bool condition, MathLib::bigint path) const
+{
+    if (!mImpl->mValues)
+        return nullptr;
+    return getCompareValue(*mImpl->mValues, condition, path, std::greater<MathLib::bigint>{});
+}
+
+const ValueFlow::Value* Token::getMinValue(bool condition, MathLib::bigint path) const
+{
+    if (!mImpl->mValues)
+        return nullptr;
+    return getCompareValue(*mImpl->mValues, condition, path, std::less<MathLib::bigint>{});
+}
+
 const ValueFlow::Value* Token::getMovedValue() const
 {
     if (!mImpl->mValues)

lib/token.h

@@ -1224,6 +1224,7 @@ class CPPCHECKLIB Token {
     const ValueFlow::Value* getValue(const MathLib::bigint val) const;
 
     const ValueFlow::Value* getMaxValue(bool condition, MathLib::bigint path = 0) const;
+    const ValueFlow::Value* getMinValue(bool condition, MathLib::bigint path = 0) const;
 
     const ValueFlow::Value* getMovedValue() const;
 

test/testbufferoverrun.cpp

@@ -190,6 +190,7 @@ class TestBufferOverrun : public TestFixture {
         TEST_CASE(array_index_negative7);    // #5685
         TEST_CASE(array_index_negative8);    // #11651
         TEST_CASE(array_index_negative9);
+        TEST_CASE(array_index_negative10);
         TEST_CASE(array_index_for_decr);
         TEST_CASE(array_index_varnames);     // FP: struct member #1576, FN: #1586
         TEST_CASE(array_index_for_continue); // for,continue
@@ -2336,6 +2337,20 @@ class TestBufferOverrun : public TestFixture {
         ASSERT_EQUALS("[test.cpp:8]: (error) Array 'a[3]' accessed at index -1, which is out of bounds.\n", errout.str());
     }
 
+    // #11844
+    void array_index_negative10()
+    {
+        check("struct S { int a[4]; };\n"
+              "void f(S* p, int k) {\n"
+              "  int m = 3;\n"
+              "  if (k)\n"
+              "    m = 2;\n"
+              "  for (int j = m + 1; j <= 4; j++)\n"
+              "    p->a[j-1];\n"
+              "}\n");
+        ASSERT_EQUALS("", errout.str());
+    }
+
     void array_index_for_decr() {
         check("void f()\n"
               "{\n"