refs #12171 / fixed #12347 - improved handling of (unmatched) local and inline suppressions

Makefile

@@ -682,7 +682,7 @@ cli/singleexecutor.o: cli/singleexecutor.cpp cli/executor.h cli/singleexecutor.h
 cli/stacktrace.o: cli/stacktrace.cpp cli/stacktrace.h lib/config.h lib/utils.h
 	$(CXX) ${INCLUDE_FOR_CLI} $(CPPFLAGS) $(CXXFLAGS) -c -o $@ cli/stacktrace.cpp
 
-cli/threadexecutor.o: cli/threadexecutor.cpp cli/executor.h cli/threadexecutor.h lib/addoninfo.h lib/check.h lib/checkers.h lib/color.h lib/config.h lib/cppcheck.h lib/errorlogger.h lib/errortypes.h lib/filesettings.h lib/library.h lib/mathlib.h lib/path.h lib/platform.h lib/settings.h lib/standards.h lib/timer.h lib/utils.h
+cli/threadexecutor.o: cli/threadexecutor.cpp cli/executor.h cli/threadexecutor.h lib/addoninfo.h lib/check.h lib/checkers.h lib/color.h lib/config.h lib/cppcheck.h lib/errorlogger.h lib/errortypes.h lib/filesettings.h lib/library.h lib/mathlib.h lib/path.h lib/platform.h lib/settings.h lib/standards.h lib/suppressions.h lib/timer.h lib/utils.h
 	$(CXX) ${INCLUDE_FOR_CLI} $(CPPFLAGS) $(CXXFLAGS) -c -o $@ cli/threadexecutor.cpp
 
 test/fixture.o: test/fixture.cpp externals/simplecpp/simplecpp.h externals/tinyxml2/tinyxml2.h lib/addoninfo.h lib/check.h lib/checkers.h lib/color.h lib/config.h lib/cppcheck.h lib/errorlogger.h lib/errortypes.h lib/library.h lib/mathlib.h lib/path.h lib/platform.h lib/preprocessor.h lib/settings.h lib/standards.h lib/tokenize.h lib/tokenlist.h lib/utils.h lib/xml.h test/fixture.h test/helpers.h test/options.h test/redirect.h

cli/cppcheckexecutor.cpp

@@ -412,6 +412,12 @@ bool CppCheckExecutor::reportSuppressions(const Settings &settings, const Suppre
                 suppressions.getUnmatchedLocalSuppressions(i->file, unusedFunctionCheckEnabled), errorLogger);
         }
     }
+    if (settings.inlineSuppressions) {
+        // report unmatched unusedFunction suppressions
+        err |= SuppressionList::reportUnmatchedSuppressions(
+            suppressions.getUnmatchedInlineSuppressions(), errorLogger);
+    }
+
     err |= SuppressionList::reportUnmatchedSuppressions(suppressions.getUnmatchedGlobalSuppressions(unusedFunctionCheckEnabled), errorLogger);
     return err;
 }

cli/processexecutor.cpp

