Fixed #3232 (Check if container is modified inside BOOST_FOREACH)

This commit is contained in:
Thomas Jarosch 2011-10-23 13:07:43 +02:00 committed by Daniel Marjamäki
parent 5ac9b00e84
commit db8c7deb30
7 changed files with 248 additions and 3 deletions

View File

@ -51,6 +51,7 @@ MAN_SOURCE=man/cppcheck.1.xml
LIBOBJ = lib/check64bit.o \
lib/checkassignif.o \
lib/checkautovariables.o \
lib/checkboost.o \
lib/checkbufferoverrun.o \
lib/checkclass.o \
lib/checkexceptionsafety.o \
@ -88,6 +89,7 @@ TESTOBJ = test/options.o \
test/test64bit.o \
test/testassignif.o \
test/testautovariables.o \
test/testboost.o \
test/testbufferoverrun.o \
test/testcharvar.o \
test/testclass.o \
@ -181,6 +183,9 @@ lib/checkassignif.o: lib/checkassignif.cpp lib/checkassignif.h lib/check.h lib/t
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
$(CXX) $(CPPFLAGS) $(CXXFLAGS) ${INCLUDE_FOR_LIB} -c -o lib/checkautovariables.o lib/checkautovariables.cpp
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
$(CXX) $(CPPFLAGS) $(CXXFLAGS) ${INCLUDE_FOR_LIB} -c -o lib/checkboost.o lib/checkboost.cpp
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
$(CXX) $(CPPFLAGS) $(CXXFLAGS) ${INCLUDE_FOR_LIB} -c -o lib/checkbufferoverrun.o lib/checkbufferoverrun.cpp
@ -286,6 +291,9 @@ test/testassignif.o: test/testassignif.cpp lib/tokenize.h lib/checkassignif.h li
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
$(CXX) $(CPPFLAGS) $(CXXFLAGS) ${INCLUDE_FOR_TEST} -c -o test/testautovariables.o test/testautovariables.cpp
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
$(CXX) $(CPPFLAGS) $(CXXFLAGS) ${INCLUDE_FOR_TEST} -c -o test/testboost.o test/testboost.cpp
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
$(CXX) $(CPPFLAGS) $(CXXFLAGS) ${INCLUDE_FOR_TEST} -c -o test/testbufferoverrun.o test/testbufferoverrun.cpp

58
lib/checkboost.cpp Normal file
View File

@ -0,0 +1,58 @@
/*
* Cppcheck - A tool for static C/C++ code analysis
* Copyright (C) 2007-2011 Daniel Marjamäki and Cppcheck team.
*
* This program is free software: you can redistribute it and/or modify
* it under the terms of the GNU General Public License as published by
* the Free Software Foundation, either version 3 of the License, or
* (at your option) any later version.
*
* This program is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
* GNU General Public License for more details.
*
* You should have received a copy of the GNU General Public License
* along with this program. If not, see <http://www.gnu.org/licenses/>.
*/
#include "checkboost.h"
#include "symboldatabase.h"
#include <sstream>
// Register this check class (by creating a static instance of it)
namespace {
CheckBoost instance;
}
void CheckBoost::checkBoostForeachModification()
{
for (const Token *tok = _tokenizer->tokens(); tok; tok = tok->next()) {
if (!Token::simpleMatch(tok, "BOOST_FOREACH ("))
continue;
const Token *container_tok = tok->next()->link()->tokAt(-1);
if (!Token::Match(container_tok, "%var% ) {"))
continue;
const unsigned int container_id = container_tok->varId();
if (container_id == 0)
continue;
const Token *tok2 = container_tok->tokAt(2);
const Token *end = tok2->link();
for (; tok2 != end; tok2 = tok2->next()) {
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)) {
boostForeachError(tok2);
break;
}
}
}
}
void CheckBoost::boostForeachError(const Token *tok)
{
reportError(tok, Severity::error, "boostForeachError",
"BOOST_FOREACH caches the end() iterator. It's undefined behavior if you modify the container."
);
}

75
lib/checkboost.h Normal file
View File

@ -0,0 +1,75 @@
/*
* Cppcheck - A tool for static C/C++ code analysis
* Copyright (C) 2007-2011 Daniel Marjamäki and Cppcheck team.
*
* This program is free software: you can redistribute it and/or modify
* it under the terms of the GNU General Public License as published by
* the Free Software Foundation, either version 3 of the License, or
* (at your option) any later version.
*
* This program is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
* GNU General Public License for more details.
*
* You should have received a copy of the GNU General Public License
* along with this program. If not, see <http://www.gnu.org/licenses/>.
*/
//---------------------------------------------------------------------------
#ifndef CHECKBOOST_H
#define CHECKBOOST_H
//---------------------------------------------------------------------------
#include "check.h"
class Token;
/// @addtogroup Checks
/// @{
/** @brief %Check Boost usage */
class CheckBoost : public Check {
public:
/** This constructor is used when registering the CheckClass */
CheckBoost() : Check(myName())
{ }
/** This constructor is used when running checks. */
CheckBoost(const Tokenizer *tokenizer, const Settings *settings, ErrorLogger *errorLogger)
: Check(myName(), tokenizer, settings, errorLogger)
{ }
/** Simplified checks. The token list is simplified. */
void runSimplifiedChecks(const Tokenizer *tokenizer, const Settings *settings, ErrorLogger *errorLogger) {
CheckBoost checkBoost(tokenizer, settings, errorLogger);
checkBoost.checkBoostForeachModification();
}
/** @brief %Check for container modification while using the BOOST_FOREACH macro */
void checkBoostForeachModification();
private:
void boostForeachError(const Token *tok);
void getErrorMessages(ErrorLogger *errorLogger, const Settings *settings) {
CheckBoost c(0, settings, errorLogger);
c.boostForeachError(0);
}
std::string myName() const {
return "Boost usage";
}
std::string classInfo() const {
return "Check for invalid usage of Boost:\n"
"* container modification during BOOST_FOREACH\n";
}
};
/// @}
//---------------------------------------------------------------------------
#endif

