From 3601b18415ab1deb32922c5be572fbbbab2bd6a7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Marjam=C3=A4ki?= Date: Fri, 19 Apr 2024 13:57:38 +0200 Subject: [PATCH] Fixed #12440 (Misra violations found but code exited with 0 even after specifying exit code) (#6308) --- cli/cppcheckexecutor.cpp | 4 ++-- lib/cppcheck.cpp | 25 ++++++------------------- lib/cppcheck.h | 2 +- lib/settings.cpp | 17 +++++++++++++++++ lib/settings.h | 3 +++ test/cli/other_test.py | 11 +++++++++++ 6 files changed, 40 insertions(+), 22 deletions(-) diff --git a/cli/cppcheckexecutor.cpp b/cli/cppcheckexecutor.cpp index 82673af9c14..27f1d1951db 100644 --- a/cli/cppcheckexecutor.cpp +++ b/cli/cppcheckexecutor.cpp @@ -292,7 +292,7 @@ int CppCheckExecutor::check_internal(const Settings& settings) const #endif } - cppcheck.analyseWholeProgram(settings.buildDir, mFiles, mFileSettings); + returnValue |= cppcheck.analyseWholeProgram(settings.buildDir, mFiles, mFileSettings); if (settings.severity.isEnabled(Severity::information) || settings.checkConfiguration) { const bool err = reportSuppressions(settings, suppressions, settings.checks.isEnabled(Checks::unusedFunction), mFiles, mFileSettings, stdLogger); @@ -311,7 +311,7 @@ int CppCheckExecutor::check_internal(const Settings& settings) const stdLogger.reportErr(ErrorMessage::getXMLFooter()); } - if (settings.safety && stdLogger.hasCriticalErrors()) + if (settings.safety && (stdLogger.hasCriticalErrors() || returnValue != 0)) return EXIT_FAILURE; if (returnValue) diff --git a/lib/cppcheck.cpp b/lib/cppcheck.cpp index cfe7b624fbe..87d8808899f 100644 --- a/lib/cppcheck.cpp +++ b/lib/cppcheck.cpp @@ -60,12 +60,6 @@ #include #include -#ifndef _WIN32 -#include -#else -#include -#endif - #include "json.h" #include @@ -139,15 +133,6 @@ static std::vector split(const std::string &str, const std::string return ret; } -static int getPid() -{ -#ifndef _WIN32 - return getpid(); -#else - return _getpid(); -#endif -} - static std::string getDumpFileName(const Settings& settings, const std::string& filename) { if (!settings.dumpFile.empty()) @@ -157,7 +142,7 @@ static std::string getDumpFileName(const Settings& settings, const std::string& if (settings.dump) extension = ".dump"; else - extension = "." + std::to_string(getPid()) + ".dump"; + extension = "." + std::to_string(settings.pid) + ".dump"; if (!settings.dump && !settings.buildDir.empty()) return AnalyzerInformation::getAnalyzerInfoFile(settings.buildDir, filename, emptyString) + extension; @@ -1409,7 +1394,7 @@ void CppCheck::executeAddons(const std::vector& files, const std::s std::string fileList; if (files.size() >= 2 || endsWith(files[0], ".ctu-info")) { - fileList = Path::getPathFromFilename(files[0]) + FILELIST + std::to_string(getPid()); + fileList = Path::getPathFromFilename(files[0]) + FILELIST + std::to_string(mSettings.pid); filesDeleter.addFile(fileList); std::ofstream fout(fileList); for (const std::string& f: files) @@ -1770,12 +1755,12 @@ bool CppCheck::analyseWholeProgram() return errors && (mExitCode > 0); } -void CppCheck::analyseWholeProgram(const std::string &buildDir, const std::list> &files, const std::list& fileSettings) +unsigned int CppCheck::analyseWholeProgram(const std::string &buildDir, const std::list> &files, const std::list& fileSettings) { executeAddonsWholeProgram(files); // TODO: pass FileSettings if (buildDir.empty()) { removeCtuInfoFiles(files, fileSettings); - return; + return mExitCode; } if (mSettings.checks.isEnabled(Checks::unusedFunction)) CheckUnusedFunctions::analyseWholeProgram(mSettings, *this, buildDir); @@ -1836,6 +1821,8 @@ void CppCheck::analyseWholeProgram(const std::string &buildDir, const std::list< for (Check::FileInfo *fi : fileInfoList) delete fi; + + return mExitCode; } void CppCheck::removeCtuInfoFiles(const std::list> &files, const std::list& fileSettings) diff --git a/lib/cppcheck.h b/lib/cppcheck.h index abe815377bb..ed942ebca8b 100644 --- a/lib/cppcheck.h +++ b/lib/cppcheck.h @@ -142,7 +142,7 @@ class CPPCHECKLIB CppCheck : ErrorLogger { void analyseClangTidy(const FileSettings &fileSettings); /** analyse whole program use .analyzeinfo files */ - void analyseWholeProgram(const std::string &buildDir, const std::list> &files, const std::list& fileSettings); + unsigned int analyseWholeProgram(const std::string &buildDir, const std::list> &files, const std::list& fileSettings); /** Remove *.ctu-info files */ void removeCtuInfoFiles(const std::list>& files, const std::list& fileSettings); // cppcheck-suppress functionConst // has side effects diff --git a/lib/settings.cpp b/lib/settings.cpp index 1940a07ced3..c91abf5f7e5 100644 --- a/lib/settings.cpp +++ b/lib/settings.cpp @@ -29,6 +29,13 @@ #include "json.h" +#ifndef _WIN32 +#include // for getpid() +#else +#include // for getpid() +#endif + + std::atomic Settings::mTerminated; const char Settings::SafeChecks::XmlRootName[] = "safe-checks"; @@ -37,12 +44,22 @@ const char Settings::SafeChecks::XmlExternalFunctions[] = "external-functions"; const char Settings::SafeChecks::XmlInternalFunctions[] = "internal-functions"; const char Settings::SafeChecks::XmlExternalVariables[] = "external-variables"; +static int getPid() +{ +#ifndef _WIN32 + return getpid(); +#else + return _getpid(); +#endif +} + Settings::Settings() { severity.setEnabled(Severity::error, true); certainty.setEnabled(Certainty::normal, true); setCheckLevel(Settings::CheckLevel::exhaustive); executor = defaultExecutor(); + pid = getPid(); } std::string Settings::loadCppcheckCfg(Settings& settings, Suppressions& suppressions) diff --git a/lib/settings.h b/lib/settings.h index 55ee9669310..3f0023493c1 100644 --- a/lib/settings.h +++ b/lib/settings.h @@ -263,6 +263,9 @@ class CPPCHECKLIB WARN_UNUSED Settings { /** @brief max number of sets of arguments to pass to subfuncions in valueflow */ int performanceValueFlowMaxSubFunctionArgs = 256; + /** @brief pid of cppcheck. Intention is that this is set in the main process. */ + int pid; + /** @brief plist output (--plist-output=<dir>) */ std::string plistOutput; diff --git a/test/cli/other_test.py b/test/cli/other_test.py index 7cdd3d9e841..1ac8e4a341e 100644 --- a/test/cli/other_test.py +++ b/test/cli/other_test.py @@ -207,6 +207,17 @@ def test_internal_error(tmpdir): assert stderr == '{}:0:0: error: Bailing from out analysis: Checking file failed: converting \'1f\' to integer failed - not an integer [internalError]\n\n^\n'.format(test_file) +def test_addon_ctu_exitcode(tmpdir): + """ #12440 - Misra ctu violations found => exit code should be non-zero """ + test_file = os.path.join(tmpdir, 'test.c') + with open(test_file, 'wt') as f: + f.write("""typedef enum { BLOCK = 0x80U, } E;""") + args = ['--addon=misra', '--enable=style', '--error-exitcode=1', test_file] + exitcode, stdout, stderr = cppcheck(args) + assert '2.3' in stderr, stderr + assert exitcode == 1 + + # TODO: test with -j2 def test_addon_misra(tmpdir): test_file = os.path.join(tmpdir, 'test.cpp')