@@ -78,7 +78,7 @@ ProcessExecutor::ProcessExecutor(const std::list<FileWithDetails> &files, const
 namespace {
     class PipeWriter : public ErrorLogger {
     public:
-        enum PipeSignal : std::uint8_t {REPORT_OUT='1',REPORT_ERROR='2', CHILD_END='5'};
+        enum PipeSignal : std::uint8_t {REPORT_OUT='1',REPORT_ERROR='2',REPORT_SUPPR_INLINE='3',CHILD_END='5'};
 
         explicit PipeWriter(int pipe) : mWpipe(pipe) {}
 
@@ -90,11 +90,32 @@ namespace {
             writeToPipe(REPORT_ERROR, msg.serialize());
         }
 
+        void writeSuppr(const SuppressionList &supprs) const {
+            for (const auto& suppr : supprs.getSuppressions())
+            {
+                if (!suppr.isInline)
+                    continue;
+
+                writeToPipe(REPORT_SUPPR_INLINE, suppressionToString(suppr));
+            }
+            // TODO: update suppression states?
+        }
+
         void writeEnd(const std::string& str) const {
             writeToPipe(CHILD_END, str);
         }
 
     private:
+        static std::string suppressionToString(const SuppressionList::Suppression &suppr)
+        {
+            std::string suppr_str = suppr.toString();
+            suppr_str += ";";
+            suppr_str += suppr.checked ? "1" : "0";
+            suppr_str += ";";
+            suppr_str += suppr.matched ? "1" : "0";
+            return suppr_str;
+        }
+
         // TODO: how to log file name in error?
         void writeToPipeInternal(PipeSignal type, const void* data, std::size_t to_write) const
         {
@@ -124,7 +145,7 @@ namespace {
                 writeToPipeInternal(type, &len, l_size);
             }
 
-            if (len > 0)
+            if (len > 0) // TODO: unexpected - write a warning?
                 writeToPipeInternal(type, data.c_str(), len);
         }
 
@@ -155,7 +176,10 @@ bool ProcessExecutor::handleRead(int rpipe, unsigned int &result, const std::str
         std::exit(EXIT_FAILURE);
     }
 
-    if (type != PipeWriter::REPORT_OUT && type != PipeWriter::REPORT_ERROR && type != PipeWriter::CHILD_END) {
+    if (type != PipeWriter::REPORT_OUT &&
+        type != PipeWriter::REPORT_ERROR &&
+        type != PipeWriter::REPORT_SUPPR_INLINE &&
+        type != PipeWriter::CHILD_END) {
         std::cerr << "#### ThreadExecutor::handleRead(" << filename << ") invalid type " << int(type) << std::endl;
         std::exit(EXIT_FAILURE);
     }
@@ -174,7 +198,7 @@ bool ProcessExecutor::handleRead(int rpipe, unsigned int &result, const std::str
     }
 
     std::string buf(len, '\0');
-    if (len > 0) {
+    if (len > 0) { // TODO: unexpected - write a warning?
         char *data_start = &buf[0];
         bytes_to_read = len;
         do {
@@ -206,6 +230,29 @@ bool ProcessExecutor::handleRead(int rpipe, unsigned int &result, const std::str
 
         if (hasToLog(msg))
             mErrorLogger.reportErr(msg);
+    } else if (type == PipeWriter::REPORT_SUPPR_INLINE) {
+        if (!buf.empty()) {
+            // TODO: avoid string splitting
+            auto parts = splitString(buf, ';');
+            if (parts.size() != 3)
+            {
+                // TODO: make this non-fatal
+                std::cerr << "#### ThreadExecutor::handleRead(" << filename << ") adding of inline suppression failed - insufficient data" << std::endl;
+                std::exit(EXIT_FAILURE);
+            }
+            auto suppr = SuppressionList::parseLine(parts[0]);
+            suppr.isInline = true;
+            suppr.checked = parts[1] == "1";
+            suppr.matched = parts[2] == "1";
+            const std::string err = mSuppressions.nomsg.addSuppression(suppr);
+            if (!err.empty()) {
+                // TODO: only update state if it doesn't exist - otherwise propagate error
+                mSuppressions.nomsg.updateSuppressionState(suppr); // TODO: check result
+                // TODO: make this non-fatal
+                //std::cerr << "#### ThreadExecutor::handleRead(" << filename << ") adding of inline suppression failed - " << err << std::endl;
+                //std::exit(EXIT_FAILURE);
+            }
+        }
     } else if (type == PipeWriter::CHILD_END) {
         result += std::stoi(buf);
         res = false;
@@ -297,6 +344,8 @@ unsigned int ProcessExecutor::check()
                     // TODO: call analyseClangTidy()?
                 }
 
+                pipewriter.writeSuppr(mSuppressions.nomsg);
+
                 pipewriter.writeEnd(std::to_string(resultOfCheck));
                 std::exit(EXIT_SUCCESS);
             }

cli/threadexecutor.cpp

@@ -23,6 +23,7 @@
 #include "errorlogger.h"
 #include "filesettings.h"
 #include "settings.h"
+#include "suppressions.h"
 #include "timer.h"
 
 #include <cassert>
@@ -125,6 +126,23 @@ class ThreadData
             result = fileChecker.check(*file);
             // TODO: call analyseClangTidy()?
         }
+        for (const auto& suppr : mSuppressions.nomsg.getSuppressions()) {
+            // need to transfer all inline suppressions because these are used later on
+            if (suppr.isInline) {
+                const std::string err = mSuppressions.nomsg.addSuppression(suppr);
+                if (!err.empty()) {
+                    // TODO: only update state if it doesn't exist - otherwise propagate error
+                    mSuppressions.nomsg.updateSuppressionState(suppr); // TODO: check result
+                }
+                continue;
+            }
+
+            // propagate state of global suppressions
+            if (!suppr.isLocal()) {
+                mSuppressions.nomsg.updateSuppressionState(suppr); // TODO: check result
+                continue;
+            }
+        }
         return result;
     }
 

lib/cppcheck.cpp

@@ -1168,8 +1168,10 @@ unsigned int CppCheck::checkFile(const FileWithDetails& file, const std::string
                     fdump << "</dump>" << std::endl;
                 }
 
-                // Need to call this even if the hash will skip this configuration
-                mSuppressions.nomsg.markUnmatchedInlineSuppressionsAsChecked(tokenizer);
+                if (mSettings.inlineSuppressions) {
+                    // Need to call this even if the hash will skip this configuration
+                    mSuppressions.nomsg.markUnmatchedInlineSuppressionsAsChecked(tokenizer);
+                }
 
                 // Skip if we already met the same simplified token list
                 if (mSettings.force || mSettings.maxConfigs > 1) {
@@ -1258,10 +1260,14 @@ unsigned int CppCheck::checkFile(const FileWithDetails& file, const std::string
         mErrorLogger.reportErr(errmsg);
     }
 
-    // In jointSuppressionReport mode, unmatched suppressions are
-    // collected after all files are processed
-    if (!mSettings.useSingleJob() && (mSettings.severity.isEnabled(Severity::information) || mSettings.checkConfiguration)) {
-        SuppressionList::reportUnmatchedSuppressions(mSuppressions.nomsg.getUnmatchedLocalSuppressions(file, (bool)mUnusedFunctionsCheck), mErrorLogger);
+    // TODO: this is done too early causing the whole program analysis suppressions to be reported as unmatched
+    if (mSettings.severity.isEnabled(Severity::information) || mSettings.checkConfiguration) {
+        // In jointSuppressionReport mode, unmatched suppressions are
+        // collected after all files are processed
+        if (!mSettings.useSingleJob()) {
+            // TODO: check result?
+            SuppressionList::reportUnmatchedSuppressions(mSuppressions.nomsg.getUnmatchedLocalSuppressions(file, (bool)mUnusedFunctionsCheck), mErrorLogger);
+        }
     }
 
     if (analyzerInformation) {

lib/preprocessor.cpp

@@ -135,6 +135,7 @@ static bool parseInlineSuppressionCommentToken(const simplecpp::Token *tok, std:
         std::vector<SuppressionList::Suppression> suppressions = SuppressionList::parseMultiSuppressComment(comment, &errmsg);
 
         for (SuppressionList::Suppression &s : suppressions) {
+            s.isInline = true;
             s.type = errorType;
             s.lineNumber = tok->location.line;
         }
@@ -152,6 +153,7 @@ static bool parseInlineSuppressionCommentToken(const simplecpp::Token *tok, std:
         if (!s.parseComment(comment, &errmsg))
             return false;
 
+        s.isInline = true;
         s.type = errorType;
         s.lineNumber = tok->location.line;
 

lib/suppressions.cpp

@@ -204,7 +204,7 @@ std::vector<SuppressionList::Suppression> SuppressionList::parseMultiSuppressCom
     return suppressions;
 }
 
-std::string SuppressionList::addSuppressionLine(const std::string &line)
+SuppressionList::Suppression SuppressionList::parseLine(const std::string &line)
 {
     std::istringstream lineStream;
     SuppressionList::Suppression suppression;
@@ -249,7 +249,12 @@ std::string SuppressionList::addSuppressionLine(const std::string &line)
 
     suppression.fileName = Path::simplifyPath(suppression.fileName);
 
-    return addSuppression(std::move(suppression));
+    return suppression;
+}
+
+std::string SuppressionList::addSuppressionLine(const std::string &line)
+{
+    return addSuppression(parseLine(line));
 }
 
 std::string SuppressionList::addSuppression(SuppressionList::Suppression suppression)
@@ -511,12 +516,14 @@ void SuppressionList::dump(std::ostream & out) const
     out << "  </suppressions>" << std::endl;
 }
 
-std::list<SuppressionList::Suppression> SuppressionList::getUnmatchedLocalSuppressions(const FileWithDetails &file, const bool unusedFunctionChecking) const
+std::list<SuppressionList::Suppression> SuppressionList::getUnmatchedLocalSuppressions(const FileWithDetails &file, const bool includeUnusedFunction) const
 {
     std::lock_guard<std::mutex> lg(mSuppressionsSync);
 
     std::list<Suppression> result;
     for (const Suppression &s : mSuppressions) {
+        if (s.isInline)
+            continue;
         if (s.matched || ((s.lineNumber != Suppression::NO_LINE) && !s.checked))
             continue;
         if (s.type == SuppressionList::Type::macro)
@@ -525,7 +532,7 @@ std::list<SuppressionList::Suppression> SuppressionList::getUnmatchedLocalSuppre
             continue;
         if (s.errorId == ID_CHECKERSREPORT)
             continue;
-        if (!unusedFunctionChecking && s.errorId == ID_UNUSEDFUNCTION)
+        if (!includeUnusedFunction && s.errorId == ID_UNUSEDFUNCTION)
             continue;
         if (!s.isLocal() || s.fileName != file.spath())
             continue;
@@ -534,17 +541,19 @@ std::list<SuppressionList::Suppression> SuppressionList::getUnmatchedLocalSuppre
     return result;
 }
 
-std::list<SuppressionList::Suppression> SuppressionList::getUnmatchedGlobalSuppressions(const bool unusedFunctionChecking) const
+std::list<SuppressionList::Suppression> SuppressionList::getUnmatchedGlobalSuppressions(const bool includeUnusedFunction) const
 {
     std::lock_guard<std::mutex> lg(mSuppressionsSync);
 
     std::list<Suppression> result;
     for (const Suppression &s : mSuppressions) {
+        if (s.isInline)
+            continue;
         if (s.matched || ((s.lineNumber != Suppression::NO_LINE) && !s.checked))
             continue;
         if (s.hash > 0)
             continue;
-        if (!unusedFunctionChecking && s.errorId == ID_UNUSEDFUNCTION)
+        if (!includeUnusedFunction && s.errorId == ID_UNUSEDFUNCTION)
             continue;
         if (s.errorId == ID_CHECKERSREPORT)
             continue;
@@ -555,6 +564,23 @@ std::list<SuppressionList::Suppression> SuppressionList::getUnmatchedGlobalSuppr
     return result;
 }
 
+std::list<SuppressionList::Suppression> SuppressionList::getUnmatchedInlineSuppressions() const
+{
+    std::list<SuppressionList::Suppression> result;
+    for (const SuppressionList::Suppression &s : SuppressionList::mSuppressions) {
+        if (!s.isInline)
+            continue;
+        if (!s.checked)
+            continue;
+        if (s.matched)
+            continue;
+        if (s.hash > 0)
+            continue;
+        result.push_back(s);
+    }
+    return result;
+}
+
 std::list<SuppressionList::Suppression> SuppressionList::getSuppressions() const
 {
     std::lock_guard<std::mutex> lg(mSuppressionsSync);

lib/suppressions.h

@@ -151,6 +151,7 @@ class CPPCHECKLIB SuppressionList {
         bool thisAndNextLine{}; // Special case for backwards compatibility: { // cppcheck-suppress something
         bool matched{};
         bool checked{}; // for inline suppressions, checked or not
+        bool isInline{};
 
         enum : std::int8_t { NO_LINE = -1 };
     };
@@ -177,6 +178,13 @@ class CPPCHECKLIB SuppressionList {
      */
     static std::vector<Suppression> parseMultiSuppressComment(const std::string &comment, std::string *errorMessage);
 
+    /**
+     * Create a Suppression object from a suppression line
+     * @param line The line to parse.
+     * @return a suppression object
+     */
+    static Suppression parseLine(const std::string &line);
+
     /**
      * @brief Don't show the given error.
      * @param line Description of error to suppress (in id:file:line format).
@@ -239,13 +247,19 @@ class CPPCHECKLIB SuppressionList {
      * @brief Returns list of unmatched local (per-file) suppressions.
      * @return list of unmatched suppressions
      */
-    std::list<Suppression> getUnmatchedLocalSuppressions(const FileWithDetails &file, bool unusedFunctionChecking) const;
+    std::list<Suppression> getUnmatchedLocalSuppressions(const FileWithDetails &file, bool includeUnusedFunction) const;
 
     /**
      * @brief Returns list of unmatched global (glob pattern) suppressions.
      * @return list of unmatched suppressions
      */
-    std::list<Suppression> getUnmatchedGlobalSuppressions(bool unusedFunctionChecking) const;
+    std::list<Suppression> getUnmatchedGlobalSuppressions(bool includeUnusedFunction) const;
+
+    /**
+     * @brief Returns list of unmatched inline suppressions.
+     * @return list of unmatched suppressions
+     */
+    std::list<Suppression> getUnmatchedInlineSuppressions() const;
 
     /**
      * @brief Returns list of all suppressions.