Skip to content

Commit aef62ee

Browse files
firewavefirewave
authored
bail out on invalid tokenlist in --rule-file / some rule related cleanups (#6205)
1 parent 356b2bd commit aef62ee

10 files changed

Lines changed: 120 additions & 59 deletions

File tree

‎Makefile‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ ifndef VERBOSE
55
endif
66
# To compile with rules, use 'make HAVE_RULES=yes'
77
ifndef HAVE_RULES
8-
HAVE_RULES=no
8+
HAVE_RULES=
99
endif
1010

1111
ifndef MATCHCOMPILER
@@ -160,6 +160,8 @@ ifeq ($(HAVE_RULES),yes)
160160
else
161161
LIBS=$(shell $(PCRE_CONFIG) --libs)
162162
endif
163+
else ifneq ($(HAVE_RULES),)
164+
$(error invalid HAVE_RULES value '$(HAVE_RULES)')
163165
endif
164166

165167
ifndef PREFIX

‎cli/cmdlineparser.cpp‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1121,6 +1121,16 @@ CmdLineParser::Result CmdLineParser::parseFromArgs(int argc, const char* const a
11211121
return Result::Fail;
11221122
}
11231123

1124+
if (rule.tokenlist.empty()) {
1125+
mLogger.printError("unable to load rule-file '" + ruleFile + "' - a rule is lacking a tokenlist.");
1126+
return Result::Fail;
1127+
}
1128+
1129+
if (rule.tokenlist != "normal" && rule.tokenlist != "define" && rule.tokenlist != "raw") {
1130+
mLogger.printError("unable to load rule-file '" + ruleFile + "' - a rule is using the unsupported tokenlist '" + rule.tokenlist + "'.");
1131+
return Result::Fail;
1132+
}
1133+
11241134
mSettings.rules.emplace_back(std::move(rule));
11251135
}
11261136
} else {

‎lib/cppcheck.cpp‎

Lines changed: 17 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -819,20 +819,17 @@ unsigned int CppCheck::checkFile(const std::string& filename, const std::string
819819

820820
#ifdef HAVE_RULES
821821
// Run define rules on raw code
822-
const auto rules_it = std::find_if(mSettings.rules.cbegin(), mSettings.rules.cend(), [](const Settings::Rule& rule) {
823-
return rule.tokenlist == "define";
824-
});
825-
if (rules_it != mSettings.rules.cend()) {
822+
if (hasRule("define")) {
826823
std::string code;
827-
const std::list<Directive> &directives = preprocessor.getDirectives();
828-
for (const Directive &dir : directives) {
824+
for (const Directive &dir : preprocessor.getDirectives()) {
829825
if (startsWith(dir.str,"#define ") || startsWith(dir.str,"#include "))
830826
code += "#line " + std::to_string(dir.linenr) + " \"" + dir.file + "\"\n" + dir.str + '\n';
831827
}
832-
Tokenizer tokenizer2(mSettings, this);
828+
TokenList tokenlist(&mSettings);
833829
std::istringstream istr2(code);
834-
tokenizer2.list.createTokens(istr2, Path::identify(*files.begin()));
835-
executeRules("define", tokenizer2);
830+
// TODO: asserts when file has unknown extension
831+
tokenlist.createTokens(istr2, Path::identify(*files.begin())); // TODO: check result?
832+
executeRules("define", tokenlist);
836833
}
837834
#endif
838835

@@ -924,8 +921,10 @@ unsigned int CppCheck::checkFile(const std::string& filename, const std::string
924921
if (mSettings.checkConfiguration)
925922
continue;
926923

927-
// Check raw tokens
928-
checkRawTokens(tokenizer);
924+
#ifdef HAVE_RULES
925+
// Execute rules for "raw" code
926+
executeRules("raw", tokenizer.list);
927+
#endif
929928

930929
// Simplify tokens into normal form, skip rest of iteration if failed
931930
if (!tokenizer.simplifyTokens1(mCurrentConfig))
@@ -959,13 +958,6 @@ unsigned int CppCheck::checkFile(const std::string& filename, const std::string
959958

960959
// Check normal tokens
961960
checkNormalTokens(tokenizer);
962-
963-
#ifdef HAVE_RULES
964-
// handling of "simple" rules has been removed.
965-
if (hasRule("simple"))
966-
throw InternalError(nullptr, "Handling of \"simple\" rules has been removed in Cppcheck. Use --addon instead.");
967-
#endif
968-
969961
} catch (const simplecpp::Output &o) {
970962
// #error etc during preprocessing
971963
configurationError.push_back((mCurrentConfig.empty() ? "\'\'" : mCurrentConfig) + " : [" + o.location.file() + ':' + std::to_string(o.location.line) + "] " + o.msg);
@@ -1072,19 +1064,6 @@ void CppCheck::internalError(const std::string &filename, const std::string &msg
10721064
mErrorLogger.reportErr(errmsg);
10731065
}
10741066

1075-
//---------------------------------------------------------------------------
1076-
// CppCheck - A function that checks a raw token list
1077-
//---------------------------------------------------------------------------
1078-
void CppCheck::checkRawTokens(const Tokenizer &tokenizer)
1079-
{
1080-
#ifdef HAVE_RULES
1081-
// Execute rules for "raw" code
1082-
executeRules("raw", tokenizer);
1083-
#else
1084-
(void)tokenizer;
1085-
#endif
1086-
}
1087-
10881067
//---------------------------------------------------------------------------
10891068
// CppCheck - A function that checks a normal token list
10901069
//---------------------------------------------------------------------------
@@ -1169,7 +1148,7 @@ void CppCheck::checkNormalTokens(const Tokenizer &tokenizer)
11691148
}
11701149

11711150
#ifdef HAVE_RULES
1172-
executeRules("normal", tokenizer);
1151+
executeRules("normal", tokenizer.list);
11731152
#endif
11741153
}
11751154

@@ -1312,15 +1291,15 @@ static const char * pcreErrorCodeToString(const int pcreExecRet)
13121291
return "";
13131292
}
13141293

1315-
void CppCheck::executeRules(const std::string &tokenlist, const Tokenizer &tokenizer)
1294+
void CppCheck::executeRules(const std::string &tokenlist, const TokenList &list)
13161295
{
13171296
// There is no rule to execute
13181297
if (!hasRule(tokenlist))
13191298
return;
13201299

13211300
// Write all tokens in a string that can be parsed by pcre
13221301
std::ostringstream ostr;
1323-
for (const Token *tok = tokenizer.tokens(); tok; tok = tok->next())
1302+
for (const Token *tok = list.front(); tok; tok = tok->next())
13241303
ostr << " " << tok->str();
13251304
const std::string str(ostr.str());
13261305

@@ -1400,14 +1379,14 @@ void CppCheck::executeRules(const std::string &tokenlist, const Tokenizer &token
14001379
pos = (int)pos2;
14011380

14021381
// determine location..
1403-
std::string file = tokenizer.list.getSourceFilePath();
1382+
std::string file = list.getSourceFilePath();
14041383
int line = 0;
14051384

14061385
std::size_t len = 0;
1407-
for (const Token *tok = tokenizer.tokens(); tok; tok = tok->next()) {
1386+
for (const Token *tok = list.front(); tok; tok = tok->next()) {
14081387
len = len + 1U + tok->str().size();
14091388
if (len > pos1) {
1410-
file = tokenizer.list.getFiles().at(tok->fileIndex());
1389+
file = list.getFiles().at(tok->fileIndex());
14111390
line = tok->linenr();
14121391
break;
14131392
}
@@ -1421,7 +1400,7 @@ void CppCheck::executeRules(const std::string &tokenlist, const Tokenizer &token
14211400
summary = "found '" + str.substr(pos1, pos2 - pos1) + "'";
14221401
else
14231402
summary = rule.summary;
1424-
const ErrorMessage errmsg({std::move(loc)}, tokenizer.list.getSourceFilePath(), rule.severity, summary, rule.id, Certainty::normal);
1403+
const ErrorMessage errmsg({std::move(loc)}, list.getSourceFilePath(), rule.severity, summary, rule.id, Certainty::normal);
14251404

14261405
// Report error
14271406
reportErr(errmsg);

‎lib/cppcheck.h‎

Lines changed: 4 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@
3939
#include <unordered_set>
4040
#include <vector>
4141

42-
class Tokenizer;
42+
class TokenList;
4343
enum class SHOWTIME_MODES;
4444
struct FileSettings;
4545
class CheckUnusedFunctions;
@@ -169,12 +169,6 @@ class CPPCHECKLIB CppCheck : ErrorLogger {
169169
*/
170170
unsigned int checkFile(const std::string& filename, const std::string &cfgname, std::istream* fileStream = nullptr);
171171

172-
/**
173-
* @brief Check raw tokens
174-
* @param tokenizer tokenizer instance
175-
*/
176-
void checkRawTokens(const Tokenizer &tokenizer);
177-
178172
/**
179173
* @brief Check normal tokens
180174
* @param tokenizer tokenizer instance
@@ -195,10 +189,10 @@ class CPPCHECKLIB CppCheck : ErrorLogger {
195189
#ifdef HAVE_RULES
196190
/**
197191
* @brief Execute rules, if any
198-
* @param tokenlist token list to use (normal / simple)
199-
* @param tokenizer tokenizer
192+
* @param tokenlist token list to use (define / normal / raw)
193+
* @param list token list
200194
*/
201-
void executeRules(const std::string &tokenlist, const Tokenizer &tokenizer);
195+
void executeRules(const std::string &tokenlist, const TokenList &list);
202196
#endif
203197

204198
unsigned int checkClang(const std::string &path);

‎lib/library.cpp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,7 @@ static std::vector<std::string> getnames(const char *names)
5656
static void gettokenlistfromvalid(const std::string& valid, bool cpp, TokenList& tokenList)
5757
{
5858
std::istringstream istr(valid + ',');
59-
tokenList.createTokens(istr, cpp ? Standards::Language::CPP : Standards::Language::C);
59+
tokenList.createTokens(istr, cpp ? Standards::Language::CPP : Standards::Language::C); // TODO: check result?
6060
for (Token *tok = tokenList.front(); tok; tok = tok->next()) {
6161
if (Token::Match(tok,"- %num%")) {
6262
tok->str("-" + tok->strAt(1));

‎lib/symboldatabase.cpp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7583,7 +7583,7 @@ void SymbolDatabase::setValueTypeInTokenList(bool reportDebugWarnings, Token *to
75837583
ValueType valuetype;
75847584
TokenList tokenList(&mSettings);
75857585
std::istringstream istr(typestr+";");
7586-
tokenList.createTokens(istr, tok->isCpp() ? Standards::Language::CPP : Standards::Language::C);
7586+
tokenList.createTokens(istr, tok->isCpp() ? Standards::Language::CPP : Standards::Language::C); // TODO: check result?
75877587
tokenList.simplifyStdType();
75887588
if (parsedecl(tokenList.front(), &valuetype, mDefaultSignedness, mSettings)) {
75897589
valuetype.originalTypeName = typestr;

‎lib/valueflow.cpp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3815,7 +3815,7 @@ static bool isNotEqual(std::pair<const Token*, const Token*> x, const std::strin
38153815
{
38163816
TokenList tokenList(nullptr);
38173817
std::istringstream istr(y);
3818-
tokenList.createTokens(istr, cpp ? Standards::Language::CPP : Standards::Language::C);
3818+
tokenList.createTokens(istr, cpp ? Standards::Language::CPP : Standards::Language::C); // TODO: check result?
38193819
return isNotEqual(x, std::make_pair(tokenList.front(), tokenList.back()));
38203820
}
38213821
static bool isNotEqual(std::pair<const Token*, const Token*> x, const ValueType* y, bool cpp)

‎test/cli/other_test.py‎

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1180,3 +1180,51 @@ def test_unknown_extension(tmpdir):
11801180
assert exitcode == 0, stderr
11811181
assert stdout == ''
11821182
assert stderr == ''
1183+
1184+
1185+
def test_multiple_define_rules(tmpdir):
1186+
rule_file = os.path.join(tmpdir, 'rule_file.xml')
1187+
with open(rule_file, 'wt') as f:
1188+
f.write("""
1189+
<rules>
1190+
<rule>
1191+
<tokenlist>define</tokenlist>
1192+
<pattern>DEF_1</pattern>
1193+
<message>
1194+
<severity>error</severity>
1195+
<id>ruleId1</id>
1196+
</message>
1197+
</rule>
1198+
<rule>
1199+
<tokenlist>define</tokenlist>
1200+
<pattern>DEF_2</pattern>
1201+
<message>
1202+
<severity>error</severity>
1203+
<id>ruleId2</id>
1204+
</message>
1205+
</rule>
1206+
</rules>""")
1207+
1208+
test_file = os.path.join(tmpdir, 'test.c')
1209+
with open(test_file, 'wt') as f:
1210+
f.write('''
1211+
#define DEF_1
1212+
#define DEF_2
1213+
void f() { }
1214+
''')
1215+
1216+
exitcode, stdout, stderr = cppcheck(['--template=simple', '--rule-file={}'.format(rule_file), test_file])
1217+
assert exitcode == 0, stderr
1218+
lines = stdout.splitlines()
1219+
assert lines == [
1220+
'Checking {} ...'.format(test_file),
1221+
'Processing rule: DEF_1',
1222+
'Processing rule: DEF_2'
1223+
]
1224+
lines = stderr.splitlines()
1225+
assert lines == [
1226+
"{}:2:0: error: found 'DEF_1' [ruleId1]".format(test_file),
1227+
"{}:3:0: error: found 'DEF_2' [ruleId2]".format(test_file)
1228+
]
1229+
1230+
# TODO: test "raw" and "normal" rules

‎test/testcmdlineparser.cpp‎

Lines changed: 32 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -344,6 +344,8 @@ class TestCmdlineParser : public TestFixture {
344344
TEST_CASE(ruleFileEmptyElements2);
345345
TEST_CASE(ruleFileUnknownElement1);
346346
TEST_CASE(ruleFileUnknownElement2);
347+
TEST_CASE(ruleFileMissingTokenList);
348+
TEST_CASE(ruleFileUnknownTokenList);
347349
#else
348350
TEST_CASE(ruleFileNotSupported);
349351
#endif
@@ -2157,7 +2159,7 @@ class TestCmdlineParser : public TestFixture {
21572159
ScopedFile file("rule.xml",
21582160
"<rules>\n"
21592161
"<rule>\n"
2160-
"<tokenlist>toklist1</tokenlist>\n"
2162+
"<tokenlist>raw</tokenlist>\n"
21612163
"<pattern>.+</pattern>\n"
21622164
"<message>\n"
21632165
"<severity>error</severity>\n"
@@ -2166,7 +2168,7 @@ class TestCmdlineParser : public TestFixture {
21662168
"</message>\n"
21672169
"</rule>\n"
21682170
"<rule>\n"
2169-
"<tokenlist>toklist2</tokenlist>\n"
2171+
"<tokenlist>define</tokenlist>\n"
21702172
"<pattern>.*</pattern>\n"
21712173
"<message>\n"
21722174
"<severity>warning</severity>\n"
@@ -2179,13 +2181,13 @@ class TestCmdlineParser : public TestFixture {
21792181
ASSERT_EQUALS_ENUM(CmdLineParser::Result::Success, parser->parseFromArgs(3, argv));
21802182
ASSERT_EQUALS(2, settings->rules.size());
21812183
auto it = settings->rules.cbegin();
2182-
ASSERT_EQUALS("toklist1", it->tokenlist);
2184+
ASSERT_EQUALS("raw", it->tokenlist);
21832185
ASSERT_EQUALS(".+", it->pattern);
21842186
ASSERT_EQUALS_ENUM(Severity::error, it->severity);
21852187
ASSERT_EQUALS("ruleId1", it->id);
21862188
ASSERT_EQUALS("ruleSummary1", it->summary);
21872189
++it;
2188-
ASSERT_EQUALS("toklist2", it->tokenlist);
2190+
ASSERT_EQUALS("define", it->tokenlist);
21892191
ASSERT_EQUALS(".*", it->pattern);
21902192
ASSERT_EQUALS_ENUM(Severity::warning, it->severity);
21912193
ASSERT_EQUALS("ruleId2", it->id);
@@ -2196,7 +2198,7 @@ class TestCmdlineParser : public TestFixture {
21962198
REDIRECT;
21972199
ScopedFile file("rule.xml",
21982200
"<rule>\n"
2199-
"<tokenlist>toklist</tokenlist>\n"
2201+
"<tokenlist>define</tokenlist>\n"
22002202
"<pattern>.+</pattern>\n"
22012203
"<message>\n"
22022204
"<severity>error</severity>\n"
@@ -2208,7 +2210,7 @@ class TestCmdlineParser : public TestFixture {
22082210
ASSERT_EQUALS_ENUM(CmdLineParser::Result::Success, parser->parseFromArgs(3, argv));
22092211
ASSERT_EQUALS(1, settings->rules.size());
22102212
auto it = settings->rules.cbegin();
2211-
ASSERT_EQUALS("toklist", it->tokenlist);
2213+
ASSERT_EQUALS("define", it->tokenlist);
22122214
ASSERT_EQUALS(".+", it->pattern);
22132215
ASSERT_EQUALS_ENUM(Severity::error, it->severity);
22142216
ASSERT_EQUALS("ruleId", it->id);
@@ -2300,6 +2302,30 @@ class TestCmdlineParser : public TestFixture {
23002302
ASSERT_EQUALS_ENUM(CmdLineParser::Result::Fail, parser->parseFromArgs(3, argv));
23012303
ASSERT_EQUALS("cppcheck: error: unable to load rule-file 'rule.xml' - unknown element 'pattern' encountered in 'message'.\n", logger->str());
23022304
}
2305+
2306+
void ruleFileMissingTokenList() {
2307+
REDIRECT;
2308+
ScopedFile file("rule.xml",
2309+
"<rule>\n"
2310+
"<tokenlist/>\n"
2311+
"<pattern>.+</pattern>\n"
2312+
"</rule>\n");
2313+
const char * const argv[] = {"cppcheck", "--rule-file=rule.xml", "file.cpp"};
2314+
ASSERT_EQUALS_ENUM(CmdLineParser::Result::Fail, parser->parseFromArgs(3, argv));
2315+
ASSERT_EQUALS("cppcheck: error: unable to load rule-file 'rule.xml' - a rule is lacking a tokenlist.\n", logger->str());
2316+
}
2317+
2318+
void ruleFileUnknownTokenList() {
2319+
REDIRECT;
2320+
ScopedFile file("rule.xml",
2321+
"<rule>\n"
2322+
"<tokenlist>simple</tokenlist>\n"
2323+
"<pattern>.+</pattern>\n"
2324+
"</rule>\n");
2325+
const char * const argv[] = {"cppcheck", "--rule-file=rule.xml", "file.cpp"};
2326+
ASSERT_EQUALS_ENUM(CmdLineParser::Result::Fail, parser->parseFromArgs(3, argv));
2327+
ASSERT_EQUALS("cppcheck: error: unable to load rule-file 'rule.xml' - a rule is using the unsupported tokenlist 'simple'.\n", logger->str());
2328+
}
23032329
#else
23042330
void ruleFileNotSupported() {
23052331
REDIRECT;

‎tools/dmake/dmake.cpp‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -584,7 +584,7 @@ int main(int argc, char **argv)
584584
<< "endif\n";
585585

586586
fout << "# To compile with rules, use 'make HAVE_RULES=yes'\n";
587-
makeConditionalVariable(fout, "HAVE_RULES", "no");
587+
makeConditionalVariable(fout, "HAVE_RULES", "");
588588

589589
makeMatchcompiler(fout, emptyString, emptyString);
590590

@@ -743,6 +743,8 @@ int main(int argc, char **argv)
743743
<< " else\n"
744744
<< " LIBS=$(shell $(PCRE_CONFIG) --libs)\n"
745745
<< " endif\n"
746+
<< "else ifneq ($(HAVE_RULES),)\n"
747+
<< " $(error invalid HAVE_RULES value '$(HAVE_RULES)')\n"
746748
<< "endif\n\n";
747749

748750
makeConditionalVariable(fout, "PREFIX", "/usr");

0 commit comments

Comments
 (0)