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

Fix issue 737: new check: Dereference end iterator

This commit is contained in:
Paul 2020-08-17 16:36:45 -05:00
parent 7ff5a208a5
commit 7776fb82a2
7 changed files with 205 additions and 24 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

8
lib/checknullpointer.cpp
View File

@ -172,11 +172,9 @@ bool CheckNullPointer::isPointerDeRef(const Token *tok, bool &unknown, const Set
if (parent->str() == "." && parent->astOperand2() == tok) if (parent->str() == "." && parent->astOperand2() == tok)
return isPointerDeRef(parent, unknown, settings); return isPointerDeRef(parent, unknown, settings);
const bool firstOperand = parent->astOperand1() == tok; const bool firstOperand = parent->astOperand1() == tok;
while (parent->str() == "(" && (parent->astOperand2() == nullptr && parent->strAt(1) != ")")) { // Skip over casts parent = astParentSkipParens(tok);
parent = parent->astParent(); if (!parent)
if (!parent) return false;
return false;
}
// Dereferencing pointer.. // Dereferencing pointer..
if (parent->isUnaryOp("*") && !Token::Match(parent->tokAt(-2), "sizeof|decltype|typeof")) if (parent->isUnaryOp("*") && !Token::Match(parent->tokAt(-2), "sizeof|decltype|typeof"))

63
lib/checkstl.cpp
View File

@ -18,6 +18,8 @@
#include "checkstl.h" #include "checkstl.h"
#include "check.h"
#include "checknullpointer.h"
#include "library.h" #include "library.h"
#include "mathlib.h" #include "mathlib.h"
#include "settings.h" #include "settings.h"
@ -2011,6 +2013,67 @@ void CheckStl::checkDereferenceInvalidIterator()
} }
} }
void CheckStl::checkDereferenceInvalidIterator2()
{
const bool printInconclusive = (mSettings->inconclusive);
for (const Token *tok = mTokenizer->tokens(); tok; tok = tok->next()) {
if (Token::Match(tok, "sizeof|decltype|typeid|typeof (")) {
tok = tok->next()->link();
continue;
}
// Can iterator point to END or before START?
for(const ValueFlow::Value& value:tok->values()) {
if (!printInconclusive && value.isInconclusive())
continue;
if (!value.isIteratorValue())
continue;
if (value.isIteratorEndValue() && value.intvalue < 0)
continue;
if (value.isIteratorStartValue() && value.intvalue >= 0)
continue;
bool unknown = false;
if (!CheckNullPointer::isPointerDeRef(tok, unknown, mSettings)) {
if (unknown)
dereferenceInvalidIteratorError(tok, &value, true);
continue;
}
dereferenceInvalidIteratorError(tok, &value, false);
}
}
}
void CheckStl::dereferenceInvalidIteratorError(const Token* tok, const ValueFlow::Value *value, bool inconclusive)
{
const std::string& varname = tok ? tok->expressionString() : "var";
const std::string errmsgcond("$symbol:" + varname + '\n' + ValueFlow::eitherTheConditionIsRedundant(value ? value->condition : nullptr) + " or there is possible dereference of an invalid iterator: $symbol.");
if (!tok || !value) {
reportError(tok, Severity::error, "derefInvalidIterator", "Dereference of an invalid iterator", CWE825, false);
reportError(tok, Severity::warning, "derefInvalidIteratorRedundantCheck", errmsgcond, CWE825, false);
return;
}
if (!mSettings->isEnabled(value, inconclusive))
return;
const ErrorPath errorPath = getErrorPath(tok, value, "Dereference of an invalid iterator");
if (value->condition) {
reportError(errorPath, Severity::warning, "derefInvalidIteratorRedundantCheck", errmsgcond, CWE825, inconclusive || value->isInconclusive());
} else {
std::string errmsg;
errmsg = std::string(value->isKnown() ? "Dereference" : "Possible dereference") + " of an invalid iterator";
if (!varname.empty())
errmsg = "$symbol:" + varname + '\n' + errmsg + ": $symbol";
reportError(errorPath,
value->isKnown() ? Severity::error : Severity::warning,
"derefInvalidIterator",
errmsg,
CWE825, inconclusive || value->isInconclusive());
}
}
void CheckStl::dereferenceInvalidIteratorError(const Token* deref, const std::string &iterName) void CheckStl::dereferenceInvalidIteratorError(const Token* deref, const std::string &iterName)
{ {
reportError(deref, Severity::warning, reportError(deref, Severity::warning,

3
lib/checkstl.h
View File

@ -84,6 +84,7 @@ public:
checkStl.stlBoundaries(); checkStl.stlBoundaries();
checkStl.checkDereferenceInvalidIterator(); checkStl.checkDereferenceInvalidIterator();
checkStl.checkDereferenceInvalidIterator2();
checkStl.checkMutexes(); checkStl.checkMutexes();
// Style check // Style check
@ -171,6 +172,7 @@ public:
/** @brief %Check for dereferencing an iterator that is invalid */ /** @brief %Check for dereferencing an iterator that is invalid */
void checkDereferenceInvalidIterator(); void checkDereferenceInvalidIterator();
void checkDereferenceInvalidIterator2();
/** /**
* Dereferencing an erased iterator * Dereferencing an erased iterator
@ -227,6 +229,7 @@ private:
void uselessCallsRemoveError(const Token* tok, const std::string& function); void uselessCallsRemoveError(const Token* tok, const std::string& function);
void dereferenceInvalidIteratorError(const Token* deref, const std::string& iterName); void dereferenceInvalidIteratorError(const Token* deref, const std::string& iterName);
void dereferenceInvalidIteratorError(const Token* tok, const ValueFlow::Value *value, bool inconclusive);
void readingEmptyStlContainerError(const Token* tok, const ValueFlow::Value *value=nullptr); void readingEmptyStlContainerError(const Token* tok, const ValueFlow::Value *value=nullptr);

14
lib/token.cpp
View File

@ -1631,6 +1631,12 @@ void Token::printValueFlow(bool xml, std::ostream &out) const
case ValueFlow::Value::CONTAINER_SIZE: case ValueFlow::Value::CONTAINER_SIZE:
out << "container-size=\"" << value.intvalue << '\"'; out << "container-size=\"" << value.intvalue << '\"';
break; break;
case ValueFlow::Value::ITERATOR_START:
out << "iterator-start=\"" << value.intvalue << '\"';
break;
case ValueFlow::Value::ITERATOR_END:
out << "iterator-end=\"" << value.intvalue << '\"';
break;
case ValueFlow::Value::LIFETIME: case ValueFlow::Value::LIFETIME:
out << "lifetime=\"" << value.tokvalue << '\"'; out << "lifetime=\"" << value.tokvalue << '\"';
break; break;
@ -1680,6 +1686,12 @@ void Token::printValueFlow(bool xml, std::ostream &out) const
case ValueFlow::Value::CONTAINER_SIZE: case ValueFlow::Value::CONTAINER_SIZE:
out << "size=" << value.intvalue; out << "size=" << value.intvalue;
break; break;
case ValueFlow::Value::ITERATOR_START:
out << "start=" << value.intvalue;
break;
case ValueFlow::Value::ITERATOR_END:
out << "end=" << value.intvalue;
break;
case ValueFlow::Value::LIFETIME: case ValueFlow::Value::LIFETIME:
out << "lifetime=" << value.tokvalue->str(); out << "lifetime=" << value.tokvalue->str();
break; break;
@ -1958,6 +1970,8 @@ bool Token::addValue(const ValueFlow::Value &value)
case ValueFlow::Value::ValueType::INT: case ValueFlow::Value::ValueType::INT:
case ValueFlow::Value::ValueType::CONTAINER_SIZE: case ValueFlow::Value::ValueType::CONTAINER_SIZE:
case ValueFlow::Value::ValueType::BUFFER_SIZE: case ValueFlow::Value::ValueType::BUFFER_SIZE:
case ValueFlow::Value::ValueType::ITERATOR_START:
case ValueFlow::Value::ValueType::ITERATOR_END:
differentValue = (it->intvalue != value.intvalue); differentValue = (it->intvalue != value.intvalue);
break; break;
case ValueFlow::Value::ValueType::TOK: case ValueFlow::Value::ValueType::TOK:

59
lib/valueflow.cpp
View File

@ -347,6 +347,10 @@ static void combineValueProperties(const ValueFlow::Value &value1, const ValueFl
result->setInconclusive(); result->setInconclusive();
else else
result->setPossible(); result->setPossible();
if (value1.isIteratorValue())
result->valueType = value1.valueType;
if (value2.isIteratorValue())
result->valueType = value2.valueType;
result->condition = value1.condition ? value1.condition : value2.condition; result->condition = value1.condition ? value1.condition : value2.condition;
result->varId = (value1.varId != 0U) ? value1.varId : value2.varId; result->varId = (value1.varId != 0U) ? value1.varId : value2.varId;
result->varvalue = (result->varId == value1.varId) ? value1.varvalue : value2.varvalue; result->varvalue = (result->varId == value1.varId) ? value1.varvalue : value2.varvalue;
@ -376,6 +380,20 @@ static const Token *getCastTypeStartToken(const Token *parent)
return nullptr; return nullptr;
} }
static bool isComputableValue(const Token* parent, const ValueFlow::Value& value)
{
const bool noninvertible = parent->isComparisonOp() || Token::Match(parent, "%|/|&|%or%");
if (noninvertible && value.isImpossible())
return false;
if (!value.isIntValue() && !value.isFloatValue() && !value.isTokValue() && !value.isIteratorValue())
return false;
if (value.isIteratorValue() && !Token::Match(parent, "+|-"))
return false;
if (value.isTokValue() && (!parent->isComparisonOp() || value.tokvalue->tokType() != Token::eString))
return false;
return true;
}
/** Set token value for cast */ /** Set token value for cast */
static void setTokenValueCast(Token *parent, const ValueType &valueType, const ValueFlow::Value &value, const Settings *settings); static void setTokenValueCast(Token *parent, const ValueType &valueType, const ValueFlow::Value &value, const Settings *settings);
@ -564,20 +582,14 @@ static void setTokenValue(Token* tok, const ValueFlow::Value &value, const Setti
} }
for (const ValueFlow::Value &value1 : parent->astOperand1()->values()) { for (const ValueFlow::Value &value1 : parent->astOperand1()->values()) {
if (noninvertible && value1.isImpossible()) if (!isComputableValue(parent, value1))
continue;
if (!value1.isIntValue() && !value1.isFloatValue() && !value1.isTokValue())
continue;
if (value1.isTokValue() && (!parent->isComparisonOp() || value1.tokvalue->tokType() != Token::eString))
continue; continue;
for (const ValueFlow::Value &value2 : parent->astOperand2()->values()) { for (const ValueFlow::Value &value2 : parent->astOperand2()->values()) {
if (value1.path != value2.path) if (value1.path != value2.path)
continue; continue;
if (noninvertible && value2.isImpossible()) if (!isComputableValue(parent, value2))
continue; continue;
if (!value2.isIntValue() && !value2.isFloatValue() && !value2.isTokValue()) if (value1.isIteratorValue() && value2.isIteratorValue())
continue;
if (value2.isTokValue() && (!parent->isComparisonOp() || value2.tokvalue->tokType() != Token::eString || value1.isTokValue()))
continue; continue;
if (value1.isKnown() || value2.isKnown() || value1.varId == 0U || value2.varId == 0U || if (value1.isKnown() || value2.isKnown() || value1.varId == 0U || value2.varId == 0U ||
(value1.varId == value2.varId && value1.varvalue == value2.varvalue && value1.isIntValue() && (value1.varId == value2.varId && value1.varvalue == value2.varvalue && value1.isIntValue() &&
@ -5803,6 +5815,30 @@ static void valueFlowSmartPointer(TokenList *tokenlist, ErrorLogger * errorLogge
} }
} }
static void valueFlowIterators(TokenList *tokenlist, ErrorLogger * errorLogger, const Settings *settings)
{
for (Token *tok = tokenlist->front(); tok; tok = tok->next()) {
if (!tok->scope())
continue;
if (!tok->scope()->isExecutable())
continue;
if (!astIsContainer(tok))
continue;
if (Token::Match(tok->astParent(), ". %name% (")) {
Library::Container::Yield yield = tok->valueType()->container->getYield(tok->astParent()->strAt(1));
ValueFlow::Value v(0);
v.setKnown();
if (yield == Library::Container::Yield::START_ITERATOR) {
v.valueType = ValueFlow::Value::ITERATOR_START;
setTokenValue(tok->astParent()->tokAt(2), v, settings);
} else if (yield == Library::Container::Yield::END_ITERATOR) {
v.valueType = ValueFlow::Value::ITERATOR_END;
setTokenValue(tok->astParent()->tokAt(2), v, settings);
}
}
}
}
static void valueFlowContainerSize(TokenList *tokenlist, SymbolDatabase* symboldatabase, ErrorLogger * /*errorLogger*/, const Settings *settings) static void valueFlowContainerSize(TokenList *tokenlist, SymbolDatabase* symboldatabase, ErrorLogger * /*errorLogger*/, const Settings *settings)
{ {
// declaration // declaration
@ -6314,6 +6350,10 @@ std::string ValueFlow::Value::infoString() const
case BUFFER_SIZE: case BUFFER_SIZE:
case CONTAINER_SIZE: case CONTAINER_SIZE:
return "size=" + MathLib::toString(intvalue); return "size=" + MathLib::toString(intvalue);
case ITERATOR_START:
return "start=" + MathLib::toString(intvalue);
case ITERATOR_END:
return "end=" + MathLib::toString(intvalue);
case LIFETIME: case LIFETIME:
return "lifetime=" + tokvalue->str(); return "lifetime=" + tokvalue->str();
} }
@ -6391,6 +6431,7 @@ void ValueFlow::setValues(TokenList *tokenlist, SymbolDatabase* symboldatabase,
valueFlowUninit(tokenlist, symboldatabase, errorLogger, settings); valueFlowUninit(tokenlist, symboldatabase, errorLogger, settings);
if (tokenlist->isCPP()) { if (tokenlist->isCPP()) {
valueFlowSmartPointer(tokenlist, errorLogger, settings); valueFlowSmartPointer(tokenlist, errorLogger, settings);
valueFlowIterators(tokenlist, errorLogger, settings);
valueFlowContainerSize(tokenlist, symboldatabase, errorLogger, settings); valueFlowContainerSize(tokenlist, symboldatabase, errorLogger, settings);
valueFlowContainerAfterCondition(tokenlist, symboldatabase, errorLogger, settings); valueFlowContainerAfterCondition(tokenlist, symboldatabase, errorLogger, settings);
} }

27
lib/valueflow.h
View File

@ -82,6 +82,10 @@ namespace ValueFlow {
return false; return false;
switch (valueType) { switch (valueType) {
case ValueType::INT: case ValueType::INT:
case ValueType::CONTAINER_SIZE:
case ValueType::BUFFER_SIZE:
case ValueType::ITERATOR_START:
case ValueType::ITERATOR_END:
if (intvalue != rhs.intvalue) if (intvalue != rhs.intvalue)
return false; return false;
break; break;
@ -100,14 +104,6 @@ namespace ValueFlow {
break; break;
case ValueType::UNINIT: case ValueType::UNINIT:
break; break;
case ValueType::BUFFER_SIZE:
if (intvalue != rhs.intvalue)
return false;
break;
case ValueType::CONTAINER_SIZE:
if (intvalue != rhs.intvalue)
return false;
break;
case ValueType::LIFETIME: case ValueType::LIFETIME:
if (tokvalue != rhs.tokvalue) if (tokvalue != rhs.tokvalue)
return false; return false;
@ -120,7 +116,9 @@ namespace ValueFlow {
switch (valueType) { switch (valueType) {
case ValueType::INT: case ValueType::INT:
case ValueType::BUFFER_SIZE: case ValueType::BUFFER_SIZE:
case ValueType::CONTAINER_SIZE: { case ValueType::CONTAINER_SIZE:
case ValueType::ITERATOR_START:
case ValueType::ITERATOR_END: {
f(intvalue); f(intvalue);
break; break;
} }
@ -174,7 +172,7 @@ namespace ValueFlow {
std::string infoString() const; std::string infoString() const;
enum ValueType { INT, TOK, FLOAT, MOVED, UNINIT, CONTAINER_SIZE, LIFETIME, BUFFER_SIZE } valueType; enum ValueType { INT, TOK, FLOAT, MOVED, UNINIT, CONTAINER_SIZE, LIFETIME, BUFFER_SIZE, ITERATOR_START, ITERATOR_END } valueType;
bool isIntValue() const { bool isIntValue() const {
return valueType == ValueType::INT; return valueType == ValueType::INT;
} }
@ -199,6 +197,15 @@ namespace ValueFlow {
bool isBufferSizeValue() const { bool isBufferSizeValue() const {
return valueType == ValueType::BUFFER_SIZE; return valueType == ValueType::BUFFER_SIZE;
} }
bool isIteratorValue() const {
return valueType == ValueType::ITERATOR_START || valueType == ValueType::ITERATOR_END;
}
bool isIteratorStartValue() const {
return valueType == ValueType::ITERATOR_START;
}
bool isIteratorEndValue() const {
return valueType == ValueType::ITERATOR_END;
}
bool isLocalLifetimeValue() const { bool isLocalLifetimeValue() const {
return valueType == ValueType::LIFETIME && lifetimeScope == LifetimeScope::Local; return valueType == ValueType::LIFETIME && lifetimeScope == LifetimeScope::Local;

55
test/teststl.cpp
View File

@ -3515,6 +3515,61 @@ private:
" }\n" " }\n"
"}\n"); "}\n");
ASSERT_EQUALS("", errout.str()); ASSERT_EQUALS("", errout.str());
check("void f() {\n"
" std::vector <int> v;\n"
" std::vector <int>::iterator i = v.end();\n"
" *i=0;\n"
"}\n");
ASSERT_EQUALS("[test.cpp:4]: (error) Dereference of an invalid iterator: i\n", errout.str());
check("void f(std::vector <int> v) {\n"
" std::vector <int>::iterator i = v.end();\n"
" *i=0;\n"
"}\n");
ASSERT_EQUALS("[test.cpp:3]: (error) Dereference of an invalid iterator: i\n", errout.str());
check("void f(std::vector <int> v) {\n"
" std::vector <int>::iterator i = v.end();\n"
" *(i+1)=0;\n"
"}\n");
ASSERT_EQUALS("[test.cpp:3]: (error) Dereference of an invalid iterator: i+1\n", errout.str());
check("void f(std::vector <int> v) {\n"
" std::vector <int>::iterator i = v.end();\n"
" *(i-1)=0;\n"
"}\n");
ASSERT_EQUALS("", errout.str());
check("void f(std::vector <int> v) {\n"
" std::vector <int>::iterator i = v.begin();\n"
" *(i-1)=0;\n"
"}\n");
ASSERT_EQUALS("[test.cpp:3]: (error) Dereference of an invalid iterator: i-1\n", errout.str());
check("void f(std::vector <int> v, bool b) {\n"
" std::vector <int>::iterator i = v.begin();\n"
" if (b)\n"
" i = v.end();\n"
" *i=0;\n"
"}\n");
ASSERT_EQUALS("[test.cpp:5]: (warning) Possible dereference of an invalid iterator: i\n", errout.str());
check("void f(std::vector <int> v, bool b) {\n"
" std::vector <int>::iterator i = v.begin();\n"
" if (b)\n"
" i = v.end();\n"
" *(i+1)=0;\n"
"}\n");
ASSERT_EQUALS("[test.cpp:5]: (warning) Possible dereference of an invalid iterator: i+1\n", errout.str());
check("void f(std::vector <int> v, bool b) {\n"
" std::vector <int>::iterator i = v.begin();\n"
" if (b)\n"
" i = v.end();\n"
" *(i-1)=0;\n"
"}\n");
ASSERT_EQUALS("[test.cpp:5]: (warning) Possible dereference of an invalid iterator: i-1\n", errout.str());
} }
void dereferenceInvalidIterator2() { void dereferenceInvalidIterator2() {