Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 16 additions & 0 deletions cfg/std.cfg
Original file line number Diff line number Diff line change
Expand Up @@ -8539,6 +8539,22 @@ initializer list (7) string& replace (const_iterator i1, const_iterator i2, init
<valid>0:</valid>
</arg>
</function>
<function name="std::begin,std::cbegin,std::rbegin,std::crbegin">
<use-retval/>
<leak-ignore/>
<noreturn>false</noreturn>
<arg nr="1" direction="in"/>
<container yields="start-iterator"/>
<returnValue type="iterator" container="1"/>
</function>
<function name="std::end,std::cend,std::rend,std::crend">
<use-retval/>
<leak-ignore/>
<noreturn>false</noreturn>
<arg nr="1" direction="in"/>
<container yields="end-iterator"/>
<returnValue type="iterator" container="1"/>
</function>
<memory>
<alloc init="false" buffer-size="malloc">malloc</alloc>
<alloc init="true" buffer-size="calloc">calloc</alloc>
Expand Down
15 changes: 15 additions & 0 deletions lib/astutils.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -282,6 +282,7 @@ Library::Container::Action astContainerAction(const Token* tok, const Token** ft
return Library::Container::Action::NO_ACTION;
return tok->valueType()->container->getAction(ftok2->str());
}

Library::Container::Yield astContainerYield(const Token* tok, const Token** ftok)
{
const Token* ftok2 = getContainerFunction(tok);
Expand All @@ -292,6 +293,20 @@ Library::Container::Yield astContainerYield(const Token* tok, const Token** ftok
return tok->valueType()->container->getYield(ftok2->str());
}

Library::Container::Yield astFunctionYield(const Token* tok, const Settings* settings, const Token** ftok)
{
if (!tok)
return Library::Container::Yield::NO_YIELD;

const auto* function = settings->library.getFunction(tok);
if (!function)
return Library::Container::Yield::NO_YIELD;

if (ftok)
*ftok = tok;
return function->containerYield;
}

bool astIsRangeBasedForDecl(const Token* tok)
{
return Token::simpleMatch(tok->astParent(), ":") && Token::simpleMatch(tok->astParent()->astParent(), "(");
Expand Down
2 changes: 2 additions & 0 deletions lib/astutils.h
Original file line number Diff line number Diff line change
Expand Up @@ -153,6 +153,8 @@ bool astIsContainerOwned(const Token* tok);
Library::Container::Action astContainerAction(const Token* tok, const Token** ftok = nullptr);
Library::Container::Yield astContainerYield(const Token* tok, const Token** ftok = nullptr);

Library::Container::Yield astFunctionYield(const Token* tok, const Settings* settings, const Token** ftok = nullptr);

/** Is given token a range-declaration in a range-based for loop */
bool astIsRangeBasedForDecl(const Token* tok);

Expand Down
24 changes: 23 additions & 1 deletion lib/symboldatabase.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -7029,6 +7029,7 @@ void SymbolDatabase::setValueTypeInTokenList(bool reportDebugWarnings, Token *to
}
}

//Is iterator fetching function invoked on container?
const bool isReturnIter = typestr == "iterator";
if (typestr.empty() || isReturnIter) {
if (Token::simpleMatch(tok->astOperand1(), ".") &&
Expand All @@ -7037,7 +7038,7 @@ void SymbolDatabase::setValueTypeInTokenList(bool reportDebugWarnings, Token *to
tok->astOperand1()->astOperand1()->valueType() &&
tok->astOperand1()->astOperand1()->valueType()->container) {
const Library::Container *cont = tok->astOperand1()->astOperand1()->valueType()->container;
const std::map<std::string, Library::Container::Function>::const_iterator it = cont->functions.find(tok->astOperand1()->astOperand2()->str());
const auto it = cont->functions.find(tok->astOperand1()->astOperand2()->str());
if (it != cont->functions.end()) {
if (it->second.yield == Library::Container::Yield::START_ITERATOR ||
it->second.yield == Library::Container::Yield::END_ITERATOR ||
Expand All @@ -7051,6 +7052,27 @@ void SymbolDatabase::setValueTypeInTokenList(bool reportDebugWarnings, Token *to
continue;
}
}
//Is iterator fetching function called?
} else if (Token::simpleMatch(tok->astOperand1(), "::") &&
tok->astOperand2() &&
tok->astOperand2()->isVariable()) {
const auto* const paramVariable = tok->astOperand2()->variable();
if (!paramVariable ||
!paramVariable->valueType() ||
!paramVariable->valueType()->container) {
continue;
}

const auto yield = astFunctionYield(tok->previous(), &mSettings);
if (yield == Library::Container::Yield::START_ITERATOR ||
yield == Library::Container::Yield::END_ITERATOR ||
yield == Library::Container::Yield::ITERATOR) {
ValueType vt;
vt.type = ValueType::Type::ITERATOR;
vt.container = paramVariable->valueType()->container;
vt.containerTypeToken = paramVariable->valueType()->containerTypeToken;
setValueType(tok, vt);
}
}
if (isReturnIter) {
const std::vector<const Token*> args = getArguments(tok);
Expand Down
4 changes: 3 additions & 1 deletion lib/tokenize.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -8897,7 +8897,9 @@ static const std::set<std::string> stdFunctions = {
"set_symmetric_difference", "push_heap", "pop_heap", "make_heap", "sort_heap",
"min", "max", "min_element", "max_element", "lexicographical_compare", "next_permutation", "prev_permutation",
"advance", "back_inserter", "distance", "front_inserter", "inserter",
"make_pair", "make_shared", "make_tuple"
"make_pair", "make_shared", "make_tuple",
"begin", "cbegin", "rbegin", "crbegin",
"end", "cend", "rend", "crend"
};


Expand Down
35 changes: 27 additions & 8 deletions lib/valueflow.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -663,7 +663,6 @@ static void setTokenValue(Token* tok,
}
}
}

else if (Token::Match(parent, ". %name% (") && parent->astParent() == parent->tokAt(2) &&
parent->astOperand1() && parent->astOperand1()->valueType()) {
const Library::Container* c = getLibraryContainer(parent->astOperand1());
Expand Down Expand Up @@ -695,7 +694,6 @@ static void setTokenValue(Token* tok,
}
}
}

return;
}

Expand Down Expand Up @@ -2203,7 +2201,7 @@ class SelectValueFromVarIdMapRange {
using pointer = value_type *;
using reference = value_type &;

explicit Iterator(const M::const_iterator &it)
explicit Iterator(const M::const_iterator & it)
: mIt(it) {}

reference operator*() const {
Expand Down Expand Up @@ -4805,7 +4803,8 @@ static void valueFlowLifetime(TokenList *tokenlist, SymbolDatabase* /*db*/, Erro
// container lifetimes
else if (astIsContainer(tok)) {
Token * parent = astParentSkipParens(tok);
if (!Token::Match(parent, ". %name% ("))
if (!Token::Match(parent, ". %name% (") &&
!Token::simpleMatch(parent, "("))
continue;

ValueFlow::Value master;
Expand All @@ -4815,8 +4814,12 @@ static void valueFlowLifetime(TokenList *tokenlist, SymbolDatabase* /*db*/, Erro
if (astIsIterator(parent->tokAt(2))) {
master.errorPath.emplace_back(parent->tokAt(2), "Iterator to container is created here.");
master.lifetimeKind = ValueFlow::Value::LifetimeKind::Iterator;
} else if ((astIsPointer(parent->tokAt(2)) && !isContainerOfPointers(tok->valueType()->containerTypeToken, settings)) ||
Token::Match(parent->next(), "data|c_str")) {
} else if (astIsIterator(parent)) {
master.errorPath.emplace_back(parent, "Iterator to container is created here.");
master.lifetimeKind = ValueFlow::Value::LifetimeKind::Iterator;
}
else if ((astIsPointer(parent->tokAt(2)) && !isContainerOfPointers(tok->valueType()->containerTypeToken, settings)) ||
Token::Match(parent->next(), "data|c_str")) {
master.errorPath.emplace_back(parent->tokAt(2), "Pointer to container is created here.");
master.lifetimeKind = ValueFlow::Value::LifetimeKind::Object;
} else {
Expand Down Expand Up @@ -4853,7 +4856,10 @@ static void valueFlowLifetime(TokenList *tokenlist, SymbolDatabase* /*db*/, Erro
ValueFlow::Value value = master;
value.tokvalue = rt.token;
value.errorPath.insert(value.errorPath.begin(), rt.errors.cbegin(), rt.errors.cend());
setTokenValue(parent->tokAt(2), std::move(value), settings);
if (Token::simpleMatch(parent, "("))
setTokenValue(parent, value, settings);
else
setTokenValue(parent->tokAt(2), value, settings);

if (!rt.token->variable()) {
LifetimeStore ls = LifetimeStore{
Expand Down Expand Up @@ -8100,6 +8106,19 @@ static void valueFlowSmartPointer(TokenList *tokenlist, ErrorLogger * errorLogge
}
}

static Library::Container::Yield findIteratorYield(Token* tok, const Token** ftok, const Settings *settings)
{
auto yield = astContainerYield(tok, ftok);
if (*ftok)
return yield;

if (!tok->astParent())
return yield;

//begin/end free functions
return astFunctionYield(tok->astParent()->previous(), settings, ftok);
}

static void valueFlowIterators(TokenList *tokenlist, const Settings *settings)
{
for (Token *tok = tokenlist->front(); tok; tok = tok->next()) {
Expand All @@ -8110,7 +8129,7 @@ static void valueFlowIterators(TokenList *tokenlist, const Settings *settings)
if (!astIsContainer(tok))
continue;
const Token* ftok = nullptr;
const Library::Container::Yield yield = astContainerYield(tok, &ftok);
const Library::Container::Yield yield = findIteratorYield(tok, &ftok, settings);
if (ftok) {
ValueFlow::Value v(0);
v.setKnown();
Expand Down
43 changes: 43 additions & 0 deletions test/cfg/std.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4661,4 +4661,47 @@ void stdspan()
spn3.last<1>();
spn3.subspan<1, 1>();
#endif
}

void beginEnd()
{
std::vector<int> v;

//cppcheck-suppress ignoredReturnValue
std::begin(v);
//cppcheck-suppress ignoredReturnValue
std::rbegin(v);
//cppcheck-suppress ignoredReturnValue
std::cbegin(v);
//cppcheck-suppress ignoredReturnValue
std::crbegin(v);

//cppcheck-suppress ignoredReturnValue
std::end(v);
//cppcheck-suppress ignoredReturnValue
std::rend(v);
//cppcheck-suppress ignoredReturnValue
std::cend(v);
//cppcheck-suppress ignoredReturnValue
std::crend(v);

int arr[4];

//cppcheck-suppress ignoredReturnValue
std::begin(arr);
//cppcheck-suppress ignoredReturnValue
std::rbegin(arr);
//cppcheck-suppress ignoredReturnValue
std::cbegin(arr);
//cppcheck-suppress ignoredReturnValue
std::crbegin(arr);

//cppcheck-suppress ignoredReturnValue
std::end(arr);
//cppcheck-suppress ignoredReturnValue
std::rend(arr);
//cppcheck-suppress ignoredReturnValue
std::cend(arr);
//cppcheck-suppress ignoredReturnValue
std::crend(arr);
}
7 changes: 7 additions & 0 deletions test/testautovariables.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2281,6 +2281,13 @@ class TestAutoVariables : public TestFixture {
"}");
ASSERT_EQUALS("[test.cpp:3] -> [test.cpp:2] -> [test.cpp:4]: (error) Returning iterator to local container 'x' that will be invalid when returning.\n", errout.str());

check("auto f() {\n"
" std::vector<int> x;\n"
" auto it = std::begin(x);\n"
" return it;\n"
"}");
ASSERT_EQUALS("[test.cpp:3] -> [test.cpp:2] -> [test.cpp:4]: (error) Returning iterator to local container 'x' that will be invalid when returning.\n", errout.str());

check("auto f() {\n"
" std::vector<int> x;\n"
" auto p = x.data();\n"
Expand Down
32 changes: 27 additions & 5 deletions test/teststl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1884,7 +1884,6 @@ class TestStl : public TestFixture {
ASSERT_EQUALS("", errout.str());

check("std::vector<int>& f();\n"
"std::vector<int>& g();\n"
"void foo() {\n"
" auto it = f().end() - 1;\n"
" f().begin() - it;\n"
Expand All @@ -1906,18 +1905,28 @@ class TestStl : public TestFixture {
" (void)std::find(begin(f()), end(f()) - 1, 0);\n"
" (void)std::find(begin(f()) + 1, end(f()) - 1, 0);\n"
"}");
ASSERT_EQUALS("[test.cpp:10]: (error) Dereference of an invalid iterator: f().end()+1\n", errout.str());
ASSERT_EQUALS("[test.cpp:9]: (error) Dereference of an invalid iterator: f().end()+1\n", errout.str());

check("std::vector<int>& f();\n"
"std::vector<int>& g();\n"
"void foo() {\n"
" if(f().begin() == f().end()) {}\n"
" if(f().begin() == f().end()+1) {}\n"
" if(f().begin()+1 == f().end()) {}\n"
" if(f().begin()+1 == f().end()+1) {}\n"
"}");
ASSERT_EQUALS("[test.cpp:5]: (error) Dereference of an invalid iterator: f().end()+1\n"
"[test.cpp:7]: (error) Dereference of an invalid iterator: f().end()+1\n",
ASSERT_EQUALS("[test.cpp:4]: (error) Dereference of an invalid iterator: f().end()+1\n"
"[test.cpp:6]: (error) Dereference of an invalid iterator: f().end()+1\n",
errout.str());

check("std::vector<int>& f();\n"
"void foo() {\n"
" if(std::begin(f()) == std::end(f())) {}\n"
" if(std::begin(f()) == std::end(f())+1) {}\n"
" if(std::begin(f())+1 == std::end(f())) {}\n"
" if(std::begin(f())+1 == std::end(f())+1) {}\n"
"}");
ASSERT_EQUALS("[test.cpp:4]: (error) Dereference of an invalid iterator: std::end(f())+1\n"
"[test.cpp:6]: (error) Dereference of an invalid iterator: std::end(f())+1\n",
errout.str());

check("template<int N>\n"
Expand Down Expand Up @@ -4495,6 +4504,13 @@ class TestStl : public TestFixture {
"}\n");
ASSERT_EQUALS("[test.cpp:4]: (error) Dereference of an invalid iterator: i\n", errout.str());

check("void f() {\n"
" std::vector <int> v;\n"
" std::vector <int>::iterator i = std::end(v);\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"
Expand All @@ -4519,6 +4535,12 @@ class TestStl : public TestFixture {
"}\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 = std::begin(v);\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"
Expand Down