walkero
/
cppcheck
1
0
Fork 0
Code Issues Pull Requests Projects Releases 1 Wiki Activity

Improve checking of mismatch iterators (#1293)

This commit is contained in:
Paul Fultz II 2018-07-26 15:00:48 -05:00 committed by Daniel Marjamäki
parent 54e2726bf3
commit 0d35a96594
4 changed files with 115 additions and 12 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

42
lib/checkstl.cpp
View File

@ -25,6 +25,7 @@
#include "symboldatabase.h"
#include "token.h"
#include "utils.h"
#include "astutils.h"
#include <cstddef>
#include <list>
@ -304,6 +305,15 @@ void CheckStl::mismatchingContainersError(const Token *tok)
reportError(tok, Severity::error, "mismatchingContainers", "Iterators of different containers are used together.", CWE664, false);
}
void CheckStl::mismatchingContainerExpressionError(const Token *tok1, const Token *tok2)
{
const std::string expr1(tok1 ? tok1->expressionString() : std::string());
const std::string expr2(tok2 ? tok2->expressionString() : std::string());
reportError(tok1, Severity::warning, "mismatchingContainerExpression",
"Iterators to containers from different expressions '" +
expr1 + "' and '" + expr2 + "' are used together.", CWE664, false);
}
static const std::set<std::string> algorithm2 = { // func(begin1, end1
"binary_search", "copy", "copy_if", "equal_range"
, "generate", "is_heap", "is_heap_until", "is_partitioned"
@ -324,6 +334,7 @@ static const std::set<std::string> algorithm1x1 = { // func(begin1, x, end1
static const std::string iteratorBeginFuncPattern = "begin|cbegin|rbegin|crbegin";
static const std::string iteratorEndFuncPattern = "end|cend|rend|crend";
static const std::string iteratorFuncPattern = "begin|cbegin|rbegin|crbegin|end|cend|rend|crend (";
static const std::string pattern1x1_1 = "%name% . " + iteratorBeginFuncPattern + " ( ) , ";
static const std::string pattern1x1_2 = "%name% . " + iteratorEndFuncPattern + " ( ) ,|)";
@ -341,6 +352,20 @@ static const Variable *getContainer(const Token *argtok)
return nullptr;
}
static const Token * getIteratorExpression(const Token * tok, const Token * end)
{
for(;tok != end;tok = tok->next()) {
if(Token::Match(tok, iteratorFuncPattern.c_str())) {
if(Token::Match(tok->previous(), ". %name% ( )")) {
return tok->previous()->astOperand1();
} else if(Token::Match(tok, "%name% ( !!)")) {
return tok->next()->astOperand2();
}
}
}
return nullptr;
}
void CheckStl::mismatchingContainers()
{
// Check if different containers are used in various calls of standard functions
@ -351,6 +376,7 @@ void CheckStl::mismatchingContainers()
continue;
const Token * const ftok = tok;
const Token * const arg1 = tok->tokAt(2);
const Token * firstArg = nullptr;
int argnr = 1;
std::map<const Variable *, unsigned int> containerNr;
@ -359,8 +385,7 @@ void CheckStl::mismatchingContainers()
if (!i)
continue;
const Variable *c = getContainer(argTok);
if (!c)
continue;
if(c) {
std::map<const Variable *, unsigned int>::const_iterator it = containerNr.find(c);
if (it == containerNr.end()) {
for (it = containerNr.begin(); it != containerNr.end(); ++it) {
@ -373,6 +398,19 @@ void CheckStl::mismatchingContainers()
} else if (it->second != i->container) {
mismatchingContainersError(argTok);
}
} else {
if(i->first) {
firstArg = argTok;
} else if(i->last && firstArg && argTok) {
const Token * firstArgNext = firstArg->nextArgument() ? firstArg->nextArgument() : tok->linkAt(1);
const Token * iter1 = getIteratorExpression(firstArg, firstArgNext);
const Token * argTokNext = argTok->nextArgument() ? argTok->nextArgument() : tok->linkAt(1);
const Token * iter2 = getIteratorExpression(argTok, argTokNext);
if(iter1 && iter2 && !isSameExpression(true, false, iter1, iter2, mSettings->library, false)) {
mismatchingContainerExpressionError(iter1, iter2);
}
}
}
}
const int ret = mSettings->library.returnValueContainer(ftok);
if (ret != -1 && Token::Match(ftok->next()->astParent(), "==|!=")) {

2
lib/checkstl.h
View File

@ -183,6 +183,7 @@ private:
void invalidIteratorError(const Token* tok, const std::string& iteratorName);
void iteratorsError(const Token* tok, const std::string& container1, const std::string& container2);
void mismatchingContainersError(const Token* tok);
void mismatchingContainerExpressionError(const Token *tok1, const Token *tok2);
void invalidIteratorError(const Token* tok, const std::string& func, const std::string& iterator_name);
void invalidPointerError(const Token* tok, const std::string& func, const std::string& pointer_name);
void stlBoundariesError(const Token* tok);
@ -210,6 +211,7 @@ private:
c.invalidIteratorError(nullptr, "iterator");
c.iteratorsError(nullptr, "container1", "container2");
c.mismatchingContainersError(nullptr);
c.mismatchingContainerExpressionError(nullptr, nullptr);
c.dereferenceErasedError(nullptr, nullptr, "iter", false);
c.stlOutOfBoundsError(nullptr, "i", "foo", false);
c.negativeIndexError(nullptr, ValueFlow::Value(-1));

2
lib/token.cpp
View File

@ -1248,7 +1248,7 @@ std::string Token::expressionString() const
const Token * const top = this;
const Token *start = top;
while (start->astOperand1() &&
(start->astOperand2() || !start->isUnaryPreOp() || Token::simpleMatch(start, "( )")))
(start->astOperand2() || !start->isUnaryPreOp() || Token::simpleMatch(start, "( )") || start->str() == "{"))
start = start->astOperand1();
const Token *end = top;
while (end->astOperand1() && (end->astOperand2() || end->isUnaryPreOp())) {

63
test/teststl.cpp
View File

@ -54,6 +54,7 @@ private:
TEST_CASE(iterator12);
TEST_CASE(iterator13);
TEST_CASE(iterator14); // #8191
TEST_CASE(iteratorExpression);
TEST_CASE(dereference);
TEST_CASE(dereference_break); // #3644 - handle "break"
@ -532,6 +533,68 @@ private:
ASSERT_EQUALS("", errout.str());
}
void iteratorExpression() {
check("std::vector<int>& f();\n"
"std::vector<int>& g();\n"
"void foo() {\n"
" (void)std::find(f().begin(), g().end(), 0);\n"
"}\n");
ASSERT_EQUALS("[test.cpp:4]: (warning) Iterators to containers from different expressions 'f()' and 'g()' are used together.\n", errout.str());
check("struct A {\n"
" std::vector<int>& f();\n"
" std::vector<int>& g();\n"
"};\n"
"void foo() {\n"
" (void)std::find(A().f().begin(), A().g().end(), 0);\n"
"}\n");
ASSERT_EQUALS("[test.cpp:6]: (warning) Iterators to containers from different expressions 'A().f()' and 'A().g()' are used together.\n", errout.str());
check("struct A {\n"
" std::vector<int>& f();\n"
" std::vector<int>& g();\n"
"};\n"
"void foo() {\n"
" (void)std::find(A{}.f().begin(), A{}.g().end(), 0);\n"
"}\n");
ASSERT_EQUALS("[test.cpp:6]: (warning) Iterators to containers from different expressions 'A{}.f()' and 'A{}.g()' are used together.\n", errout.str());
check("std::vector<int>& f();\n"
"std::vector<int>& g();\n"
"void foo() {\n"
" (void)std::find(begin(f()), end(g()), 0);\n"
"}\n");
ASSERT_EQUALS("[test.cpp:4]: (warning) Iterators to containers from different expressions 'f()' and 'g()' are used together.\n", errout.str());
check("struct A {\n"
" std::vector<int>& f();\n"
" std::vector<int>& g();\n"
"};\n"
"void foo() {\n"
" (void)std::find(A().f().begin(), A().f().end(), 0);\n"
"}\n");
ASSERT_EQUALS("", errout.str());
check("std::vector<int>& f();\n"
"std::vector<int>& g();\n"
"void foo() {\n"
" auto it = f().end();"
" (void)std::find(f().begin(), it, 0);\n"
" (void)std::find(f().begin(), it + 1, 0);\n"
" (void)std::find(f().begin() + 1, it + 1, 0);\n"
" (void)std::find(f().begin() + 1, it, 0);\n"
" (void)std::find(f().begin(), f().end(), 0);\n"
" (void)std::find(f().begin() + 1, f().end(), 0);\n"
" (void)std::find(f().begin(), f().end() - 1, 0);\n"
" (void)std::find(f().begin() + 1, f().end() - 1, 0);\n"
" (void)std::find(begin(f()), end(f()));\n"
" (void)std::find(begin(f()) + 1, end(f()), 0);\n"
" (void)std::find(begin(f()), end(f()) - 1, 0);\n"
" (void)std::find(begin(f()) + 1, end(f()) - 1, 0);\n"
"}\n");
ASSERT_EQUALS("", errout.str());
}
// Dereferencing invalid pointer
void dereference() {
check("void f()\n"