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

Reimplemented CheckStl::stlBoundaries() based on Libraries; Added support for iterators to libraries

This commit is contained in:
PKEuS 2015-11-20 12:46:59 +01:00
parent fdadb3e7a9
commit 53b2eca983
7 changed files with 59 additions and 49 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

18
cfg/std.cfg
View File

@ -3959,7 +3959,7 @@
<alloc init="true">tmpfile</alloc> <alloc init="true">tmpfile</alloc>
<dealloc>fclose</dealloc> <dealloc>fclose</dealloc>
</resource> </resource>
<container id="stdContainer" endPattern="&gt; !!::"> <container id="stdContainer" endPattern="&gt; !!::" itEndPattern="&gt; :: iterator|const_iterator|reverse_iterator|const_reverse_iterator" opLessAllowed="false">
<type templateParameter="0"/> <type templateParameter="0"/>
<size> <size>
<function name="resize" action="resize"/> <function name="resize" action="resize"/>
@ -3978,7 +3978,7 @@
<function name="crend" yields="end-iterator"/> <function name="crend" yields="end-iterator"/>
</access> </access>
</container> </container>
<container id="stdVectorDeque" startPattern="std :: vector|deque &lt;" inherits="stdContainer"> <container id="stdVectorDeque" startPattern="std :: vector|deque &lt;" inherits="stdContainer" opLessAllowed="true">
<size> <size>
<function name="push_back" action="push"/> <function name="push_back" action="push"/>
<function name="pop_back" action="pop"/> <function name="pop_back" action="pop"/>
@ -3992,7 +3992,7 @@
<function name="data" yields="buffer"/> <function name="data" yields="buffer"/>
</access> </access>
</container> </container>
<container id="stdArray" startPattern="std :: array &lt;" inherits="stdContainer"> <container id="stdArray" startPattern="std :: array &lt;" inherits="stdContainer" opLessAllowed="true">
<size templateParameter="1"/> <size templateParameter="1"/>
<access indexOperator="array-like"> <access indexOperator="array-like">
<function name="at" yields="at_index"/> <function name="at" yields="at_index"/>
@ -4001,6 +4001,14 @@
<function name="data" yields="buffer"/> <function name="data" yields="buffer"/>
</access> </access>
</container> </container>
<container id="stdBitset" startPattern="std :: bitset &lt;" inherits="stdContainer" itEndPattern="">
<size templateParameter="0"/>
<access indexOperator="array-like" />
</container>
<container id="stdQueue" startPattern="std :: queue|priority_queue &lt;" inherits="stdContainer">
</container>
<container id="stdStack" startPattern="std :: stack &lt;" inherits="stdContainer">
</container>
<container id="stdSet" startPattern="std :: set|unoredered_set|multiset|unoredered_multiset &lt;" inherits="stdContainer"> <container id="stdSet" startPattern="std :: set|unoredered_set|multiset|unoredered_multiset &lt;" inherits="stdContainer">
<access> <access>
<function name="find" action="find"/> <function name="find" action="find"/>
@ -4013,7 +4021,7 @@
<function name="find" action="find"/> <function name="find" action="find"/>
</access> </access>
</container> </container>
<container id="stdList" startPattern="std :: list &lt;" inherits="stdContainer"> <container id="stdList" startPattern="std :: list|forward_list &lt;" inherits="stdContainer">
<size> <size>
<function name="push_back" action="push"/> <function name="push_back" action="push"/>
<function name="pop_back" action="pop"/> <function name="pop_back" action="pop"/>
@ -4028,7 +4036,7 @@
<container id="stdComplex" startPattern="std :: complex &lt;" endPattern="&gt; !!::"> <container id="stdComplex" startPattern="std :: complex &lt;" endPattern="&gt; !!::">
<type templateParameter="1"/> <type templateParameter="1"/>
</container> </container>
<container id="stdAllString" inherits="stdContainer"> <container id="stdAllString" inherits="stdContainer" opLessAllowed="true">
<type string="std-like"/> <type string="std-like"/>
<size> <size>
<function name="push_back" action="push"/> <function name="push_back" action="push"/>

