Skip to content

Commit 5594b4a

Browse files
author
Lena Herscheid
committed
more extensive assert statement checks (warn for side effects), moved to separate class
1 parent de8ee5b commit 5594b4a

13 files changed

Lines changed: 410 additions & 106 deletions

Makefile

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -93,8 +93,10 @@ MAN_SOURCE=man/cppcheck.1.xml
9393
###### Object Files
9494

9595
LIBOBJ = $(SRCDIR)/check64bit.o \
96+
$(SRCDIR)/checkassert.o \
9697
$(SRCDIR)/checkassignif.o \
9798
$(SRCDIR)/checkautovariables.o \
99+
$(SRCDIR)/checkbool.o \
98100
$(SRCDIR)/checkboost.o \
99101
$(SRCDIR)/checkbufferoverrun.o \
100102
$(SRCDIR)/checkclass.o \
@@ -108,6 +110,7 @@ LIBOBJ = $(SRCDIR)/check64bit.o \
108110
$(SRCDIR)/checkobsoletefunctions.o \
109111
$(SRCDIR)/checkother.o \
110112
$(SRCDIR)/checkpostfixoperator.o \
113+
$(SRCDIR)/checksizeof.o \
111114
$(SRCDIR)/checkstl.o \
112115
$(SRCDIR)/checkuninitvar.o \
113116
$(SRCDIR)/checkunusedfunctions.o \
@@ -136,8 +139,10 @@ CLIOBJ = cli/cmdlineparser.o \
136139

137140
TESTOBJ = test/options.o \
138141
test/test64bit.o \
142+
test/testassert.o \
139143
test/testassignif.o \
140144
test/testautovariables.o \
145+
test/testbool.o \
141146
test/testboost.o \
142147
test/testbufferoverrun.o \
143148
test/testcharvar.o \
@@ -166,6 +171,7 @@ TESTOBJ = test/options.o \
166171
test/testpreprocessor.o \
167172
test/testrunner.o \
168173
test/testsimplifytokens.o \
174+
test/testsizeof.o \
169175
test/teststl.o \
170176
test/testsuite.o \
171177
test/testsuppressions.o \
@@ -230,12 +236,18 @@ install: cppcheck
230236
$(SRCDIR)/check64bit.o: lib/check64bit.cpp lib/check64bit.h lib/config.h lib/check.h lib/token.h lib/tokenize.h lib/errorlogger.h lib/suppressions.h lib/tokenlist.h lib/settings.h lib/standards.h lib/symboldatabase.h lib/mathlib.h
231237
$(CXX) $(CPPFLAGS) $(CXXFLAGS) ${INCLUDE_FOR_LIB} -c -o $(SRCDIR)/check64bit.o $(SRCDIR)/check64bit.cpp
232238

239+
$(SRCDIR)/checkassert.o: lib/checkassert.cpp lib/checkassert.h lib/config.h lib/check.h lib/token.h lib/tokenize.h lib/errorlogger.h lib/suppressions.h lib/tokenlist.h lib/settings.h lib/standards.h lib/symboldatabase.h lib/mathlib.h
240+
$(CXX) $(CPPFLAGS) $(CXXFLAGS) ${INCLUDE_FOR_LIB} -c -o $(SRCDIR)/checkassert.o $(SRCDIR)/checkassert.cpp
241+
233242
$(SRCDIR)/checkassignif.o: lib/checkassignif.cpp lib/checkassignif.h lib/config.h lib/check.h lib/token.h lib/tokenize.h lib/errorlogger.h lib/suppressions.h lib/tokenlist.h lib/settings.h lib/standards.h lib/mathlib.h lib/symboldatabase.h
234243
$(CXX) $(CPPFLAGS) $(CXXFLAGS) ${INCLUDE_FOR_LIB} -c -o $(SRCDIR)/checkassignif.o $(SRCDIR)/checkassignif.cpp
235244

236245
$(SRCDIR)/checkautovariables.o: lib/checkautovariables.cpp lib/checkautovariables.h lib/config.h lib/check.h lib/token.h lib/tokenize.h lib/errorlogger.h lib/suppressions.h lib/tokenlist.h lib/settings.h lib/standards.h lib/symboldatabase.h lib/mathlib.h lib/checkuninitvar.h
237246
$(CXX) $(CPPFLAGS) $(CXXFLAGS) ${INCLUDE_FOR_LIB} -c -o $(SRCDIR)/checkautovariables.o $(SRCDIR)/checkautovariables.cpp
238247

