diff --git a/lib/astutils.cpp b/lib/astutils.cpp index e69b78659..4acb370fa 100644 --- a/lib/astutils.cpp +++ b/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 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%|&&")) diff --git a/lib/token.cpp b/lib/token.cpp index 0e9184626..a1d2cfd6f 100644 --- a/lib/token.cpp +++ b/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 +static const ValueFlow::Value* getCompareValue(const std::list& 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{}); +} + +const ValueFlow::Value* Token::getMinValue(bool condition, MathLib::bigint path) const +{ + if (!mImpl->mValues) + return nullptr; + return getCompareValue(*mImpl->mValues, condition, path, std::less{}); +} + const ValueFlow::Value* Token::getMovedValue() const { if (!mImpl->mValues) diff --git a/lib/token.h b/lib/token.h index d04d365d4..4163ab754 100644 --- a/lib/token.h +++ b/lib/token.h @@ -1224,6 +1224,7 @@ public: 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; diff --git a/test/testbufferoverrun.cpp b/test/testbufferoverrun.cpp index 862add419..17a0898bd 100644 --- a/test/testbufferoverrun.cpp +++ b/test/testbufferoverrun.cpp @@ -190,6 +190,7 @@ private: 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 @@ private: 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"