44
lib/checkstl.cpp
View File

@ -597,43 +597,33 @@ void CheckStl::invalidPointerError(const Token *tok, const std::string &func, co
void CheckStl::stlBoundaries() void CheckStl::stlBoundaries()
{ {
const SymbolDatabase* const symbolDatabase = _tokenizer->getSymbolDatabase(); const SymbolDatabase* const symbolDatabase = _tokenizer->getSymbolDatabase();
const std::size_t functions = symbolDatabase->functionScopes.size(); for (unsigned int iteratorId = 1; iteratorId < symbolDatabase->getVariableListSize(); iteratorId++) {
for (std::size_t i = 0; i < functions; ++i) { const Variable* var = symbolDatabase->getVariableFromVarId(iteratorId);
const Scope * scope = symbolDatabase->functionScopes[i]; if (!var || !var->scope() || !var->scope()->isExecutable())
for (const Token* tok = scope->classStart->next(); tok != scope->classEnd; tok = tok->next()) { continue;
// Declaring iterator..
if (tok->str() == "<" && Token::Match(tok->previous(), "bitset|list|forward_list|map|multimap|multiset|priority_queue|queue|set|stack|hash_map|hash_multimap|hash_set|unordered_map|unordered_multimap|unordered_set|unordered_multiset")) {
if (!tok->link())
continue;
const std::string& container_name(tok->strAt(-1));
tok = tok->link();
if (Token::Match(tok, "> :: iterator|const_iterator %var% =|;")) { const Library::Container* container = _settings->library.detectContainer(var->typeStartToken(), true);
const unsigned int iteratorid(tok->tokAt(3)->varId()); if (!container || container->opLessAllowed)
continue;
// Using "iterator < ..." is not allowed const Token* const end = var->scope()->classEnd;
const Token* const end = tok->scope()->classEnd; for (const Token *tok = var->nameToken(); tok != end; tok = tok->next()) {
for (const Token *tok2 = tok; tok2 != end; tok2 = tok2->next()) { if (Token::Match(tok, "!!* %varid% <", var->declarationId())) {
if (Token::Match(tok2, "!!* %varid% <", iteratorid)) { stlBoundariesError(tok);
stlBoundariesError(tok2, container_name); } else if (Token::Match(tok, "> %varid% !!.", var->declarationId())) {
} else if (Token::Match(tok2, "> %varid% !!.", iteratorid)) { stlBoundariesError(tok);
stlBoundariesError(tok2, container_name);
}
}
}
} }
} }
} }
} }
// Error message for bad boundary usage.. // Error message for bad boundary usage..
void CheckStl::stlBoundariesError(const Token *tok, const std::string &container_name) void CheckStl::stlBoundariesError(const Token *tok)
{ {
reportError(tok, Severity::error, "stlBoundaries", reportError(tok, Severity::error, "stlBoundaries",
"Dangerous iterator comparison using operator< on 'std::" + container_name + "'.\n" "Dangerous comparison using operator< on iterator.\n"
"Iterator of container 'std::" + container_name + "' compared with operator<. " "Iterator compared with operator<. This is dangerous since the order of items in the "
"This is dangerous since the order of items in the container is not guaranteed. " "container is not guaranteed. One should use operator!= instead to compare iterators.");
"One should use operator!= instead to compare iterators.");
} }
static bool if_findCompare(const Token * const tokBack) static bool if_findCompare(const Token * const tokBack)

4
lib/checkstl.h
View File

@ -165,7 +165,7 @@ private:
void mismatchingContainersError(const Token *tok); void mismatchingContainersError(const Token *tok);
void invalidIteratorError(const Token *tok, const std::string &func, const std::string &iterator_name); 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 invalidPointerError(const Token *tok, const std::string &func, const std::string &pointer_name);
void stlBoundariesError(const Token *tok, const std::string &container_name); void stlBoundariesError(const Token *tok);
void if_findError(const Token *tok, bool str); void if_findError(const Token *tok, bool str);
void sizeError(const Token *tok); void sizeError(const Token *tok);
void redundantIfRemoveError(const Token *tok); void redundantIfRemoveError(const Token *tok);
@ -194,7 +194,7 @@ private:
c.stlOutOfBoundsError(0, "i", "foo", false); c.stlOutOfBoundsError(0, "i", "foo", false);
c.invalidIteratorError(0, "push_back|push_front|insert", "iterator"); c.invalidIteratorError(0, "push_back|push_front|insert", "iterator");
c.invalidPointerError(0, "push_back", "pointer"); c.invalidPointerError(0, "push_back", "pointer");
c.stlBoundariesError(0, "container"); c.stlBoundariesError(0);
c.if_findError(0, false); c.if_findError(0, false);
c.if_findError(0, true); c.if_findError(0, true);
c.string_c_strError(0); c.string_c_strError(0);

13
lib/library.cpp
View File

@ -322,6 +322,12 @@ Library::Error Library::load(const tinyxml2::XMLDocument &doc)
const char* const endPattern = node->Attribute("endPattern"); const char* const endPattern = node->Attribute("endPattern");
if (endPattern) if (endPattern)
container.endPattern = endPattern; container.endPattern = endPattern;
const char* const itEndPattern = node->Attribute("itEndPattern");
if (itEndPattern)
container.itEndPattern = itEndPattern;
const char* const opLessAllowed = node->Attribute("opLessAllowed");
if (opLessAllowed)
container.opLessAllowed = std::string(opLessAllowed) == "true";
for (const tinyxml2::XMLElement *containerNode = node->FirstChildElement(); containerNode; containerNode = containerNode->NextSiblingElement()) { for (const tinyxml2::XMLElement *containerNode = node->FirstChildElement(); containerNode; containerNode = containerNode->NextSiblingElement()) {
const std::string containerNodeName = containerNode->Name(); const std::string containerNodeName = containerNode->Name();
@ -778,7 +784,7 @@ bool Library::isScopeNoReturn(const Token *end, std::string *unknownFunc) const
return false; return false;
} }
const Library::Container* Library::detectContainer(const Token* typeStart) const const Library::Container* Library::detectContainer(const Token* typeStart, bool iterator) const
{ {
for (std::map<std::string, Container>::const_iterator i = containers.begin(); i != containers.end(); ++i) { for (std::map<std::string, Container>::const_iterator i = containers.begin(); i != containers.end(); ++i) {
const Container& container = i->second; const Container& container = i->second;
@ -786,12 +792,13 @@ const Library::Container* Library::detectContainer(const Token* typeStart) const
continue; continue;
if (Token::Match(typeStart, container.startPattern.c_str())) { if (Token::Match(typeStart, container.startPattern.c_str())) {
if (container.endPattern.empty()) if (!iterator && container.endPattern.empty()) // If endPattern is undefined, it will always match, but itEndPattern has to be defined.
return &container; return &container;
for (const Token* tok = typeStart; tok && !tok->varId(); tok = tok->next()) { for (const Token* tok = typeStart; tok && !tok->varId(); tok = tok->next()) {
if (tok->link()) { if (tok->link()) {
if (Token::Match(tok->link(), container.endPattern.c_str())) const std::string& endPattern = iterator ? container.itEndPattern : container.endPattern;
if (Token::Match(tok->link(), endPattern.c_str()))
return &container; return &container;
break; break;
} }

8
lib/library.h
View File

@ -137,7 +137,8 @@ public:
type_templateArgNo(-1), type_templateArgNo(-1),
size_templateArgNo(-1), size_templateArgNo(-1),
arrayLike_indexOp(false), arrayLike_indexOp(false),
stdStringLike(false) { stdStringLike(false),
opLessAllowed(true) {
} }
enum Action { enum Action {
@ -152,12 +153,13 @@ public:
Action action; Action action;
Yield yield; Yield yield;
}; };
std::string startPattern, endPattern; std::string startPattern, endPattern, itEndPattern;
std::map<std::string, Function> functions; std::map<std::string, Function> functions;
int type_templateArgNo; int type_templateArgNo;
int size_templateArgNo; int size_templateArgNo;
bool arrayLike_indexOp; bool arrayLike_indexOp;
bool stdStringLike; bool stdStringLike;
bool opLessAllowed;
Action getAction(const std::string& function) const { Action getAction(const std::string& function) const {
std::map<std::string, Function>::const_iterator i = functions.find(function); std::map<std::string, Function>::const_iterator i = functions.find(function);
@ -174,7 +176,7 @@ public:
} }
}; };
std::map<std::string, Container> containers; std::map<std::string, Container> containers;
const Container* detectContainer(const Token* typeStart) const; const Container* detectContainer(const Token* typeStart, bool iterator = false) const;
class ArgumentChecks { class ArgumentChecks {
public: public:

8
test/testlibrary.cpp
View File

@ -382,7 +382,7 @@ private:
void container() const { void container() const {
const char xmldata[] = "<?xml version=\"1.0\"?>\n" const char xmldata[] = "<?xml version=\"1.0\"?>\n"
"<def>\n" "<def>\n"
" <container id=\"A\" startPattern=\"std :: A &lt;\" endPattern=\"&gt; !!::\">\n" " <container id=\"A\" startPattern=\"std :: A &lt;\" endPattern=\"&gt; !!::\" itEndPattern=\"&gt; :: iterator\">\n"
" <type templateParameter=\"1\"/>\n" " <type templateParameter=\"1\"/>\n"
" <size templateParameter=\"4\">\n" " <size templateParameter=\"4\">\n"
" <function name=\"resize\" action=\"resize\"/>\n" " <function name=\"resize\" action=\"resize\"/>\n"
@ -402,7 +402,7 @@ private:
" <function name=\"find\" action=\"find\"/>\n" " <function name=\"find\" action=\"find\"/>\n"
" </access>\n" " </access>\n"
" </container>\n" " </container>\n"
" <container id=\"B\" startPattern=\"std :: B &lt;\" inherits=\"A\">\n" " <container id=\"B\" startPattern=\"std :: B &lt;\" inherits=\"A\" opLessAllowed=\"false\">\n"
" <size templateParameter=\"3\"/>\n" // Inherits all but templateParameter " <size templateParameter=\"3\"/>\n" // Inherits all but templateParameter
" </container>\n" " </container>\n"
" <container id=\"C\">\n" " <container id=\"C\">\n"
@ -422,8 +422,10 @@ private:
ASSERT_EQUALS(A.size_templateArgNo, 4); ASSERT_EQUALS(A.size_templateArgNo, 4);
ASSERT_EQUALS(A.startPattern, "std :: A <"); ASSERT_EQUALS(A.startPattern, "std :: A <");
ASSERT_EQUALS(A.endPattern, "> !!::"); ASSERT_EQUALS(A.endPattern, "> !!::");
ASSERT_EQUALS(A.itEndPattern, "> :: iterator");
ASSERT_EQUALS(A.stdStringLike, false); ASSERT_EQUALS(A.stdStringLike, false);
ASSERT_EQUALS(A.arrayLike_indexOp, false); ASSERT_EQUALS(A.arrayLike_indexOp, false);
ASSERT_EQUALS(A.opLessAllowed, true);
ASSERT_EQUALS(Library::Container::SIZE, A.getYield("size")); ASSERT_EQUALS(Library::Container::SIZE, A.getYield("size"));
ASSERT_EQUALS(Library::Container::EMPTY, A.getYield("empty")); ASSERT_EQUALS(Library::Container::EMPTY, A.getYield("empty"));
ASSERT_EQUALS(Library::Container::AT_INDEX, A.getYield("at")); ASSERT_EQUALS(Library::Container::AT_INDEX, A.getYield("at"));
@ -444,7 +446,9 @@ private:
ASSERT_EQUALS(B.size_templateArgNo, 3); ASSERT_EQUALS(B.size_templateArgNo, 3);
ASSERT_EQUALS(B.startPattern, "std :: B <"); ASSERT_EQUALS(B.startPattern, "std :: B <");
ASSERT_EQUALS(B.endPattern, "> !!::"); ASSERT_EQUALS(B.endPattern, "> !!::");
ASSERT_EQUALS(B.itEndPattern, "> :: iterator");
ASSERT_EQUALS(B.functions.size(), A.functions.size()); ASSERT_EQUALS(B.functions.size(), A.functions.size());
ASSERT_EQUALS(B.opLessAllowed, false);
ASSERT(C.functions.empty()); ASSERT(C.functions.empty());
ASSERT_EQUALS(C.type_templateArgNo, -1); ASSERT_EQUALS(C.type_templateArgNo, -1);

13
test/teststl.cpp
View File

@ -1406,8 +1406,7 @@ private:
void stlBoundaries1() { void stlBoundaries1() {
const std::string stlCont[] = { const std::string stlCont[] = {
"list", "set", "multiset", "map", "list", "set", "multiset", "map", "multimap"
"multimap", "hash_map", "hash_multimap", "hash_set"
}; };
for (size_t i = 0; i < getArraylength(stlCont); ++i) { for (size_t i = 0; i < getArraylength(stlCont); ++i) {
@ -1418,16 +1417,16 @@ private:
" ;\n" " ;\n"
"}"); "}");
ASSERT_EQUALS("[test.cpp:4]: (error) Dangerous iterator comparison using operator< on 'std::" + stlCont[i] + "'.\n", errout.str()); ASSERT_EQUALS("[test.cpp:4]: (error) Dangerous comparison using operator< on iterator.\n", errout.str());
} }
check("void f() {\n" check("void f() {\n"
" std::forward_list<int>::iterator it;\n" " std::forward_list<int>::iterator it;\n"
" for (it = ab.begin(); ab.end() > it; ++it) {}\n" " for (it = ab.begin(); ab.end() > it; ++it) {}\n"
"}"); "}");
ASSERT_EQUALS("[test.cpp:3]: (error) Dangerous iterator comparison using operator< on 'std::forward_list'.\n", errout.str()); ASSERT_EQUALS("[test.cpp:3]: (error) Dangerous comparison using operator< on iterator.\n", errout.str());
// #5926 no FP Dangerous iterator comparison using operator< on 'std::deque'. // #5926 no FP Dangerous comparison using operator< on iterator on std::deque
check("void f() {\n" check("void f() {\n"
" std::deque<int>::iterator it;\n" " std::deque<int>::iterator it;\n"
" for (it = ab.begin(); ab.end() > it; ++it) {}\n" " for (it = ab.begin(); ab.end() > it; ++it) {}\n"
@ -1473,7 +1472,7 @@ private:
" std::forward_list<std::vector<std::vector<int>>>::iterator it;\n" " std::forward_list<std::vector<std::vector<int>>>::iterator it;\n"
" for (it = ab.begin(); ab.end() > it; ++it) {}\n" " for (it = ab.begin(); ab.end() > it; ++it) {}\n"
"}"); "}");
ASSERT_EQUALS("[test.cpp:3]: (error) Dangerous iterator comparison using operator< on 'std::forward_list'.\n", errout.str()); ASSERT_EQUALS("[test.cpp:3]: (error) Dangerous comparison using operator< on iterator.\n", errout.str());
// don't crash // don't crash
check("void f() {\n" check("void f() {\n"
@ -1487,7 +1486,7 @@ private:
" for (it = ab.begin(); ab.end() > it; ++it) {}\n" " for (it = ab.begin(); ab.end() > it; ++it) {}\n"
" }\n" " }\n"
"}"); "}");
ASSERT_EQUALS("[test.cpp:4]: (error) Dangerous iterator comparison using operator< on 'std::forward_list'.\n", errout.str()); ASSERT_EQUALS("[test.cpp:4]: (error) Dangerous comparison using operator< on iterator.\n", errout.str());
} }
void stlBoundaries5() { void stlBoundaries5() {