248+
$(SRCDIR)/checkbool.o: lib/checkbool.cpp lib/checkbool.h lib/config.h lib/check.h lib/token.h lib/tokenize.h lib/errorlogger.h lib/suppressions.h lib/tokenlist.h lib/settings.h lib/standards.h lib/mathlib.h lib/symboldatabase.h
249+
$(CXX) $(CPPFLAGS) $(CXXFLAGS) ${INCLUDE_FOR_LIB} -c -o $(SRCDIR)/checkbool.o $(SRCDIR)/checkbool.cpp
250+
239251
$(SRCDIR)/checkboost.o: lib/checkboost.cpp lib/checkboost.h lib/config.h lib/check.h lib/token.h lib/tokenize.h lib/errorlogger.h lib/suppressions.h lib/tokenlist.h lib/settings.h lib/standards.h lib/symboldatabase.h lib/mathlib.h
240252
$(CXX) $(CPPFLAGS) $(CXXFLAGS) ${INCLUDE_FOR_LIB} -c -o $(SRCDIR)/checkboost.o $(SRCDIR)/checkboost.cpp
241253

@@ -275,6 +287,9 @@ $(SRCDIR)/checkother.o: lib/checkother.cpp lib/checkother.h lib/config.h lib/che
275287
$(SRCDIR)/checkpostfixoperator.o: lib/checkpostfixoperator.cpp lib/checkpostfixoperator.h lib/config.h lib/check.h lib/token.h lib/tokenize.h lib/errorlogger.h lib/suppressions.h lib/tokenlist.h lib/settings.h lib/standards.h lib/symboldatabase.h lib/mathlib.h
276288
$(CXX) $(CPPFLAGS) $(CXXFLAGS) ${INCLUDE_FOR_LIB} -c -o $(SRCDIR)/checkpostfixoperator.o $(SRCDIR)/checkpostfixoperator.cpp
277289

290+
$(SRCDIR)/checksizeof.o: lib/checksizeof.cpp lib/checksizeof.h lib/config.h lib/check.h lib/token.h lib/tokenize.h lib/errorlogger.h lib/suppressions.h lib/tokenlist.h lib/settings.h lib/standards.h lib/symboldatabase.h lib/mathlib.h
291+
$(CXX) $(CPPFLAGS) $(CXXFLAGS) ${INCLUDE_FOR_LIB} -c -o $(SRCDIR)/checksizeof.o $(SRCDIR)/checksizeof.cpp
292+
278293
$(SRCDIR)/checkstl.o: lib/checkstl.cpp lib/checkstl.h lib/config.h lib/check.h lib/token.h lib/tokenize.h lib/errorlogger.h lib/suppressions.h lib/tokenlist.h lib/settings.h lib/standards.h lib/executionpath.h lib/symboldatabase.h lib/mathlib.h
279294
$(CXX) $(CPPFLAGS) $(CXXFLAGS) ${INCLUDE_FOR_LIB} -c -o $(SRCDIR)/checkstl.o $(SRCDIR)/checkstl.cpp
280295

@@ -353,12 +368,18 @@ test/options.o: test/options.cpp test/options.h
353368
test/test64bit.o: test/test64bit.cpp lib/tokenize.h lib/errorlogger.h lib/config.h lib/suppressions.h lib/tokenlist.h lib/check64bit.h lib/check.h lib/token.h lib/settings.h lib/standards.h test/testsuite.h test/redirect.h
354369
$(CXX) $(CPPFLAGS) $(CXXFLAGS) ${INCLUDE_FOR_TEST} -c -o test/test64bit.o test/test64bit.cpp
355370

371+
test/testassert.o: test/testassert.cpp lib/tokenize.h lib/errorlogger.h lib/config.h lib/suppressions.h lib/tokenlist.h lib/checkassert.h lib/check.h lib/token.h lib/settings.h lib/standards.h test/testsuite.h test/redirect.h
372+
$(CXX) $(CPPFLAGS) $(CXXFLAGS) ${INCLUDE_FOR_TEST} -c -o test/testassert.o test/testassert.cpp
373+
356374
test/testassignif.o: test/testassignif.cpp lib/tokenize.h lib/errorlogger.h lib/config.h lib/suppressions.h lib/tokenlist.h lib/checkassignif.h lib/check.h lib/token.h lib/settings.h lib/standards.h lib/mathlib.h test/testsuite.h test/redirect.h
357375
$(CXX) $(CPPFLAGS) $(CXXFLAGS) ${INCLUDE_FOR_TEST} -c -o test/testassignif.o test/testassignif.cpp
358376