View File

@ -6,6 +6,7 @@ HEADERS += $${BASEPATH}check.h \
$${BASEPATH}check64bit.h \
$${BASEPATH}checkassignif.h \
$${BASEPATH}checkautovariables.h \
$${BASEPATH}checkboost.h \
$${BASEPATH}checkbufferoverrun.h \
$${BASEPATH}checkclass.h \
$${BASEPATH}checkexceptionsafety.h \
@ -35,6 +36,7 @@ HEADERS += $${BASEPATH}check.h \
SOURCES += $${BASEPATH}check64bit.cpp \
$${BASEPATH}checkassignif.cpp \
$${BASEPATH}checkautovariables.cpp \
$${BASEPATH}checkboost.cpp \
$${BASEPATH}checkbufferoverrun.cpp \
$${BASEPATH}checkclass.cpp \
$${BASEPATH}checkexceptionsafety.cpp \

View File

@ -2374,9 +2374,12 @@ std::string Preprocessor::expandMacros(const std::string &code, std::string file
// defining a macro..
if (line.compare(0, 8, "#define ") == 0) {
PreprocessorMacro *macro = new PreprocessorMacro(line.substr(8));
if (macro->name().empty())
if (macro->name().empty()) {
delete macro;
else {
} else if (macro->name() == "BOOST_FOREACH") {
// BOOST_FOREACH is currently too complex to parse, so skip it.
delete macro;
} else {
std::map<std::string, PreprocessorMacro *>::iterator it;
it = macros.find(macro->name());
if (it != macros.end())

View File

@ -4680,7 +4680,7 @@ bool Tokenizer::simplifyIfAddBraces()
continue;
}
if (Token::Match(tok, "if|for|while (")) {
if (Token::Match(tok, "if|for|while|BOOST_FOREACH (")) {
// don't add "{}" around ";" in "do {} while();" (#609)
const Token *prev = tok->previous();
if (Token::simpleMatch(prev, "} while") &&

99
test/testboost.cpp Normal file
View File

@ -0,0 +1,99 @@
/*
* Cppcheck - A tool for static C/C++ code analysis
* Copyright (C) 2007-2011 Daniel Marjamäki and Cppcheck team.
*
* This program is free software: you can redistribute it and/or modify
* it under the terms of the GNU General Public License as published by
* the Free Software Foundation, either version 3 of the License, or
* (at your option) any later version.
*
* This program is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
* GNU General Public License for more details.
*
* You should have received a copy of the GNU General Public License
* along with this program. If not, see <http://www.gnu.org/licenses/>.
*/
#include "tokenize.h"
#include "checkboost.h"
#include "testsuite.h"
#include <sstream>
extern std::ostringstream errout;
class TestBoost : public TestFixture {
public:
TestBoost() : TestFixture("TestBoost")
{ }
private:
void run() {
TEST_CASE(BoostForeachContainerModification)
}
void check(const std::string &code) {
// Clear the error buffer..
errout.str("");
Settings settings;
settings.addEnabled("style");
settings.addEnabled("performance");
// Tokenize..
Tokenizer tokenizer(&settings, this);
std::istringstream istr(code.c_str());
tokenizer.tokenize(istr, "test.cpp");
tokenizer.simplifyTokenList();
// Check..
CheckBoost checkBoost;
checkBoost.runSimplifiedChecks(&tokenizer, &settings, this);
}
void BoostForeachContainerModification() {
check("void f() {\n"
" vector<int> data;\n"
" BOOST_FOREACH(int i, data) {\n"
" data.push_back(123);\n"
" }\n"
"}");
ASSERT_EQUALS("[test.cpp:4]: (error) BOOST_FOREACH caches the end() iterator. It's undefined behavior if you modify the container.\n", errout.str());
check("void f() {\n"
" set<int> data;\n"
" BOOST_FOREACH(int i, data) {\n"
" data.insert(123);\n"
" }\n"
"}");
ASSERT_EQUALS("[test.cpp:4]: (error) BOOST_FOREACH caches the end() iterator. It's undefined behavior if you modify the container.\n", errout.str());
check("void f() {\n"
" set<int> data;\n"
" BOOST_FOREACH(const int &i, data) {\n"
" data.erase(123);\n"
" }\n"
"}");
ASSERT_EQUALS("[test.cpp:4]: (error) BOOST_FOREACH caches the end() iterator. It's undefined behavior if you modify the container.\n", errout.str());
// Check single line usage
check("void f() {\n"
" set<int> data;\n"
" BOOST_FOREACH(const int &i, data)\n"
" data.clear();\n"
"}");
ASSERT_EQUALS("[test.cpp:4]: (error) BOOST_FOREACH caches the end() iterator. It's undefined behavior if you modify the container.\n", errout.str());
// Container returned as result of a function -> Be quiet
check("void f() {\n"
" BOOST_FOREACH(const int &i, get_data())\n"
" data.insert(i);\n"
"}");
ASSERT_EQUALS("", errout.str());
}
};
REGISTER_TEST(TestBoost)