Skip to content

Commit db8c7de

Browse files
thomasjfoxDaniel Marjamäki
thomasjfox
authored and
Daniel Marjamäki
committed
Fixed cppcheck-opensource#3232 (Check if container is modified inside BOOST_FOREACH)
1 parent 5ac9b00 commit db8c7de

7 files changed

Lines changed: 248 additions & 3 deletions

File tree

‎Makefile‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,7 @@ MAN_SOURCE=man/cppcheck.1.xml
5151
LIBOBJ = lib/check64bit.o \
5252
lib/checkassignif.o \
5353
lib/checkautovariables.o \
54+
lib/checkboost.o \
5455
lib/checkbufferoverrun.o \
5556
lib/checkclass.o \
5657
lib/checkexceptionsafety.o \
@@ -88,6 +89,7 @@ TESTOBJ = test/options.o \
8889
test/test64bit.o \
8990
test/testassignif.o \
9091
test/testautovariables.o \
92+
test/testboost.o \
9193
test/testbufferoverrun.o \
9294
test/testcharvar.o \
9395
test/testclass.o \
@@ -181,6 +183,9 @@ lib/checkassignif.o: lib/checkassignif.cpp lib/checkassignif.h lib/check.h lib/t
181183
lib/checkautovariables.o: lib/checkautovariables.cpp lib/checkautovariables.h lib/check.h lib/token.h lib/tokenize.h lib/settings.h lib/suppressions.h lib/standards.h lib/errorlogger.h lib/symboldatabase.h lib/mathlib.h
182184
$(CXX) $(CPPFLAGS) $(CXXFLAGS) ${INCLUDE_FOR_LIB} -c -o lib/checkautovariables.o lib/checkautovariables.cpp
183185

186+
lib/checkboost.o: lib/checkboost.cpp lib/checkboost.h lib/check.h lib/token.h lib/tokenize.h lib/settings.h lib/suppressions.h lib/standards.h lib/errorlogger.h lib/symboldatabase.h lib/mathlib.h
187+
$(CXX) $(CPPFLAGS) $(CXXFLAGS) ${INCLUDE_FOR_LIB} -c -o lib/checkboost.o lib/checkboost.cpp
188+
184189
lib/checkbufferoverrun.o: lib/checkbufferoverrun.cpp lib/checkbufferoverrun.h lib/check.h lib/token.h lib/tokenize.h lib/settings.h lib/suppressions.h lib/standards.h lib/errorlogger.h lib/mathlib.h lib/symboldatabase.h lib/executionpath.h
185190
$(CXX) $(CPPFLAGS) $(CXXFLAGS) ${INCLUDE_FOR_LIB} -c -o lib/checkbufferoverrun.o lib/checkbufferoverrun.cpp
186191

@@ -286,6 +291,9 @@ test/testassignif.o: test/testassignif.cpp lib/tokenize.h lib/checkassignif.h li
286291
test/testautovariables.o: test/testautovariables.cpp lib/tokenize.h lib/checkautovariables.h lib/check.h lib/token.h lib/settings.h lib/suppressions.h lib/standards.h lib/errorlogger.h test/testsuite.h test/redirect.h
287292
$(CXX) $(CPPFLAGS) $(CXXFLAGS) ${INCLUDE_FOR_TEST} -c -o test/testautovariables.o test/testautovariables.cpp
288293

294+
test/testboost.o: test/testboost.cpp lib/tokenize.h lib/checkboost.h lib/check.h lib/token.h lib/settings.h lib/suppressions.h lib/standards.h lib/errorlogger.h test/testsuite.h test/redirect.h
295+
$(CXX) $(CPPFLAGS) $(CXXFLAGS) ${INCLUDE_FOR_TEST} -c -o test/testboost.o test/testboost.cpp
296+
289297
test/testbufferoverrun.o: test/testbufferoverrun.cpp lib/tokenize.h lib/checkbufferoverrun.h lib/check.h lib/token.h lib/settings.h lib/suppressions.h lib/standards.h lib/errorlogger.h lib/mathlib.h test/testsuite.h test/redirect.h
290298
$(CXX) $(CPPFLAGS) $(CXXFLAGS) ${INCLUDE_FOR_TEST} -c -o test/testbufferoverrun.o test/testbufferoverrun.cpp
291299

