From 6542816ba9eb684336025869cd64d7b4cd040d01 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Oliver=20St=C3=B6neberg?= Date: Mon, 11 Mar 2024 03:27:05 +0100 Subject: [PATCH] fixed #12465 - added (undocumented) command-line option `--executor` to specify the used executor implementation (#6043) --- .github/workflows/tsan.yml | 5 +- cli/cmdlineparser.cpp | 37 +++++++++++- cli/cppcheckexecutor.cpp | 23 +++++--- cmake/compilerDefinitions.cmake | 4 +- cmake/options.cmake | 6 +- cmake/printInfo.cmake | 2 +- lib/config.h | 10 ++-- lib/settings.cpp | 13 +++++ lib/settings.h | 14 +++++ releasenotes.txt | 2 + test/cli/testutils.py | 15 +++++ test/testcmdlineparser.cpp | 100 +++++++++++++++++++++++++++++++- 12 files changed, 210 insertions(+), 21 deletions(-) diff --git a/.github/workflows/tsan.yml b/.github/workflows/tsan.yml index 8ace8797f25..0cabb7b2271 100644 --- a/.github/workflows/tsan.yml +++ b/.github/workflows/tsan.yml @@ -67,7 +67,7 @@ jobs: - name: CMake run: | - cmake -S . -B cmake.output -DCMAKE_BUILD_TYPE=RelWithDebInfo -DHAVE_RULES=On -DBUILD_TESTS=On -DBUILD_GUI=Off -DWITH_QCHART=Off -DUSE_MATCHCOMPILER=Verify -DANALYZE_THREAD=On -DUSE_THREADS=On -DENABLE_CHECK_INTERNAL=On -DUSE_BOOST=On -DCPPCHK_GLIBCXX_DEBUG=Off -DCMAKE_DISABLE_PRECOMPILE_HEADERS=On -DCMAKE_GLOBAL_AUTOGEN_TARGET=Off -DDISABLE_DMAKE=On -DCMAKE_C_COMPILER_LAUNCHER=ccache -DCMAKE_CXX_COMPILER_LAUNCHER=ccache + cmake -S . -B cmake.output -DCMAKE_BUILD_TYPE=RelWithDebInfo -DHAVE_RULES=On -DBUILD_TESTS=On -DBUILD_GUI=Off -DWITH_QCHART=Off -DUSE_MATCHCOMPILER=Verify -DANALYZE_THREAD=On -DENABLE_CHECK_INTERNAL=On -DUSE_BOOST=On -DCPPCHK_GLIBCXX_DEBUG=Off -DCMAKE_DISABLE_PRECOMPILE_HEADERS=On -DCMAKE_GLOBAL_AUTOGEN_TARGET=Off -DDISABLE_DMAKE=On -DCMAKE_C_COMPILER_LAUNCHER=ccache -DCMAKE_CXX_COMPILER_LAUNCHER=ccache env: CC: clang-18 CXX: clang++-18 @@ -98,6 +98,8 @@ jobs: pwd=$(pwd) cd test/cli TEST_CPPCHECK_EXE_LOOKUP_PATH="$pwd/cmake.output" python3 -m pytest -Werror --strict-markers -vv + env: + TEST_CPPCHECK_INJECT_EXECUTOR: thread - name: Run test/cli (-j2) run: | @@ -129,6 +131,7 @@ jobs: if: false run: | selfcheck_options="-q -j$(nproc) --std=c++11 --template=selfcheck --showtime=top5_summary -D__GNUC__ --error-exitcode=0 --inline-suppr --suppressions-list=.selfcheck_suppressions --library=gnu --inconclusive --enable=style,performance,portability,warning,missingInclude,internal --exception-handling --debug-warnings --check-level=exhaustive" + selfcheck_options="$selfcheck_options --executor=thread" cppcheck_options="-D__CPPCHECK__ -DCHECK_INTERNAL -DHAVE_RULES --library=cppcheck-lib -Ilib -Iexternals/simplecpp/ -Iexternals/tinyxml2" ec=0 ./cmake.output/bin/cppcheck $selfcheck_options externals/simplecpp || ec=1 diff --git a/cli/cmdlineparser.cpp b/cli/cmdlineparser.cpp index 15e2492d609..7f275ab00df 100644 --- a/cli/cmdlineparser.cpp +++ b/cli/cmdlineparser.cpp @@ -374,6 +374,7 @@ CmdLineParser::Result CmdLineParser::parseFromArgs(int argc, const char* const a ImportProject project; + bool executorAuto = true; int8_t logMissingInclude{0}; for (int i = 1; i < argc; i++) { @@ -614,6 +615,36 @@ CmdLineParser::Result CmdLineParser::parseFromArgs(int argc, const char* const a #endif } + else if (std::strncmp(argv[i], "--executor=", 11) == 0) { + const std::string type = 11 + argv[i]; + if (type == "auto") { + executorAuto = true; + mSettings.executor = Settings::defaultExecutor(); + } + else if (type == "thread") { +#if defined(HAS_THREADING_MODEL_THREAD) + executorAuto = false; + mSettings.executor = Settings::ExecutorType::Thread; +#else + mLogger.printError("executor type 'thread' cannot be used as Cppcheck has not been built with a respective threading model."); + return Result::Fail; +#endif + } + else if (type == "process") { +#if defined(HAS_THREADING_MODEL_FORK) + executorAuto = false; + mSettings.executor = Settings::ExecutorType::Process; +#else + mLogger.printError("executor type 'process' cannot be used as Cppcheck has not been built with a respective threading model."); + return Result::Fail; +#endif + } + else { + mLogger.printError("unknown executor: '" + type + "'."); + return Result::Fail; + } + } + // Filter errors else if (std::strncmp(argv[i], "--exitcode-suppressions=", 24) == 0) { // exitcode-suppressions=filename.txt @@ -751,7 +782,7 @@ CmdLineParser::Result CmdLineParser::parseFromArgs(int argc, const char* const a } else if (std::strncmp(argv[i], "-l", 2) == 0) { -#ifdef THREADING_MODEL_FORK +#ifdef HAS_THREADING_MODEL_FORK std::string numberString; // "-l 3" @@ -1276,6 +1307,10 @@ CmdLineParser::Result CmdLineParser::parseFromArgs(int argc, const char* const a if (!loadCppcheckCfg()) return Result::Fail; + // TODO: bail out? + if (!executorAuto && mSettings.useSingleJob()) + mLogger.printMessage("'--executor' has no effect as only a single job will be used."); + // Default template format.. if (mSettings.templateFormat.empty()) { mSettings.templateFormat = "{bold}{file}:{line}:{column}: {red}{inconclusive:{magenta}}{severity}:{inconclusive: inconclusive:}{default} {message} [{id}]{reset}\\n{code}"; diff --git a/cli/cppcheckexecutor.cpp b/cli/cppcheckexecutor.cpp index b734077ca6c..7bebf81e5da 100644 --- a/cli/cppcheckexecutor.cpp +++ b/cli/cppcheckexecutor.cpp @@ -33,9 +33,10 @@ #include "suppressions.h" #include "utils.h" -#if defined(THREADING_MODEL_THREAD) +#if defined(HAS_THREADING_MODEL_THREAD) #include "threadexecutor.h" -#elif defined(THREADING_MODEL_FORK) +#endif +#if defined(HAS_THREADING_MODEL_FORK) #include "processexecutor.h" #endif @@ -271,18 +272,24 @@ int CppCheckExecutor::check_internal(const Settings& settings) const cppcheck.settings() = settings; // this is a copy auto& suppressions = cppcheck.settings().supprs.nomsg; - unsigned int returnValue; + unsigned int returnValue = 0; if (settings.useSingleJob()) { // Single process SingleExecutor executor(cppcheck, mFiles, mFileSettings, settings, suppressions, stdLogger); returnValue = executor.check(); } else { -#if defined(THREADING_MODEL_THREAD) - ThreadExecutor executor(mFiles, mFileSettings, settings, suppressions, stdLogger, CppCheckExecutor::executeCommand); -#elif defined(THREADING_MODEL_FORK) - ProcessExecutor executor(mFiles, mFileSettings, settings, suppressions, stdLogger, CppCheckExecutor::executeCommand); +#if defined(HAS_THREADING_MODEL_THREAD) + if (settings.executor == Settings::ExecutorType::Thread) { + ThreadExecutor executor(mFiles, mFileSettings, settings, suppressions, stdLogger, CppCheckExecutor::executeCommand); + returnValue = executor.check(); + } +#endif +#if defined(HAS_THREADING_MODEL_FORK) + if (settings.executor == Settings::ExecutorType::Process) { + ProcessExecutor executor(mFiles, mFileSettings, settings, suppressions, stdLogger, CppCheckExecutor::executeCommand); + returnValue = executor.check(); + } #endif - returnValue = executor.check(); } cppcheck.analyseWholeProgram(settings.buildDir, mFiles, mFileSettings); diff --git a/cmake/compilerDefinitions.cmake b/cmake/compilerDefinitions.cmake index 8ff3ba1d432..f4cb59f3bbc 100644 --- a/cmake/compilerDefinitions.cmake +++ b/cmake/compilerDefinitions.cmake @@ -41,8 +41,8 @@ if (ENABLE_CHECK_INTERNAL) add_definitions(-DCHECK_INTERNAL) endif() -if (USE_THREADS) - add_definitions(-DUSE_THREADS) +if (DISALLOW_THREAD_EXECUTOR) + add_definitions(-DDISALLOW_THREAD_EXECUTOR) endif() if (MSVC AND DISABLE_CRTDBG_MAP_ALLOC) diff --git a/cmake/options.cmake b/cmake/options.cmake index 3e3b37f7a6e..d63d67079af 100644 --- a/cmake/options.cmake +++ b/cmake/options.cmake @@ -58,10 +58,14 @@ if (BUILD_CORE_DLL) set(USE_BUNDLED_TINYXML2 ON) endif() option(CPPCHK_GLIBCXX_DEBUG "Usage of STL debug checks in Debug build" ON) -option(USE_THREADS "Usage of threads instead of fork() for -j" OFF) +option(DISALLOW_THREAD_EXECUTOR "Disallow usage of ThreadExecutor for -j" OFF) option(USE_BOOST "Usage of Boost" OFF) option(USE_LIBCXX "Use libc++ instead of libstdc++" OFF) +if (DISALLOW_THREAD_EXECUTOR AND WIN32) + message(FATAL_ERROR "Cannot disable usage of ThreadExecutor on Windows as no other executor implementation is currently available") +endif() + option(DISABLE_CRTDBG_MAP_ALLOC "Disable usage of Visual Studio C++ memory leak detection in Debug build" OFF) option(NO_UNIX_SIGNAL_HANDLING "Disable usage of Unix Signal Handling" OFF) option(NO_UNIX_BACKTRACE_SUPPORT "Disable usage of Unix Backtrace support" OFF) diff --git a/cmake/printInfo.cmake b/cmake/printInfo.cmake index 77b9f997946..244c6b1e6bf 100644 --- a/cmake/printInfo.cmake +++ b/cmake/printInfo.cmake @@ -71,7 +71,7 @@ if (HAVE_RULES) message( STATUS "PCRE_LIBRARY = ${PCRE_LIBRARY}" ) endif() message( STATUS ) -message( STATUS "USE_THREADS = ${USE_THREADS}" ) +message( STATUS "DISALLOW_THREAD_EXECUTOR = ${DISALLOW_THREAD_EXECUTOR}" ) message( STATUS "CMAKE_THREAD_LIBS_INIT = ${CMAKE_THREAD_LIBS_INIT}" ) message( STATUS ) message( STATUS "USE_BUNDLED_TINYXML2 = ${USE_BUNDLED_TINYXML2}" ) diff --git a/lib/config.h b/lib/config.h index 060462e6474..538e20a7ad6 100644 --- a/lib/config.h +++ b/lib/config.h @@ -151,13 +151,13 @@ static const std::string emptyString; #endif #if defined(_WIN32) -#define THREADING_MODEL_THREAD +#define HAS_THREADING_MODEL_THREAD #define STDCALL __stdcall -#elif defined(USE_THREADS) -#define THREADING_MODEL_THREAD -#define STDCALL #elif ((defined(__GNUC__) || defined(__sun)) && !defined(__MINGW32__)) || defined(__CPPCHECK__) -#define THREADING_MODEL_FORK +#define HAS_THREADING_MODEL_FORK +#if !defined(DISALLOW_THREAD_EXECUTOR) +#define HAS_THREADING_MODEL_THREAD +#endif #define STDCALL #else #error "No threading model defined" diff --git a/lib/settings.cpp b/lib/settings.cpp index 27c4ecdebef..ae03f2476de 100644 --- a/lib/settings.cpp +++ b/lib/settings.cpp @@ -16,6 +16,7 @@ * along with this program. If not, see . */ +#include "config.h" #include "settings.h" #include "path.h" #include "summaries.h" @@ -41,6 +42,7 @@ Settings::Settings() severity.setEnabled(Severity::error, true); certainty.setEnabled(Certainty::normal, true); setCheckLevelNormal(); + executor = defaultExecutor(); } std::string Settings::loadCppcheckCfg(Settings& settings, Suppressions& suppressions) @@ -593,3 +595,14 @@ std::string Settings::getMisraRuleText(const std::string& id, const std::string& const auto it = mMisraRuleTexts.find(id.substr(id.rfind('-') + 1)); return it != mMisraRuleTexts.end() ? it->second : text; } + +Settings::ExecutorType Settings::defaultExecutor() +{ + static constexpr ExecutorType defaultExecutor = +#if defined(HAS_THREADING_MODEL_FORK) + ExecutorType::Process; +#elif defined(HAS_THREADING_MODEL_THREAD) + ExecutorType::Thread; +#endif + return defaultExecutor; +} diff --git a/lib/settings.h b/lib/settings.h index aad106fed50..a4aa03eee46 100644 --- a/lib/settings.h +++ b/lib/settings.h @@ -195,6 +195,18 @@ class CPPCHECKLIB WARN_UNUSED Settings { bool exceptionHandling{}; #endif + enum class ExecutorType + { +#ifdef HAS_THREADING_MODEL_THREAD + Thread, +#endif +#ifdef HAS_THREADING_MODEL_FORK + Process +#endif + }; + + ExecutorType executor; + // argv[0] std::string exename; @@ -465,6 +477,8 @@ class CPPCHECKLIB WARN_UNUSED Settings { void setMisraRuleTexts(const std::string& data); std::string getMisraRuleText(const std::string& id, const std::string& text) const; + static ExecutorType defaultExecutor(); + private: static std::string parseEnabled(const std::string &str, std::tuple, SimpleEnableGroup> &groups); std::string applyEnabled(const std::string &str, bool enable); diff --git a/releasenotes.txt b/releasenotes.txt index a645a24a3cb..65c9e9ef90a 100644 --- a/releasenotes.txt +++ b/releasenotes.txt @@ -23,3 +23,5 @@ Other: - Added '--template=simple'. It is expands to '{file}:{line}:{column}: {severity}:{inconclusive:inconclusive:} {message} [{id}]' without any additional location details. - Removed deprecated platform type 'Unspecified'. Please use 'unspecified' instead. - Removed deprecated 'Makefile' option 'SRCDIR'. +- Added CMake option 'DISALLOW_THREAD_EXECUTOR' to control the inclusion of the executor which performs the analysis within a thread of the main process. +- Removed CMake option 'USE_THREADS' in favor of 'DISALLOW_THREAD_EXECUTOR'. \ No newline at end of file diff --git a/test/cli/testutils.py b/test/cli/testutils.py index b076f27d4d2..a6692bde8bc 100644 --- a/test/cli/testutils.py +++ b/test/cli/testutils.py @@ -97,6 +97,21 @@ def cppcheck(args, env=None, remove_checkers_report=True, cwd=None, cppcheck_exe arg_clang = '--clang=' + str(os.environ['TEST_CPPCHECK_INJECT_CLANG']) args.append(arg_clang) + if 'TEST_CPPCHECK_INJECT_EXECUTOR' in os.environ: + found_jn = False + found_executor = False + for arg in args: + if arg.startswith('-j') and arg != '-j1': + found_jn = True + continue + if arg.startswith('--executor'): + found_executor = True + continue + # only add '--executor' if we are actually using multiple jobs + if found_jn and not found_executor: + arg_executor = '--executor=' + str(os.environ['TEST_CPPCHECK_INJECT_EXECUTOR']) + args.append(arg_executor) + logging.info(exe + ' ' + ' '.join(args)) p = subprocess.Popen([exe] + args, stdout=subprocess.PIPE, stderr=subprocess.PIPE, env=env, cwd=cwd) try: diff --git a/test/testcmdlineparser.cpp b/test/testcmdlineparser.cpp index 52e293fe7e9..5437ca9f013 100644 --- a/test/testcmdlineparser.cpp +++ b/test/testcmdlineparser.cpp @@ -299,7 +299,7 @@ class TestCmdlineParser : public TestFixture { TEST_CASE(checksMaxTime); TEST_CASE(checksMaxTime2); TEST_CASE(checksMaxTimeInvalid); -#ifdef THREADING_MODEL_FORK +#ifdef HAS_THREADING_MODEL_FORK TEST_CASE(loadAverage); TEST_CASE(loadAverage2); TEST_CASE(loadAverageInvalid); @@ -354,6 +354,21 @@ class TestCmdlineParser : public TestFixture { TEST_CASE(suppressXmlMissing); TEST_CASE(suppressXmlInvalid); TEST_CASE(suppressXmlNoRoot); + TEST_CASE(executorDefault); + TEST_CASE(executorAuto); + TEST_CASE(executorAutoNoJobs); +#if defined(HAS_THREADING_MODEL_THREAD) + TEST_CASE(executorThread); + TEST_CASE(executorThreadNoJobs); +#else + TEST_CASE(executorThreadNotSupported); +#endif +#if defined(HAS_THREADING_MODEL_FORK) + TEST_CASE(executorProcess); + TEST_CASE(executorProcessNoJobs); +#else + TEST_CASE(executorProcessNotSupported); +#endif TEST_CASE(ignorepaths1); TEST_CASE(ignorepaths2); @@ -1895,7 +1910,7 @@ class TestCmdlineParser : public TestFixture { ASSERT_EQUALS("cppcheck: error: argument to '--checks-max-time=' is not valid - not an integer.\n", logger->str()); } -#ifdef THREADING_MODEL_FORK +#ifdef HAS_THREADING_MODEL_FORK void loadAverage() { REDIRECT; const char * const argv[] = {"cppcheck", "-l", "12", "file.cpp"}; @@ -2247,6 +2262,87 @@ class TestCmdlineParser : public TestFixture { ASSERT_EQUALS("cppcheck: error: failed to load suppressions XML 'suppress.xml' (no root node found).\n", logger->str()); } + void executorDefault() { + REDIRECT; + const char * const argv[] = {"cppcheck", "file.cpp"}; + ASSERT_EQUALS_ENUM(CmdLineParser::Result::Success, parser->parseFromArgs(2, argv)); +#if defined(HAS_THREADING_MODEL_FORK) + ASSERT_EQUALS_ENUM(Settings::ExecutorType::Process, settings->executor); +#elif defined(HAS_THREADING_MODEL_THREAD) + ASSERT_EQUALS_ENUM(Settings::ExecutorType::Thread, settings->executor); +#endif + } + + void executorAuto() { + REDIRECT; + const char * const argv[] = {"cppcheck", "-j2", "--executor=auto", "file.cpp"}; + ASSERT_EQUALS_ENUM(CmdLineParser::Result::Success, parser->parseFromArgs(4, argv)); +#if defined(HAS_THREADING_MODEL_FORK) + ASSERT_EQUALS_ENUM(Settings::ExecutorType::Process, settings->executor); +#elif defined(HAS_THREADING_MODEL_THREAD) + ASSERT_EQUALS_ENUM(Settings::ExecutorType::Thread, settings->executor); +#endif + } + + void executorAutoNoJobs() { + REDIRECT; + const char * const argv[] = {"cppcheck", "--executor=auto", "file.cpp"}; + ASSERT_EQUALS_ENUM(CmdLineParser::Result::Success, parser->parseFromArgs(3, argv)); +#if defined(HAS_THREADING_MODEL_FORK) + ASSERT_EQUALS_ENUM(Settings::ExecutorType::Process, settings->executor); +#elif defined(HAS_THREADING_MODEL_THREAD) + ASSERT_EQUALS_ENUM(Settings::ExecutorType::Thread, settings->executor); +#endif + } + +#if defined(HAS_THREADING_MODEL_THREAD) + void executorThread() { + REDIRECT; + const char * const argv[] = {"cppcheck", "-j2", "--executor=thread", "file.cpp"}; + ASSERT_EQUALS_ENUM(CmdLineParser::Result::Success, parser->parseFromArgs(4, argv)); + ASSERT_EQUALS_ENUM(Settings::ExecutorType::Thread, settings->executor); + } + + void executorThreadNoJobs() { + REDIRECT; + const char * const argv[] = {"cppcheck", "--executor=thread", "file.cpp"}; + ASSERT_EQUALS_ENUM(CmdLineParser::Result::Success, parser->parseFromArgs(3, argv)); + ASSERT_EQUALS_ENUM(Settings::ExecutorType::Thread, settings->executor); + ASSERT_EQUALS("cppcheck: '--executor' has no effect as only a single job will be used.\n", logger->str()); + } +#else + void executorThreadNotSupported() { + REDIRECT; + const char * const argv[] = {"cppcheck", "-j2", "--executor=thread", "file.cpp"}; + ASSERT_EQUALS_ENUM(CmdLineParser::Result::Fail, parser->parseFromArgs(4, argv)); + ASSERT_EQUALS("cppcheck: error: executor type 'thread' cannot be used as Cppcheck has not been built with a respective threading model.\n", logger->str()); + } +#endif + +#if defined(HAS_THREADING_MODEL_FORK) + void executorProcess() { + REDIRECT; + const char * const argv[] = {"cppcheck", "-j2", "--executor=process", "file.cpp"}; + ASSERT_EQUALS_ENUM(CmdLineParser::Result::Success, parser->parseFromArgs(4, argv)); + ASSERT_EQUALS_ENUM(Settings::ExecutorType::Process, settings->executor); + } + + void executorProcessNoJobs() { + REDIRECT; + const char * const argv[] = {"cppcheck", "--executor=process", "file.cpp"}; + ASSERT_EQUALS_ENUM(CmdLineParser::Result::Success, parser->parseFromArgs(3, argv)); + ASSERT_EQUALS_ENUM(Settings::ExecutorType::Process, settings->executor); + ASSERT_EQUALS("cppcheck: '--executor' has no effect as only a single job will be used.\n", logger->str()); + } +#else + void executorProcessNotSupported() { + REDIRECT; + const char * const argv[] = {"cppcheck", "-j2", "--executor=process", "file.cpp"}; + ASSERT_EQUALS_ENUM(CmdLineParser::Result::Fail, parser->parseFromArgs(4, argv)); + ASSERT_EQUALS("cppcheck: error: executor type 'process' cannot be used as Cppcheck has not been built with a respective threading model.\n", logger->str()); + } +#endif + void ignorepaths1() { REDIRECT; const char * const argv[] = {"cppcheck", "-isrc", "file.cpp"};