Revert "Fixed #12071 (suppressing critical error, no indication to user that analysis of file fails) (#5771)" (#5775)

danmar · web-flow · commit 2932ab759219 · 2023-12-17T19:13:14.000+01:00
This reverts commit 7c316fb.
diff --git a/.github/workflows/CI-unixish.yml b/.github/workflows/CI-unixish.yml
@@ -423,8 +423,7 @@ jobs:
         run: |
           ./cppcheck --error-exitcode=1 --inline-suppr --addon=threadsafety addons/test/threadsafety
           ./cppcheck --error-exitcode=1 --inline-suppr --addon=threadsafety --std=c++03 addons/test/threadsafety
-          ./cppcheck --error-exitcode=1 --dump addons/test/misra/crash*.c
-          python3 addons/misra.py --cli addons/test/misra/crash*.c.dump > /dev/null
+          ./cppcheck --error-exitcode=1 --inline-suppr --addon=misra addons/test/misra/crash*.c
           ./cppcheck --error-exitcode=1 --inline-suppr --addon=misra --enable=information addons/test/misra/config*.c
 
           ./cppcheck --addon=misra --enable=style --inline-suppr --enable=information --error-exitcode=1 addons/test/misra/misra-ctu-*-test.c
diff --git a/CMakeLists.txt b/CMakeLists.txt
@@ -47,7 +47,7 @@ if(LIBXML2_XMLLINT_EXECUTABLE)
     add_custom_target(errorlist-xml $<TARGET_FILE:cppcheck> --errorlist > ${CMAKE_BINARY_DIR}/errorlist.xml
             DEPENDS cppcheck)
 
-    add_custom_target(example-xml $<TARGET_FILE:cppcheck> --xml --enable=all --inconclusive --max-configs=1 ${CMAKE_SOURCE_DIR}/samples -i${CMAKE_SOURCE_DIR}/samples/syntaxError/bad.c 2> ${CMAKE_BINARY_DIR}/example.xml
+    add_custom_target(example-xml $<TARGET_FILE:cppcheck> --xml --enable=all --inconclusive --max-configs=1 ${CMAKE_SOURCE_DIR}/samples 2> ${CMAKE_BINARY_DIR}/example.xml
             DEPENDS cppcheck)
 
     add_custom_target(createXMLExamples DEPENDS errorlist-xml example-xml)
diff --git a/Makefile b/Makefile
@@ -439,7 +439,7 @@ validatePlatforms: ${PlatformFilesCHECKED}
 /tmp/errorlist.xml: cppcheck
 	./cppcheck --errorlist >$@
 /tmp/example.xml: cppcheck
-	./cppcheck --xml --enable=all --inconclusive --max-configs=1 samples -isamples/syntaxError/bad.c 2>/tmp/example.xml
+	./cppcheck --xml --enable=all --inconclusive --max-configs=1 samples 2>/tmp/example.xml
 createXMLExamples:/tmp/errorlist.xml /tmp/example.xml
 .PHONY: validateXML
 validateXML: createXMLExamples
diff --git a/cli/cmdlineparser.cpp b/cli/cmdlineparser.cpp
@@ -587,10 +587,6 @@ CmdLineParser::Result CmdLineParser::parseFromArgs(int argc, const char* const a
                 if (!parseNumberArg(argv[i], 17, mSettings.exitCode))
                     return Result::Fail;
             }
-            // --unsafe-exitcode => for tests, critical errors will not force non-zero exitcode
-            else if (std::strcmp(argv[i], "--unsafe-exitcode") == 0) {
-                mSettings.unsafeExitCode = true;
-            }
 
             // Exception handling inside cppcheck client
             else if (std::strcmp(argv[i], "--exception-handling") == 0) {
diff --git a/cli/cppcheckexecutor.cpp b/cli/cppcheckexecutor.cpp
@@ -118,10 +118,6 @@ class CppCheckExecutor::StdLogger : public ErrorLogger
      */
     void writeCheckersReport() const;
 
-    bool hasCriticalErrors() const {
-        return !mCriticalErrors.empty();
-    }
-
 private:
     /**
      * Information about progress is directed here. This should be
@@ -291,12 +287,9 @@ int CppCheckExecutor::check_internal(CppCheck& cppcheck) const
 
     mStdLogger->writeCheckersReport();
 
-    if (!settings.unsafeExitCode && mStdLogger->hasCriticalErrors())
-        return settings.exitCode > 0 ? settings.exitCode : EXIT_FAILURE;
-
     if (returnValue)
         return settings.exitCode;
-    return EXIT_SUCCESS;
+    return 0;
 }
 
 void CppCheckExecutor::StdLogger::writeCheckersReport() const
@@ -405,10 +398,6 @@ 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)
diff --git a/gui/resultstree.cpp b/gui/resultstree.cpp
@@ -666,11 +666,6 @@ 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);
 
diff --git a/gui/resultsview.cpp b/gui/resultsview.cpp
@@ -168,9 +168,6 @@ 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));
@@ -565,14 +562,10 @@ 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);
diff --git a/htmlreport/check.sh b/htmlreport/check.sh
@@ -40,7 +40,7 @@ validate_html "$INDEX_HTML"
 validate_html "$STATS_HTML"
 
 
-../cppcheck ../samples -i../samples/syntaxError/bad.c --enable=all --inconclusive --xml-version=2 2> "$GUI_TEST_XML"
+../cppcheck ../samples --enable=all --inconclusive --xml-version=2 2> "$GUI_TEST_XML"
 xmllint --noout "$GUI_TEST_XML"
 $PYTHON cppcheck-htmlreport --file "$GUI_TEST_XML" --title "xml2 + inconclusive test" --report-dir "$REPORT_DIR"
 echo ""
@@ -49,7 +49,7 @@ validate_html "$INDEX_HTML"
 validate_html "$STATS_HTML"
 
 
-../cppcheck ../samples -i../samples/syntaxError/bad.c --enable=all --inconclusive --verbose --xml-version=2 2> "$GUI_TEST_XML"
+../cppcheck ../samples --enable=all --inconclusive --verbose --xml-version=2 2> "$GUI_TEST_XML"
 xmllint --noout "$GUI_TEST_XML"
 $PYTHON cppcheck-htmlreport --file "$GUI_TEST_XML" --title "xml2 + inconclusive + verbose test" --report-dir "$REPORT_DIR"
 echo -e "\n"
diff --git a/lib/cppcheck.cpp b/lib/cppcheck.cpp
@@ -1599,19 +1599,6 @@ void CppCheck::reportErr(const ErrorMessage &msg)
     const auto errorMessage = Suppressions::ErrorMessage::fromErrorMessage(msg, macroNames);
 
     if (mSettings.nomsg.isSuppressed(errorMessage, mUseGlobalSuppressions)) {
-        if (ErrorLogger::isCriticalErrorId(msg.id)) {
-            mExitCode = 1;
-
-            if (mSettings.nomsg.isSuppressedExplicitly(errorMessage, mUseGlobalSuppressions)) {
-                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;
     }
 
diff --git a/lib/settings.h b/lib/settings.h
@@ -358,9 +358,6 @@ class CPPCHECKLIB WARN_UNUSED Settings {
     /** @brief The maximum time in seconds for the typedef simplification */
     std::size_t typedefMaxTime{};
 
-    /** @brief Unsafe exitcode => do not force non-zero exitcode when there are critical errors */
-    bool unsafeExitCode = false;
-
     /** @brief defines given by the user */
     std::string userDefines;
 
diff --git a/lib/suppressions.cpp b/lib/suppressions.cpp
@@ -418,19 +418,6 @@ 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())
diff --git a/lib/suppressions.h b/lib/suppressions.h
@@ -202,14 +202,6 @@ 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
diff --git a/releasenotes.txt b/releasenotes.txt
@@ -44,6 +44,3 @@ 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.
diff --git a/test/cli/test-other.py b/test/cli/test-other.py
@@ -411,7 +411,7 @@ def test_invalid_addon_py(tmpdir):
     args = ['--addon={}'.format(addon_file), '--enable=all', test_file]
 
     exitcode, stdout, stderr = cppcheck(args)
-    assert exitcode == 1
+    assert exitcode == 0  # TODO: needs to be 1
     lines = stdout.splitlines()
     assert lines == [
         'Checking {} ...'.format(test_file)
@@ -435,7 +435,7 @@ def test_invalid_addon_py_verbose(tmpdir):
     args = ['--addon={}'.format(addon_file), '--enable=all', '--verbose', test_file]
 
     exitcode, stdout, stderr = cppcheck(args)
-    assert exitcode == 1
+    assert exitcode == 0  # TODO: needs to be 1
     lines = stdout.splitlines()
     assert lines == [
         'Checking {} ...'.format(test_file),
diff --git a/test/cli/test-suppress-syntaxError.py b/test/cli/test-suppress-syntaxError.py
@@ -13,15 +13,3 @@ def test_j2_suppress():
     assert ret == 0
     assert len(stderr) == 0
 
-def test_suppress_syntax_error_implicitly():
-    ret, stdout, stderr = cppcheck(['--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_suppress_syntax_error_explicitly():
-    ret, stdout, stderr = cppcheck(['--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
-
diff --git a/test/cli/testutils.py b/test/cli/testutils.py
@@ -71,7 +71,7 @@ def __lookup_cppcheck_exe():
 
 
 # Run Cppcheck with args
-def cppcheck(args, env=None, remove_active_checkers=True):
+def cppcheck(args, env=None):
     exe = __lookup_cppcheck_exe()
     assert exe is not None, 'no cppcheck binary found'
 
@@ -80,7 +80,7 @@ def cppcheck(args, env=None, remove_active_checkers=True):
     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 remove_active_checkers and stdout.find('\nActive checkers:') > 0:
+    if stdout.find('\nActive checkers:') > 0:
         stdout = stdout[:1 + stdout.find('\nActive checkers:')]
     return p.returncode, stdout, stderr
 
diff --git a/test/testsuppressions.cpp b/test/testsuppressions.cpp
@@ -1144,7 +1144,7 @@ class TestSuppressions : public TestFixture {
     void suppressingSyntaxErrors() { // syntaxErrors should be suppressible (#7076)
         const char code[] = "if if\n";
 
-        ASSERT_EQUALS(1, checkSuppression(code, "syntaxError:test.cpp:1"));
+        ASSERT_EQUALS(0, checkSuppression(code, "syntaxError:test.cpp:1"));
         ASSERT_EQUALS("", errout.str());
     }
 
@@ -1159,7 +1159,7 @@ class TestSuppressions : public TestFixture {
                             "   fstp  QWORD PTR result  ; store a double (8 bytes)\n"
                             "   pop   EAX               ; restore EAX\n"
                             "}";
-        ASSERT_EQUALS(1, checkSuppression(code, ""));
+        ASSERT_EQUALS(0, checkSuppression(code, ""));
         ASSERT_EQUALS("", errout.str());
     }
 
@@ -1177,7 +1177,7 @@ class TestSuppressions : public TestFixture {
                             "[!VAR \"BC\" = \"$BC + 1\"!][!//\n"
                             "[!ENDIF!][!//\n"
                             "};";
-        ASSERT_EQUALS(1, checkSuppression(code, "syntaxError:test.cpp:4"));
+        ASSERT_EQUALS(0, checkSuppression(code, "syntaxError:test.cpp:4"));
         ASSERT_EQUALS("", errout.str());
     }
 
@@ -1211,17 +1211,15 @@ class TestSuppressions : public TestFixture {
     void suppressingSyntaxErrorAndExitCode() {
         const char code[] = "fi if;";
 
-        ASSERT_EQUALS(2, checkSuppression(code, "*:test.cpp"));
-        ASSERT_EQUALS("[test.cpp:1]: (error) syntax error\n", errout.str());  // <- critical errors can't be suppressed with globbed id
+        ASSERT_EQUALS(0, checkSuppression(code, "*:test.cpp"));
+        ASSERT_EQUALS("", errout.str());
 
         // multi files, but only suppression one
         std::map<std::string, std::string> mfiles;
         mfiles["test.cpp"] = "fi if;";
         mfiles["test2.cpp"] = "fi if";
-        ASSERT_EQUALS(3, checkSuppression(mfiles, "*:test.cpp"));
-        ASSERT_EQUALS("[test.cpp:1]: (error) syntax error\n" // <- critical errors can't be suppressed with globbed id
-                      "[test2.cpp:1]: (error) syntax error\n",
-                      errout.str());
+        ASSERT_EQUALS(2, checkSuppression(mfiles, "*:test.cpp"));
+        ASSERT_EQUALS("[test2.cpp:1]: (error) syntax error\n", errout.str());
 
         // multi error in file, but only suppression one error
         const char code2[] = "fi fi\n"
diff --git a/tools/dmake.cpp b/tools/dmake.cpp
@@ -740,7 +740,7 @@ int main(int argc, char **argv)
     fout << "/tmp/errorlist.xml: cppcheck\n";
     fout << "\t./cppcheck --errorlist >$@\n";
     fout << "/tmp/example.xml: cppcheck\n";
-    fout << "\t./cppcheck --xml --enable=all --inconclusive --max-configs=1 samples -isamples/syntaxError/bad.c 2>/tmp/example.xml\n";
+    fout << "\t./cppcheck --xml --enable=all --inconclusive --max-configs=1 samples 2>/tmp/example.xml\n";
     fout << "createXMLExamples:/tmp/errorlist.xml /tmp/example.xml\n";
     fout << ".PHONY: validateXML\n";
     fout << "validateXML: createXMLExamples\n";
diff --git a/tools/donate_cpu_lib.py b/tools/donate_cpu_lib.py
@@ -16,7 +16,7 @@
 # Version scheme (MAJOR.MINOR.PATCH) should orientate on "Semantic Versioning" https://semver.org/
 # Every change in this script should result in increasing the version number accordingly (exceptions may be cosmetic
 # changes)
-CLIENT_VERSION = "1.4.0"
+CLIENT_VERSION = "1.3.53"
 
 # Timeout for analysis with Cppcheck in seconds
 CPPCHECK_TIMEOUT = 30 * 60
@@ -443,7 +443,7 @@ def scan_package(cppcheck_path, source_path, libraries, capture_callstack=True):
     # TODO: temporarily disabled timing information - use --showtime=top5_summary when next version is released
     # TODO: remove missingInclude disabling when it no longer is implied by --enable=information
     # Reference for GNU C: https://gcc.gnu.org/onlinedocs/cpp/Common-Predefined-Macros.html
-    options = libs + ' --unsafe-exitcode --check-library --inconclusive --enable=style,information --inline-suppr --disable=missingInclude --suppress=unmatchedSuppression --template=daca2'
+    options = libs + ' --check-library --inconclusive --enable=style,information --inline-suppr --disable=missingInclude --suppress=unmatchedSuppression --template=daca2'
     options += ' --debug-warnings --suppress=autoNoType --suppress=valueFlowBailout --suppress=bailoutUninitVar --suppress=symbolDatabaseWarning'
     options += ' -D__GNUC__ --platform=unix64'
     options_rp = options + ' -rp={}'.format(dir_to_scan)
@@ -457,8 +457,6 @@ def scan_package(cppcheck_path, source_path, libraries, capture_callstack=True):
         cppcheck_cmd = os.path.join(cppcheck_path, 'cppcheck') + ' ' + options_rp
         cmd = nice_cmd + ' ' + cppcheck_cmd + ' ' + __jobs + ' ' + dir_to_scan
     returncode, stdout, stderr, elapsed_time = __run_command(cmd)
-    if returncode >= 1 and ('--unsafe-exitcode' in stdout):
-        returncode, stdout, stderr, elapsed_time = __run_command(cmd.replace(' --unsafe-exitcode', ''))
 
     # collect messages
     information_messages_list = []
diff --git a/tools/extracttests.py b/tools/extracttests.py
@@ -94,7 +94,8 @@ def parseFile(self, filename):
         start_code = None
         disable = False
 
-        for line in open(filename, 'r'):
+        fin = open(filename, 'r')
+        for line in fin:
             # testclass starts
             res = re.match('class (' + name + ')', line)
             if res is not None:
@@ -136,10 +137,6 @@ def parseFile(self, filename):
             if code is not None:
                 res = re.match('\\s+' + string, line)
                 if res is not None:
-                    if line.find('",') > line.find('"'):
-                        code = None
-                        continue
-
                     code = code + res.group(1)
                     if res.group(1).find('"') > 0:
                         code = None
@@ -162,8 +159,10 @@ def parseFile(self, filename):
                         'expected': expected}
                 self.nodes.append(node)
                 code = None
-            elif re.match('\\s+[TOD_]*ASSERT', line) is not None:
-                code = None
+
+        # close test file
+        fin.close()
+
 
 def strtoxml(s):
     """Convert string to xml/html format"""
diff --git a/tools/run_more_tests.sh b/tools/run_more_tests.sh
@@ -18,8 +18,6 @@ python3 $DIR/extracttests.py --code=$(pwd)/test1 $1
 
 cd test1
 
-$CPPCHECK -q --template=cppcheck1 --suppress="*" .
-
 $CPPCHECK -q --template=cppcheck1 . 2> 1.txt
 
 echo "(!x) => (x==0)"