‎lib/checkboost.cpp‎

Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,58 @@
1+
/*
2+
* Cppcheck - A tool for static C/C++ code analysis
3+
* Copyright (C) 2007-2011 Daniel Marjamäki and Cppcheck team.
4+
*
5+
* This program is free software: you can redistribute it and/or modify
6+
* it under the terms of the GNU General Public License as published by
7+
* the Free Software Foundation, either version 3 of the License, or
8+
* (at your option) any later version.
9+
*
10+
* This program is distributed in the hope that it will be useful,
11+
* but WITHOUT ANY WARRANTY; without even the implied warranty of
12+
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
13+
* GNU General Public License for more details.
14+
*
15+
* You should have received a copy of the GNU General Public License
16+
* along with this program. If not, see <http://www.gnu.org/licenses/>.
17+
*/
18+
19+
#include "checkboost.h"
20+
#include "symboldatabase.h"
21+
#include <sstream>
22+
23+
// Register this check class (by creating a static instance of it)
24+
namespace {
25+
CheckBoost instance;
26+
}
27+
28+
void CheckBoost::checkBoostForeachModification()
29+
{
30+
for (const Token *tok = _tokenizer->tokens(); tok; tok = tok->next()) {
31+
if (!Token::simpleMatch(tok, "BOOST_FOREACH ("))
32+
continue;
33+
34+
const Token *container_tok = tok->next()->link()->tokAt(-1);
35+
if (!Token::Match(container_tok, "%var% ) {"))
36+
continue;
37+
38+
const unsigned int container_id = container_tok->varId();
39+
if (container_id == 0)
40+
continue;
41+
42+
const Token *tok2 = container_tok->tokAt(2);
43+
const Token *end = tok2->link();
44+
for (; tok2 != end; tok2 = tok2->next()) {
45+
if (Token::Match(tok2, "%varid% . insert|erase|push_back|push_front|pop_front|pop_back|clear|swap|resize|assign|merge|remove|remove_if|reverse|sort|splice|unique|pop|push", container_id)) {
46+
boostForeachError(tok2);
47+
break;
48+
}
49+
}
50+
}
51+
}
52+
53+
void CheckBoost::boostForeachError(const Token *tok)
54+
{
55+
reportError(tok, Severity::error, "boostForeachError",
56+
"BOOST_FOREACH caches the end() iterator. It's undefined behavior if you modify the container."
57+
);
58+
}

‎lib/checkboost.h‎