359377
test/testautovariables.o: test/testautovariables.cpp lib/tokenize.h lib/errorlogger.h lib/config.h lib/suppressions.h lib/tokenlist.h lib/checkautovariables.h lib/check.h lib/token.h lib/settings.h lib/standards.h test/testsuite.h test/redirect.h
360378
$(CXX) $(CPPFLAGS) $(CXXFLAGS) ${INCLUDE_FOR_TEST} -c -o test/testautovariables.o test/testautovariables.cpp
361379

380+
test/testbool.o: test/testbool.cpp lib/tokenize.h lib/errorlogger.h lib/config.h lib/suppressions.h lib/tokenlist.h lib/checkbool.h lib/check.h lib/token.h lib/settings.h lib/standards.h test/testsuite.h test/redirect.h
381+
$(CXX) $(CPPFLAGS) $(CXXFLAGS) ${INCLUDE_FOR_TEST} -c -o test/testbool.o test/testbool.cpp
382+
362383
test/testboost.o: test/testboost.cpp lib/tokenize.h lib/errorlogger.h lib/config.h lib/suppressions.h lib/tokenlist.h lib/checkboost.h lib/check.h lib/token.h lib/settings.h lib/standards.h test/testsuite.h test/redirect.h
363384
$(CXX) $(CPPFLAGS) $(CXXFLAGS) ${INCLUDE_FOR_TEST} -c -o test/testboost.o test/testboost.cpp
364385

@@ -443,6 +464,9 @@ test/testrunner.o: test/testrunner.cpp test/testsuite.h lib/errorlogger.h lib/co
443464
test/testsimplifytokens.o: test/testsimplifytokens.cpp test/testsuite.h lib/errorlogger.h lib/config.h lib/suppressions.h test/redirect.h lib/tokenize.h lib/tokenlist.h lib/token.h lib/settings.h lib/standards.h lib/templatesimplifier.h lib/path.h
444465
$(CXX) $(CPPFLAGS) $(CXXFLAGS) ${INCLUDE_FOR_TEST} -c -o test/testsimplifytokens.o test/testsimplifytokens.cpp
445466

467+
test/testsizeof.o: test/testsizeof.cpp lib/tokenize.h lib/errorlogger.h lib/config.h lib/suppressions.h lib/tokenlist.h lib/checksizeof.h lib/check.h lib/token.h lib/settings.h lib/standards.h test/testsuite.h test/redirect.h
468+
$(CXX) $(CPPFLAGS) $(CXXFLAGS) ${INCLUDE_FOR_TEST} -c -o test/testsizeof.o test/testsizeof.cpp
469+
446470
test/teststl.o: test/teststl.cpp lib/tokenize.h lib/errorlogger.h lib/config.h lib/suppressions.h lib/tokenlist.h lib/checkstl.h lib/check.h lib/token.h lib/settings.h lib/standards.h test/testsuite.h test/redirect.h
447471
$(CXX) $(CPPFLAGS) $(CXXFLAGS) ${INCLUDE_FOR_TEST} -c -o test/teststl.o test/teststl.cpp
448472

lib/checkassert.cpp

