Fixed #8681 (ValueFlow: Container size)

This commit is contained in:
Daniel Marjamäki 2018-08-10 11:29:16 +02:00
parent 8032f64c15
commit 81f54f7094
8 changed files with 156 additions and 23 deletions

View File

@ -1718,8 +1718,42 @@ void CheckStl::readingEmptyStlContainer()
}
}
void CheckStl::readingEmptyStlContainerError(const Token *tok)
void CheckStl::readingEmptyStlContainer2()
{
for (const Scope *function : mTokenizer->getSymbolDatabase()->functionScopes) {
for (const Token *tok = function->bodyStart; tok != function->bodyEnd; tok = tok->next()) {
if (!tok->isName() || !tok->valueType())
continue;
const Library::Container *container = tok->valueType()->container;
if (!container)
continue;
const ValueFlow::Value *value = tok->getContainerSizeValue(0);
if (!value)
continue;
if (value->isInconclusive() && !mSettings->inconclusive)
continue;
if (!value->errorSeverity() && !mSettings->isEnabled(Settings::WARNING))
continue;
if (Token::Match(tok, "%name% . %name% (")) {
if (container->getYield(tok->strAt(2)) == Library::Container::Yield::ITEM)
readingEmptyStlContainerError(tok,value);
}
}
}
}
void CheckStl::readingEmptyStlContainerError(const Token *tok, const ValueFlow::Value *value)
{
const std::string varname = tok ? tok->str() : std::string("var");
reportError(tok, Severity::style, "reademptycontainer", "$symbol:" + varname +"\nReading from empty STL container '$symbol'", CWE398, true);
std::string errmsg;
if (value && value->condition)
errmsg = "Reading from container '$symbol'. " + ValueFlow::eitherTheConditionIsRedundant(value->condition) + " or '$symbol' can be empty.";
else
errmsg = "Reading from empty STL container '$symbol'";
const ErrorPath errorPath = getErrorPath(tok, value, "Reading from empty container");
reportError(errorPath, value ? (value->errorSeverity() ? Severity::error : Severity::warning) : Severity::style, "reademptycontainer", "$symbol:" + varname +"\n" + errmsg, CWE398, !value);
}

View File

