From 09c8cfb2ae503e834409a6e05483cdc80070381d Mon Sep 17 00:00:00 2001 From: Paul Fultz II Date: Wed, 6 Apr 2022 23:47:15 -0500 Subject: [PATCH] Fix 6624: false negative: std::vector out of bounds access not detected (#3980) * Fix 6624: false negative: std::vector out of bounds access not detected * Format * Add test for auto * Fix tests * Format --- lib/library.cpp | 14 ++++++++++++++ lib/library.h | 1 + lib/symboldatabase.cpp | 23 ++++++++++++++++------- test/teststl.cpp | 34 +++++++++++++++++++++++----------- 4 files changed, 54 insertions(+), 18 deletions(-) diff --git a/lib/library.cpp b/lib/library.cpp index 6a0377cf2..2faf4b910 100644 --- a/lib/library.cpp +++ b/lib/library.cpp @@ -1198,6 +1198,20 @@ const Library::Container* Library::detectContainer(const Token* typeStart, bool return nullptr; } +const Library::Container* Library::detectContainerOrIterator(const Token* typeStart, bool* isIterator) const +{ + const Library::Container* c = detectContainer(typeStart); + if (c) { + if (isIterator) + *isIterator = false; + return c; + } + c = detectContainer(typeStart, true); + if (c && isIterator) + *isIterator = true; + return c; +} + bool Library::isContainerYield(const Token * const cond, Library::Container::Yield y, const std::string& fallback) { if (!cond) diff --git a/lib/library.h b/lib/library.h index 99e70d764..3e64983cc 100644 --- a/lib/library.h +++ b/lib/library.h @@ -285,6 +285,7 @@ public: }; std::map containers; const Container* detectContainer(const Token* typeStart, bool iterator = false) const; + const Container* detectContainerOrIterator(const Token* typeStart, bool* isIterator = nullptr) const; class ArgumentChecks { public: diff --git a/lib/symboldatabase.cpp b/lib/symboldatabase.cpp index 1213a941d..2716c27a6 100644 --- a/lib/symboldatabase.cpp +++ b/lib/symboldatabase.cpp @@ -6324,6 +6324,7 @@ static const Token * parsedecl(const Token *type, ValueType * const valuetype, V bool par = false; while (Token::Match(type, "%name%|*|&|&&|::|(") && !Token::Match(type, "typename|template") && type->varId() == 0 && !type->variable() && !type->function()) { + bool isIterator = false; if (type->str() == "(") { if (Token::Match(type->link(), ") const| {")) break; @@ -6364,9 +6365,13 @@ static const Token * parsedecl(const Token *type, ValueType * const valuetype, V typeTokens.addtoken("::", 0, 0, 0, false); pos1 = pos2 + 2; } while (pos1 < type->str().size()); - const Library::Container *container = settings->library.detectContainer(typeTokens.front()); + const Library::Container* container = + settings->library.detectContainerOrIterator(typeTokens.front(), &isIterator); if (container) { - valuetype->type = ValueType::Type::CONTAINER; + if (isIterator) + valuetype->type = ValueType::Type::ITERATOR; + else + valuetype->type = ValueType::Type::CONTAINER; valuetype->container = container; } else { const Scope *scope = type->scope(); @@ -6374,8 +6379,11 @@ static const Token * parsedecl(const Token *type, ValueType * const valuetype, V if (valuetype->typeScope) valuetype->type = (scope->type == Scope::ScopeType::eClass) ? ValueType::Type::RECORD : ValueType::Type::NONSTD; } - } else if (const Library::Container *container = settings->library.detectContainer(type)) { - valuetype->type = ValueType::Type::CONTAINER; + } else if (const Library::Container* container = settings->library.detectContainerOrIterator(type, &isIterator)) { + if (isIterator) + valuetype->type = ValueType::Type::ITERATOR; + else + valuetype->type = ValueType::Type::CONTAINER; valuetype->container = container; while (Token::Match(type, "%type%|::|<") && type->str() != "const") { if (type->str() == "<" && type->link()) { @@ -6696,7 +6704,7 @@ void SymbolDatabase::setValueTypeInTokenList(bool reportDebugWarnings, Token *to const Token *typeStartToken = tok->astOperand1(); while (typeStartToken && typeStartToken->str() == "::") typeStartToken = typeStartToken->astOperand1(); - if (mSettings->library.detectContainer(typeStartToken) || + if (mSettings->library.detectContainerOrIterator(typeStartToken) || mSettings->library.detectSmartPointer(typeStartToken)) { ValueType vt; if (parsedecl(typeStartToken, &vt, mDefaultSignedness, mSettings)) { @@ -6800,11 +6808,12 @@ void SymbolDatabase::setValueTypeInTokenList(bool reportDebugWarnings, Token *to const Token *typeTok = tok->next(); if (Token::Match(typeTok, "( std| ::| nothrow )")) typeTok = typeTok->link()->next(); - if (const Library::Container *c = mSettings->library.detectContainer(typeTok)) { + bool isIterator = false; + if (const Library::Container* c = mSettings->library.detectContainerOrIterator(typeTok, &isIterator)) { ValueType vt; vt.pointer = 1; vt.container = c; - vt.type = ValueType::Type::CONTAINER; + vt.type = isIterator ? ValueType::Type::ITERATOR : ValueType::Type::CONTAINER; setValueType(tok, vt); continue; } diff --git a/test/teststl.cpp b/test/teststl.cpp index b886383f8..33813a374 100644 --- a/test/teststl.cpp +++ b/test/teststl.cpp @@ -904,6 +904,22 @@ private: " }\n" "}\n"); ASSERT_EQUALS("", errout.str()); + + check("void f() {\n" + " std::vector vec;\n" + " std::vector::iterator it = vec.begin();\n" + " *it = 1;\n" + "}\n"); + ASSERT_EQUALS("[test.cpp:4]: (error) Out of bounds access in expression 'it' because 'vec' is empty.\n", + errout.str()); + + check("void f() {\n" + " std::vector vec;\n" + " auto it = vec.begin();\n" + " *it = 1;\n" + "}\n"); + ASSERT_EQUALS("[test.cpp:4]: (error) Out of bounds access in expression 'it' because 'vec' is empty.\n", + errout.str()); } void iterator1() { @@ -984,7 +1000,9 @@ private: " std::list::iterator it = l1.begin();\n" " l2.insert(it, 0);\n" "}"); - ASSERT_EQUALS("[test.cpp:6]: (error) Same iterator is used with different containers 'l1' and 'l2'.\n", errout.str()); + ASSERT_EQUALS("[test.cpp:6]: (error) Same iterator is used with different containers 'l1' and 'l2'.\n" + "[test.cpp:6]: (error) Iterator 'it' from different container 'l2' are used together.\n", + errout.str()); check("void foo() {\n" // #5803 " std::list l1;\n" @@ -1416,10 +1434,8 @@ private: } void iterator18() { - check("void foo()\n" + check("void foo(std::list l1, std::list l2)\n" "{\n" - " std::list l1;\n" - " std::list l2;\n" " std::list::iterator it1 = l1.begin();\n" " std::list::iterator it2 = l1.end();\n" " while (++it1 != --it2)\n" @@ -1428,10 +1444,8 @@ private: "}"); ASSERT_EQUALS("", errout.str()); - check("void foo()\n" + check("void foo(std::list l1, std::list l2)\n" "{\n" - " std::list l1;\n" - " std::list l2;\n" " std::list::iterator it1 = l1.begin();\n" " std::list::iterator it2 = l1.end();\n" " while (it1++ != --it2)\n" @@ -1440,17 +1454,15 @@ private: "}"); ASSERT_EQUALS("", errout.str()); - check("void foo()\n" + check("void foo(std::list l1, std::list l2)\n" "{\n" - " std::list l1;\n" - " std::list l2;\n" " std::list::iterator it1 = l1.begin();\n" " std::list::iterator it2 = l1.end();\n" " if (--it2 > it1++)\n" " {\n" " }\n" "}"); - TODO_ASSERT_EQUALS("", "[test.cpp:7]: (error) Dangerous comparison using operator< on iterator.\n", errout.str()); + TODO_ASSERT_EQUALS("", "[test.cpp:5]: (error) Dangerous comparison using operator< on iterator.\n", errout.str()); } void iterator19() {