Fix unmatched suppression (#5704) (#3886)

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

lib/cppcheck.cpp

@@ -861,6 +861,9 @@ unsigned int CppCheck::checkFile(const std::string& filename, const std::string
                     fdump << "</dump>" << 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();

lib/suppressions.cpp

@@ -22,6 +22,7 @@
 #include "mathlib.h"
 #include "path.h"
 #include "utils.h"
+#include "tokenize.h"
 
 #include <algorithm>
 #include <cctype>   // 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::Suppression> Suppressions::getUnmatchedLocalSuppressions
     std::string tmpFile = Path::simplifyPath(file);
     std::list<Suppression> 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::Suppression> Suppressions::getUnmatchedGlobalSuppression
 {
     std::list<Suppression> 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::Suppression> &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;
+                }
+            }
+        }
+    }
+}

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<Suppression> &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<Suppression> mSuppressions;

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()

test/cli/trac5704/trac5704a.c

@@ -0,0 +1,11 @@
+#include <stdio.h>
+
+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;
+}

test/cli/trac5704/trac5704b.c

@@ -0,0 +1,11 @@
+#include <stdio.h>
+
+int main(int argc, char *argv[])
+{
+#ifdef 0
+    // cppcheck-suppress zerodiv
+    int i = 1/0;
+    printf("i = %d\n", i);
+#endif
+    return 0;
+}

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());