@ -79,6 +79,7 @@ public:
checkStl.redundantCondition();
checkStl.missingComparison();
checkStl.readingEmptyStlContainer();
checkStl.readingEmptyStlContainer2();
}
@ -169,6 +170,9 @@ public:
/** @brief Reading from empty stl container */
void readingEmptyStlContainer();
/** @brief Reading from empty stl container (using valueflow) */
void readingEmptyStlContainer2();
private:
void readingEmptyStlContainer_parseUsage(const Token* tok, const Library::Container* container, std::map<unsigned int, const Library::Container*>& empty, bool noerror);
@ -205,7 +209,7 @@ private:
void dereferenceInvalidIteratorError(const Token* deref, const std::string& iterName);
void readingEmptyStlContainerError(const Token* tok);
void readingEmptyStlContainerError(const Token* tok, const ValueFlow::Value *value=nullptr);
void getErrorMessages(ErrorLogger* errorLogger, const Settings* settings) const override {
CheckStl c(nullptr, settings, errorLogger);

View File

@ -1374,62 +1374,68 @@ void Token::printValueFlow(bool xml, std::ostream &out) const
if (tok->mValues->size() > 1U)
out << '{';
}
for (std::list<ValueFlow::Value>::const_iterator it=tok->mValues->begin(); it!=tok->mValues->end(); ++it) {
for (const ValueFlow::Value &value : *tok->mValues) {
if (xml) {
out << " <value ";
switch (it->valueType) {
switch (value.valueType) {
case ValueFlow::Value::INT:
if (tok->valueType() && tok->valueType()->sign == ValueType::UNSIGNED)
out << "intvalue=\"" << (MathLib::biguint)it->intvalue << '\"';
out << "intvalue=\"" << (MathLib::biguint)value.intvalue << '\"';
else
out << "intvalue=\"" << it->intvalue << '\"';
out << "intvalue=\"" << value.intvalue << '\"';
break;
case ValueFlow::Value::TOK:
out << "tokvalue=\"" << it->tokvalue << '\"';
out << "tokvalue=\"" << value.tokvalue << '\"';
break;
case ValueFlow::Value::FLOAT:
out << "floatvalue=\"" << it->floatValue << '\"';
out << "floatvalue=\"" << value.floatValue << '\"';
break;
case ValueFlow::Value::MOVED:
out << "movedvalue=\"" << ValueFlow::Value::toString(it->moveKind) << '\"';
out << "movedvalue=\"" << ValueFlow::Value::toString(value.moveKind) << '\"';
break;
case ValueFlow::Value::UNINIT:
out << "uninit=\"1\"";
break;
case ValueFlow::Value::CONTAINER_SIZE:
out << "container-size=\"" << value.intvalue << '\"';
break;
}
if (it->condition)
out << " condition-line=\"" << it->condition->linenr() << '\"';
if (it->isKnown())
if (value.condition)
out << " condition-line=\"" << value.condition->linenr() << '\"';
if (value.isKnown())
out << " known=\"true\"";
else if (it->isPossible())
else if (value.isPossible())
out << " possible=\"true\"";
else if (it->isInconclusive())
else if (value.isInconclusive())
out << " inconclusive=\"true\"";
out << "/>" << std::endl;
}
else {
if (it != tok->mValues->begin())
if (&value != &tok->mValues->front())
out << ",";
switch (it->valueType) {
switch (value.valueType) {
case ValueFlow::Value::INT:
if (tok->valueType() && tok->valueType()->sign == ValueType::UNSIGNED)
out << (MathLib::biguint)it->intvalue;
out << (MathLib::biguint)value.intvalue;
else
out << it->intvalue;
out << value.intvalue;
break;
case ValueFlow::Value::TOK:
out << it->tokvalue->str();
out << value.tokvalue->str();
break;
case ValueFlow::Value::FLOAT:
out << it->floatValue;
out << value.floatValue;
break;
case ValueFlow::Value::MOVED:
out << ValueFlow::Value::toString(it->moveKind);
out << ValueFlow::Value::toString(value.moveKind);
break;
case ValueFlow::Value::UNINIT:
out << "Uninit";
break;
case ValueFlow::Value::CONTAINER_SIZE:
out << "size=" << value.intvalue;
break;
}
}
}

View File

@ -875,6 +875,16 @@ public:
const ValueFlow::Value * getInvalidValue(const Token *ftok, unsigned int argnr, const Settings *settings) const;
const ValueFlow::Value * getContainerSizeValue(const MathLib::bigint val) const {
if (!mValues)
return nullptr;
for (const ValueFlow::Value &value : *mValues) {
if (value.isContainerSizeValue() && value.intvalue == val)
return &value;
}
return nullptr;
}
const Token *getValueTokenMaxStrLength() const;
const Token *getValueTokenMinStrSize() const;

View File

@ -3413,6 +3413,38 @@ static void valueFlowUninit(TokenList *tokenlist, SymbolDatabase * /*symbolDatab
}
}
static void valueFlowContainerReverse(const Token *tok, unsigned int containerId, const ValueFlow::Value &value, const Settings *settings)
{
while (nullptr != (tok = tok->previous())) {
if (Token::Match(tok, "[{}]"))
break;
if (tok->varId() != containerId)
continue;
if (!tok->valueType() || !tok->valueType()->container)
continue;
setTokenValue(const_cast<Token *>(tok), value, settings);
}
}
static void valueFlowContainerSize(TokenList * /*tokenlist*/, SymbolDatabase* symboldatabase, ErrorLogger * /*errorLogger*/, const Settings *settings)
{
for (const Scope &scope : symboldatabase->scopeList) {
if (scope.type != Scope::ScopeType::eIf) // TODO: while
continue;
for (const Token *tok = scope.classDef; tok && tok->str() != "{"; tok = tok->next()) {
if (!tok->isName() || !tok->valueType() || tok->valueType()->type != ValueType::CONTAINER)
continue;
if (!Token::Match(tok, "%name% . %name% ("))
continue;
if (tok->valueType()->container->getYield(tok->strAt(2)) != Library::Container::Yield::EMPTY)
continue;
ValueFlow::Value value(tok->tokAt(3), 0LL);
value.valueType = ValueFlow::Value::ValueType::CONTAINER_SIZE;
valueFlowContainerReverse(scope.classDef, tok->varId(), value, settings);
}
}
}
ValueFlow::Value::Value(const Token *c, long long val)
: valueType(INT),
intvalue(val),
@ -3442,6 +3474,8 @@ std::string ValueFlow::Value::infoString() const
return "<Moved>";
case UNINIT:
return "<Uninit>";
case CONTAINER_SIZE:
return "size=" + MathLib::toString(intvalue);
};
throw InternalError(nullptr, "Invalid ValueFlow Value type");
}
@ -3479,6 +3513,8 @@ void ValueFlow::setValues(TokenList *tokenlist, SymbolDatabase* symboldatabase,
valueFlowSubFunction(tokenlist, errorLogger, settings);
valueFlowFunctionDefaultParameter(tokenlist, symboldatabase, errorLogger, settings);
valueFlowUninit(tokenlist, symboldatabase, errorLogger, settings);
if (tokenlist->isCPP())
valueFlowContainerSize(tokenlist, symboldatabase, errorLogger, settings);
}

View File

@ -65,6 +65,9 @@ namespace ValueFlow {
break;
case UNINIT:
break;
case CONTAINER_SIZE:
if (intvalue != rhs.intvalue)
return false;
};
return varvalue == rhs.varvalue &&
@ -77,7 +80,7 @@ namespace ValueFlow {
std::string infoString() const;
enum ValueType { INT, TOK, FLOAT, MOVED, UNINIT } valueType;
enum ValueType { INT, TOK, FLOAT, MOVED, UNINIT, CONTAINER_SIZE } valueType;
bool isIntValue() const {
return valueType == INT;
}
@ -93,6 +96,9 @@ namespace ValueFlow {
bool isUninitValue() const {
return valueType == UNINIT;
}
bool isContainerSizeValue() const {
return valueType == CONTAINER_SIZE;
}
/** int value */
long long intvalue;

View File

@ -143,6 +143,7 @@ private:
TEST_CASE(dereference_auto);
TEST_CASE(readingEmptyStlContainer);
TEST_CASE(readingEmptyStlContainer2);
}
void check(const char code[], const bool inconclusive=false, const Standards::cppstd_t cppstandard=Standards::CPP11) {
@ -3254,6 +3255,15 @@ private:
"}", true);
ASSERT_EQUALS("", errout.str());
}
void readingEmptyStlContainer2() {
check("void f(std::vector<int> v) {\n"
" v.front();\n"
" if (v.empty());\n"
"}\n",true);
ASSERT_EQUALS("[test.cpp:3] -> [test.cpp:2]: (warning) Reading from container 'v'. Either the condition 'v.empty()' is redundant or 'v' can be empty.\n", errout.str());
}
};
REGISTER_TEST(TestStl)