Lines changed: 147 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,147 @@
1+
/*
2+
* Cppcheck - A tool for static C/C++ code analysis
3+
* Copyright (C) 2007-2013 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+
// You should not write statements with side effects in assert()
21+
//---------------------------------------------------------------------------
22+
23+
#include "checkassert.h"
24+
#include "symboldatabase.h"
25+
26+
//---------------------------------------------------------------------------
27+
28+
29+
// Register this check class (by creating a static instance of it)
30+
namespace {
31+
CheckAssert instance;
32+
}
33+
34+
void CheckAssert::assertWithSideEffects()
35+
{
36+
if (!_settings->isEnabled("warning"))
37+
return;
38+
39+
const Token *tok = findAssertPattern(_tokenizer->tokens());
40+
const Token *endTok = tok ? tok->next()->link() : NULL;
41+
42+
while (tok && endTok) {
43+
for (const Token* tmp = tok->tokAt(1); tmp != endTok; tmp = tmp->next()) {
44+
checkVariableAssignment(tmp, true);
45+
46+
if (tmp->isName() && tmp->type() == Token::eFunction) {
47+
const Function* f = tmp->function();
48+
if (f->argCount() == 0 && f->isConst) continue;
49+
50+
// functions with non-const references
51+
else if (f->argCount() != 0) {
52+
for (std::list<Variable>::const_iterator it = f->argumentList.begin(); it != f->argumentList.end(); ++it) {
53+
if (it->isConst() || it->isLocal()) continue;
54+
else if (it->isReference()) {
55+
const Token* next = it->nameToken()->next();
56+
bool isAssigned = checkVariableAssignment(next, false);
57+
if (isAssigned)
58+
sideEffectInAssertError(tmp, f->name());
59+
continue;
60+
}
61+
}
62+
}
63+
64+
// variables in function scope
65+
const Scope* scope = f->functionScope;
66+
if (!scope) continue;
67+
68+
for (const Token *tok2 = scope->classStart; tok2 != scope->classEnd; tok2 = tok2->next()) {
69+
if (tok2 && tok2->type() != Token::eAssignmentOp && tok2->type() != Token::eIncDecOp) continue;
70+
const Variable* var = tok2->previous()->variable();
71+
if (!var) continue;
72+
73+
std::vector<const Token*> returnTokens; // find all return statements
74+
for (const Token *rt = scope->classStart; rt != scope->classEnd; rt = rt->next()) {
75+
if (!Token::Match(rt, "return %any%")) continue;
76+
returnTokens.push_back(rt);
77+
}
78+
79+
bool noReturnInScope = true;
80+
for (std::vector<const Token*>::iterator rt = returnTokens.begin(); rt != returnTokens.end(); ++rt) {
81+
noReturnInScope &= !inSameScope(*rt, tok2);
82+
}
83+
if (noReturnInScope) continue;
84+
bool isAssigned = checkVariableAssignment(tok2, false);
85+
if (isAssigned)
86+
sideEffectInAssertError(tmp, f->name());
87+
}
88+
}
89+
}
90+
91+
tok = findAssertPattern(endTok->next());
92+
endTok = tok ? tok->next()->link() : NULL;
93+
}
94+
}
95+
//---------------------------------------------------------------------------
96+
97+
98+
void CheckAssert::sideEffectInAssertError(const Token *tok, const std::string& functionName)
99+
{
100+
reportError(tok, Severity::warning,
101+
"assertWithSideEffect", "Assert statement calls a function which may have desired side effects: '" + functionName + "'.\n"
102+
"Non-pure function: '" + functionName + "' is called inside assert statement. "
103+
"Assert statements are removed from release builds so the code inside "
104+
"assert statement is not executed. If the code is needed also in release "
105+
"builds, this is a bug.");
106+
}
107+
108+
void CheckAssert::assignmentInAssertError(const Token *tok, const std::string& varname)
109+
{
110+
reportError(tok, Severity::warning,
111+
"assignmentInAssert", "Assert statement modifies '" + varname + "'.\n"
112+
"Variable '" + varname + "' is modified insert assert statement. "
113+
"Assert statements are removed from release builds so the code inside "
114+
"assert statement is not executed. If the code is needed also in release "
115+
"builds, this is a bug.");
116+
}
117+
118+
// checks if side effects happen on the variable prior to tmp
119+
bool CheckAssert::checkVariableAssignment(const Token* assignTok, bool reportError /*= true*/)
120+
{
121+
const Variable* v = assignTok->previous()->variable();
122+
if (!v) return false;
123+
124+
// assignment
125+
if (assignTok->isAssignmentOp() || assignTok->type() == Token::eIncDecOp) {
126+
127+
if (v->isConst()) return false;
128+
129+
if (reportError) // report as variable assignment error
130+
assignmentInAssertError(assignTok, v->name());
131+
132+
return true;
133+
}
134+
return false;
135+
// TODO: function calls on v
136+
}
137+
138+
const Token* CheckAssert::findAssertPattern(const Token* start)
139+
{
140+
return Token::findmatch(start, "assert ( %any%");
141+
}
142+
143+
bool CheckAssert::inSameScope(const Token* returnTok, const Token* assignTok)
144+
{
145+
// TODO: even if a return is in the same scope, the assignment might not affect it.
146+
return returnTok->scope() == assignTok->scope();
147+
}

lib/checkassert.h

