refactored SEH wrapper and added unit test

.github/workflows/CI-windows.yml

@@ -207,6 +207,17 @@ jobs:
         env:
           TEST_CPPCHECK_INJECT_BUILDDIR: injected
 
+      # TODO: test with Release configuration?
+      - name: Test SEH wrapper
+        if: matrix.config == 'release'
+        run: |
+          cmake -S . -B build.cmake.seh -DBUILD_TESTS=On || exit /b !errorlevel!
+          cmake --build build.cmake.seh --target test-sehwrapper || exit /b !errorlevel!
+          :: TODO: how to run this without copying the file?
+          copy build.cmake.seh\bin\Debug\test-sehwrapper.exe . || exit /b !errorlevel!
+          python3 -m pytest -Werror --strict-markers -vv test/seh/test-sehwrapper.py || exit /b !errorlevel!
+          del test-sehwrapper.exe || exit /b !errorlevel!
+
       - name: Test addons
         if: matrix.config == 'release'
         run: |

Makefile

@@ -254,11 +254,11 @@ EXTOBJ =      externals/simplecpp/simplecpp.o \
 
 CLIOBJ =      cli/cmdlineparser.o \
               cli/cppcheckexecutor.o \
-              cli/cppcheckexecutorseh.o \
               cli/executor.o \
               cli/filelister.o \
               cli/main.o \
               cli/processexecutor.o \
+              cli/sehwrapper.o \
               cli/signalhandler.o \
               cli/singleexecutor.o \
               cli/stacktrace.o \
@@ -348,7 +348,7 @@ cppcheck: $(EXTOBJ) $(LIBOBJ) $(CLIOBJ)
 
 all:	cppcheck testrunner
 
-testrunner: $(EXTOBJ) $(TESTOBJ) $(LIBOBJ) cli/executor.o cli/processexecutor.o cli/singleexecutor.o cli/threadexecutor.o cli/cmdlineparser.o cli/cppcheckexecutor.o cli/cppcheckexecutorseh.o cli/signalhandler.o cli/stacktrace.o cli/filelister.o
+testrunner: $(EXTOBJ) $(TESTOBJ) $(LIBOBJ) cli/cmdlineparser.o cli/cppcheckexecutor.o cli/executor.o cli/filelister.o cli/processexecutor.o cli/sehwrapper.o cli/signalhandler.o cli/singleexecutor.o cli/stacktrace.o cli/threadexecutor.o
 	$(CXX) $(CPPFLAGS) $(CXXFLAGS) -o $@ $^ $(LIBS) $(LDFLAGS) $(RDYNAMIC)
 
 test:	all
@@ -655,12 +655,9 @@ $(libcppdir)/vfvalue.o: lib/vfvalue.cpp lib/config.h lib/errortypes.h lib/mathli
 cli/cmdlineparser.o: cli/cmdlineparser.cpp cli/cmdlinelogger.h cli/cmdlineparser.h cli/filelister.h externals/tinyxml2/tinyxml2.h lib/addoninfo.h lib/check.h lib/color.h lib/config.h lib/cppcheck.h lib/errorlogger.h lib/errortypes.h lib/filesettings.h lib/importproject.h lib/library.h lib/mathlib.h lib/path.h lib/pathmatch.h lib/platform.h lib/settings.h lib/standards.h lib/suppressions.h lib/timer.h lib/utils.h lib/xml.h
 	$(CXX) ${INCLUDE_FOR_CLI} $(CPPFLAGS) $(CXXFLAGS) -c -o $@ cli/cmdlineparser.cpp
 
-cli/cppcheckexecutor.o: cli/cppcheckexecutor.cpp cli/cmdlinelogger.h cli/cmdlineparser.h cli/cppcheckexecutor.h cli/cppcheckexecutorseh.h cli/executor.h cli/processexecutor.h cli/signalhandler.h cli/singleexecutor.h cli/threadexecutor.h externals/picojson/picojson.h lib/addoninfo.h lib/analyzerinfo.h lib/check.h lib/checkersreport.h lib/color.h lib/config.h lib/cppcheck.h lib/errorlogger.h lib/errortypes.h lib/filesettings.h lib/json.h lib/library.h lib/mathlib.h lib/path.h lib/platform.h lib/settings.h lib/standards.h lib/suppressions.h lib/utils.h
+cli/cppcheckexecutor.o: cli/cppcheckexecutor.cpp cli/cmdlinelogger.h cli/cmdlineparser.h cli/cppcheckexecutor.h cli/executor.h cli/processexecutor.h cli/sehwrapper.h cli/signalhandler.h cli/singleexecutor.h cli/threadexecutor.h externals/picojson/picojson.h lib/addoninfo.h lib/analyzerinfo.h lib/check.h lib/checkersreport.h lib/color.h lib/config.h lib/cppcheck.h lib/errorlogger.h lib/errortypes.h lib/filesettings.h lib/json.h lib/library.h lib/mathlib.h lib/path.h lib/platform.h lib/settings.h lib/standards.h lib/suppressions.h lib/utils.h
 	$(CXX) ${INCLUDE_FOR_CLI} $(CPPFLAGS) $(CXXFLAGS) -c -o $@ cli/cppcheckexecutor.cpp
 