View File

@ -103,6 +103,8 @@ private:
TEST_CASE(valueFlowInlineAssembly);
TEST_CASE(valueFlowUninit);
TEST_CASE(valueFlowContainerSize);
}
bool testValueOfX(const char code[], unsigned int linenr, int value) {
@ -3202,6 +3204,31 @@ private:
ASSERT_EQUALS(true, values.front().isPossible() || values.back().isPossible());
ASSERT_EQUALS(true, values.front().intvalue == 0 || values.back().intvalue == 0);
}
void valueFlowContainerSize() {
const char *code;
std::list<ValueFlow::Value> values;
LOAD_LIB_2(settings.library, "std.cfg");
// valueFlowContainerReverse
code = "void f(const std::list<int> &ints) {\n"
" ints.front();\n"
" if (ints.empty()) {}\n"
"}";
values = tokenValues(code, "ints . front");
ASSERT_EQUALS(1, values.size());
ASSERT_EQUALS(true, values.empty() ? true : values.front().isContainerSizeValue());
ASSERT_EQUALS(0, values.empty() ? 0 : values.front().intvalue);
code = "void f(std::list<int> ints) {\n"
" ints.front();\n"
" ints.pop_back();\n"
" if (ints.empty()) {}\n"
"}";
values = tokenValues(code, "ints . front");
ASSERT_EQUALS(true, values.empty());
}
};
REGISTER_TEST(TestValueFlow)