Skip to content

Commit

Permalink
refactored SEH wrapper and added unit test (#7145)
Browse files Browse the repository at this point in the history
  • Loading branch information
firewave authored Jan 2, 2025
1 parent a3cb101 commit 9d34327
Show file tree
Hide file tree
Showing 13 changed files with 251 additions and 43 deletions.
11 changes: 11 additions & 0 deletions .github/workflows/CI-windows.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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: |
Expand Down
12 changes: 6 additions & 6 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -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 \
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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

Expand All @@ -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

Expand Down
4 changes: 2 additions & 2 deletions cli/cli.vcxproj
Original file line number Diff line number Diff line change
Expand Up @@ -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" />
Expand All @@ -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>
Expand All @@ -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" />
Expand Down
7 changes: 4 additions & 3 deletions cli/cppcheckexecutor.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@
#endif

#ifdef USE_WINDOWS_SEH
#include "cppcheckexecutorseh.h"
#include "sehwrapper.h"
#endif

#ifdef _WIN32
Expand Down Expand Up @@ -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);
Expand Down
28 changes: 13 additions & 15 deletions cli/cppcheckexecutorseh.cpp → cli/sehwrapper.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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));
}
}

Expand Down
24 changes: 21 additions & 3 deletions cli/cppcheckexecutorseh.h → cli/sehwrapper.h
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
1 change: 1 addition & 0 deletions test/CMakeLists.txt
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
if (BUILD_TESTS)
add_subdirectory(signal)
add_subdirectory(seh)

file(GLOB hdrs "*.h")
file(GLOB srcs "*.cpp")
Expand Down
4 changes: 4 additions & 0 deletions test/seh/CMakeLists.txt
Original file line number Diff line number Diff line change
@@ -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)
86 changes: 86 additions & 0 deletions test/seh/test-sehwrapper.cpp
Original file line number Diff line number Diff line change
@@ -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
}
78 changes: 78 additions & 0 deletions test/seh/test-sehwrapper.py
Original file line number Diff line number Diff line change
@@ -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
Loading

0 comments on commit 9d34327

Please sign in to comment.