From 0e9810b7f6306cfe25d100bfb3cb5b0e62cdf971 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Marjam=C3=A4ki?= Date: Tue, 1 Nov 2016 14:08:42 +0100 Subject: [PATCH] CheckStl: validation of iterators returned from functions --- cfg/std.cfg | 36 ++++++++++++++++++++++++++++++++++-- lib/checkstl.cpp | 13 +++++++++++++ lib/library.cpp | 12 +++++++++++- lib/library.h | 2 ++ lib/symboldatabase.cpp | 2 +- test/cfg/std.cpp | 7 ++++--- 6 files changed, 65 insertions(+), 7 deletions(-) diff --git a/cfg/std.cfg b/cfg/std.cfg index 168047181..0942c132e 100644 --- a/cfg/std.cfg +++ b/cfg/std.cfg @@ -4038,12 +4038,44 @@ The obsolete function 'gets' is called. With 'gets' you'll get a buffer overrun + + + false + + + + + + + + + + + + + - - + + false + + + + + + + + + + + + + + + + false diff --git a/lib/checkstl.cpp b/lib/checkstl.cpp index e320e8fdc..4225ffb86 100644 --- a/lib/checkstl.cpp +++ b/lib/checkstl.cpp @@ -298,6 +298,8 @@ static const std::string pattern2 = pattern1x1_1 + pattern1x1_2; static const Variable *getContainer(const Token *argtok) { + while (argtok && argtok->astOperand1()) + argtok = argtok->astOperand1(); if (!Token::Match(argtok, "%var% . begin|end|rbegin|rend ( )")) // TODO: use Library yield return nullptr; const Variable *var = argtok->variable(); @@ -341,6 +343,17 @@ void CheckStl::mismatchingContainers() mismatchingContainersError(argTok); } } + int ret = _settings->library.returnValueContainer(ftok); + if (ret != -1 && Token::Match(ftok->next()->astParent(), "==|!=")) { + const Token *parent = ftok->next()->astParent(); + const Token *other = (parent->astOperand1() == ftok->next()) ? parent->astOperand2() : parent->astOperand1(); + const Variable *c = getContainer(other); + if (c) { + std::map::const_iterator it = containerNr.find(c); + if (it == containerNr.end() || it->second != ret) + mismatchingContainersError(other); + } + } } } for (unsigned int varid = 0; varid < symbolDatabase->getVariableListSize(); varid++) { diff --git a/lib/library.cpp b/lib/library.cpp index 187895709..1472f2494 100644 --- a/lib/library.cpp +++ b/lib/library.cpp @@ -550,12 +550,14 @@ Library::Error Library::loadFunction(const tinyxml2::XMLElement * const node, co _returnValue[name] = expr; if (const char *type = functionnode->Attribute("type")) _returnValueType[name] = type; + if (const char *container = functionnode->Attribute("container")) + _returnValueContainer[name] = std::atoi(container); } else if (functionnodename == "arg") { const char* argNrString = functionnode->Attribute("nr"); if (!argNrString) return Error(MISSING_ATTRIBUTE, "nr"); const bool bAnyArg = strcmp(argNrString, "any")==0; - const int nr = (bAnyArg) ? -1 : atoi(argNrString); + const int nr = bAnyArg ? -1 : std::atoi(argNrString); ArgumentChecks &ac = argumentChecks[name][nr]; ac.optional = functionnode->Attribute("default") != nullptr; for (const tinyxml2::XMLElement *argnode = functionnode->FirstChildElement(); argnode; argnode = argnode->NextSiblingElement()) { @@ -1000,6 +1002,14 @@ std::string Library::returnValueType(const Token *ftok) const return it != _returnValueType.end() ? it->second : std::string(); } +int Library::returnValueContainer(const Token *ftok) const +{ + if (isNotLibraryFunction(ftok)) + return -1; + std::map::const_iterator it = _returnValueContainer.find(getFunctionName(ftok)); + return it != _returnValueContainer.end() ? it->second : -1; +} + bool Library::isnoreturn(const Token *ftok) const { if (ftok->function() && ftok->function()->isAttributeNoreturn()) diff --git a/lib/library.h b/lib/library.h index 6fcc25b2a..9d95b5bd5 100644 --- a/lib/library.h +++ b/lib/library.h @@ -168,6 +168,7 @@ public: std::string returnValue(const Token *ftok) const; std::string returnValueType(const Token *ftok) const; + int returnValueContainer(const Token *ftok) const; bool isnoreturn(const Token *ftok) const; bool isnotnoreturn(const Token *ftok) const; @@ -510,6 +511,7 @@ private: std::map _noreturn; // is function noreturn? std::map _returnValue; std::map _returnValueType; + std::map _returnValueContainer; std::set _ignorefunction; // ignore functions/macros from a library (gtk, qt etc) std::map _reporterrors; std::map _processAfterCode; diff --git a/lib/symboldatabase.cpp b/lib/symboldatabase.cpp index 7a66bc735..32a447ca1 100644 --- a/lib/symboldatabase.cpp +++ b/lib/symboldatabase.cpp @@ -4619,7 +4619,7 @@ void SymbolDatabase::setValueTypeInTokenList(Token *tokens, bool cpp, const Sett // library function else if (tok->previous()) { const std::string typestr(settings->library.returnValueType(tok->previous())); - if (typestr.empty()) + if (typestr.empty() || typestr == "iterator") continue; TokenList tokenList(settings); std::istringstream istr(typestr+";"); diff --git a/test/cfg/std.cpp b/test/cfg/std.cpp index 7cf917d65..02d651978 100644 --- a/test/cfg/std.cpp +++ b/test/cfg/std.cpp @@ -3165,10 +3165,11 @@ void nullPointer_wmemcmp(wchar_t *p) #include #include -void stdfind(const std::list &ints1, const std::list &ints2) { +void stdfind(const std::list &ints1, const std::list &ints2) +{ // cppcheck-suppress mismatchingContainers // cppcheck-suppress ignoredReturnValue std::find(ints1.begin(), ints2.end(), 123); - // TODO cppcheck-suppress mismatchingContainers - if (std::find(ints1.begin(), ints1.end(), 123) == ints2.end()){} + // cppcheck-suppress mismatchingContainers + if (std::find(ints1.begin(), ints1.end(), 123) == ints2.end()) {} }