From 34a11c16734e361a0a21f652dcce316a867cf9d2 Mon Sep 17 00:00:00 2001 From: Jens Yllman Date: Wed, 30 Mar 2022 19:24:53 +0200 Subject: [PATCH] Fix unmatched suppression (#5704) (#3886) --- Makefile | 2 +- lib/cppcheck.cpp | 3 +++ lib/suppressions.cpp | 22 ++++++++++++++++++++-- lib/suppressions.h | 14 ++++++++++++-- test/cli/test-inline-suppress.py | 12 ++++++++++++ test/cli/trac5704/trac5704a.c | 11 +++++++++++ test/cli/trac5704/trac5704b.c | 11 +++++++++++ test/testsuppressions.cpp | 4 +++- 8 files changed, 73 insertions(+), 6 deletions(-) create mode 100644 test/cli/trac5704/trac5704a.c create mode 100644 test/cli/trac5704/trac5704b.c diff --git a/Makefile b/Makefile index 44cb7d455..fec318778 100644 --- a/Makefile +++ b/Makefile @@ -554,7 +554,7 @@ $(libcppdir)/settings.o: lib/settings.cpp externals/picojson/picojson.h lib/conf $(libcppdir)/summaries.o: lib/summaries.cpp lib/analyzerinfo.h lib/config.h lib/errortypes.h lib/importproject.h lib/library.h lib/mathlib.h lib/platform.h lib/settings.h lib/standards.h lib/summaries.h lib/suppressions.h lib/symboldatabase.h lib/templatesimplifier.h lib/timer.h lib/token.h lib/tokenize.h lib/tokenlist.h lib/utils.h lib/valueflow.h $(CXX) ${INCLUDE_FOR_LIB} $(CPPFLAGS) $(CPPFILESDIR) $(CXXFLAGS) $(UNDEF_STRICT_ANSI) -c -o $(libcppdir)/summaries.o $(libcppdir)/summaries.cpp -$(libcppdir)/suppressions.o: lib/suppressions.cpp externals/tinyxml2/tinyxml2.h lib/color.h lib/config.h lib/errorlogger.h lib/errortypes.h lib/mathlib.h lib/path.h lib/suppressions.h lib/utils.h +$(libcppdir)/suppressions.o: lib/suppressions.cpp externals/tinyxml2/tinyxml2.h lib/color.h lib/config.h lib/errorlogger.h lib/errortypes.h lib/mathlib.h lib/path.h lib/suppressions.h lib/templatesimplifier.h lib/token.h lib/tokenize.h lib/tokenlist.h lib/utils.h lib/valueflow.h $(CXX) ${INCLUDE_FOR_LIB} $(CPPFLAGS) $(CPPFILESDIR) $(CXXFLAGS) $(UNDEF_STRICT_ANSI) -c -o $(libcppdir)/suppressions.o $(libcppdir)/suppressions.cpp $(libcppdir)/symboldatabase.o: lib/symboldatabase.cpp lib/astutils.h lib/color.h lib/config.h lib/errorlogger.h lib/errortypes.h lib/importproject.h lib/library.h lib/mathlib.h lib/platform.h lib/settings.h lib/standards.h lib/suppressions.h lib/symboldatabase.h lib/templatesimplifier.h lib/timer.h lib/token.h lib/tokenize.h lib/tokenlist.h lib/utils.h lib/valueflow.h diff --git a/lib/cppcheck.cpp b/lib/cppcheck.cpp index 80e002c66..2493f903a 100644 --- a/lib/cppcheck.cpp +++ b/lib/cppcheck.cpp @@ -861,6 +861,9 @@ unsigned int CppCheck::checkFile(const std::string& filename, const std::string fdump << "" << std::endl; } + // Need to call this even if the checksum will skip this configuration + mSettings.nomsg.markUnmatchedInlineSuppressionsAsChecked(tokenizer); + // Skip if we already met the same simplified token list if (mSettings.force || mSettings.maxConfigs > 1) { const unsigned long long checksum = tokenizer.list.calculateChecksum(); diff --git a/lib/suppressions.cpp b/lib/suppressions.cpp index 031c7be69..782711f9d 100644 --- a/lib/suppressions.cpp +++ b/lib/suppressions.cpp @@ -22,6 +22,7 @@ #include "mathlib.h" #include "path.h" #include "utils.h" +#include "tokenize.h" #include #include // std::isdigit, std::isalnum, etc @@ -340,6 +341,7 @@ bool Suppressions::Suppression::isMatch(const Suppressions::ErrorMessage &errmsg if (!isSuppressed(errmsg)) return false; matched = true; + checked = true; return true; } @@ -411,7 +413,7 @@ std::list Suppressions::getUnmatchedLocalSuppressions std::string tmpFile = Path::simplifyPath(file); std::list result; for (const Suppression &s : mSuppressions) { - if (s.matched) + if (s.matched || ((s.lineNumber != Suppression::NO_LINE) && !s.checked)) continue; if (s.hash > 0) continue; @@ -428,7 +430,7 @@ std::list Suppressions::getUnmatchedGlobalSuppression { std::list result; for (const Suppression &s : mSuppressions) { - if (s.matched) + if (s.matched || ((s.lineNumber != Suppression::NO_LINE) && !s.checked)) continue; if (s.hash > 0) continue; @@ -445,3 +447,19 @@ const std::list &Suppressions::getSuppressions() cons { return mSuppressions; } + +void Suppressions::markUnmatchedInlineSuppressionsAsChecked(const Tokenizer &tokenizer) { + int currLineNr = -1; + int currFileIdx = -1; + for (const Token *tok = tokenizer.tokens(); tok; tok = tok->next()) { + if (currFileIdx != tok->fileIndex() || currLineNr != tok->linenr()) { + currLineNr = tok->linenr(); + currFileIdx = tok->fileIndex(); + for (auto &suppression : mSuppressions) { + if (!suppression.checked && (suppression.lineNumber == currLineNr) && (suppression.fileName == tokenizer.list.file(tok))) { + suppression.checked = true; + } + } + } + } +} diff --git a/lib/suppressions.h b/lib/suppressions.h index 430837561..c381eb75b 100644 --- a/lib/suppressions.h +++ b/lib/suppressions.h @@ -32,6 +32,8 @@ /// @addtogroup Core /// @{ +class Tokenizer; + /** @brief class for handling suppressions */ class CPPCHECKLIB Suppressions { public: @@ -51,11 +53,11 @@ public: }; struct CPPCHECKLIB Suppression { - Suppression() : lineNumber(NO_LINE), hash(0), thisAndNextLine(false), matched(false) {} + Suppression() : lineNumber(NO_LINE), hash(0), thisAndNextLine(false), matched(false), checked(false) {} Suppression(const Suppression &other) { *this = other; } - Suppression(const std::string &id, const std::string &file, int line=NO_LINE) : errorId(id), fileName(file), lineNumber(line), hash(0), thisAndNextLine(false), matched(false) {} + Suppression(const std::string &id, const std::string &file, int line=NO_LINE) : errorId(id), fileName(file), lineNumber(line), hash(0), thisAndNextLine(false), matched(false), checked(false) {} Suppression & operator=(const Suppression &other) { errorId = other.errorId; @@ -65,6 +67,7 @@ public: hash = other.hash; thisAndNextLine = other.thisAndNextLine; matched = other.matched; + checked = other.checked; return *this; } @@ -118,6 +121,7 @@ public: std::size_t hash; bool thisAndNextLine; // Special case for backwards compatibility: { // cppcheck-suppress something bool matched; + bool checked; // for inline suppressions, checked or not enum { NO_LINE = -1 }; }; @@ -204,6 +208,12 @@ public: */ const std::list &getSuppressions() const; + /** + * @brief Marks Inline Suppressions as checked if source line is in the token stream + * @return No return. + */ + void markUnmatchedInlineSuppressionsAsChecked(const Tokenizer &tokenizer); + private: /** @brief List of error which the user doesn't want to see. */ std::list mSuppressions; diff --git a/test/cli/test-inline-suppress.py b/test/cli/test-inline-suppress.py index 1dfef87cc..ebd7a4558 100644 --- a/test/cli/test-inline-suppress.py +++ b/test/cli/test-inline-suppress.py @@ -59,6 +59,18 @@ def test_compile_commands_unused_function_suppression(): assert 'unusedFunction' not in stderr +def test_unmatched_suppression_ifdef(): + ret, stdout, stderr = cppcheck(['--enable=all', '--suppress=missingIncludeSystem', '--inline-suppr', '-DNO_ZERO_DIV', 'trac5704/trac5704a.c']) + assert ret == 0, stdout + assert 'unmatchedSuppression' not in stderr + + +def test_unmatched_suppression_ifdef_0(): + ret, stdout, stderr = cppcheck(['--enable=all', '--suppress=missingIncludeSystem', '--inline-suppr', 'trac5704/trac5704b.c']) + assert ret == 0, stdout + assert 'unmatchedSuppression' not in stderr + + def test_build_dir(): with tempfile.TemporaryDirectory() as tempdir: args = f'--cppcheck-build-dir={tempdir} --enable=all --inline-suppr proj-inline-suppress/4.c'.split() diff --git a/test/cli/trac5704/trac5704a.c b/test/cli/trac5704/trac5704a.c new file mode 100644 index 000000000..e25010413 --- /dev/null +++ b/test/cli/trac5704/trac5704a.c @@ -0,0 +1,11 @@ +#include + +int main(int argc, char *argv[]) +{ +#ifdef INCLUDE_ZERO_DIV + // cppcheck-suppress zerodiv + int i = 1/0; + printf("i = %d\n", i); +#endif + return 0; +} diff --git a/test/cli/trac5704/trac5704b.c b/test/cli/trac5704/trac5704b.c new file mode 100644 index 000000000..17b281016 --- /dev/null +++ b/test/cli/trac5704/trac5704b.c @@ -0,0 +1,11 @@ +#include + +int main(int argc, char *argv[]) +{ +#ifdef 0 + // cppcheck-suppress zerodiv + int i = 1/0; + printf("i = %d\n", i); +#endif + return 0; +} diff --git a/test/testsuppressions.cpp b/test/testsuppressions.cpp index b6e33d89b..98ae2e29d 100644 --- a/test/testsuppressions.cpp +++ b/test/testsuppressions.cpp @@ -669,7 +669,9 @@ private: void inlinesuppress_unusedFunction() const { // #4210, #4946 - wrong report of "unmatchedSuppression" for "unusedFunction" Suppressions suppressions; - suppressions.addSuppression(Suppressions::Suppression("unusedFunction", "test.c", 3)); + auto suppression = Suppressions::Suppression("unusedFunction", "test.c", 3); + suppression.checked = true; // have to do this because fixes for #5704 + suppressions.addSuppression(suppression); ASSERT_EQUALS(true, !suppressions.getUnmatchedLocalSuppressions("test.c", true).empty()); ASSERT_EQUALS(false, !suppressions.getUnmatchedGlobalSuppressions(true).empty()); ASSERT_EQUALS(false, !suppressions.getUnmatchedLocalSuppressions("test.c", false).empty());