Switch to use lifetime analysis for iterators and pointers to invalid containers

This will diagnose more issues such as:

```cpp
void f(std::vector<int> &v) {
    auto v0 = v.begin();
    v.push_back(123);
    std::cout << *v0 << std::endl;
}
```
This commit is contained in:
Paul Fultz II 2019-07-18 10:56:44 +02:00 committed by Daniel Marjamäki
parent 512c1b12c6
commit a08a9c1349
9 changed files with 408 additions and 236 deletions

View File

@ -1172,6 +1172,165 @@ static bool hasFunctionCall(const Token *tok)
return hasFunctionCall(tok->astOperand1()) || hasFunctionCall(tok->astOperand2());
}
const Scope* PathAnalysis::findOuterScope(const Scope * scope)
{
if (!scope)
return nullptr;
if (scope->isLocal() && scope->type != Scope::eSwitch)
return findOuterScope(scope->nestedIn);
return scope;
}
static const Token* getCondTok(const Token* tok)
{
if (!tok)
return nullptr;
if (Token::simpleMatch(tok, "("))
return getCondTok(tok->previous());
if (Token::simpleMatch(tok, "for") && Token::simpleMatch(tok->next()->astOperand2(), ";") && tok->next()->astOperand2()->astOperand2())
return tok->next()->astOperand2()->astOperand2()->astOperand1();
if (Token::simpleMatch(tok->next()->astOperand2(), ";"))
return tok->next()->astOperand2()->astOperand1();
return tok->next()->astOperand2();
}
std::pair<bool, bool> PathAnalysis::checkCond(const Token * tok, bool& known)
{
if (tok->hasKnownIntValue()) {
known = true;
return std::make_pair(tok->values().front().intvalue, !tok->values().front().intvalue);
}
auto it = std::find_if(tok->values().begin(), tok->values().end(), [](const ValueFlow::Value& v) {
return v.isIntValue();
});
// If all possible values are the same, then assume all paths have the same value
if (it != tok->values().end() && std::all_of(it, tok->values().end(), [&](const ValueFlow::Value& v) {
if (v.isIntValue())
return v.intvalue == it->intvalue;
return true;
})) {
known = false;
return std::make_pair(it->intvalue, !it->intvalue);
}
return std::make_pair(true, true);
}
PathAnalysis::Progress PathAnalysis::forwardRecursive(const Token* tok, Info info, const std::function<PathAnalysis::Progress(const Info&)>& f) const
{
if (!tok)
return Progress::Continue;
if (tok->astOperand1() && forwardRecursive(tok->astOperand1(), info, f) == Progress::Break)
return Progress::Break;
info.tok = tok;
if (f(info) == Progress::Break)
return Progress::Break;
if (tok->astOperand2() && forwardRecursive(tok->astOperand2(), info, f) == Progress::Break)
return Progress::Break;
return Progress::Continue;
}
PathAnalysis::Progress PathAnalysis::forwardRange(const Token* startToken, const Token* endToken, Info info, const std::function<PathAnalysis::Progress(const Info&)>& f) const
{
for (const Token *tok = startToken; tok && tok != endToken; tok = tok->next()) {
if (Token::Match(tok, "asm|goto|break|continue"))
return Progress::Break;
if (Token::Match(tok, "return|throw")) {
forwardRecursive(tok, info, f);
return Progress::Break;
}
if (Token::simpleMatch(tok, "}") && Token::simpleMatch(tok->link()->previous(), ") {") && Token::Match(tok->link()->linkAt(-1)->previous(), "if|while|for (")) {
const Token * blockStart = tok->link()->linkAt(-1)->previous();
const Token * condTok = getCondTok(blockStart);
if (!condTok)
continue;
info.errorPath.emplace_back(condTok, "Assuming condition is true.");
// Traverse a loop a second time
if (Token::Match(blockStart, "for|while (")) {
const Token* endCond = blockStart->linkAt(1);
bool traverseLoop = true;
// Only traverse simple for loops
if (Token::simpleMatch(blockStart, "for") && !Token::Match(endCond->tokAt(-3), "; ++|--|%var% %var%|++|-- ) {"))
traverseLoop = false;
// Traverse loop a second time
if (traverseLoop) {
// Traverse condition
if (forwardRecursive(condTok, info, f) == Progress::Break)
return Progress::Break;
// TODO: Should we traverse the body: forwardRange(tok->link(), tok, info, f)?
}
}
}
if (Token::Match(tok, "if|while|for (") && Token::simpleMatch(tok->next()->link(), ") {")) {
const Token * endCond = tok->next()->link();
const Token * endBlock = endCond->next()->link();
const Token * condTok = getCondTok(tok);
if (!condTok)
continue;
// Traverse condition
if (forwardRange(tok->next(), tok->next()->link(), info, f) == Progress::Break)
return Progress::Break;
Info i = info;
i.known = false;
i.errorPath.emplace_back(condTok, "Assuming condition is true.");
// Check if condition is true or false
bool checkThen = false;
bool checkElse = false;
std::tie(checkThen, checkElse) = checkCond(condTok, i.known);
// Traverse then block
if (checkThen) {
if (forwardRange(endCond->next(), endBlock, i, f) == Progress::Break)
return Progress::Break;
}
// Traverse else block
if (Token::simpleMatch(endBlock, "} else {")) {
if (checkElse) {
i.errorPath.back().second = "Assuming condition is false.";
Progress result = forwardRange(endCond->next(), endBlock, i, f);
if (result == Progress::Break)
return Progress::Break;
}
tok = endBlock->linkAt(2);
} else {
tok = endBlock;
}
} else if (Token::simpleMatch(tok, "} else {")) {
tok = tok->linkAt(2);
} else {
info.tok = tok;
if (f(info) == Progress::Break)
return Progress::Break;
}
// Prevent inifinite recursion
if (tok->next() == start)
break;
}
return Progress::Continue;
}
void PathAnalysis::forward(const std::function<Progress(const Info&)>& f) const
{
const Scope * endScope = findOuterScope(start->scope());
if (!endScope)
return;
const Token * endToken = endScope->bodyEnd;
Info info{start, ErrorPath{}, true};
forwardRange(start, endToken, info, f);
}
bool reaches(const Token * start, const Token * dest, const Library& library, ErrorPath* errorPath)
{
PathAnalysis::Info info = PathAnalysis{start, library} .forwardFind([&](const PathAnalysis::Info& i) {
return (i.tok == dest);
});
if (!info.tok)
return false;
if (errorPath)
errorPath->insert(errorPath->end(), info.errorPath.begin(), info.errorPath.end());
return true;
}
struct FwdAnalysis::Result FwdAnalysis::checkRecursive(const Token *expr, const Token *startToken, const Token *endToken, const std::set<int> &exprVarIds, bool local, bool inInnerClass)
{
// Parse the given tokens

View File

@ -31,6 +31,7 @@
class Library;
class Settings;
class Scope;
class Token;
class Variable;
@ -169,6 +170,62 @@ bool isConstVarExpression(const Token *tok);
const Variable *getLHSVariable(const Token *tok);
struct PathAnalysis {
enum Progress {
Continue,
Break
};
PathAnalysis(const Token* start, const Library& library)
: start(start), library(&library)
{}
const Token * start;
const Library * library;
struct Info {
const Token* tok;
ErrorPath errorPath;
bool known;
};
void forward(const std::function<Progress(const Info&)>& f) const;
template<class F>
void forwardAll(F f) {
forward([&](const Info& info) {
f(info);
return Progress::Continue;
});
}
template<class Predicate>
Info forwardFind(Predicate pred) {
Info result{};
forward([&](const Info& info) {
if (pred(info)) {
result = info;
return Progress::Break;
}
return Progress::Continue;
});
return result;
}
private:
Progress forwardRecursive(const Token* tok, Info info, const std::function<PathAnalysis::Progress(const Info&)>& f) const;
Progress forwardRange(const Token* startToken, const Token* endToken, Info info, const std::function<Progress(const Info&)>& f) const;
static const Scope* findOuterScope(const Scope * scope);
static std::pair<bool, bool> checkCond(const Token * tok, bool& known);
};
/**
* @brief Returns true if there is a path between the two tokens
*
* @param start Starting point of the path
* @param dest The path destination
* @param errorPath Adds the path traversal to the errorPath
*/
bool reaches(const Token * start, const Token * dest, const Library& library, ErrorPath* errorPath);
/**
* Forward data flow analysis for checks
* - unused value

View File

@ -631,38 +631,6 @@ void CheckAutoVariables::checkVarLifetime()
}
}
static std::string lifetimeMessage(const Token *tok, const ValueFlow::Value *val, ErrorPath &errorPath)
{
const Token *tokvalue = val ? val->tokvalue : nullptr;
const Variable *tokvar = tokvalue ? tokvalue->variable() : nullptr;
const Token *vartok = tokvar ? tokvar->nameToken() : nullptr;
std::string type = lifetimeType(tok, val);
std::string msg = type;
if (vartok) {
errorPath.emplace_back(vartok, "Variable created here.");
const Variable * var = vartok->variable();
if (var) {
switch (val->lifetimeKind) {
case ValueFlow::Value::LifetimeKind::Object:
case ValueFlow::Value::LifetimeKind::Address:
if (type == "pointer")
msg += " to local variable";
else
msg += " that points to local variable";
break;
case ValueFlow::Value::LifetimeKind::Lambda:
msg += " that captures local variable";
break;
case ValueFlow::Value::LifetimeKind::Iterator:
msg += " to local container";
break;
}
msg += " '" + var->name() + "'";
}
}
return msg;
}
void CheckAutoVariables::errorReturnDanglingLifetime(const Token *tok, const ValueFlow::Value *val)
{
ErrorPath errorPath = val ? val->errorPath : ErrorPath();

View File

@ -377,6 +377,15 @@ static const Token* findIteratorContainer(const Token* start, const Token* end,
return containerToken;
}
static bool isVector(const Token* tok)
{
if (!tok)
return false;
const Variable *var = tok->variable();
const Token *decltok = var ? var->typeStartToken() : nullptr;
return Token::simpleMatch(decltok, "std :: vector");
}
void CheckStl::iterators()
{
const SymbolDatabase *symbolDatabase = mTokenizer->getSymbolDatabase();
@ -439,7 +448,7 @@ void CheckStl::iterators()
}
// Is the iterator used in a insert/erase operation?
else if (Token::Match(tok2, "%name% . insert|erase ( *| %varid% )|,", iteratorId)) {
else if (Token::Match(tok2, "%name% . insert|erase ( *| %varid% )|,", iteratorId) && !isVector(tok2)) {
const Token* itTok = tok2->tokAt(4);
if (itTok->str() == "*") {
if (tok2->strAt(2) == "insert")
@ -772,6 +781,83 @@ void CheckStl::mismatchingContainers()
}
}
static bool isInvalidMethod(const Token * tok)
{
if (Token::Match(tok->next(), ". assign|clear"))
return true;
if (Token::Match(tok->next(), "%assign%"))
return true;
if (isVector(tok) && Token::Match(tok->next(), ". insert|emplace|emplace_back|push_back|erase|pop_back|reserve ("))
return true;
return false;
}
void CheckStl::invalidContainer()
{
const SymbolDatabase *symbolDatabase = mTokenizer->getSymbolDatabase();
const Library& library = mSettings->library;
for (const Scope * scope : symbolDatabase->functionScopes) {
for (const Token* tok = scope->bodyStart->next(); tok != scope->bodyEnd; tok = tok->next()) {
if (!Token::Match(tok, "%var%"))
continue;
if (tok->varId() == 0)
continue;
if (!astIsContainer(tok))
continue;
if (!isInvalidMethod(tok))
continue;
// Skip if the variable is assigned to
unsigned int skipVarId = 0;
if (Token::Match(tok->astTop(), "%assign%") && Token::Match(tok->astTop()->previous(), "%var%"))
skipVarId = tok->astTop()->previous()->varId();
const Token * endToken = nextAfterAstRightmostLeaf(tok->next()->astParent());
if (!endToken)
endToken = tok->next();
const ValueFlow::Value* v = nullptr;
ErrorPath errorPath;
PathAnalysis::Info info = PathAnalysis{endToken, library} .forwardFind([&](const PathAnalysis::Info& info) {
if (!info.tok->variable())
return false;
if (info.tok->varId() == skipVarId)
return false;
for (const ValueFlow::Value& val:info.tok->values()) {
if (!val.isLocalLifetimeValue())
continue;
if (!val.tokvalue->variable())
continue;
if (val.tokvalue->varId() != tok->varId())
continue;
// Skip possible temporaries
if (val.tokvalue == tok)
continue;
ErrorPath ep;
// Check the iterator is created before the change
if (reaches(val.tokvalue, tok, library, &ep)) {
v = &val;
errorPath = ep;
return true;
}
}
return false;
});
if (!info.tok || !v)
continue;
errorPath.insert(errorPath.end(), info.errorPath.begin(), info.errorPath.end());
invalidContainerError(info.tok, tok, v, errorPath);
}
}
}
void CheckStl::invalidContainerError(const Token *tok, const Token * contTok, const ValueFlow::Value *val, ErrorPath errorPath)
{
std::string method = contTok ? contTok->strAt(2) : "erase";
errorPath.emplace_back(contTok, "After calling '" + method + "', iterators or references to the container's data may be invalid .");
if (val)
errorPath.insert(errorPath.begin(), val->errorPath.begin(), val->errorPath.end());
std::string msg = "Using " + lifetimeMessage(tok, val, errorPath);
errorPath.emplace_back(tok, "");
reportError(errorPath, Severity::error, "invalidContainer", msg + " that may be invalid.", CWE664, false);
}
void CheckStl::stlOutOfBounds()
{
@ -921,6 +1007,9 @@ void CheckStl::eraseCheckLoopVar(const Scope &scope, const Variable *var)
continue;
if (!Token::Match(tok->tokAt(-2), ". erase ( ++| %varid% )", var->declarationId()))
continue;
// Vector erases are handled by invalidContainer check
if (isVector(tok->tokAt(-3)))
continue;
if (Token::simpleMatch(tok->astParent(), "="))
continue;
// Iterator is invalid..
@ -952,162 +1041,6 @@ void CheckStl::eraseCheckLoopVar(const Scope &scope, const Variable *var)
}
}
void CheckStl::pushback()
{
// Pointer can become invalid after push_back, push_front, reserve or resize..
const SymbolDatabase* const symbolDatabase = mTokenizer->getSymbolDatabase();
for (const Scope * scope : symbolDatabase->functionScopes) {
for (const Token* tok = scope->bodyStart->next(); tok != scope->bodyEnd; tok = tok->next()) {
if (Token::Match(tok, "%var% = & %var% [")) {
// Skip it directly if it is a pointer or an array
const Token* containerTok = tok->tokAt(3);
if (containerTok->variable() && containerTok->variable()->isArrayOrPointer())
continue;
// Variable id for pointer
const int pointerId(tok->varId());
bool invalidPointer = false;
const Token* function = nullptr;
const Token* end2 = tok->scope()->bodyEnd;
for (const Token *tok2 = tok; tok2 != end2; tok2 = tok2->next()) {
// push_back on vector..
if (Token::Match(tok2, "%varid% . push_front|push_back|insert|reserve|resize|clear", containerTok->varId())) {
invalidPointer = true;
function = tok2->tokAt(2);
}
// Using invalid pointer..
if (invalidPointer && tok2->varId() == pointerId) {
bool unknown = false;
if (CheckNullPointer::isPointerDeRef(tok2, unknown, mSettings))
invalidPointerError(tok2, function->str(), tok2->str());
break;
}
}
}
}
}
// Iterator becomes invalid after reserve, resize, insert, push_back or push_front..
for (const Variable* var : symbolDatabase->variableList()) {
// Check that its an iterator
if (!var || !var->isLocal() || !Token::Match(var->typeEndToken(), "iterator|const_iterator|reverse_iterator|const_reverse_iterator"))
continue;
const int iteratorId = var->declarationId();
// ... on std::vector
if (!Token::Match(var->typeStartToken(), "std| ::| vector <"))
continue;
// the variable id for the vector
int vectorid = 0;
const Token* validatingToken = nullptr;
std::string invalidIterator;
const Token* end2 = var->scope()->bodyEnd;
for (const Token *tok2 = var->nameToken(); tok2 != end2; tok2 = tok2->next()) {
if (validatingToken == tok2) {
invalidIterator.clear();
validatingToken = nullptr;
}
// Using push_back or push_front inside a loop..
if (Token::simpleMatch(tok2, "for (")) {
tok2 = tok2->tokAt(2);
}
if (Token::Match(tok2, "%varid% = %var% . begin|rbegin|cbegin|crbegin ( ) ; %varid% != %var% . end|rend|cend|crend ( ) ; ++| %varid% ++| ) {", iteratorId)) {
// variable id for the loop iterator
const int varId(tok2->tokAt(2)->varId());
const Token *pushbackTok = nullptr;
// Count { and } for tok3
const Token *tok3 = tok2->tokAt(20);
for (const Token* const end3 = tok3->linkAt(-1); tok3 != end3; tok3 = tok3->next()) {
if (tok3->str() == "break" || tok3->str() == "return") {
pushbackTok = nullptr;
break;
} else if (Token::Match(tok3, "%varid% . push_front|push_back|insert|reserve|resize|clear|erase (", varId) && !tok3->previous()->isAssignmentOp()) {
if (tok3->strAt(2) != "erase" || (tok3->tokAt(4)->varId() != iteratorId && tok3->tokAt(5)->varId() != iteratorId)) // This case is handled in: CheckStl::iterators()
pushbackTok = tok3->tokAt(2);
}
}
if (pushbackTok)
invalidIteratorError(pushbackTok, pushbackTok->str(), tok2->str());
}
// Assigning iterator..
if (Token::Match(tok2, "%varid% =", iteratorId)) {
if (Token::Match(tok2->tokAt(2), "%var% . begin|end|rbegin|rend|cbegin|cend|crbegin|crend|insert|erase|find (")) {
if (!invalidIterator.empty() && Token::Match(tok2->tokAt(4), "insert|erase ( *| %varid% )|,", iteratorId)) {
invalidIteratorError(tok2, invalidIterator, var->name());
break;
}
vectorid = tok2->tokAt(2)->varId();
tok2 = tok2->linkAt(5);
} else {
vectorid = 0;
}
invalidIterator.clear();
}
// push_back on vector..
if (vectorid > 0 && Token::Match(tok2, "%varid% . push_front|push_back|insert|reserve|resize|clear|erase (", vectorid)) {
if (!invalidIterator.empty() && Token::Match(tok2->tokAt(2), "insert|erase ( *| %varid% ,|)", iteratorId)) {
invalidIteratorError(tok2, invalidIterator, var->name());
break;
}
if (tok2->strAt(2) != "erase" || (tok2->tokAt(4)->varId() != iteratorId && tok2->tokAt(5)->varId() != iteratorId)) // This case is handled in: CheckStl::iterators()
invalidIterator = tok2->strAt(2);
tok2 = tok2->linkAt(3);
}
else if (tok2->str() == "return" || tok2->str() == "throw")
validatingToken = Token::findsimplematch(tok2->next(), ";");
// TODO: instead of bail out for 'else' try to check all execution paths.
else if (tok2->str() == "break" || tok2->str() == "else")
invalidIterator.clear();
// Using invalid iterator..
if (!invalidIterator.empty()) {
if (Token::Match(tok2, "++|--|*|+|-|(|,|=|!= %varid%", iteratorId))
invalidIteratorError(tok2, invalidIterator, tok2->strAt(1));
if (Token::Match(tok2, "%varid% ++|--|+|-|.", iteratorId))
invalidIteratorError(tok2, invalidIterator, tok2->str());
}
}
}
}
// Error message for bad iterator usage..
void CheckStl::invalidIteratorError(const Token *tok, const std::string &func, const std::string &iterator_name)
{
reportError(tok, Severity::error, "invalidIterator2",
"$symbol:" + func + "\n"
"$symbol:" + iterator_name + "\n"
"After " + func + "(), the iterator '" + iterator_name + "' may be invalid.", CWE664, false);
}
// Error message for bad iterator usage..
void CheckStl::invalidPointerError(const Token *tok, const std::string &func, const std::string &pointer_name)
{
reportError(tok, Severity::error, "invalidPointer",
"$symbol:" + func + "\n"
"$symbol:" + pointer_name + "\n"
"Invalid pointer '" + pointer_name + "' after " + func + "().", CWE664, false);
}
void CheckStl::stlBoundaries()
{
const SymbolDatabase* const symbolDatabase = mTokenizer->getSymbolDatabase();

View File

@ -68,7 +68,6 @@ public:
checkStl.missingComparison();
checkStl.outOfBounds();
checkStl.outOfBoundsIndexExpression();
checkStl.pushback();
checkStl.redundantCondition();
checkStl.string_c_str();
checkStl.uselessCalls();
@ -76,6 +75,10 @@ public:
checkStl.stlOutOfBounds();
checkStl.negativeIndex();
checkStl.invalidContainer();
checkStl.mismatchingContainers();
checkStl.stlBoundaries();
checkStl.checkDereferenceInvalidIterator();
@ -106,6 +109,8 @@ public:
*/
void iterators();
void invalidContainer();
/**
* Mismatching containers:
* std::find(foo.begin(), bar.end(), x)
@ -119,12 +124,6 @@ public:
void erase();
void eraseCheckLoopVar(const Scope& scope, const Variable* var);
/**
* Dangerous usage of push_back and insert
*/
void pushback();
/**
* bad condition.. "it < alist.end()"
*/
@ -201,13 +200,12 @@ private:
void mismatchingContainersError(const Token* tok);
void mismatchingContainerExpressionError(const Token *tok1, const Token *tok2);
void sameIteratorExpressionError(const Token *tok);
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);
void if_findError(const Token* tok, bool str);
void checkFindInsertError(const Token *tok);
void sizeError(const Token* tok);
void redundantIfRemoveError(const Token* tok);
void invalidContainerError(const Token *tok, const Token * contTok, const ValueFlow::Value *val, ErrorPath errorPath);
void uselessCallsReturnValueError(const Token* tok, const std::string& varname, const std::string& function);
void uselessCallsSwapError(const Token* tok, const std::string& varname);
@ -224,6 +222,7 @@ private:
bool compareIteratorAgainstDifferentContainer(const Token* operatorTok, const Token* containerTok, const nonneg int iteratorId, const std::map<int, const Token*>& iteratorScopeBeginInfo);
void getErrorMessages(ErrorLogger* errorLogger, const Settings* settings) const OVERRIDE {
ErrorPath errorPath;
CheckStl c(nullptr, settings, errorLogger);
c.outOfBoundsError(nullptr, "container", nullptr, "x", nullptr);
c.invalidIteratorError(nullptr, "iterator");
@ -232,14 +231,13 @@ private:
c.iteratorsError(nullptr, nullptr, "container");
c.iteratorsCmpError(nullptr, nullptr, nullptr, "container1", "container2");
c.iteratorsCmpError(nullptr, nullptr, nullptr, "container");
c.invalidContainerError(nullptr, nullptr, nullptr, errorPath);
c.mismatchingContainersError(nullptr);
c.mismatchingContainerExpressionError(nullptr, nullptr);
c.sameIteratorExpressionError(nullptr);
c.dereferenceErasedError(nullptr, nullptr, "iter", false);
c.stlOutOfBoundsError(nullptr, "i", "foo", false);
c.negativeIndexError(nullptr, ValueFlow::Value(-1));
c.invalidIteratorError(nullptr, "push_back|push_front|insert", "iterator");
c.invalidPointerError(nullptr, "push_back", "pointer");
c.stlBoundariesError(nullptr);
c.if_findError(nullptr, false);
c.if_findError(nullptr, true);

View File

@ -2706,6 +2706,38 @@ std::string lifetimeType(const Token *tok, const ValueFlow::Value *val)
return result;
}
std::string lifetimeMessage(const Token *tok, const ValueFlow::Value *val, ErrorPath &errorPath)
{
const Token *tokvalue = val ? val->tokvalue : nullptr;
const Variable *tokvar = tokvalue ? tokvalue->variable() : nullptr;
const Token *vartok = tokvar ? tokvar->nameToken() : nullptr;
std::string type = lifetimeType(tok, val);
std::string msg = type;
if (vartok) {
errorPath.emplace_back(vartok, "Variable created here.");
const Variable * var = vartok->variable();
if (var) {
switch (val->lifetimeKind) {
case ValueFlow::Value::LifetimeKind::Object:
case ValueFlow::Value::LifetimeKind::Address:
if (type == "pointer")
msg += " to local variable";
else
msg += " that points to local variable";
break;
case ValueFlow::Value::LifetimeKind::Lambda:
msg += " that captures local variable";
break;
case ValueFlow::Value::LifetimeKind::Iterator:
msg += " to local container";
break;
}
msg += " '" + var->name() + "'";
}
}
return msg;
}
ValueFlow::Value getLifetimeObjValue(const Token *tok)
{
ValueFlow::Value result;
@ -3339,7 +3371,7 @@ 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 (")) {
else if (tok->variable() && Token::Match(tok, "%var% . begin|cbegin|rbegin|crbegin|end|cend|rend|crend|data|c_str|find|insert (")) {
if (Token::simpleMatch(tok->tokAt(2), "find") && !astIsIterator(tok->tokAt(3)))
continue;
ErrorPath errorPath;

View File

@ -243,6 +243,8 @@ bool isLifetimeBorrowed(const Token *tok, const Settings *settings);
std::string lifetimeType(const Token *tok, const ValueFlow::Value *val);
std::string lifetimeMessage(const Token *tok, const ValueFlow::Value *val, ValueFlow::Value::ErrorPath &errorPath);
ValueFlow::Value getLifetimeObjValue(const Token *tok);
#endif // valueflowH

View File

@ -1 +1 @@
[samples\erase\bad.cpp:9] -> [samples\erase\bad.cpp:11]: (error) Iterator 'iter' used after element has been erased.
[samples\erase\bad.cpp:9] -> [samples\erase\bad.cpp:10] -> [samples\erase\bad.cpp:10] -> [samples\erase\bad.cpp:9] -> [samples\erase\bad.cpp:11] -> [samples\erase\bad.cpp:4] -> [samples\erase\bad.cpp:9]: (error) Using iterator to local container 'items' that may be invalid.

View File

@ -156,6 +156,8 @@ private:
TEST_CASE(loopAlgoIncrement);
TEST_CASE(loopAlgoConditional);
TEST_CASE(loopAlgoMinMax);
TEST_CASE(invalidContainer);
TEST_CASE(findInsert);
}
@ -1149,7 +1151,7 @@ private:
" ints.erase(iter);\n"
" std::cout << (*iter) << std::endl;\n"
"}");
ASSERT_EQUALS("[test.cpp:7] -> [test.cpp:6]: (error) Iterator 'iter' used after element has been erased.\n", errout.str());
ASSERT_EQUALS("[test.cpp:5] -> [test.cpp:6] -> [test.cpp:3] -> [test.cpp:7]: (error) Using iterator to local container 'ints' that may be invalid.\n", errout.str());
// #6554 "False positive eraseDereference - erase in while() loop"
check("typedef std::map<Packet> packetMap;\n"
@ -1220,7 +1222,7 @@ private:
" ints.erase(iter);\n"
" std::cout << (*iter) << std::endl;\n"
"}");
ASSERT_EQUALS("[test.cpp:6] -> [test.cpp:5]: (error) Iterator 'iter' used after element has been erased.\n", errout.str());
ASSERT_EQUALS("[test.cpp:4] -> [test.cpp:5] -> [test.cpp:3] -> [test.cpp:6]: (error) Using iterator to local container 'ints' that may be invalid.\n", errout.str());
check("void f() {\n"
" auto x = *myList.begin();\n"
@ -1631,7 +1633,7 @@ private:
" }\n"
" }\n"
"}");
ASSERT_EQUALS("[test.cpp:5] -> [test.cpp:9]: (error) Iterator 'it' used after element has been erased.\n", errout.str());
ASSERT_EQUALS("[test.cpp:5] -> [test.cpp:7] -> [test.cpp:8] -> [test.cpp:8] -> [test.cpp:7] -> [test.cpp:5] -> [test.cpp:9] -> [test.cpp:3] -> [test.cpp:5]: (error) Using iterator to local container 'foo' that may be invalid.\n", errout.str());
}
void eraseGoto() {
@ -1719,7 +1721,7 @@ private:
" ints.erase(iter);\n"
" ints.erase(iter);\n"
"}");
ASSERT_EQUALS("[test.cpp:6]: (error) Invalid iterator: iter\n", errout.str());
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());
}
void eraseByValue() {
@ -1757,15 +1759,15 @@ private:
ASSERT_EQUALS("", errout.str());
check("void f(const std::list<int>& m_ImplementationMap) {\n"
" std::list<int>::iterator aIt = m_ImplementationMap.find( xEle );\n"
" std::list<int>::iterator aIt = m_ImplementationMap.begin();\n"
" m_ImplementationMap.erase(*aIt);\n"
" m_ImplementationMap.erase(aIt);\n"
"}");
ASSERT_EQUALS("[test.cpp:4]: (error) Invalid iterator: aIt\n", errout.str());
check("void f(const std::list<int>& m_ImplementationMap) {\n"
" std::list<int>::iterator aIt = m_ImplementationMap.find( xEle1 );\n"
" std::list<int>::iterator bIt = m_ImplementationMap.find( xEle2 );\n"
" std::list<int>::iterator aIt = m_ImplementationMap.begin();\n"
" std::list<int>::iterator bIt = m_ImplementationMap.begin();\n"
" m_ImplementationMap.erase(*bIt);\n"
" m_ImplementationMap.erase(aIt);\n"
"}");
@ -1788,26 +1790,26 @@ private:
void eraseOnVector() {
check("void f(const std::vector<int>& m_ImplementationMap) {\n"
" std::vector<int>::iterator aIt = m_ImplementationMap.find( xEle );\n"
" std::vector<int>::iterator aIt = m_ImplementationMap.begin();\n"
" m_ImplementationMap.erase(something(unknown));\n" // All iterators become invalidated when erasing from std::vector
" m_ImplementationMap.erase(aIt);\n"
"}");
ASSERT_EQUALS("[test.cpp:4]: (error) After erase(), the iterator 'aIt' may be invalid.\n", errout.str());
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());
check("void f(const std::vector<int>& m_ImplementationMap) {\n"
" std::vector<int>::iterator aIt = m_ImplementationMap.find( xEle );\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:4]: (error) Invalid iterator: aIt\n", errout.str());
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());
check("void f(const std::vector<int>& m_ImplementationMap) {\n"
" std::vector<int>::iterator aIt = m_ImplementationMap.find( xEle1 );\n"
" std::vector<int>::iterator bIt = m_ImplementationMap.find( xEle2 );\n"
" std::vector<int>::iterator aIt = m_ImplementationMap.begin();\n"
" std::vector<int>::iterator bIt = m_ImplementationMap.begin();\n"
" m_ImplementationMap.erase(*bIt);\n" // All iterators become invalidated when erasing from std::vector
" aIt = m_ImplementationMap.erase(aIt);\n"
"}");
ASSERT_EQUALS("[test.cpp:5]: (error) After erase(), the iterator 'aIt' may be invalid.\n", errout.str());
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());
}
void pushback1() {
@ -1817,7 +1819,7 @@ private:
" foo.push_back(123);\n"
" *it;\n"
"}");
ASSERT_EQUALS("[test.cpp:5]: (error) After push_back(), the iterator 'it' may be invalid.\n", errout.str());
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());
}
void pushback2() {
@ -1844,7 +1846,7 @@ private:
" foo.push_back(123);\n"
" }\n"
"}");
ASSERT_EQUALS("[test.cpp:8]: (error) After push_back(), the iterator 'it' may be invalid.\n", errout.str());
ASSERT_EQUALS("[test.cpp:6] -> [test.cpp:6] -> [test.cpp:8] -> [test.cpp:3] -> [test.cpp:6]: (error) Using iterator to local container 'foo' that may be invalid.\n", errout.str());
}
void pushback4() {
@ -1856,7 +1858,7 @@ private:
" ints.push_back(2);\n"
" *first;\n"
"}");
ASSERT_EQUALS("[test.cpp:7]: (error) Invalid pointer 'first' after push_back().\n", errout.str());
ASSERT_EQUALS("[test.cpp:5] -> [test.cpp:6] -> [test.cpp:3] -> [test.cpp:7]: (error) Using pointer to local variable 'ints' that may be invalid.\n", errout.str());
}
void pushback5() {
@ -1880,16 +1882,16 @@ private:
// ticket #735
check("void f()\n"
"{\n"
" vector<int> v;\n"
" std::vector<int> v;\n"
" v.push_back(1);\n"
" v.push_back(2);\n"
" for (vector<int>::iterator it = v.begin(); it != v.end(); ++it)\n"
" for (std::vector<int>::iterator it = v.begin(); it != v.end(); ++it)\n"
" {\n"
" if (*it == 1)\n"
" v.push_back(10);\n"
" }\n"
"}");
ASSERT_EQUALS("[test.cpp:9]: (error) After push_back(), the iterator 'it' may be invalid.\n", errout.str());
ASSERT_EQUALS("[test.cpp:6] -> [test.cpp:8] -> [test.cpp:8] -> [test.cpp:6] -> [test.cpp:9] -> [test.cpp:3] -> [test.cpp:6]: (error) Using iterator to local container 'v' that may be invalid.\n", errout.str());
check("void f()\n"
"{\n"
@ -1902,7 +1904,7 @@ private:
" v.push_back(10);\n"
" }\n"
"}");
ASSERT_EQUALS("[test.cpp:9]: (error) After push_back(), the iterator 'it' may be invalid.\n", errout.str());
ASSERT_EQUALS("[test.cpp:6] -> [test.cpp:8] -> [test.cpp:8] -> [test.cpp:6] -> [test.cpp:9] -> [test.cpp:3] -> [test.cpp:6]: (error) Using iterator to local container 'v' that may be invalid.\n", errout.str());
}
void pushback7() {
@ -1916,7 +1918,7 @@ private:
" foo.push_back(123);\n"
" }\n"
"}");
ASSERT_EQUALS("[test.cpp:8]: (error) After push_back(), the iterator 'it' may be invalid.\n", errout.str());
ASSERT_EQUALS("[test.cpp:6] -> [test.cpp:6] -> [test.cpp:8] -> [test.cpp:3] -> [test.cpp:6]: (error) Using iterator to local container 'foo' that may be invalid.\n", errout.str());
}
void pushback8() {
@ -1932,7 +1934,7 @@ private:
" sum += *it;\n"
" }\n"
"}");
ASSERT_EQUALS("[test.cpp:8]: (error) After push_back(), the iterator 'end' may be invalid.\n", errout.str());
ASSERT_EQUALS("[test.cpp:4] -> [test.cpp:5] -> [test.cpp:3] -> [test.cpp:8]: (error) Using iterator to local container 'ints' that may be invalid.\n", errout.str());
}
void pushback9() {
@ -1962,7 +1964,7 @@ private:
" foo.reserve(100);\n"
" *it = 0;\n"
"}");
ASSERT_EQUALS("[test.cpp:5]: (error) After reserve(), the iterator 'it' may be invalid.\n", errout.str());
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());
// in loop
check("void f()\n"
@ -1975,7 +1977,7 @@ private:
" foo.reserve(123);\n"
" }\n"
"}");
ASSERT_EQUALS("[test.cpp:8]: (error) After reserve(), the iterator 'it' may be invalid.\n", errout.str());
ASSERT_EQUALS("[test.cpp:6] -> [test.cpp:6] -> [test.cpp:8] -> [test.cpp:3] -> [test.cpp:6]: (error) Using iterator to local container 'foo' that may be invalid.\n", errout.str());
}
void pushback11() {
@ -2005,8 +2007,7 @@ private:
" *it;\n"
" }\n"
"}");
ASSERT_EQUALS("[test.cpp:6]: (error) After insert(), the iterator 'it' may be invalid.\n"
"[test.cpp:9]: (error) After insert(), the iterator 'it' may be invalid.\n", errout.str());
ASSERT_EQUALS("[test.cpp:4] -> [test.cpp:6] -> [test.cpp:3] -> [test.cpp:9]: (error) Using iterator to local container 'vec' that may be invalid.\n", errout.str());
}
void pushback13() {
@ -2026,7 +2027,7 @@ private:
" ints.insert(ints.begin(), 1);\n"
" ++iter;\n"
"}");
ASSERT_EQUALS("[test.cpp:5]: (error) After insert(), the iterator 'iter' may be invalid.\n", errout.str());
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());
check("void f()\n"
"{\n"
@ -2043,7 +2044,7 @@ private:
" ints.insert(iter, 1);\n"
" ints.insert(iter, 2);\n"
"}");
ASSERT_EQUALS("[test.cpp:6]: (error) After insert(), the iterator 'iter' may be invalid.\n", errout.str());
ASSERT_EQUALS("[test.cpp:4] -> [test.cpp:5] -> [test.cpp:3] -> [test.cpp:6]: (error) Using iterator to local container 'ints' that may be invalid.\n", errout.str());
check("void* f(const std::vector<Bar>& bars) {\n"
" std::vector<Bar>::iterator i = bars.begin();\n"
@ -2051,14 +2052,14 @@ private:
" void* v = &i->foo;\n"
" return v;\n"
"}");
ASSERT_EQUALS("[test.cpp:4]: (error) After insert(), the iterator 'i' may be invalid.\n", errout.str());
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());
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:4]: (error) After insert(), the iterator 'i' may be invalid.\n", errout.str());
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());
check("void f(const std::vector<Bar>& bars) {\n"
" for(std::vector<Bar>::iterator i = bars.begin(); i != bars.end(); ++i) {\n"
@ -2073,8 +2074,7 @@ private:
" i = bars.insert(i, bar);\n"
" }\n"
"}");
ASSERT_EQUALS("[test.cpp:3]: (error) After insert(), the iterator 'i' may be invalid.\n"
"[test.cpp:4]: (error) After insert(), the iterator 'i' may be invalid.\n", errout.str());
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());
check("void* f(const std::vector<Bar>& bars) {\n"
" std::vector<Bar>::iterator i = bars.begin();\n"
@ -2083,7 +2083,8 @@ private:
" void* v = &i->foo;\n"
" return v;\n"
"}");
ASSERT_EQUALS("[test.cpp:4]: (error) After insert(), the iterator 'i' may be invalid.\n", errout.str());
ASSERT_EQUALS("[test.cpp:4] -> [test.cpp:1] -> [test.cpp:5] -> [test.cpp:3] -> [test.cpp:1] -> [test.cpp:6]: (error) Using pointer to local variable 'bars' that may be invalid.\n"
"[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() {
@ -3846,6 +3847,28 @@ private:
ASSERT_EQUALS("[test.cpp:4]: (style) Consider using std::accumulate algorithm instead of a raw loop.\n", errout.str());
}
void invalidContainer() {
check("void f(std::vector<int> &v) {\n"
" auto v0 = v.begin();\n"
" v.push_back(123);\n"
" 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());
check("std::string e();\n"
"void a() {\n"
" std::vector<std::string> b;\n"
" for (std::vector<std::string>::const_iterator c; c != b.end(); ++c) {\n"
" std::string f = e();\n"
" std::string::const_iterator d = f.begin();\n"
" if (d != f.end()) {}\n"
" }\n"
"}\n",
true);
ASSERT_EQUALS("", errout.str());
}
void findInsert() {
check("void f1(std::set<unsigned>& s, unsigned x) {\n"
" if (s.find(x) == s.end()) {\n"