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

Use library to track container lifetimes

This commit is contained in:
Paul Fultz II 2019-08-15 21:14:27 +02:00 committed by Daniel Marjamäki
parent 9f7f446c59
commit ef714225bb
7 changed files with 70 additions and 53 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

18
lib/astutils.cpp
View File

@ -106,7 +106,7 @@ bool astIsIterator(const Token *tok)
bool astIsContainer(const Token *tok)
{
return tok && tok->valueType() && tok->valueType()->type == ValueType::Type::CONTAINER;
return getLibraryContainer(tok) != nullptr && tok->valueType()->type != ValueType::Type::ITERATOR;
}
std::string astCanonicalType(const Token *expr)
@ -214,6 +214,22 @@ const Token * nextAfterAstRightmostLeaf(const Token * tok)
return rightmostLeaf->next();
}
const Token* astParentSkipParens(const Token* tok)
{
return astParentSkipParens(const_cast<Token*>(tok));
}
Token* astParentSkipParens(Token* tok)
{
if (!tok)
return nullptr;
Token * parent = tok->astParent();
if (!Token::simpleMatch(parent, "("))
return parent;
if (parent->link() != nextAfterAstRightmostLeaf(tok))
return parent;
return astParentSkipParens(parent);
}
static const Token * getVariableInitExpression(const Variable * var)
{
if (!var || !var->declEndToken())

3
lib/astutils.h
View File

@ -81,6 +81,9 @@ const Token * astIsVariableComparison(const Token *tok, const std::string &comp,
const Token * nextAfterAstRightmostLeaf(const Token * tok);
Token* astParentSkipParens(Token* tok);
const Token* astParentSkipParens(const Token* tok);
bool precedes(const Token * tok1, const Token * tok2);
bool isSameExpression(bool cpp, bool macro, const Token *tok1, const Token *tok2, const Library& library, bool pure, bool followVar, ErrorPath* errors=nullptr);

20
lib/checkstl.cpp
View File

@ -51,22 +51,6 @@ static const struct CWE CWE788(788U); // Access of Memory Location After End o
static const struct CWE CWE825(825U); // Expired Pointer Dereference
static const struct CWE CWE834(834U); // Excessive Iteration
static const Library::Container * getLibraryContainer(const Token * tok)
{
if (!tok)
return nullptr;
if (tok->isUnaryOp("*") && astIsPointer(tok->astOperand1())) {
for (const ValueFlow::Value& v:tok->astOperand1()->values()) {
if (!v.isLocalLifetimeValue())
continue;
return getLibraryContainer(v.tokvalue);
}
}
if (!tok->valueType())
return nullptr;
return tok->valueType()->container;
}
void CheckStl::outOfBounds()
{
for (const Scope *function : mTokenizer->getSymbolDatabase()->functionScopes) {
@ -74,9 +58,7 @@ void CheckStl::outOfBounds()
const Library::Container *container = getLibraryContainer(tok);
if (!container)
continue;
const Token * parent = tok->astParent();
while (Token::simpleMatch(parent, "(") && !Token::Match(parent->previous(), "%name%"))
parent = parent->astParent();
const Token * parent = astParentSkipParens(tok);
for (const ValueFlow::Value &value : tok->values()) {
if (!value.isContainerSizeValue())
continue;

20
lib/library.cpp
View File

@ -1361,3 +1361,23 @@ bool Library::isSmartPointer(const Token *tok) const
return smartPointers.find(typestr) != smartPointers.end();
}
CPPCHECKLIB const Library::Container * getLibraryContainer(const Token * tok)
{
if (!tok)
return nullptr;
// TODO: Support dereferencing iterators
// TODO: Support dereferencing with ->
if (tok->isUnaryOp("*") && astIsPointer(tok->astOperand1())) {
for (const ValueFlow::Value& v:tok->astOperand1()->values()) {
if (!v.isLocalLifetimeValue())
continue;
if (v.lifetimeKind != ValueFlow::Value::LifetimeKind::Address)
continue;
return getLibraryContainer(v.tokvalue);
}
}
if (!tok->valueType())
return nullptr;
return tok->valueType()->container;
}

2
lib/library.h
View File

@ -565,6 +565,8 @@ private:
}
};
CPPCHECKLIB const Library::Container * getLibraryContainer(const Token * tok);
/// @}
//---------------------------------------------------------------------------
#endif // libraryH

36
lib/valueflow.cpp
View File

@ -3445,31 +3445,25 @@ static void valueFlowLifetime(TokenList *tokenlist, SymbolDatabase*, ErrorLogger
valueFlowForwardLifetime(tok, tokenlist, errorLogger, settings);
}
// container lifetimes
else if (tok->variable() &&
Token::Match(tok, "%var% . begin|cbegin|rbegin|crbegin|end|cend|rend|crend|data|c_str|find|insert (") &&
tok->next()->originalName() != "->") {
if (Token::simpleMatch(tok->tokAt(2), "find") && !astIsIterator(tok->tokAt(3)))
continue;
ErrorPath errorPath;
const Library::Container * container = settings->library.detectContainer(tok->variable()->typeStartToken());
if (!container)
else if (astIsContainer(tok)) {
Token * parent = astParentSkipParens(tok);
if (!Token::Match(parent, ". %name% ("))
continue;
bool isIterator = !Token::Match(tok->tokAt(2), "data|c_str");
if (isIterator)
errorPath.emplace_back(tok, "Iterator to container is created here.");
LifetimeStore ls;
if (astIsIterator(parent->tokAt(2)))
ls = LifetimeStore{tok, "Iterator to container is created here.", ValueFlow::Value::LifetimeKind::Iterator};
else if (astIsPointer(parent->tokAt(2)) || Token::Match(parent->next(), "data|c_str"))
ls = LifetimeStore{tok, "Pointer to container is created here.", ValueFlow::Value::LifetimeKind::Object};
else
errorPath.emplace_back(tok, "Pointer to container is created here.");
continue;
ValueFlow::Value value;
value.valueType = ValueFlow::Value::ValueType::LIFETIME;
value.lifetimeScope = ValueFlow::Value::LifetimeScope::Local;
value.tokvalue = tok;
value.errorPath = errorPath;
value.lifetimeKind = isIterator ? ValueFlow::Value::LifetimeKind::Iterator : ValueFlow::Value::LifetimeKind::Object;
setTokenValue(tok->tokAt(3), value, tokenlist->getSettings());
valueFlowForwardLifetime(tok->tokAt(3), tokenlist, errorLogger, settings);
// Dereferencing
if (tok->isUnaryOp("*") || parent->originalName() == "->")
ls.byDerefCopy(parent->tokAt(2), tokenlist, errorLogger, settings);
else
ls.byRef(parent->tokAt(2), tokenlist, errorLogger, settings);
}
// Check constructors

24
test/teststl.cpp
View File

@ -1745,7 +1745,7 @@ private:
" ints.erase(iter);\n"
" ints.erase(iter);\n"
"}");
ASSERT_EQUALS("[test.cpp:4] -> [test.cpp:5] -> [test.cpp:1] -> [test.cpp:6]: (error) Using iterator to local container 'ints' that may be invalid.\n", errout.str());
ASSERT_EQUALS("[test.cpp:1] -> [test.cpp:4] -> [test.cpp:5] -> [test.cpp:1] -> [test.cpp:6]: (error) Using iterator to local container 'ints' that may be invalid.\n", errout.str());
}
void eraseByValue() {
@ -1818,14 +1818,14 @@ private:
" m_ImplementationMap.erase(something(unknown));\n" // All iterators become invalidated when erasing from std::vector
" m_ImplementationMap.erase(aIt);\n"
"}");
ASSERT_EQUALS("[test.cpp:2] -> [test.cpp:3] -> [test.cpp:1] -> [test.cpp:4]: (error) Using iterator to local container 'm_ImplementationMap' that may be invalid.\n", errout.str());
ASSERT_EQUALS("[test.cpp:1] -> [test.cpp:2] -> [test.cpp:3] -> [test.cpp:1] -> [test.cpp:4]: (error) Using iterator to local container 'm_ImplementationMap' that may be invalid.\n", errout.str());
check("void f(const std::vector<int>& m_ImplementationMap) {\n"
" std::vector<int>::iterator aIt = m_ImplementationMap.begin();\n"
" m_ImplementationMap.erase(*aIt);\n" // All iterators become invalidated when erasing from std::vector
" m_ImplementationMap.erase(aIt);\n"
"}");
ASSERT_EQUALS("[test.cpp:2] -> [test.cpp:3] -> [test.cpp:1] -> [test.cpp:4]: (error) Using iterator to local container 'm_ImplementationMap' that may be invalid.\n", errout.str());
ASSERT_EQUALS("[test.cpp:1] -> [test.cpp:2] -> [test.cpp:3] -> [test.cpp:1] -> [test.cpp:4]: (error) Using iterator to local container 'm_ImplementationMap' that may be invalid.\n", errout.str());
check("void f(const std::vector<int>& m_ImplementationMap) {\n"
" std::vector<int>::iterator aIt = m_ImplementationMap.begin();\n"
@ -1833,7 +1833,7 @@ private:
" m_ImplementationMap.erase(*bIt);\n" // All iterators become invalidated when erasing from std::vector
" aIt = m_ImplementationMap.erase(aIt);\n"
"}");
ASSERT_EQUALS("[test.cpp:2] -> [test.cpp:4] -> [test.cpp:1] -> [test.cpp:5]: (error) Using iterator to local container 'm_ImplementationMap' that may be invalid.\n", errout.str());
ASSERT_EQUALS("[test.cpp:1] -> [test.cpp:2] -> [test.cpp:4] -> [test.cpp:1] -> [test.cpp:5]: (error) Using iterator to local container 'm_ImplementationMap' that may be invalid.\n", errout.str());
}
void pushback1() {
@ -1843,7 +1843,7 @@ private:
" foo.push_back(123);\n"
" *it;\n"
"}");
ASSERT_EQUALS("[test.cpp:3] -> [test.cpp:4] -> [test.cpp:1] -> [test.cpp:5]: (error) Using iterator to local container 'foo' that may be invalid.\n", errout.str());
ASSERT_EQUALS("[test.cpp:1] -> [test.cpp:3] -> [test.cpp:4] -> [test.cpp:1] -> [test.cpp:5]: (error) Using iterator to local container 'foo' that may be invalid.\n", errout.str());
}
void pushback2() {
@ -1988,7 +1988,7 @@ private:
" foo.reserve(100);\n"
" *it = 0;\n"
"}");
ASSERT_EQUALS("[test.cpp:3] -> [test.cpp:4] -> [test.cpp:1] -> [test.cpp:5]: (error) Using iterator to local container 'foo' that may be invalid.\n", errout.str());
ASSERT_EQUALS("[test.cpp:1] -> [test.cpp:3] -> [test.cpp:4] -> [test.cpp:1] -> [test.cpp:5]: (error) Using iterator to local container 'foo' that may be invalid.\n", errout.str());
// in loop
check("void f()\n"
@ -2051,7 +2051,7 @@ private:
" ints.insert(ints.begin(), 1);\n"
" ++iter;\n"
"}");
ASSERT_EQUALS("[test.cpp:3] -> [test.cpp:4] -> [test.cpp:1] -> [test.cpp:5]: (error) Using iterator to local container 'ints' that may be invalid.\n", errout.str());
ASSERT_EQUALS("[test.cpp:1] -> [test.cpp:3] -> [test.cpp:4] -> [test.cpp:1] -> [test.cpp:5]: (error) Using iterator to local container 'ints' that may be invalid.\n", errout.str());
check("void f()\n"
"{\n"
@ -2076,14 +2076,14 @@ private:
" void* v = &i->foo;\n"
" return v;\n"
"}");
ASSERT_EQUALS("[test.cpp:2] -> [test.cpp:3] -> [test.cpp:1] -> [test.cpp:4]: (error) Using iterator to local container 'bars' that may be invalid.\n", errout.str());
ASSERT_EQUALS("[test.cpp:1] -> [test.cpp:2] -> [test.cpp:3] -> [test.cpp:1] -> [test.cpp:4]: (error) Using iterator to local container 'bars' that may be invalid.\n", errout.str());
check("Foo f(const std::vector<Bar>& bars) {\n"
" std::vector<Bar>::iterator i = bars.begin();\n"
" bars.insert(Bar());\n"
" return i->foo;\n"
"}");
ASSERT_EQUALS("[test.cpp:2] -> [test.cpp:3] -> [test.cpp:1] -> [test.cpp:4]: (error) Using iterator to local container 'bars' that may be invalid.\n", errout.str());
ASSERT_EQUALS("[test.cpp:1] -> [test.cpp:2] -> [test.cpp:3] -> [test.cpp:1] -> [test.cpp:4]: (error) Using iterator to local container 'bars' that may be invalid.\n", errout.str());
check("void f(const std::vector<Bar>& bars) {\n"
" for(std::vector<Bar>::iterator i = bars.begin(); i != bars.end(); ++i) {\n"
@ -2098,7 +2098,7 @@ private:
" i = bars.insert(i, bar);\n"
" }\n"
"}");
ASSERT_EQUALS("[test.cpp:2] -> [test.cpp:2] -> [test.cpp:3] -> [test.cpp:1] -> [test.cpp:2]: (error) Using iterator to local container 'bars' that may be invalid.\n", errout.str());
ASSERT_EQUALS("[test.cpp:1] -> [test.cpp:2] -> [test.cpp:2] -> [test.cpp:3] -> [test.cpp:1] -> [test.cpp:2]: (error) Using iterator to local container 'bars' that may be invalid.\n", errout.str());
check("void* f(const std::vector<Bar>& bars) {\n"
" std::vector<Bar>::iterator i = bars.begin();\n"
@ -2107,7 +2107,7 @@ private:
" void* v = &i->foo;\n"
" return v;\n"
"}");
ASSERT_EQUALS("[test.cpp:4] -> [test.cpp:1] -> [test.cpp:5] -> [test.cpp:4] -> [test.cpp:1] -> [test.cpp:6]: (error) Using pointer to local variable 'bars' that may be invalid.\n", errout.str());
ASSERT_EQUALS("[test.cpp:1] -> [test.cpp:4] -> [test.cpp:1] -> [test.cpp:5] -> [test.cpp:4] -> [test.cpp:1] -> [test.cpp:6]: (error) Using pointer to local variable 'bars' that may be invalid.\n", errout.str());
}
void insert2() {
@ -3877,7 +3877,7 @@ private:
" std::cout << *v0 << std::endl;\n"
"}\n",
true);
ASSERT_EQUALS("[test.cpp:2] -> [test.cpp:3] -> [test.cpp:1] -> [test.cpp:4]: (error) Using iterator to local container 'v' that may be invalid.\n", errout.str());
ASSERT_EQUALS("[test.cpp:1] -> [test.cpp:2] -> [test.cpp:3] -> [test.cpp:1] -> [test.cpp:4]: (error) Using iterator to local container 'v' that may be invalid.\n", errout.str());
check("std::string e();\n"
"void a() {\n"