Lines changed: 75 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,75 @@
1+
/*
2+
* Cppcheck - A tool for static C/C++ code analysis
3+
* Copyright (C) 2007-2011 Daniel Marjamäki and Cppcheck team.
4+
*
5+
* This program is free software: you can redistribute it and/or modify
6+
* it under the terms of the GNU General Public License as published by
7+
* the Free Software Foundation, either version 3 of the License, or
8+
* (at your option) any later version.
9+
*
10+
* This program is distributed in the hope that it will be useful,
11+
* but WITHOUT ANY WARRANTY; without even the implied warranty of
12+
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
13+
* GNU General Public License for more details.
14+
*
15+
* You should have received a copy of the GNU General Public License
16+
* along with this program. If not, see <http://www.gnu.org/licenses/>.
17+
*/
18+
19+
20+
//---------------------------------------------------------------------------
21+
#ifndef CHECKBOOST_H
22+
#define CHECKBOOST_H
23+
//---------------------------------------------------------------------------
24+
25+
#include "check.h"
26+
27+
class Token;
28+
29+
/// @addtogroup Checks
30+
/// @{
31+
32+
33+
/** @brief %Check Boost usage */
34+
class CheckBoost : public Check {
35+
public:
36+
/** This constructor is used when registering the CheckClass */
37+
CheckBoost() : Check(myName())
38+
{ }
39+
40+
/** This constructor is used when running checks. */
41+
CheckBoost(const Tokenizer *tokenizer, const Settings *settings, ErrorLogger *errorLogger)
42+
: Check(myName(), tokenizer, settings, errorLogger)
43+
{ }
44+
45+
/** Simplified checks. The token list is simplified. */
46+
void runSimplifiedChecks(const Tokenizer *tokenizer, const Settings *settings, ErrorLogger *errorLogger) {
47+
CheckBoost checkBoost(tokenizer, settings, errorLogger);
48+
49+
checkBoost.checkBoostForeachModification();
50+
}
51+
52+
/** @brief %Check for container modification while using the BOOST_FOREACH macro */
53+
void checkBoostForeachModification();
54+
55+
private:
56+
void boostForeachError(const Token *tok);
57+
58+
void getErrorMessages(ErrorLogger *errorLogger, const Settings *settings) {
59+
CheckBoost c(0, settings, errorLogger);
60+
c.boostForeachError(0);
61+
}
62+
63+
std::string myName() const {
64+
return "Boost usage";
65+
}
66+
67+
std::string classInfo() const {
68+
return "Check for invalid usage of Boost:\n"
69+
"* container modification during BOOST_FOREACH\n";
70+
}
71+
};
72+
/// @}
73+
//---------------------------------------------------------------------------
74+
#endif
75+

‎lib/lib.pri‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ HEADERS += $${BASEPATH}check.h \
66
$${BASEPATH}check64bit.h \
77
$${BASEPATH}checkassignif.h \
88
$${BASEPATH}checkautovariables.h \
9+
$${BASEPATH}checkboost.h \
910
$${BASEPATH}checkbufferoverrun.h \
1011
$${BASEPATH}checkclass.h \
1112
$${BASEPATH}checkexceptionsafety.h \
@@ -35,6 +36,7 @@ HEADERS += $${BASEPATH}check.h \
3536
SOURCES += $${BASEPATH}check64bit.cpp \
3637
$${BASEPATH}checkassignif.cpp \
3738
$${BASEPATH}checkautovariables.cpp \
39+
$${BASEPATH}checkboost.cpp \
3840
$${BASEPATH}checkbufferoverrun.cpp \
3941
$${BASEPATH}checkclass.cpp \
4042
$${BASEPATH}checkexceptionsafety.cpp \

