fixed #12465 - added command-line option --executor to specify the …

…used executor implementation
danmar · Mar 4, 2024 · f6d994d · f6d994d
1 parent 7125290
commit f6d994d
Show file tree

Hide file tree

Showing 12 changed files with 211 additions and 21 deletions.
diff --git 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: Generate dependencies
         if: false
@@ -112,6 +114,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
@@ -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"
@@ -1274,6 +1305,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}";
@@ -1430,6 +1465,8 @@ void CmdLineParser::printHelp() const
         "                         provided. Note that your operating system can modify\n"
         "                         this value, e.g. '256' can become '0'.\n"
         "    --errorlist          Print a list of all the error messages in XML format.\n"
+        "    --executor=<type>\n"
+        "                         Specifies the executor to use. Possible values: auto, thread, processor.\n"
         "    --exitcode-suppressions=<file>\n"
         "                         Used when certain messages should be displayed but\n"
         "                         should not cause a non-zero exitcode.\n"

diff --git 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
 
@@ -270,18 +271,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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -16,6 +16,7 @@
  * along with this program.  If not, see <http://www.gnu.org/licenses/>.
  */
 
+#include "config.h"
 #include "settings.h"
 #include "path.h"
 #include "summaries.h"
@@ -37,9 +38,11 @@ const char Settings::SafeChecks::XmlExternalVariables[] = "external-variables";
 
 Settings::Settings()
 {
+    // TODO: set default executor
     severity.setEnabled(Severity::error, true);
     certainty.setEnabled(Certainty::normal, true);
     setCheckLevelNormal();
+    executor = defaultExecutor();
 }
 
 std::string Settings::loadCppcheckCfg(Settings& settings, Suppressions& suppressions)
@@ -592,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
@@ -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<Severity>, SimpleEnableGroup<Checks>> &groups);
     std::string applyEnabled(const std::string &str, bool enable);

diff --git a/man/cppcheck.1.xml b/man/cppcheck.1.xml
@@ -117,6 +117,9 @@ man(1), man(7), http://www.tldp.org/HOWTO/Man-Page/
       <arg choice="opt">
         <option>--errorlist</option>
       </arg>
+      <arg choice="opt">
+        <option>--executor=&lt;type&gt;</option>
+      </arg>
       <arg choice="opt">
         <option>--exitcode-suppressions=&lt;file&gt;</option>
       </arg>
@@ -328,6 +331,15 @@ Example: '-UDEBUG'</para>
           <para>Print a list of all possible error messages in XML format.</para>
         </listitem>
       </varlistentry>
+      <varlistentry>
+        <term>
+          <option>--executor=&lt;type&gt;</option>
+        </term>
+        <listitem>
+          <para>Specifies the executor to use. Possible values:</para>
+          <glosslist><glossentry><glossterm>auto</glossterm><glossdef><para>Automatically select (default)</para></glossdef></glossentry><glossentry><glossterm>thread</glossterm><glossdef><para>Perform analysis within a thread in the main process</para></glossdef></glossentry><glossentry><glossterm>proicess</glossterm><glossdef><para>Perform analysis of each file in a separate child process</para></glossdef></glossentry></glosslist>
+        </listitem>
+      </varlistentry>
       <varlistentry>
         <term>
           <option>--exitcode-suppressions=&lt;file&gt;</option>

diff --git a/releasenotes.txt b/releasenotes.txt
@@ -23,3 +23,6 @@ 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'.
+- Added command-line option `--executor=` to specify the executor. Available options are 'auto', 'process' and 'thread'. 'auto' will prefer 'process' if it is available. Note: Windows has no implementation of 'process' as of now so it will always chose 'thread' for now.