-cli/cppcheckexecutorseh.o: cli/cppcheckexecutorseh.cpp cli/cppcheckexecutor.h cli/cppcheckexecutorseh.h lib/config.h lib/filesettings.h lib/mathlib.h lib/path.h lib/platform.h lib/standards.h lib/utils.h
-	$(CXX) ${INCLUDE_FOR_CLI} $(CPPFLAGS) $(CXXFLAGS) -c -o $@ cli/cppcheckexecutorseh.cpp
-
 cli/executor.o: cli/executor.cpp cli/executor.h lib/addoninfo.h lib/color.h lib/config.h lib/errorlogger.h lib/errortypes.h lib/library.h lib/mathlib.h lib/platform.h lib/settings.h lib/standards.h lib/suppressions.h lib/utils.h
 	$(CXX) ${INCLUDE_FOR_CLI} $(CPPFLAGS) $(CXXFLAGS) -c -o $@ cli/executor.cpp
 
@@ -673,6 +670,9 @@ cli/main.o: cli/main.cpp cli/cppcheckexecutor.h lib/config.h lib/errortypes.h li
 cli/processexecutor.o: cli/processexecutor.cpp cli/executor.h cli/processexecutor.h lib/addoninfo.h lib/check.h lib/color.h lib/config.h lib/cppcheck.h lib/errorlogger.h lib/errortypes.h lib/filesettings.h lib/library.h lib/mathlib.h lib/path.h lib/platform.h lib/settings.h lib/standards.h lib/suppressions.h lib/timer.h lib/utils.h
 	$(CXX) ${INCLUDE_FOR_CLI} $(CPPFLAGS) $(CXXFLAGS) -c -o $@ cli/processexecutor.cpp
 
+cli/sehwrapper.o: cli/sehwrapper.cpp cli/sehwrapper.h lib/config.h lib/utils.h
+	$(CXX) ${INCLUDE_FOR_CLI} $(CPPFLAGS) $(CXXFLAGS) -c -o $@ cli/sehwrapper.cpp
+
 cli/signalhandler.o: cli/signalhandler.cpp cli/signalhandler.h cli/stacktrace.h lib/config.h lib/utils.h
 	$(CXX) ${INCLUDE_FOR_CLI} $(CPPFLAGS) $(CXXFLAGS) -c -o $@ cli/signalhandler.cpp
 

cli/cli.vcxproj

@@ -223,10 +223,10 @@
   <ItemGroup Label="HeaderFiles">
     <ClInclude Include="cmdlineparser.h" />
     <ClInclude Include="cppcheckexecutor.h" />
-    <ClInclude Include="cppcheckexecutorseh.h" />
     <ClInclude Include="executor.h" />
     <ClInclude Include="filelister.h" />
     <ClInclude Include="processexecutor.h" />
+    <ClInclude Include="sehwrapper.h" />
     <ClInclude Include="signalhandler.h" />
     <ClInclude Include="singleexecutor.h" />
     <ClInclude Include="stacktrace.h" />
@@ -240,7 +240,6 @@
   <ItemGroup Label="SourceFiles">
     <ClCompile Include="cmdlineparser.cpp" />
     <ClCompile Include="cppcheckexecutor.cpp" />
-    <ClCompile Include="cppcheckexecutorseh.cpp" />
     <ClCompile Include="executor.cpp">
       <PrecompiledHeader Condition="'$(Configuration)|$(Platform)'=='Release|x64'">Create</PrecompiledHeader>
       <PrecompiledHeader Condition="'$(Configuration)|$(Platform)'=='Debug|x64'">Create</PrecompiledHeader>
@@ -250,6 +249,7 @@
     <ClCompile Include="filelister.cpp" />
     <ClCompile Include="main.cpp" />
     <ClCompile Include="processexecutor.cpp" />
+    <ClCompile Include="sehwrapper.cpp" />
     <ClCompile Include="signalhandler.cpp" />
     <ClCompile Include="singleexecutor.cpp" />
     <ClCompile Include="stacktrace.cpp" />

cli/cppcheckexecutor.cpp

@@ -65,7 +65,7 @@
 #endif
 
 #ifdef USE_WINDOWS_SEH
-#include "cppcheckexecutorseh.h"
+#include "sehwrapper.h"
 #endif
 
 #ifdef _WIN32