‎lib/preprocessor.cpp‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2374,9 +2374,12 @@ std::string Preprocessor::expandMacros(const std::string &code, std::string file
23742374
// defining a macro..
23752375
if (line.compare(0, 8, "#define ") == 0) {
23762376
PreprocessorMacro *macro = new PreprocessorMacro(line.substr(8));
2377-
if (macro->name().empty())
2377+
if (macro->name().empty()) {
23782378
delete macro;
2379-
else {
2379+
} else if (macro->name() == "BOOST_FOREACH") {
2380+
// BOOST_FOREACH is currently too complex to parse, so skip it.
2381+
delete macro;
2382+
} else {
23802383
std::map<std::string, PreprocessorMacro *>::iterator it;
23812384
it = macros.find(macro->name());
23822385
if (it != macros.end())

‎lib/tokenize.cpp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4680,7 +4680,7 @@ bool Tokenizer::simplifyIfAddBraces()
46804680
continue;
46814681
}
46824682

4683-
if (Token::Match(tok, "if|for|while (")) {
4683+
if (Token::Match(tok, "if|for|while|BOOST_FOREACH (")) {
46844684
// don't add "{}" around ";" in "do {} while();" (#609)
46854685
const Token *prev = tok->previous();
46864686
if (Token::simpleMatch(prev, "} while") &&

‎test/testboost.cpp‎

Lines changed: 99 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,99 @@
1+
/*
2+
* Cppcheck - A tool for static C/C++ code analysis
3+
* Copyright (C) 2007-2011 Daniel Marjamäki and Cppcheck team.
4+
*
5+
* This program is free software: you can redistribute it and/or modify
6+
* it under the terms of the GNU General Public License as published by
7+
* the Free Software Foundation, either version 3 of the License, or
8+
* (at your option) any later version.
9+
*
10+
* This program is distributed in the hope that it will be useful,
11+
* but WITHOUT ANY WARRANTY; without even the implied warranty of
12+
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
13+
* GNU General Public License for more details.
14+
*
15+
* You should have received a copy of the GNU General Public License
16+
* along with this program. If not, see <http://www.gnu.org/licenses/>.
17+
*/
18+
19+
20+
21+
#include "tokenize.h"
22+
#include "checkboost.h"
23+
#include "testsuite.h"
24+
#include <sstream>
25+
26+
extern std::ostringstream errout;
27+
28+
class TestBoost : public TestFixture {
29+
public:
30+
TestBoost() : TestFixture("TestBoost")
31+
{ }
32+
33+
private:
34+
void run() {
35+
TEST_CASE(BoostForeachContainerModification)
36+
}
37+
38+
void check(const std::string &code) {
39+
// Clear the error buffer..
40+
errout.str("");
41+
42+
Settings settings;
43+
settings.addEnabled("style");
44+
settings.addEnabled("performance");
45+
46+
// Tokenize..
47+
Tokenizer tokenizer(&settings, this);
48+
std::istringstream istr(code.c_str());
49+
tokenizer.tokenize(istr, "test.cpp");
50+
tokenizer.simplifyTokenList();
51+
52+
// Check..
53+
CheckBoost checkBoost;
54+
checkBoost.runSimplifiedChecks(&tokenizer, &settings, this);
55+
}
56+
57+
void BoostForeachContainerModification() {
58+
check("void f() {\n"
59+
" vector<int> data;\n"
60+
" BOOST_FOREACH(int i, data) {\n"
61+
" data.push_back(123);\n"
62+
" }\n"
63+
"}");
64+
ASSERT_EQUALS("[test.cpp:4]: (error) BOOST_FOREACH caches the end() iterator. It's undefined behavior if you modify the container.\n", errout.str());
65+
66+
check("void f() {\n"
67+
" set<int> data;\n"
68+
" BOOST_FOREACH(int i, data) {\n"
69+
" data.insert(123);\n"
70+
" }\n"
71+
"}");
72+
ASSERT_EQUALS("[test.cpp:4]: (error) BOOST_FOREACH caches the end() iterator. It's undefined behavior if you modify the container.\n", errout.str());
73+
74+
check("void f() {\n"
75+
" set<int> data;\n"
76+
" BOOST_FOREACH(const int &i, data) {\n"
77+
" data.erase(123);\n"
78+
" }\n"
79+
"}");
80+
ASSERT_EQUALS("[test.cpp:4]: (error) BOOST_FOREACH caches the end() iterator. It's undefined behavior if you modify the container.\n", errout.str());
81+
82+
// Check single line usage
83+
check("void f() {\n"
84+
" set<int> data;\n"
85+
" BOOST_FOREACH(const int &i, data)\n"
86+
" data.clear();\n"
87+
"}");
88+
ASSERT_EQUALS("[test.cpp:4]: (error) BOOST_FOREACH caches the end() iterator. It's undefined behavior if you modify the container.\n", errout.str());
89+
90+
// Container returned as result of a function -> Be quiet
91+
check("void f() {\n"
92+
" BOOST_FOREACH(const int &i, get_data())\n"
93+
" data.insert(i);\n"
94+
"}");
95+
ASSERT_EQUALS("", errout.str());
96+
}
97+
};
98+
99+
REGISTER_TEST(TestBoost)

0 commit comments

Comments
 (0)