From cac1cbf5d68a3b8d27dddca853157001cca7c853 Mon Sep 17 00:00:00 2001 From: chrchr-github Date: Wed, 8 Feb 2023 23:21:32 +0100 Subject: [PATCH 1/9] Partial fix for #11543 checkLibraryFunction warning for smartpointer in container --- lib/symboldatabase.cpp | 23 +++++++++++++++-------- test/testfunctions.cpp | 11 +++++++++++ 2 files changed, 26 insertions(+), 8 deletions(-) diff --git a/lib/symboldatabase.cpp b/lib/symboldatabase.cpp index f1b68d4268b..0abf706ef7e 100644 --- a/lib/symboldatabase.cpp +++ b/lib/symboldatabase.cpp @@ -1323,7 +1323,7 @@ void SymbolDatabase::createSymbolDatabaseSetVariablePointers() Token* memberTok = tok->next()->link()->tokAt(2); const Scope* scope = vt->containerTypeToken->scope(); const Type* contType{}; - const std::string typeStr = vt->containerTypeToken->expressionString(); + const std::string& typeStr = vt->containerTypeToken->str(); // TODO: handle complex type expressions while (scope && !contType) { contType = scope->findType(typeStr); // find the type stored in the container scope = scope->nestedIn; @@ -2272,7 +2272,7 @@ void Variable::setValueType(const ValueType &valueType) setFlag(fIsSmartPointer, true); } -const Type *Variable::smartPointerType() const +const Type* Variable::smartPointerType() const { if (!isSmartPointer()) return nullptr; @@ -2280,12 +2280,19 @@ const Type *Variable::smartPointerType() const if (mValueType->smartPointerType) return mValueType->smartPointerType; - // TODO: Cache result - const Token *ptrType = typeStartToken(); - while (Token::Match(ptrType, "%name%|::")) - ptrType = ptrType->next(); - if (Token::Match(ptrType, "< %name% >")) - return ptrType->scope()->findType(ptrType->next()->str()); + // TODO: Cache result, handle more complex type expression + const Token* typeTok = typeStartToken(); + while (Token::Match(typeTok, "%name%|::")) + typeTok = typeTok->next(); + if (Token::Match(typeTok, "< %name% >")) { + const Scope* scope = typeTok->scope(); + const Type* ptrType{}; + while (scope && !ptrType) { + ptrType = scope->findType(typeTok->next()->str()); + scope = scope->nestedIn; + } + return ptrType; + } return nullptr; } diff --git a/test/testfunctions.cpp b/test/testfunctions.cpp index a79715dfd71..36a1bdf4e7f 100644 --- a/test/testfunctions.cpp +++ b/test/testfunctions.cpp @@ -1962,6 +1962,17 @@ class TestFunctions : public TestFixture { "}\n"); ASSERT_EQUALS("", errout.str()); + check("struct S {\n" // #11543 + " S() {}\n" + " std::vector> v;\n" + " void f(int i) const;\n" + "};\n" + "void S::f(int i) const {\n" + " for (const std::shared_ptr& c : v)\n" + " c->f(i);\n" + "}\n"); + ASSERT_EQUALS("", errout.str()); + settings = settings_old; } From 561e7779787fe023985ddb922d5283cd6ceb1f9e Mon Sep 17 00:00:00 2001 From: chrchr-github Date: Sat, 11 Feb 2023 20:51:19 +0100 Subject: [PATCH 2/9] Fix #11553 pop_back on empty container is UB --- cfg/std.cfg | 2 +- lib/checkstl.cpp | 7 +++++-- test/cfg/std.cpp | 10 ++++++++++ 3 files changed, 16 insertions(+), 3 deletions(-) diff --git a/cfg/std.cfg b/cfg/std.cfg index 2737d69f17b..f542c9813b9 100644 --- a/cfg/std.cfg +++ b/cfg/std.cfg @@ -8570,7 +8570,6 @@ initializer list (7) string& replace (const_iterator i1, const_iterator i2, init - @@ -8590,6 +8589,7 @@ initializer list (7) string& replace (const_iterator i1, const_iterator i2, init + diff --git a/lib/checkstl.cpp b/lib/checkstl.cpp index b5b441aaea5..5ec1344046f 100644 --- a/lib/checkstl.cpp +++ b/lib/checkstl.cpp @@ -87,12 +87,15 @@ static bool containerAppendsElement(const Library::Container* container, const T return false; } -static bool containerYieldsElement(const Library::Container* container, const Token* parent) +static bool isContainerAccess(const Library::Container* container, const Token* parent) { if (Token::Match(parent, ". %name% (")) { const Library::Container::Yield yield = container->getYield(parent->strAt(1)); if (isElementAccessYield(yield)) return true; + const Library::Container::Action action = container->getAction(parent->strAt(1)); + if (contains({ Library::Container::Action::POP }, action)) + return true; } return false; } @@ -148,7 +151,7 @@ void CheckStl::outOfBounds() continue; if (!value.errorSeverity() && !mSettings->severity.isEnabled(Severity::warning)) continue; - if (value.intvalue == 0 && (indexTok || (containerYieldsElement(container, parent) && + if (value.intvalue == 0 && (indexTok || (isContainerAccess(container, parent) && !containerAppendsElement(container, parent)))) { std::string indexExpr; if (indexTok && !indexTok->hasKnownValue()) diff --git a/test/cfg/std.cpp b/test/cfg/std.cpp index 00734726ad2..894a3ac8d36 100644 --- a/test/cfg/std.cpp +++ b/test/cfg/std.cpp @@ -35,6 +35,7 @@ #include #include #include +#include #include #include #include @@ -4508,6 +4509,15 @@ void stdvector() v.front(); } +void stdcontainer_pop_empty() { // #11553 + std::vector v; + // cppcheck-suppress containerOutOfBounds + v.pop_back(); + std::list l; + // cppcheck-suppress containerOutOfBounds + l.pop_front(); +} + void stdbind_helper(int a) { printf("%d", a); From ee2d6546c358c24689bdcf4c6e218e0285fb6e21 Mon Sep 17 00:00:00 2001 From: chrchr-github Date: Sat, 11 Feb 2023 21:05:57 +0100 Subject: [PATCH 3/9] Revert --- lib/symboldatabase.cpp | 23 ++++++++--------------- test/testfunctions.cpp | 11 ----------- 2 files changed, 8 insertions(+), 26 deletions(-) diff --git a/lib/symboldatabase.cpp b/lib/symboldatabase.cpp index 0abf706ef7e..f1b68d4268b 100644 --- a/lib/symboldatabase.cpp +++ b/lib/symboldatabase.cpp @@ -1323,7 +1323,7 @@ void SymbolDatabase::createSymbolDatabaseSetVariablePointers() Token* memberTok = tok->next()->link()->tokAt(2); const Scope* scope = vt->containerTypeToken->scope(); const Type* contType{}; - const std::string& typeStr = vt->containerTypeToken->str(); // TODO: handle complex type expressions + const std::string typeStr = vt->containerTypeToken->expressionString(); while (scope && !contType) { contType = scope->findType(typeStr); // find the type stored in the container scope = scope->nestedIn; @@ -2272,7 +2272,7 @@ void Variable::setValueType(const ValueType &valueType) setFlag(fIsSmartPointer, true); } -const Type* Variable::smartPointerType() const +const Type *Variable::smartPointerType() const { if (!isSmartPointer()) return nullptr; @@ -2280,19 +2280,12 @@ const Type* Variable::smartPointerType() const if (mValueType->smartPointerType) return mValueType->smartPointerType; - // TODO: Cache result, handle more complex type expression - const Token* typeTok = typeStartToken(); - while (Token::Match(typeTok, "%name%|::")) - typeTok = typeTok->next(); - if (Token::Match(typeTok, "< %name% >")) { - const Scope* scope = typeTok->scope(); - const Type* ptrType{}; - while (scope && !ptrType) { - ptrType = scope->findType(typeTok->next()->str()); - scope = scope->nestedIn; - } - return ptrType; - } + // TODO: Cache result + const Token *ptrType = typeStartToken(); + while (Token::Match(ptrType, "%name%|::")) + ptrType = ptrType->next(); + if (Token::Match(ptrType, "< %name% >")) + return ptrType->scope()->findType(ptrType->next()->str()); return nullptr; } diff --git a/test/testfunctions.cpp b/test/testfunctions.cpp index 36a1bdf4e7f..a79715dfd71 100644 --- a/test/testfunctions.cpp +++ b/test/testfunctions.cpp @@ -1962,17 +1962,6 @@ class TestFunctions : public TestFixture { "}\n"); ASSERT_EQUALS("", errout.str()); - check("struct S {\n" // #11543 - " S() {}\n" - " std::vector> v;\n" - " void f(int i) const;\n" - "};\n" - "void S::f(int i) const {\n" - " for (const std::shared_ptr& c : v)\n" - " c->f(i);\n" - "}\n"); - ASSERT_EQUALS("", errout.str()); - settings = settings_old; } From f7b50f01c3b2b415f194d95f5731563ae26086ef Mon Sep 17 00:00:00 2001 From: chrchr-github Date: Sat, 11 Feb 2023 21:30:21 +0100 Subject: [PATCH 4/9] Format --- cfg/std.cfg | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cfg/std.cfg b/cfg/std.cfg index f542c9813b9..cd80cdda949 100644 --- a/cfg/std.cfg +++ b/cfg/std.cfg @@ -8589,7 +8589,7 @@ initializer list (7) string& replace (const_iterator i1, const_iterator i2, init - + From 834d32f4e904034388c92d6e0e088da221a59f67 Mon Sep 17 00:00:00 2001 From: chrchr-github Date: Sat, 11 Feb 2023 22:49:00 +0100 Subject: [PATCH 5/9] Fix std.cfg --- cfg/std.cfg | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/cfg/std.cfg b/cfg/std.cfg index 25df50351ea..dec4d89887c 100644 --- a/cfg/std.cfg +++ b/cfg/std.cfg @@ -8593,7 +8593,9 @@ initializer list (7) string& replace (const_iterator i1, const_iterator i2, init - + + + From a6a0d761cdeb1f5baf62fab36ba56b4b3575c455 Mon Sep 17 00:00:00 2001 From: chrchr-github Date: Sat, 11 Feb 2023 23:39:40 +0100 Subject: [PATCH 6/9] Only warn for known empty containers --- lib/checkstl.cpp | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/lib/checkstl.cpp b/lib/checkstl.cpp index 5ec1344046f..438e7bbb2c4 100644 --- a/lib/checkstl.cpp +++ b/lib/checkstl.cpp @@ -87,12 +87,19 @@ static bool containerAppendsElement(const Library::Container* container, const T return false; } -static bool isContainerAccess(const Library::Container* container, const Token* parent) +static bool containerYieldsElement(const Library::Container* container, const Token* parent) { if (Token::Match(parent, ". %name% (")) { const Library::Container::Yield yield = container->getYield(parent->strAt(1)); if (isElementAccessYield(yield)) return true; + } + return false; +} + +static bool containerPopsElement(const Library::Container* container, const Token* parent) +{ + if (Token::Match(parent, ". %name% (")) { const Library::Container::Action action = container->getAction(parent->strAt(1)); if (contains({ Library::Container::Action::POP }, action)) return true; @@ -151,8 +158,9 @@ void CheckStl::outOfBounds() continue; if (!value.errorSeverity() && !mSettings->severity.isEnabled(Severity::warning)) continue; - if (value.intvalue == 0 && (indexTok || (isContainerAccess(container, parent) && - !containerAppendsElement(container, parent)))) { + if (value.intvalue == 0 && (indexTok || + (containerYieldsElement(container, parent) && !containerAppendsElement(container, parent)) || + (value.isKnown() && containerPopsElement(container, parent)))) { std::string indexExpr; if (indexTok && !indexTok->hasKnownValue()) indexExpr = indexTok->expressionString(); From 3aa87d0cb47f59a0a0f6c96951243f9508df35bd Mon Sep 17 00:00:00 2001 From: chrchr Date: Thu, 23 Feb 2023 18:16:02 +0100 Subject: [PATCH 7/9] Warn for unknown values, add test --- lib/checkstl.cpp | 2 +- test/cfg/std.cpp | 11 +++++++++++ 2 files changed, 12 insertions(+), 1 deletion(-) diff --git a/lib/checkstl.cpp b/lib/checkstl.cpp index 438e7bbb2c4..28e0e4ee69d 100644 --- a/lib/checkstl.cpp +++ b/lib/checkstl.cpp @@ -160,7 +160,7 @@ void CheckStl::outOfBounds() continue; if (value.intvalue == 0 && (indexTok || (containerYieldsElement(container, parent) && !containerAppendsElement(container, parent)) || - (value.isKnown() && containerPopsElement(container, parent)))) { + containerPopsElement(container, parent))) { std::string indexExpr; if (indexTok && !indexTok->hasKnownValue()) indexExpr = indexTok->expressionString(); diff --git a/test/cfg/std.cpp b/test/cfg/std.cpp index 3434d7b80f8..3597205d592 100644 --- a/test/cfg/std.cpp +++ b/test/cfg/std.cpp @@ -4553,6 +4553,17 @@ void stdcontainer_pop_empty() { // #11553 l.pop_front(); } +void stdcontainer_pop_empty2(std::vector& v) { + if (v.empty()) {} + // cppcheck-suppress containerOutOfBounds + v.pop_back(); +} + +void stdcontainer_pop_unknown(std::vector& v) { + v.pop_back(); + if (v.empty()) {} +} + void stdbind_helper(int a) { printf("%d", a); From e17ebd4bc4cb3da78e65c20309e290368f26bef3 Mon Sep 17 00:00:00 2001 From: chrchr-github Date: Thu, 23 Feb 2023 22:13:33 +0100 Subject: [PATCH 8/9] Move tests --- test/cfg/std.cpp | 20 -------------------- test/teststl.cpp | 26 ++++++++++++++++++++++++++ 2 files changed, 26 insertions(+), 20 deletions(-) diff --git a/test/cfg/std.cpp b/test/cfg/std.cpp index 3597205d592..f7d2eb0ebeb 100644 --- a/test/cfg/std.cpp +++ b/test/cfg/std.cpp @@ -4544,26 +4544,6 @@ void stdvector() v.front(); } -void stdcontainer_pop_empty() { // #11553 - std::vector v; - // cppcheck-suppress containerOutOfBounds - v.pop_back(); - std::list l; - // cppcheck-suppress containerOutOfBounds - l.pop_front(); -} - -void stdcontainer_pop_empty2(std::vector& v) { - if (v.empty()) {} - // cppcheck-suppress containerOutOfBounds - v.pop_back(); -} - -void stdcontainer_pop_unknown(std::vector& v) { - v.pop_back(); - if (v.empty()) {} -} - void stdbind_helper(int a) { printf("%d", a); diff --git a/test/teststl.cpp b/test/teststl.cpp index 43590966d03..0f2efccecce 100644 --- a/test/teststl.cpp +++ b/test/teststl.cpp @@ -123,6 +123,7 @@ class TestStl : public TestFixture { TEST_CASE(pushback13); TEST_CASE(insert1); TEST_CASE(insert2); + TEST_CASE(popback1); TEST_CASE(stlBoundaries1); TEST_CASE(stlBoundaries2); @@ -3061,6 +3062,31 @@ class TestStl : public TestFixture { ASSERT_EQUALS("", errout.str()); } + void popback1() { // #11553 + check("void f() {\n" + " std::vector v;\n" + " v.pop_back();\n" + " std::list l;\n" + " l.pop_front();\n" + "}\n"); + ASSERT_EQUALS("[test.cpp:3]: (error) Out of bounds access in expression 'v.pop_back()' because 'v' is empty.\n" + "[test.cpp:5]: (error) Out of bounds access in expression 'l.pop_front()' because 'l' is empty.\n", + errout.str()); + + check("void f(std::vector& v) {\n" + " if (v.empty()) {}\n" + " v.pop_back();\n" + "}\n"); + ASSERT_EQUALS("[test.cpp:2] -> [test.cpp:3]: (warning) Either the condition 'v.empty()' is redundant or expression 'v.pop_back()' cause access out of bounds.\n", + errout.str()); + + check("void f(std::vector& v) {\n" + " v.pop_back();\n" + " if (v.empty()) {}\n" + "}\n"); + ASSERT_EQUALS("", errout.str()); + } + void stlBoundaries1() { const std::string stlCont[] = { "list", "set", "multiset", "map", "multimap" From 42ffe0289e3dccf2679588fe86f2019713fa1efb Mon Sep 17 00:00:00 2001 From: chrchr-github Date: Thu, 23 Feb 2023 22:52:46 +0100 Subject: [PATCH 9/9] Undo --- test/cfg/std.cpp | 1 - 1 file changed, 1 deletion(-) diff --git a/test/cfg/std.cpp b/test/cfg/std.cpp index f7d2eb0ebeb..423f9508c89 100644 --- a/test/cfg/std.cpp +++ b/test/cfg/std.cpp @@ -35,7 +35,6 @@ #include #include #include -#include #include #include #include