@@ -372,8 +372,9 @@ int CppCheckExecutor::check(int argc, const char* const argv[])
 int CppCheckExecutor::check_wrapper(const Settings& settings)
 {
 #ifdef USE_WINDOWS_SEH
-    if (settings.exceptionHandling)
-        return check_wrapper_seh(*this, &CppCheckExecutor::check_internal, settings);
+    if (settings.exceptionHandling) {
+        CALL_WITH_SEH_WRAPPER(check_internal(settings));
+    }
 #elif defined(USE_UNIX_SIGNAL_HANDLING)
     if (settings.exceptionHandling)
         register_signal_handler(settings.exceptionOutput);

cli/cppcheckexecutorseh.cpp → cli/sehwrapper.cpp

@@ -16,17 +16,23 @@
  * along with this program.  If not, see <http://www.gnu.org/licenses/>.
  */
 
-#include "cppcheckexecutorseh.h"
+#include "sehwrapper.h"
 
 #ifdef USE_WINDOWS_SEH
 
-#include "cppcheckexecutor.h"
 #include "utils.h"
 
 #include <windows.h>
 #include <dbghelp.h>
 #include <tchar.h>
 
+static FILE* sehOutput = stdout;
+
+void set_seh_output(FILE* f)
+{
+    sehOutput = f;
+}
+
 namespace {
     const ULONG maxnamelength = 512;
     struct IMAGEHLP_SYMBOL64_EXT : public IMAGEHLP_SYMBOL64 {
@@ -247,25 +253,17 @@ namespace {
         }
         fputc('\n', outputFile);
         printCallstack(outputFile, ex);
+        fputs("Please report this to the cppcheck developers!\n", outputFile);
         fflush(outputFile);
         return EXCEPTION_EXECUTE_HANDLER;
     }
 }
 
-/**
- * Signal/SEH handling
- * Has to be clean for using with SEH on windows, i.e. no construction of C++ object instances is allowed!
- * TODO Check for multi-threading issues!
- *
- */
-int check_wrapper_seh(CppCheckExecutor& executor, int (CppCheckExecutor::*f)(const Settings&) const, const Settings& settings)
+namespace internal
 {
-    FILE * const outputFile = settings.exceptionOutput;
-    __try {
-        return (&executor->*f)(settings);
-    } __except (filterException(outputFile, GetExceptionCode(), GetExceptionInformation())) {
-        fputs("Please report this to the cppcheck developers!\n", outputFile);
-        return -1;
+    int filter_seh_exeception(int code, void* ex)
+    {
+        return filterException(sehOutput, code, static_cast<PEXCEPTION_POINTERS>(ex));
     }
 }
 

cli/cppcheckexecutorseh.h → cli/sehwrapper.h

@@ -23,10 +23,28 @@
 
 #ifdef USE_WINDOWS_SEH
 
-class CppCheckExecutor;
-class Settings;
+#include <cstdio>
 
-int check_wrapper_seh(CppCheckExecutor& executor, int (CppCheckExecutor::*f)(const Settings&) const, const Settings& settings);
+/**
+ * @param f Output file
+ */
+void set_seh_output(FILE* f);
+
+namespace internal
+{
+    int filter_seh_exeception(int code, void* ex);
+}
+
+/**
+ * Signal/SEH handling
+ * Has to be clean for using with SEH on windows, i.e. no construction of C++ object instances is allowed!
+ */
+#define CALL_WITH_SEH_WRAPPER(f) \
+    __try {                      \
+        return (f);              \
+    } __except (internal::filter_seh_exeception(GetExceptionCode(), GetExceptionInformation())) { \
+        return -1; \
+    }
 
 #endif
 

test/CMakeLists.txt

@@ -1,5 +1,6 @@
 if (BUILD_TESTS)
     add_subdirectory(signal)
+    add_subdirectory(seh)
 
     file(GLOB hdrs "*.h")
     file(GLOB srcs "*.cpp")

test/seh/CMakeLists.txt

@@ -0,0 +1,4 @@
+add_executable(test-sehwrapper
+    test-sehwrapper.cpp
+    ${PROJECT_SOURCE_DIR}/cli/sehwrapper.cpp)
+target_include_directories(test-sehwrapper PRIVATE ${PROJECT_SOURCE_DIR}/cli ${PROJECT_SOURCE_DIR}/lib)

test/seh/test-sehwrapper.cpp

@@ -0,0 +1,86 @@
+/*
+ * Cppcheck - A tool for static C/C++ code analysis
+ * Copyright (C) 2007-2024 Cppcheck team.
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation, either version 3 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#include "config.h"
+
+#if defined(USE_WINDOWS_SEH)
+#include "sehwrapper.h"
+
+#include <cassert>
+#include <cfenv>
+#include <cstdlib>
+#include <cstring>
+
+#include <windows.h>
+
+static int my_assert()
+{
+    assert(false);
+    return 0;
+}
+
+static int my_abort()
+{
+    abort();
+}
+
+static int my_segv()
+{
+    // cppcheck-suppress nullPointer
+    ++*(int*)nullptr;
+    return 0;
+}
+
+// TODO: how to catch these?
+static int my_fpe()
+{
+    const int fpe_res = std::feraiseexcept(FE_ALL_EXCEPT);
+    if (fpe_res != 0)
+        return 11;
+    return sqrt(-1.0); // invalid operation
+}
+#endif
+
+int main(int argc, const char * const argv[])
+{
+#if defined(USE_WINDOWS_SEH)
+    if (argc != 2)
+        return 1;
+
+    if (strcmp(argv[1], "assert") == 0) {
+        _set_abort_behavior(0, _WRITE_ABORT_MSG); // suppress the "Debug Error!" MessageBox
+        CALL_WITH_SEH_WRAPPER(my_assert());
+    }
+    if (strcmp(argv[1], "abort") == 0) {
+        _set_abort_behavior(0, _WRITE_ABORT_MSG); // suppress the "Debug Error!" MessageBox
+        CALL_WITH_SEH_WRAPPER(my_abort());
+    }
+    if (strcmp(argv[1], "fpe") == 0) {
+        CALL_WITH_SEH_WRAPPER(my_fpe());
+    }
+    if (strcmp(argv[1], "segv") == 0) {
+        CALL_WITH_SEH_WRAPPER(my_segv());
+    }
+
+    return 0;
+#else
+    (void)argc;
+    (void)argv;
+    return 111;
+#endif
+}

test/seh/test-sehwrapper.py

@@ -0,0 +1,78 @@
+import subprocess
+import os
+import sys
+import pytest
+
+# TODO: only run on Windows
+
+def _lookup_cppcheck_exe(exe_name):
+    # path the script is located in
+    script_path = os.path.dirname(os.path.realpath(__file__))
+
+    if sys.platform == "win32":
+        exe_name += ".exe"
+
+    for base in (script_path + '/../../', './'):
+        for path in ('', 'bin/', 'bin/debug/'):
+            exe_path = base + path + exe_name
+            if os.path.isfile(exe_path):
+                print("using '{}'".format(exe_path))
+                return exe_path
+
+    return None
+
+def _call_process(arg):
+    exe = _lookup_cppcheck_exe('test-sehwrapper')
+    if exe is None:
+        raise Exception('executable not found')
+    p = subprocess.Popen([exe, arg], stdout=subprocess.PIPE, stderr=subprocess.PIPE)
+    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')
+    return p.returncode, stdout, stderr
+
+
+def test_assert():
+    exitcode, stdout, stderr = _call_process('assert')
+    assert stdout == ''
+    # Assertion failed: false, file S:\GitHub\cppcheck-fw\test\seh\test-sehwrapper.cpp, line 33
+    assert stderr.startswith("Assertion failed: false, file "), stderr
+    assert stderr.endswith("test-sehwrapper.cpp, line 33\n"), stderr
+    assert exitcode == 3
+
+
+def test_abort():
+    exitcode, stdout, stderr = _call_process('abort')
+    # nothing is written in case of abort()
+    # it will show the "Debug Error!" MessageBox though which we suppress in the test
+    assert stdout == ''
+    assert stderr == ''
+    assert exitcode == 3
+
+
+def test_segv():
+    exitcode, stdout, stderr = _call_process('segv')
+    assert stderr == ''
+    lines = stdout.splitlines()
+    assert lines[0].startswith('Internal error: Access violation (instruction: 0x'), lines[0]
+    assert lines[0].endswith(') reading from 0x0000000000000000'), lines[0]
+    assert lines[1].startswith('0. 0x'), lines[1]
+    assert lines[1].endswith(' in my_segv'), lines[1]
+    assert lines[2].startswith('1. 0x'), lines[2]
+    assert lines[2].endswith(' in main'), lines[2]
+    assert lines[len(lines)-1] == 'Please report this to the cppcheck developers!'
+    assert exitcode == 4294967295  # returns -1
+
+
+# TODO: make this work
+@pytest.mark.skip
+def test_fpe():
+    exitcode, stdout, stderr = _call_process('fpe')
+    assert stderr == ''
+    lines = stdout.splitlines()
+    assert lines[0].startswith('Internal error: cppcheck received signal SIGFPE - FPE_FLTDIV (at 0x7f'), lines[0]
+    assert lines[0].endswith(').'), lines[0]
+    assert lines[1] == 'Callstack:'
+    assert lines[2].endswith('my_fpe()'), lines[2]
+    assert lines[len(lines)-1] == 'Please report this to the cppcheck developers!'
+    assert exitcode == 4294967295  # returns -1