From d3b2e25d007d337ddc2b49792ab9584c8fd78ea7 Mon Sep 17 00:00:00 2001 From: firewave Date: Fri, 29 Mar 2024 10:11:01 +0100 Subject: [PATCH 1/7] CppCheck: pass `TokenList` to `executeRules()` --- lib/cppcheck.cpp | 28 ++++++++++++++-------------- lib/cppcheck.h | 12 ++++++------ 2 files changed, 20 insertions(+), 20 deletions(-) diff --git a/lib/cppcheck.cpp b/lib/cppcheck.cpp index 51643b8ab95..acd1742016b 100644 --- a/lib/cppcheck.cpp +++ b/lib/cppcheck.cpp @@ -829,10 +829,10 @@ unsigned int CppCheck::checkFile(const std::string& filename, const std::string if (startsWith(dir.str,"#define ") || startsWith(dir.str,"#include ")) code += "#line " + std::to_string(dir.linenr) + " \"" + dir.file + "\"\n" + dir.str + '\n'; } - Tokenizer tokenizer2(mSettings, this); + TokenList tokenlist(&mSettings); std::istringstream istr2(code); - tokenizer2.list.createTokens(istr2, Path::identify(*files.begin())); - executeRules("define", tokenizer2); + tokenlist.createTokens(istr2, Path::identify(*files.begin())); + executeRules("define", tokenlist); } #endif @@ -925,7 +925,7 @@ unsigned int CppCheck::checkFile(const std::string& filename, const std::string continue; // Check raw tokens - checkRawTokens(tokenizer); + checkRawTokens(tokenizer.list); // Simplify tokens into normal form, skip rest of iteration if failed if (!tokenizer.simplifyTokens1(mCurrentConfig)) @@ -1075,13 +1075,13 @@ void CppCheck::internalError(const std::string &filename, const std::string &msg //--------------------------------------------------------------------------- // CppCheck - A function that checks a raw token list //--------------------------------------------------------------------------- -void CppCheck::checkRawTokens(const Tokenizer &tokenizer) +void CppCheck::checkRawTokens(const TokenList &tokenlist) { #ifdef HAVE_RULES // Execute rules for "raw" code - executeRules("raw", tokenizer); + executeRules("raw", tokenlist); #else - (void)tokenizer; + (void)tokenlist; #endif } @@ -1169,7 +1169,7 @@ void CppCheck::checkNormalTokens(const Tokenizer &tokenizer) } #ifdef HAVE_RULES - executeRules("normal", tokenizer); + executeRules("normal", tokenizer.list); #endif } @@ -1312,7 +1312,7 @@ static const char * pcreErrorCodeToString(const int pcreExecRet) return ""; } -void CppCheck::executeRules(const std::string &tokenlist, const Tokenizer &tokenizer) +void CppCheck::executeRules(const std::string &tokenlist, const TokenList &list) { // There is no rule to execute if (!hasRule(tokenlist)) @@ -1320,7 +1320,7 @@ void CppCheck::executeRules(const std::string &tokenlist, const Tokenizer &token // Write all tokens in a string that can be parsed by pcre std::ostringstream ostr; - for (const Token *tok = tokenizer.tokens(); tok; tok = tok->next()) + for (const Token *tok = list.front(); tok; tok = tok->next()) ostr << " " << tok->str(); const std::string str(ostr.str()); @@ -1400,14 +1400,14 @@ void CppCheck::executeRules(const std::string &tokenlist, const Tokenizer &token pos = (int)pos2; // determine location.. - std::string file = tokenizer.list.getSourceFilePath(); + std::string file = list.getSourceFilePath(); int line = 0; std::size_t len = 0; - for (const Token *tok = tokenizer.tokens(); tok; tok = tok->next()) { + for (const Token *tok = list.front(); tok; tok = tok->next()) { len = len + 1U + tok->str().size(); if (len > pos1) { - file = tokenizer.list.getFiles().at(tok->fileIndex()); + file = list.getFiles().at(tok->fileIndex()); line = tok->linenr(); break; } @@ -1421,7 +1421,7 @@ void CppCheck::executeRules(const std::string &tokenlist, const Tokenizer &token summary = "found '" + str.substr(pos1, pos2 - pos1) + "'"; else summary = rule.summary; - const ErrorMessage errmsg({std::move(loc)}, tokenizer.list.getSourceFilePath(), rule.severity, summary, rule.id, Certainty::normal); + const ErrorMessage errmsg({std::move(loc)}, list.getSourceFilePath(), rule.severity, summary, rule.id, Certainty::normal); // Report error reportErr(errmsg); diff --git a/lib/cppcheck.h b/lib/cppcheck.h index f17e66541df..076979d1283 100644 --- a/lib/cppcheck.h +++ b/lib/cppcheck.h @@ -39,7 +39,7 @@ #include #include -class Tokenizer; +class TokenList; enum class SHOWTIME_MODES; struct FileSettings; class CheckUnusedFunctions; @@ -171,9 +171,9 @@ class CPPCHECKLIB CppCheck : ErrorLogger { /** * @brief Check raw tokens - * @param tokenizer tokenizer instance + * @param tokenlist token list */ - void checkRawTokens(const Tokenizer &tokenizer); + void checkRawTokens(const TokenList &tokenlist); /** * @brief Check normal tokens @@ -195,10 +195,10 @@ class CPPCHECKLIB CppCheck : ErrorLogger { #ifdef HAVE_RULES /** * @brief Execute rules, if any - * @param tokenlist token list to use (normal / simple) - * @param tokenizer tokenizer + * @param tokenlist token list to use (define / normal / raw) + * @param list token list */ - void executeRules(const std::string &tokenlist, const Tokenizer &tokenizer); + void executeRules(const std::string &tokenlist, const TokenList &list); #endif unsigned int checkClang(const std::string &path); From 947edd8d2d53fdb40c3d25225a2a4bbcb7998e24 Mon Sep 17 00:00:00 2001 From: firewave Date: Fri, 29 Mar 2024 10:28:34 +0100 Subject: [PATCH 2/7] bail out on invalid tokenlist in `--rule-file` --- cli/cmdlineparser.cpp | 10 ++++++++++ lib/cppcheck.cpp | 7 ------- test/testcmdlineparser.cpp | 38 ++++++++++++++++++++++++++++++++------ 3 files changed, 42 insertions(+), 13 deletions(-) diff --git a/cli/cmdlineparser.cpp b/cli/cmdlineparser.cpp index 59cf67847fa..920bede12de 100644 --- a/cli/cmdlineparser.cpp +++ b/cli/cmdlineparser.cpp @@ -1121,6 +1121,16 @@ CmdLineParser::Result CmdLineParser::parseFromArgs(int argc, const char* const a return Result::Fail; } + if (rule.tokenlist.empty()) { + mLogger.printError("unable to load rule-file '" + ruleFile + "' - a rule is lacking a tokenlist."); + return Result::Fail; + } + + if (rule.tokenlist != "normal" && rule.tokenlist != "define" && rule.tokenlist != "raw") { + mLogger.printError("unable to load rule-file '" + ruleFile + "' - a rule is using the unsupported tokenlist '" + rule.tokenlist + "'."); + return Result::Fail; + } + mSettings.rules.emplace_back(std::move(rule)); } } else { diff --git a/lib/cppcheck.cpp b/lib/cppcheck.cpp index acd1742016b..b8700dc0132 100644 --- a/lib/cppcheck.cpp +++ b/lib/cppcheck.cpp @@ -959,13 +959,6 @@ unsigned int CppCheck::checkFile(const std::string& filename, const std::string // Check normal tokens checkNormalTokens(tokenizer); - -#ifdef HAVE_RULES - // handling of "simple" rules has been removed. - if (hasRule("simple")) - throw InternalError(nullptr, "Handling of \"simple\" rules has been removed in Cppcheck. Use --addon instead."); -#endif - } catch (const simplecpp::Output &o) { // #error etc during preprocessing configurationError.push_back((mCurrentConfig.empty() ? "\'\'" : mCurrentConfig) + " : [" + o.location.file() + ':' + std::to_string(o.location.line) + "] " + o.msg); diff --git a/test/testcmdlineparser.cpp b/test/testcmdlineparser.cpp index 60a56efbccc..d01abd2d440 100644 --- a/test/testcmdlineparser.cpp +++ b/test/testcmdlineparser.cpp @@ -344,6 +344,8 @@ class TestCmdlineParser : public TestFixture { TEST_CASE(ruleFileEmptyElements2); TEST_CASE(ruleFileUnknownElement1); TEST_CASE(ruleFileUnknownElement2); + TEST_CASE(ruleFileMissingTokenList); + TEST_CASE(ruleFileUnknownTokenList); #else TEST_CASE(ruleFileNotSupported); #endif @@ -2157,7 +2159,7 @@ class TestCmdlineParser : public TestFixture { ScopedFile file("rule.xml", "\n" "\n" - "toklist1\n" + "raw\n" ".+\n" "\n" "error\n" @@ -2166,7 +2168,7 @@ class TestCmdlineParser : public TestFixture { "\n" "\n" "\n" - "toklist2\n" + "define\n" ".*\n" "\n" "warning\n" @@ -2179,13 +2181,13 @@ class TestCmdlineParser : public TestFixture { ASSERT_EQUALS_ENUM(CmdLineParser::Result::Success, parser->parseFromArgs(3, argv)); ASSERT_EQUALS(2, settings->rules.size()); auto it = settings->rules.cbegin(); - ASSERT_EQUALS("toklist1", it->tokenlist); + ASSERT_EQUALS("raw", it->tokenlist); ASSERT_EQUALS(".+", it->pattern); ASSERT_EQUALS_ENUM(Severity::error, it->severity); ASSERT_EQUALS("ruleId1", it->id); ASSERT_EQUALS("ruleSummary1", it->summary); ++it; - ASSERT_EQUALS("toklist2", it->tokenlist); + ASSERT_EQUALS("define", it->tokenlist); ASSERT_EQUALS(".*", it->pattern); ASSERT_EQUALS_ENUM(Severity::warning, it->severity); ASSERT_EQUALS("ruleId2", it->id); @@ -2196,7 +2198,7 @@ class TestCmdlineParser : public TestFixture { REDIRECT; ScopedFile file("rule.xml", "\n" - "toklist\n" + "define\n" ".+\n" "\n" "error\n" @@ -2208,7 +2210,7 @@ class TestCmdlineParser : public TestFixture { ASSERT_EQUALS_ENUM(CmdLineParser::Result::Success, parser->parseFromArgs(3, argv)); ASSERT_EQUALS(1, settings->rules.size()); auto it = settings->rules.cbegin(); - ASSERT_EQUALS("toklist", it->tokenlist); + ASSERT_EQUALS("define", it->tokenlist); ASSERT_EQUALS(".+", it->pattern); ASSERT_EQUALS_ENUM(Severity::error, it->severity); ASSERT_EQUALS("ruleId", it->id); @@ -2300,6 +2302,30 @@ class TestCmdlineParser : public TestFixture { ASSERT_EQUALS_ENUM(CmdLineParser::Result::Fail, parser->parseFromArgs(3, argv)); ASSERT_EQUALS("cppcheck: error: unable to load rule-file 'rule.xml' - unknown element 'pattern' encountered in 'message'.\n", logger->str()); } + + void ruleFileMissingTokenList() { + REDIRECT; + ScopedFile file("rule.xml", + "\n" + "\n" + ".+\n" + "\n"); + const char * const argv[] = {"cppcheck", "--rule-file=rule.xml", "file.cpp"}; + ASSERT_EQUALS_ENUM(CmdLineParser::Result::Fail, parser->parseFromArgs(3, argv)); + ASSERT_EQUALS("cppcheck: error: unable to load rule-file 'rule.xml' - a rule is lacking a tokenlist.\n", logger->str()); + } + + void ruleFileUnknownTokenList() { + REDIRECT; + ScopedFile file("rule.xml", + "\n" + "simple\n" + ".+\n" + "\n"); + const char * const argv[] = {"cppcheck", "--rule-file=rule.xml", "file.cpp"}; + ASSERT_EQUALS_ENUM(CmdLineParser::Result::Fail, parser->parseFromArgs(3, argv)); + ASSERT_EQUALS("cppcheck: error: unable to load rule-file 'rule.xml' - a rule is using the unsupported tokenlist 'simple'.\n", logger->str()); + } #else void ruleFileNotSupported() { REDIRECT; From 5b4fa66faacba7b872214b324eef55ac13adf8f1 Mon Sep 17 00:00:00 2001 From: firewave Date: Fri, 29 Mar 2024 10:32:19 +0100 Subject: [PATCH 3/7] added TODOs --- lib/cppcheck.cpp | 2 +- lib/library.cpp | 2 +- lib/symboldatabase.cpp | 2 +- lib/valueflow.cpp | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/lib/cppcheck.cpp b/lib/cppcheck.cpp index b8700dc0132..3a6dbec3ef5 100644 --- a/lib/cppcheck.cpp +++ b/lib/cppcheck.cpp @@ -831,7 +831,7 @@ unsigned int CppCheck::checkFile(const std::string& filename, const std::string } TokenList tokenlist(&mSettings); std::istringstream istr2(code); - tokenlist.createTokens(istr2, Path::identify(*files.begin())); + tokenlist.createTokens(istr2, Path::identify(*files.begin())); // TODO: check result? executeRules("define", tokenlist); } #endif diff --git a/lib/library.cpp b/lib/library.cpp index 15ba68c31c6..e8feeb07502 100644 --- a/lib/library.cpp +++ b/lib/library.cpp @@ -56,7 +56,7 @@ static std::vector getnames(const char *names) static void gettokenlistfromvalid(const std::string& valid, bool cpp, TokenList& tokenList) { std::istringstream istr(valid + ','); - tokenList.createTokens(istr, cpp ? Standards::Language::CPP : Standards::Language::C); + tokenList.createTokens(istr, cpp ? Standards::Language::CPP : Standards::Language::C); // TODO: check result? for (Token *tok = tokenList.front(); tok; tok = tok->next()) { if (Token::Match(tok,"- %num%")) { tok->str("-" + tok->strAt(1)); diff --git a/lib/symboldatabase.cpp b/lib/symboldatabase.cpp index 6c70ae7acf0..b8451b90005 100644 --- a/lib/symboldatabase.cpp +++ b/lib/symboldatabase.cpp @@ -7583,7 +7583,7 @@ void SymbolDatabase::setValueTypeInTokenList(bool reportDebugWarnings, Token *to ValueType valuetype; TokenList tokenList(&mSettings); std::istringstream istr(typestr+";"); - tokenList.createTokens(istr, tok->isCpp() ? Standards::Language::CPP : Standards::Language::C); + tokenList.createTokens(istr, tok->isCpp() ? Standards::Language::CPP : Standards::Language::C); // TODO: check result? tokenList.simplifyStdType(); if (parsedecl(tokenList.front(), &valuetype, mDefaultSignedness, mSettings)) { valuetype.originalTypeName = typestr; diff --git a/lib/valueflow.cpp b/lib/valueflow.cpp index 024f87cb59b..f9cfae0cb98 100644 --- a/lib/valueflow.cpp +++ b/lib/valueflow.cpp @@ -3815,7 +3815,7 @@ static bool isNotEqual(std::pair x, const std::strin { TokenList tokenList(nullptr); std::istringstream istr(y); - tokenList.createTokens(istr, cpp ? Standards::Language::CPP : Standards::Language::C); + tokenList.createTokens(istr, cpp ? Standards::Language::CPP : Standards::Language::C); // TODO: check result? return isNotEqual(x, std::make_pair(tokenList.front(), tokenList.back())); } static bool isNotEqual(std::pair x, const ValueType* y, bool cpp) From d1804b0fbc1ef8c4ae14098c18894d5d7866a996 Mon Sep 17 00:00:00 2001 From: firewave Date: Fri, 29 Mar 2024 10:32:51 +0100 Subject: [PATCH 4/7] CppCheck: folded single `checkRawTokens()` call --- lib/cppcheck.cpp | 19 ++++--------------- lib/cppcheck.h | 6 ------ 2 files changed, 4 insertions(+), 21 deletions(-) diff --git a/lib/cppcheck.cpp b/lib/cppcheck.cpp index 3a6dbec3ef5..5e257c9f86a 100644 --- a/lib/cppcheck.cpp +++ b/lib/cppcheck.cpp @@ -924,8 +924,10 @@ unsigned int CppCheck::checkFile(const std::string& filename, const std::string if (mSettings.checkConfiguration) continue; - // Check raw tokens - checkRawTokens(tokenizer.list); +#ifdef HAVE_RULES + // Execute rules for "raw" code + executeRules("raw", tokenizer.list); +#endif // Simplify tokens into normal form, skip rest of iteration if failed if (!tokenizer.simplifyTokens1(mCurrentConfig)) @@ -1065,19 +1067,6 @@ void CppCheck::internalError(const std::string &filename, const std::string &msg mErrorLogger.reportErr(errmsg); } -//--------------------------------------------------------------------------- -// CppCheck - A function that checks a raw token list -//--------------------------------------------------------------------------- -void CppCheck::checkRawTokens(const TokenList &tokenlist) -{ -#ifdef HAVE_RULES - // Execute rules for "raw" code - executeRules("raw", tokenlist); -#else - (void)tokenlist; -#endif -} - //--------------------------------------------------------------------------- // CppCheck - A function that checks a normal token list //--------------------------------------------------------------------------- diff --git a/lib/cppcheck.h b/lib/cppcheck.h index 076979d1283..ef68a9c4342 100644 --- a/lib/cppcheck.h +++ b/lib/cppcheck.h @@ -169,12 +169,6 @@ class CPPCHECKLIB CppCheck : ErrorLogger { */ unsigned int checkFile(const std::string& filename, const std::string &cfgname, std::istream* fileStream = nullptr); - /** - * @brief Check raw tokens - * @param tokenlist token list - */ - void checkRawTokens(const TokenList &tokenlist); - /** * @brief Check normal tokens * @param tokenizer tokenizer instance From 5d6b532fbc15bc60216e2957a74782eb47e69d13 Mon Sep 17 00:00:00 2001 From: firewave Date: Fri, 29 Mar 2024 11:06:55 +0100 Subject: [PATCH 5/7] other_test.py: added test for multiple `define` rules --- test/cli/other_test.py | 48 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 48 insertions(+) diff --git a/test/cli/other_test.py b/test/cli/other_test.py index 2e7270e6443..529e6e8a390 100644 --- a/test/cli/other_test.py +++ b/test/cli/other_test.py @@ -1180,3 +1180,51 @@ def test_unknown_extension(tmpdir): assert exitcode == 0, stderr assert stdout == '' assert stderr == '' + + +def test_multiple_define_rules(tmpdir): + rule_file = os.path.join(tmpdir, 'rule_file.xml') + with open(rule_file, 'wt') as f: + f.write(""" + + + define + DEF_1 + + error + ruleId1 + + + + define + DEF_2 + + error + ruleId2 + + +""") + + test_file = os.path.join(tmpdir, 'test.c') + with open(test_file, 'wt') as f: + f.write(''' +#define DEF_1 +#define DEF_2 +void f() { } +''') + + exitcode, stdout, stderr = cppcheck(['--template=simple', '--rule-file={}'.format(rule_file), test_file]) + assert exitcode == 0, stderr + lines = stdout.splitlines() + assert lines == [ + 'Checking {} ...'.format(test_file), + 'Processing rule: DEF_1', + 'Processing rule: DEF_2' + ] + lines = stderr.splitlines() + assert lines == [ + "{}:2:0: error: found 'DEF_1' [ruleId1]".format(test_file), + "{}:3:0: error: found 'DEF_2' [ruleId2]".format(test_file) + ] + +# TODO: test "raw" and "normal" rules \ No newline at end of file From 058dced739084025ae662c1cd21da292e8177f9e Mon Sep 17 00:00:00 2001 From: firewave Date: Fri, 29 Mar 2024 11:07:13 +0100 Subject: [PATCH 6/7] Cppcheck: small `define` rule invocation cleanup --- lib/cppcheck.cpp | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/lib/cppcheck.cpp b/lib/cppcheck.cpp index 5e257c9f86a..3127d024e39 100644 --- a/lib/cppcheck.cpp +++ b/lib/cppcheck.cpp @@ -819,18 +819,15 @@ unsigned int CppCheck::checkFile(const std::string& filename, const std::string #ifdef HAVE_RULES // Run define rules on raw code - const auto rules_it = std::find_if(mSettings.rules.cbegin(), mSettings.rules.cend(), [](const Settings::Rule& rule) { - return rule.tokenlist == "define"; - }); - if (rules_it != mSettings.rules.cend()) { + if (hasRule("define")) { std::string code; - const std::list &directives = preprocessor.getDirectives(); - for (const Directive &dir : directives) { + for (const Directive &dir : preprocessor.getDirectives()) { if (startsWith(dir.str,"#define ") || startsWith(dir.str,"#include ")) code += "#line " + std::to_string(dir.linenr) + " \"" + dir.file + "\"\n" + dir.str + '\n'; } TokenList tokenlist(&mSettings); std::istringstream istr2(code); + // TODO: asserts when file has unknown extension tokenlist.createTokens(istr2, Path::identify(*files.begin())); // TODO: check result? executeRules("define", tokenlist); } From 0972a047d2580fa5d2b8a71ad1fb6c11789ba837 Mon Sep 17 00:00:00 2001 From: firewave Date: Fri, 29 Mar 2024 11:07:38 +0100 Subject: [PATCH 7/7] Makefile: bail out on invalid `HAVE_RULES` value --- Makefile | 4 +++- tools/dmake/dmake.cpp | 4 +++- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/Makefile b/Makefile index 23a15c49e54..32b030b8d26 100644 --- a/Makefile +++ b/Makefile @@ -5,7 +5,7 @@ ifndef VERBOSE endif # To compile with rules, use 'make HAVE_RULES=yes' ifndef HAVE_RULES - HAVE_RULES=no + HAVE_RULES= endif ifndef MATCHCOMPILER @@ -160,6 +160,8 @@ ifeq ($(HAVE_RULES),yes) else LIBS=$(shell $(PCRE_CONFIG) --libs) endif +else ifneq ($(HAVE_RULES),) + $(error invalid HAVE_RULES value '$(HAVE_RULES)') endif ifndef PREFIX diff --git a/tools/dmake/dmake.cpp b/tools/dmake/dmake.cpp index c74eeac69ef..6de023f5cea 100644 --- a/tools/dmake/dmake.cpp +++ b/tools/dmake/dmake.cpp @@ -584,7 +584,7 @@ int main(int argc, char **argv) << "endif\n"; fout << "# To compile with rules, use 'make HAVE_RULES=yes'\n"; - makeConditionalVariable(fout, "HAVE_RULES", "no"); + makeConditionalVariable(fout, "HAVE_RULES", ""); makeMatchcompiler(fout, emptyString, emptyString); @@ -743,6 +743,8 @@ int main(int argc, char **argv) << " else\n" << " LIBS=$(shell $(PCRE_CONFIG) --libs)\n" << " endif\n" + << "else ifneq ($(HAVE_RULES),)\n" + << " $(error invalid HAVE_RULES value '$(HAVE_RULES)')\n" << "endif\n\n"; makeConditionalVariable(fout, "PREFIX", "/usr");