From 2932ab75921915decf5563cb631289876117fd8c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Marjam=C3=A4ki?= Date: Sun, 17 Dec 2023 19:13:14 +0100 Subject: [PATCH] Revert "Fixed #12071 (suppressing critical error, no indication to user that analysis of file fails) (#5771)" (#5775) This reverts commit 7c316fb76ddb0a7226c4467dc4b76763eeb8bb70. --- .github/workflows/CI-unixish.yml | 3 +-- CMakeLists.txt | 2 +- Makefile | 2 +- cli/cmdlineparser.cpp | 4 ---- cli/cppcheckexecutor.cpp | 13 +------------ gui/resultstree.cpp | 5 ----- gui/resultsview.cpp | 7 ------- htmlreport/check.sh | 4 ++-- lib/cppcheck.cpp | 13 ------------- lib/settings.h | 3 --- lib/suppressions.cpp | 13 ------------- lib/suppressions.h | 8 -------- releasenotes.txt | 3 --- test/cli/test-other.py | 4 ++-- test/cli/test-suppress-syntaxError.py | 12 ------------ test/cli/testutils.py | 4 ++-- test/testsuppressions.cpp | 16 +++++++--------- tools/dmake.cpp | 2 +- tools/donate_cpu_lib.py | 6 ++---- tools/extracttests.py | 13 ++++++------- tools/run_more_tests.sh | 2 -- 21 files changed, 26 insertions(+), 113 deletions(-) diff --git a/.github/workflows/CI-unixish.yml b/.github/workflows/CI-unixish.yml index 919cd3927..af056b560 100644 --- 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 index 3a04d9951..34dd57867 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -47,7 +47,7 @@ if(LIBXML2_XMLLINT_EXECUTABLE) add_custom_target(errorlist-xml $ --errorlist > ${CMAKE_BINARY_DIR}/errorlist.xml DEPENDS cppcheck) - add_custom_target(example-xml $ --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 $ --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 index f9fac8f38..d4e8ad837 100644 --- 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 index 0e35986cc..417c46e5a 100644 --- 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 index 576a87f3c..4320c020f 100644 --- a/cli/cppcheckexecutor.cpp +++ b/cli/cppcheckexecutor.cpp @@ -118,10 +118,6 @@ public: */ 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 index 46d77dd3b..e73475cf8 100644 --- 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 index b81708202..2b5baa5fb 100644 --- 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 index 4a316f480..a4515ec06 100755 --- 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 index 742cde203..53705ed10 100644 --- 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 index f6123bdaf..6833d8569 100644 --- a/lib/settings.h +++ b/lib/settings.h @@ -358,9 +358,6 @@ public: /** @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 index e5f4e7b47..782d657d1 100644 --- 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& macroNames) { if (mSuppressions.empty()) diff --git a/lib/suppressions.h b/lib/suppressions.h index e6f6da830..d859ed02f 100644 --- a/lib/suppressions.h +++ b/lib/suppressions.h @@ -202,14 +202,6 @@ public: */ 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 index bc70765cc..75d4427d6 100644 --- 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 index 2ad2586b5..819f1d6a6 100644 --- a/test/cli/test-other.py +++ b/test/cli/test-other.py @@ -411,7 +411,7 @@ typedef int MISRA_5_6_VIOLATION; 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 @@ typedef int MISRA_5_6_VIOLATION; 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 index 62da28c83..00d383eab 100644 --- 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 index dd529e1b6..1dd932a9e 100644 --- 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 index 95e225516..2f91cbdde 100644 --- a/test/testsuppressions.cpp +++ b/test/testsuppressions.cpp @@ -1144,7 +1144,7 @@ private: 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 @@ private: " 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 @@ private: "[!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 @@ private: 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 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 index 2595f037f..1ca4262c0 100644 --- 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 index 82518bd2a..f09db15d2 100644 --- a/tools/donate_cpu_lib.py +++ b/tools/donate_cpu_lib.py @@ -16,7 +16,7 @@ import copy # 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 index a45c8261d..42a80d6c0 100755 --- a/tools/extracttests.py +++ b/tools/extracttests.py @@ -94,7 +94,8 @@ class Extract: 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 @@ class Extract: 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 @@ class Extract: '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 index 4dde28df5..12633c41f 100755 --- 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)"