Lines changed: 76 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,76 @@
1+
/*
2+
* Cppcheck - A tool for static C/C++ code analysis
3+
* Copyright (C) 2007-2013 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 CheckAssertH
22+
#define CheckAssertH
23+
//---------------------------------------------------------------------------
24+
25+
#include "config.h"
26+
#include "check.h"
27+
28+
/// @addtogroup Checks
29+
/// @{
30+
31+
/**
32+
* @brief Checking for side effects in assert statements
33+
*/
34+
35+
class CPPCHECKLIB CheckAssert : public Check {
36+
public:
37+
CheckAssert() : Check(myName())
38+
{}
39+
40+
CheckAssert(const Tokenizer *tokenizer, const Settings *settings, ErrorLogger *errorLogger)
41+
: Check(myName(), tokenizer, settings, errorLogger)
42+
{}
43+
44+
virtual void runSimplifiedChecks(const Tokenizer *tokenizer, const Settings *settings, ErrorLogger *errorLogger) {
45+
CheckAssert check(tokenizer, settings, errorLogger);
46+
check.assertWithSideEffects();
47+
}
48+
49+
void assertWithSideEffects();
50+
51+
protected:
52+
bool checkVariableAssignment(const Token* tmp, bool reportError = true);
53+
bool inSameScope(const Token* returnTok, const Token* assignTok);
54+
55+
static const Token* findAssertPattern(const Token *start);
56+
57+
private:
58+
void sideEffectInAssertError(const Token *tok, const std::string& functionName);
59+
void assignmentInAssertError(const Token *tok, const std::string &varname);
60+
61+
void getErrorMessages(ErrorLogger *errorLogger, const Settings *settings) const {
62+
CheckAssert c(0, settings, errorLogger);
63+
c.assertWithSideEffects();
64+
}
65+
66+
static std::string myName() {
67+
return "Side Effects in asserts";
68+
}
69+
70+
std::string classInfo() const {
71+
return "Warn if side effects in assert statements \n";
72+
}
73+
};
74+
/// @}
75+
//---------------------------------------------------------------------------
76+
#endif

lib/checkother.cpp

Lines changed: 0 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -1170,46 +1170,6 @@ void CheckOther::selfAssignmentError(const Token *tok, const std::string &varnam
11701170
"selfAssignment", "Redundant assignment of '" + varname + "' to itself.");
11711171
}
11721172

1173-
//---------------------------------------------------------------------------
1174-
// int a = 1;
1175-
// assert(a = 2); // <- assert should not have a side-effect
1176-
//---------------------------------------------------------------------------
1177-
static inline const Token *findAssertPattern(const Token *start)
1178-
{
1179-
return Token::findmatch(start, "assert ( %any%");
1180-
}
1181-
1182-
void CheckOther::checkAssignmentInAssert()
1183-
{
1184-
if (!_settings->isEnabled("warning"))
1185-
return;
1186-
1187-
const Token *tok = findAssertPattern(_tokenizer->tokens());
1188-
const Token *endTok = tok ? tok->next()->link() : NULL;
1189-
1190-
while (tok && endTok) {
1191-
for (tok = tok->tokAt(2); tok != endTok; tok = tok->next()) {
1192-
if (tok->isName() && (tok->next()->isAssignmentOp() || tok->next()->type() == Token::eIncDecOp))
1193-
assignmentInAssertError(tok, tok->str());
1194-
else if (Token::Match(tok, "--|++ %var%"))
1195-
assignmentInAssertError(tok, tok->strAt(1));
1196-
}
1197-
1198-
tok = findAssertPattern(endTok->next());
1199-
endTok = tok ? tok->next()->link() : NULL;
1200-
}
1201-
}
1202-
1203-
void CheckOther::assignmentInAssertError(const Token *tok, const std::string &varname)
1204-
{
1205-
reportError(tok, Severity::warning,
1206-
"assignmentInAssert", "Assert statement modifies '" + varname + "'.\n"
1207-
"Variable '" + varname + "' is modified insert assert statement. "
1208-
"Assert statements are removed from release builds so the code inside "
1209-
"assert statement is not executed. If the code is needed also in release "
1210-
"builds, this is a bug.");
1211-
}
1212-
12131173
//---------------------------------------------------------------------------
12141174
// if ((x != 1) || (x != 3)) // expression always true
12151175
// if ((x == 1) && (x == 3)) // expression always false

0 commit comments

Comments
 (0)