Fix #12071 (Add safety mode that makes cppcheck more strict about cri…

…tical errors)

cli/cmdlineparser.cpp

@@ -1066,6 +1066,10 @@ CmdLineParser::Result CmdLineParser::parseFromArgs(int argc, const char* const a
 #endif
             }
 
+            // Make Cppcheck more strict about critical errors
+            else if (std::strcmp(argv[i], "--safety") == 0)
+                mSettings.safety = true;
+
             // show timing information..
             else if (std::strncmp(argv[i], "--showtime=", 11) == 0) {
                 const std::string showtimeMode = argv[i] + 11;

cli/cppcheckexecutor.cpp

@@ -118,6 +118,10 @@ class CppCheckExecutor::StdLogger : public ErrorLogger
      */
     void writeCheckersReport() const;
 
+    bool hasCriticalErrors() const {
+        return !mCriticalErrors.empty();
+    }
+
 private:
     /**
      * Information about progress is directed here. This should be
@@ -287,9 +291,12 @@ int CppCheckExecutor::check_internal(CppCheck& cppcheck) const
 
     mStdLogger->writeCheckersReport();
 
+    if (settings.safety && mStdLogger->hasCriticalErrors())
+        return EXIT_FAILURE;
+
     if (returnValue)
         return settings.exitCode;
-    return 0;
+    return EXIT_SUCCESS;
 }
 
 void CppCheckExecutor::StdLogger::writeCheckersReport() const
@@ -398,6 +405,10 @@ void CppCheckExecutor::StdLogger::reportErr(const ErrorMessage &msg)
         if (!mCriticalErrors.empty())
             mCriticalErrors += ", ";
         mCriticalErrors += msg.id;
+        if (msg.severity == Severity::none) {
+            mCriticalErrors += " (suppressed)";
+            return;
+        }
     }
 
     if (mSettings.xml)

gui/resultstree.cpp

@@ -666,6 +666,11 @@ void ResultsTree::contextMenuEvent(QContextMenuEvent * e)
             menu.addAction(hideallid);
 
             QAction *suppress = new QAction(tr("Suppress selected id(s)"), &menu);
+            {
+                QVariantMap data = mContextItem->data().toMap();
+                const QString messageId = data[ERRORID].toString();
+                suppress->setEnabled(!ErrorLogger::isCriticalErrorId(messageId.toStdString()));
+            }
             menu.addAction(suppress);
             connect(suppress, &QAction::triggered, this, &ResultsTree::suppressSelectedIds);
 

gui/resultsview.cpp

@@ -168,6 +168,9 @@ void ResultsView::error(const ErrorItem &item)
 
     handleCriticalError(item);
 
+    if (item.severity == Severity::none)
+        return;
+
     if (mUI->mTree->addErrorItem(item)) {
         emit gotResults();
         mStatistics->addItem(item.tool(), ShowTypes::SeverityToShowType(item.severity));
@@ -562,10 +565,14 @@ void ResultsView::handleCriticalError(const ErrorItem &item)
             if (!mCriticalErrors.isEmpty())
                 mCriticalErrors += ",";
             mCriticalErrors += item.errorId;
+            if (item.severity == Severity::none)
+                mCriticalErrors += " (suppressed)";
         }
         QString msg = tr("There was a critical error with id '%1'").arg(item.errorId);
         if (!item.file0.isEmpty())
             msg += ", " + tr("when checking %1").arg(item.file0);
+        else
+            msg += ", " + tr("when checking a file");
         msg += ". " + tr("Analysis was aborted.");
         mUI->mLabelCriticalErrors->setText(msg);
         mUI->mLabelCriticalErrors->setVisible(true);

lib/cppcheck.cpp

@@ -1599,6 +1599,22 @@ void CppCheck::reportErr(const ErrorMessage &msg)
     const auto errorMessage = Suppressions::ErrorMessage::fromErrorMessage(msg, macroNames);
 
     if (mSettings.nomsg.isSuppressed(errorMessage, mUseGlobalSuppressions)) {
+        // Safety: Report critical errors to ErrorLogger
+        if (mSettings.safety && ErrorLogger::isCriticalErrorId(msg.id)) {
+            mExitCode = 1;
+
+            if (mSettings.nomsg.isSuppressedExplicitly(errorMessage, mUseGlobalSuppressions)) {
+                // Report with none severity to signal that there is this critical error but
+                // it is suppressed
+                ErrorMessage temp;
+                temp.severity = Severity::none;
+                temp.id = msg.id;
+                mErrorLogger.reportErr(temp);
+            } else {
+                // Report critical error that is not explicitly suppressed
+                mErrorLogger.reportErr(msg);
+            }
+        }
         return;
     }
 

lib/settings.cpp

@@ -119,6 +119,15 @@ std::string Settings::loadCppcheckCfg()
             }
         }
     }
+    {
+        const picojson::object::const_iterator it = obj.find("safety");
+        if (it != obj.cend()) {
+            const auto& v = it->second;
+            if (!v.is<bool>())
+                return "'safety' is not a bool";
+            safety = v.get<bool>();
+        }
+    }
 
     return "";
 }

lib/settings.h

@@ -291,6 +291,13 @@ class CPPCHECKLIB WARN_UNUSED Settings {
     std::list<Rule> rules;
 #endif
 
+    /**
+     * @brief make cppcheck checking more strict about critical errors
+     * - returns nonzero if there is critical errors
+     * - a critical error id is not suppressed (by mistake?) with glob pattern
+     */
+    bool safety = false;
+
     /** Do not only check how interface is used. Also check that interface is safe. */
     struct CPPCHECKLIB SafeChecks {
 

lib/suppressions.cpp

@@ -418,6 +418,19 @@ bool Suppressions::isSuppressed(const Suppressions::ErrorMessage &errmsg, bool g
     return returnValue;
 }
 
+bool Suppressions::isSuppressedExplicitly(const Suppressions::ErrorMessage &errmsg, bool global)
+{
+    for (Suppression &s : mSuppressions) {
+        if (!global && !s.isLocal())
+            continue;
+        if (s.errorId != errmsg.errorId) // Error id must match exactly
+            continue;
+        if (s.isMatch(errmsg))
+            return true;
+    }
+    return false;
+}
+
 bool Suppressions::isSuppressed(const ::ErrorMessage &errmsg, const std::set<std::string>& macroNames)
 {
     if (mSuppressions.empty())

lib/suppressions.h

@@ -202,6 +202,14 @@ class CPPCHECKLIB Suppressions {
      */
     bool isSuppressed(const ErrorMessage &errmsg, bool global = true);
 
+    /**
+     * @brief Returns true if this message is "explicitly" suppressed. The suppression "id" must match textually exactly.
+     * @param errmsg error message
+     * @param global use global suppressions
+     * @return true if this error is explicitly suppressed.
+     */
+    bool isSuppressedExplicitly(const ErrorMessage &errmsg, bool global = true);
+
     /**
      * @brief Returns true if this message should not be shown to the user.
      * @param errmsg error message

releasenotes.txt

@@ -44,3 +44,6 @@ Other:
 - Markup files will now be processed after the regular source files when using multiple threads/processes (some issues remain - see Trac #12167 for details).
 - Added file name to ValueFlow "--debug" output.
 - Fixed build when using "clang-cl" in CMake.
+
+Safety critical fixes:
+- Changed handling when critical errors are suppressed. They can only be suppressed explicitly and the exit code will never be successful after a critical error.

test/cli/test-suppress-syntaxError.py

@@ -13,3 +13,15 @@ def test_j2_suppress():
     assert ret == 0
     assert len(stderr) == 0
 
+def test_safety_suppress_syntax_error_implicitly(tmpdir):
+    ret, stdout, stderr = cppcheck(['--safety', '--suppress=*', 'proj-suppress-syntaxError'], remove_active_checkers=False)
+    assert ret == 1
+    assert '[syntaxError]' in stderr
+    assert 'Active checkers: There was critical errors' in stdout
+
+def test_safety_suppress_syntax_error_explicitly():
+    ret, stdout, stderr = cppcheck(['--safety', '--suppress=syntaxError', 'proj-suppress-syntaxError'], remove_active_checkers=False)
+    assert ret == 1
+    assert '[syntaxError]' not in stderr
+    assert 'Active checkers: There was critical errors' in stdout
+

test/cli/testutils.py

@@ -71,7 +71,7 @@ def __lookup_cppcheck_exe():
 
 
 # Run Cppcheck with args
-def cppcheck(args, env=None):
+def cppcheck(args, env=None, remove_active_checkers=True):
     exe = __lookup_cppcheck_exe()
     assert exe is not None, 'no cppcheck binary found'
 
@@ -80,7 +80,7 @@ def cppcheck(args, env=None):
     comm = p.communicate()
     stdout = comm[0].decode(encoding='utf-8', errors='ignore').replace('\r\n', '\n')
     stderr = comm[1].decode(encoding='utf-8', errors='ignore').replace('\r\n', '\n')
-    if stdout.find('\nActive checkers:') > 0:
+    if remove_active_checkers and stdout.find('\nActive checkers:') > 0:
         stdout = stdout[:1 + stdout.find('\nActive checkers:')]
     return p.returncode, stdout, stderr
 

tools/run_more_tests.sh

@@ -18,6 +18,9 @@ python3 $DIR/extracttests.py --code=$(pwd)/test1 $1
 
 cd test1
 
+# ensure there are not critical errors in extracted tests
+$CPPCHECK -q --safe --suppress="*" .
+
 $CPPCHECK -q --template=cppcheck1 . 2> 1.txt
 
 echo "(!x) => (x==0)"