From 772a76725264dbac082239fb5f016d491568795c Mon Sep 17 00:00:00 2001 From: Paul Date: Mon, 31 Jul 2023 10:31:18 -0500 Subject: [PATCH 1/3] Fix 11844: FP negativeIndex for known loop --- lib/astutils.cpp | 9 ++++++++- lib/token.cpp | 23 ++++++++++++++++++----- lib/token.h | 1 + test/testbufferoverrun.cpp | 15 +++++++++++++++ 4 files changed, 42 insertions(+), 6 deletions(-) diff --git a/lib/astutils.cpp b/lib/astutils.cpp index 94307ab276f..baa1d3188f6 100644 --- a/lib/astutils.cpp +++ b/lib/astutils.cpp @@ -888,9 +888,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 f24dc6f2773..44862d6f8f5 100644 --- a/lib/token.cpp +++ b/lib/token.cpp @@ -2421,25 +2421,38 @@ 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) && + 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::greater<>{}); +} + const ValueFlow::Value* Token::getMovedValue() const { if (!mImpl->mValues) diff --git a/lib/token.h b/lib/token.h index 5864a0db972..1460709b402 100644 --- a/lib/token.h +++ b/lib/token.h @@ -1245,6 +1245,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; diff --git a/test/testbufferoverrun.cpp b/test/testbufferoverrun.cpp index c12bb4467a2..b16e790570a 100644 --- a/test/testbufferoverrun.cpp +++ b/test/testbufferoverrun.cpp @@ -191,6 +191,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 @@ -2337,6 +2338,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" From 39e6cc05f20524f251a2d97863dce02bc9ac7bc4 Mon Sep 17 00:00:00 2001 From: Paul Date: Mon, 31 Jul 2023 10:32:08 -0500 Subject: [PATCH 2/3] Format --- lib/astutils.cpp | 4 ++-- lib/token.cpp | 8 +++++--- test/testbufferoverrun.cpp | 14 +++++++------- 3 files changed, 14 insertions(+), 12 deletions(-) diff --git a/lib/astutils.cpp b/lib/astutils.cpp index baa1d3188f6..549e92c74f9 100644 --- a/lib/astutils.cpp +++ b/lib/astutils.cpp @@ -890,10 +890,10 @@ bool extractForLoopValues(const Token *forToken, std::vector minInitValue = getMinValue(ValueFlow::makeIntegralInferModel(), initExpr->astOperand2()->values()); if (minInitValue.empty()) { const ValueFlow::Value* v = initExpr->astOperand2()->getMinValue(true); - if(v) + if (v) minInitValue.push_back(v->intvalue); } - if(minInitValue.empty()) + if (minInitValue.empty()) return false; varid = initExpr->astOperand1()->varId(); knownInitValue = initExpr->astOperand2()->hasKnownIntValue(); diff --git a/lib/token.cpp b/lib/token.cpp index 44862d6f8f5..8b5f20b9d91 100644 --- a/lib/token.cpp +++ b/lib/token.cpp @@ -2422,7 +2422,10 @@ const ValueFlow::Value* Token::getValue(const MathLib::bigint val) const } template -static const ValueFlow::Value* getCompareValue(const std::list& values, bool condition, MathLib::bigint path, Compare compare) +static const ValueFlow::Value* getCompareValue(const std::list& values, + bool condition, + MathLib::bigint path, + Compare compare) { const ValueFlow::Value* ret = nullptr; for (const ValueFlow::Value& value : values) { @@ -2432,8 +2435,7 @@ static const ValueFlow::Value* getCompareValue(const std::list continue; if (path > -0 && value.path != 0 && value.path != path) continue; - if ((!ret || compare(value.intvalue, ret->intvalue)) && - ((value.condition != nullptr) == condition)) + if ((!ret || compare(value.intvalue, ret->intvalue)) && ((value.condition != nullptr) == condition)) ret = &value; } return ret; diff --git a/test/testbufferoverrun.cpp b/test/testbufferoverrun.cpp index b16e790570a..efdede69dff 100644 --- a/test/testbufferoverrun.cpp +++ b/test/testbufferoverrun.cpp @@ -2342,13 +2342,13 @@ class TestBufferOverrun : public TestFixture { 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"); + "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()); } From 8f7fec8446444679439636689e31a2b62c2baa4d Mon Sep 17 00:00:00 2001 From: Paul Date: Sun, 13 Aug 2023 17:42:49 -0500 Subject: [PATCH 3/3] Fix compilation error --- lib/token.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/token.cpp b/lib/token.cpp index 8b5f20b9d91..0afb01572eb 100644 --- a/lib/token.cpp +++ b/lib/token.cpp @@ -2445,14 +2445,14 @@ const ValueFlow::Value* Token::getMaxValue(bool condition, MathLib::bigint path) { if (!mImpl->mValues) return nullptr; - return getCompareValue(*mImpl->mValues, condition, path, std::greater<>{}); + 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::greater<>{}); + return getCompareValue(*mImpl->mValues, condition, path, std::less{}); } const ValueFlow::Value* Token::getMovedValue() const