Skip to content

Commit f8aad8a

Browse files
firewavefirewave
authored
cleaned up signal handler and improved stack trace generation (#5581)
See #5540 (review) for related discussion.
1 parent 8c90edb commit f8aad8a

18 files changed

Lines changed: 400 additions & 91 deletions

‎.github/workflows/CI-unixish.yml‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -418,6 +418,22 @@ jobs:
418418
run: |
419419
make -j$(nproc) checkCWEEntries validateXML
420420
421+
- name: Test Signalhandler
422+
run: |
423+
cmake -S . -B cmake.output.signal -G "Unix Makefiles" -DBUILD_TESTS=On
424+
cmake --build cmake.output.signal --target test-signalhandler -- -j$(nproc)
425+
cp cmake.output.signal/bin/test-s* .
426+
python3 -m pytest -Werror --strict-markers -vv test/signal/test-signalhandler.py
427+
428+
# no unix backtrace support on MacOs
429+
- name: Test Stacktrace
430+
if: contains(matrix.os, 'ubuntu')
431+
run: |
432+
cmake -S . -B cmake.output.signal -G "Unix Makefiles" -DBUILD_TESTS=On
433+
cmake --build cmake.output.signal --target test-stacktrace -- -j$(nproc)
434+
cp cmake.output.signal/bin/test-s* .
435+
python3 -m pytest -Werror --strict-markers -vv test/signal/test-stacktrace.py
436+
421437
# TODO: move to scriptcheck.yml so these are tested with all Python versions?
422438
- name: Test addons
423439
run: |

‎Makefile‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -258,11 +258,11 @@ EXTOBJ = externals/simplecpp/simplecpp.o \
258258
CLIOBJ = cli/cmdlineparser.o \
259259
cli/cppcheckexecutor.o \
260260
cli/cppcheckexecutorseh.o \
261-
cli/cppcheckexecutorsig.o \
262261
cli/executor.o \
263262
cli/filelister.o \
264263
cli/main.o \
265264
cli/processexecutor.o \
265+
cli/signalhandler.o \
266266
cli/singleexecutor.o \
267267
cli/stacktrace.o \
268268
cli/threadexecutor.o
@@ -348,7 +348,7 @@ cppcheck: $(EXTOBJ) $(LIBOBJ) $(CLIOBJ)
348348

349349
all: cppcheck testrunner
350350

351-
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/cppcheckexecutorsig.o cli/stacktrace.o cli/filelister.o
351+
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
352352
$(CXX) $(CPPFLAGS) $(CXXFLAGS) -o $@ $^ $(LIBS) $(LDFLAGS) $(RDYNAMIC)
353353

354354
test: all
@@ -649,15 +649,12 @@ $(libcppdir)/vfvalue.o: lib/vfvalue.cpp lib/config.h lib/errortypes.h lib/mathli
649649
cli/cmdlineparser.o: cli/cmdlineparser.cpp cli/cmdlinelogger.h cli/cmdlineparser.h cli/cppcheckexecutor.h cli/filelister.h externals/tinyxml2/tinyxml2.h lib/addoninfo.h lib/analyzerinfo.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
650650
$(CXX) ${INCLUDE_FOR_CLI} $(CPPFLAGS) $(CXXFLAGS) -c -o $@ cli/cmdlineparser.cpp
651651

652-
cli/cppcheckexecutor.o: cli/cppcheckexecutor.cpp cli/cmdlinelogger.h cli/cmdlineparser.h cli/cppcheckexecutor.h cli/cppcheckexecutorseh.h cli/cppcheckexecutorsig.h cli/executor.h cli/processexecutor.h cli/singleexecutor.h cli/threadexecutor.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/library.h lib/mathlib.h lib/platform.h lib/settings.h lib/standards.h lib/suppressions.h lib/utils.h
652+
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 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/library.h lib/mathlib.h lib/platform.h lib/settings.h lib/standards.h lib/suppressions.h lib/utils.h
653653
$(CXX) ${INCLUDE_FOR_CLI} $(CPPFLAGS) $(CXXFLAGS) -c -o $@ cli/cppcheckexecutor.cpp
654654

655655
cli/cppcheckexecutorseh.o: cli/cppcheckexecutorseh.cpp cli/cppcheckexecutor.h cli/cppcheckexecutorseh.h lib/config.h lib/filesettings.h lib/platform.h lib/standards.h lib/utils.h
656656
$(CXX) ${INCLUDE_FOR_CLI} $(CPPFLAGS) $(CXXFLAGS) -c -o $@ cli/cppcheckexecutorseh.cpp
657657

658-
cli/cppcheckexecutorsig.o: cli/cppcheckexecutorsig.cpp cli/cppcheckexecutor.h cli/cppcheckexecutorsig.h cli/stacktrace.h lib/config.h lib/filesettings.h lib/platform.h lib/standards.h lib/utils.h
659-
$(CXX) ${INCLUDE_FOR_CLI} $(CPPFLAGS) $(CXXFLAGS) -c -o $@ cli/cppcheckexecutorsig.cpp
660-
661658
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
662659
$(CXX) ${INCLUDE_FOR_CLI} $(CPPFLAGS) $(CXXFLAGS) -c -o $@ cli/executor.cpp
663660

@@ -670,6 +667,9 @@ cli/main.o: cli/main.cpp cli/cppcheckexecutor.h lib/config.h lib/errortypes.h li
670667
cli/processexecutor.o: cli/processexecutor.cpp cli/executor.h cli/processexecutor.h lib/addoninfo.h lib/analyzerinfo.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/platform.h lib/settings.h lib/standards.h lib/suppressions.h lib/timer.h lib/utils.h
671668
$(CXX) ${INCLUDE_FOR_CLI} $(CPPFLAGS) $(CXXFLAGS) -c -o $@ cli/processexecutor.cpp
672669

670+
cli/signalhandler.o: cli/signalhandler.cpp cli/signalhandler.h cli/stacktrace.h lib/config.h
671+
$(CXX) ${INCLUDE_FOR_CLI} $(CPPFLAGS) $(CXXFLAGS) -c -o $@ cli/signalhandler.cpp
672+
673673
cli/singleexecutor.o: cli/singleexecutor.cpp cli/executor.h cli/singleexecutor.h lib/addoninfo.h lib/analyzerinfo.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/platform.h lib/settings.h lib/standards.h lib/suppressions.h lib/timer.h lib/utils.h
674674
$(CXX) ${INCLUDE_FOR_CLI} $(CPPFLAGS) $(CXXFLAGS) -c -o $@ cli/singleexecutor.cpp
675675

‎cli/cli.vcxproj‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -224,10 +224,10 @@
224224
<ClInclude Include="cmdlineparser.h" />
225225
<ClInclude Include="cppcheckexecutor.h" />
226226
<ClInclude Include="cppcheckexecutorseh.h" />
227-
<ClInclude Include="cppcheckexecutorsig.h" />
228227
<ClInclude Include="executor.h" />
229228
<ClInclude Include="filelister.h" />
230229
<ClInclude Include="processexecutor.h" />
230+
<ClInclude Include="signalhandler.h" />
231231
<ClInclude Include="singleexecutor.h" />
232232
<ClInclude Include="stacktrace.h" />
233233
<ClInclude Include="threadexecutor.h" />
@@ -241,7 +241,6 @@
241241
<ClCompile Include="cmdlineparser.cpp" />
242242
<ClCompile Include="cppcheckexecutor.cpp" />
243243
<ClCompile Include="cppcheckexecutorseh.cpp" />
244-
<ClCompile Include="cppcheckexecutorsig.cpp" />
245244
<ClCompile Include="executor.cpp">
246245
<PrecompiledHeader Condition="'$(Configuration)|$(Platform)'=='Release|x64'">Create</PrecompiledHeader>
247246
<PrecompiledHeader Condition="'$(Configuration)|$(Platform)'=='Debug|x64'">Create</PrecompiledHeader>
@@ -251,6 +250,7 @@
251250
<ClCompile Include="filelister.cpp" />
252251
<ClCompile Include="main.cpp" />
253252
<ClCompile Include="processexecutor.cpp" />
253+
<ClCompile Include="signalhandler.cpp" />
254254
<ClCompile Include="singleexecutor.cpp" />
255255
<ClCompile Include="stacktrace.cpp" />
256256
<ClCompile Include="threadexecutor.cpp" />

‎cli/cli.vcxproj.filters‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@
2626
<ClInclude Include="cppcheckexecutorseh.h">
2727
<Filter>Header Files</Filter>
2828
</ClInclude>
29-
<ClInclude Include="cppcheckexecutorsig.h">
29+
<ClInclude Include="signalhandler.h">
3030
<Filter>Header Files</Filter>
3131
</ClInclude>
3232
<ClInclude Include="executor.h">
@@ -58,7 +58,7 @@
5858
<ClCompile Include="cppcheckexecutorseh.cpp">
5959
<Filter>Source Files</Filter>
6060
</ClCompile>
61-
<ClCompile Include="cppcheckexecutorsig.cpp">
61+
<ClCompile Include="signalhandler.cpp">
6262
<Filter>Source Files</Filter>
6363
</ClCompile>
6464
<ClCompile Include="executor.cpp">

‎cli/cppcheckexecutor.cpp‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,7 @@
5353
#include <vector>
5454

5555
#ifdef USE_UNIX_SIGNAL_HANDLING
56-
#include "cppcheckexecutorsig.h"
56+
#include "signalhandler.h"
5757
#endif
5858

5959
#ifdef USE_WINDOWS_SEH
@@ -207,7 +207,7 @@ int CppCheckExecutor::check_wrapper(const Settings& settings)
207207
return check_wrapper_seh(*this, &CppCheckExecutor::check_internal, settings);
208208
#elif defined(USE_UNIX_SIGNAL_HANDLING)
209209
if (settings.exceptionHandling)
210-
return check_wrapper_sig(*this, &CppCheckExecutor::check_internal, settings);
210+
register_signal_handler();
211211
#endif
212212
return check_internal(settings);
213213
}
@@ -443,8 +443,12 @@ void StdLogger::reportErr(const ErrorMessage &msg)
443443
void CppCheckExecutor::setExceptionOutput(FILE* exceptionOutput)
444444
{
445445
mExceptionOutput = exceptionOutput;
446+
#if defined(USE_UNIX_SIGNAL_HANDLING)
447+
set_signal_handler_output(mExceptionOutput);
448+
#endif
446449
}
447450

451+
// cppcheck-suppress unusedFunction - only used by USE_WINDOWS_SEH code
448452
FILE* CppCheckExecutor::getExceptionOutput()
449453
{
450454
return mExceptionOutput;

cli/cppcheckexecutorsig.cpp renamed to cli/signalhandler.cpp

Lines changed: 21 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -16,12 +16,10 @@
1616
* along with this program. If not, see <http://www.gnu.org/licenses/>.
1717
*/
1818

19-
#include "cppcheckexecutorsig.h"
19+
#include "signalhandler.h"
2020

2121
#if defined(USE_UNIX_SIGNAL_HANDLING)
2222

23-
#include "cppcheckexecutor.h"
24-
2523
#ifdef USE_UNIX_BACKTRACE_SUPPORT
2624
#include "stacktrace.h"
2725
#endif
@@ -57,6 +55,12 @@ static constexpr size_t MYSTACKSIZE = 16*1024+SIGSTKSZ; // wild guess about a re
5755
#endif
5856
static char mytstack[MYSTACKSIZE]= {0}; // alternative stack for signal handler
5957
static bool bStackBelowHeap=false; // lame attempt to locate heap vs. stack address space. See CppCheckExecutor::check_wrapper()
58+
static FILE* signalOutput = stdout;
59+
60+
void set_signal_handler_output(FILE* f)
61+
{
62+
signalOutput = f;
63+
}
6064

6165
/**
6266
* \param[in] ptr address to be examined.
@@ -121,27 +125,21 @@ static void CppcheckSignalHandler(int signo, siginfo_t * info, void * context)
121125

122126
const Signalmap_t::const_iterator it=listofsignals.find(signo);
123127
const char * const signame = (it==listofsignals.end()) ? "unknown" : it->second.c_str();
124-
#ifdef USE_UNIX_BACKTRACE_SUPPORT
125-
bool lowMem=false; // was low-memory condition detected? Be careful then! Avoid allocating much more memory then.
126-
#endif
127128
bool unexpectedSignal=true; // unexpected indicates program failure
128129
bool terminate=true; // exit process/thread
129130
const bool isAddressOnStack = IsAddressOnStack(info->si_addr);
130-
FILE* output = CppCheckExecutor::getExceptionOutput();
131+
FILE * const output = signalOutput;
131132
switch (signo) {
132133
case SIGABRT:
133134
fputs("Internal error: cppcheck received signal ", output);
134135
fputs(signame, output);
135136
fputs(
136137
#ifdef NDEBUG
137-
" - out of memory?\n",
138+
" - abort\n",
138139
#else
139-
" - out of memory or assertion?\n",
140+
" - abort or assertion\n",
140141
#endif
141142
output);
142-
#ifdef USE_UNIX_BACKTRACE_SUPPORT
143-
lowMem=true; // educated guess
144-
#endif
145143
break;
146144
case SIGBUS:
147145
fputs("Internal error: cppcheck received signal ", output);
@@ -281,7 +279,9 @@ static void CppcheckSignalHandler(int signo, siginfo_t * info, void * context)
281279
break;
282280
}
283281
#ifdef USE_UNIX_BACKTRACE_SUPPORT
284-
print_stacktrace(output, true, -1, lowMem);
282+
// flush otherwise the trace might be printed earlier
283+
fflush(output);
284+
print_stacktrace(output, 1, true, -1, true);
285285
#endif
286286
if (unexpectedSignal) {
287287
fputs("\nPlease report this to the cppcheck developers!\n", output);
@@ -298,8 +298,10 @@ static void CppcheckSignalHandler(int signo, siginfo_t * info, void * context)
298298
}
299299
}
300300

301-
int check_wrapper_sig(CppCheckExecutor& executor, int (CppCheckExecutor::*f)(const Settings&) const, const Settings& settings)
301+
void register_signal_handler()
302302
{
303+
FILE * const output = signalOutput;
304+
303305
// determine stack vs. heap
304306
char stackVariable;
305307
char *heapVariable=static_cast<char*>(malloc(1));
@@ -311,7 +313,11 @@ int check_wrapper_sig(CppCheckExecutor& executor, int (CppCheckExecutor::*f)(con
311313
segv_stack.ss_sp = mytstack;
312314
segv_stack.ss_flags = 0;
313315
segv_stack.ss_size = MYSTACKSIZE;
314-
sigaltstack(&segv_stack, nullptr);
316+
if (sigaltstack(&segv_stack, nullptr) != 0) {
317+
// TODO: log errno
318+
fputs("could not set alternate signal stack context.\n", output);
319+
std::exit(EXIT_FAILURE);
320+
}
315321

316322
// install signal handler
317323
struct sigaction act;
@@ -321,7 +327,6 @@ int check_wrapper_sig(CppCheckExecutor& executor, int (CppCheckExecutor::*f)(con
321327
for (std::map<int, std::string>::const_iterator sig=listofsignals.cbegin(); sig!=listofsignals.cend(); ++sig) {
322328
sigaction(sig->first, &act, nullptr);
323329
}
324-
return (executor.*f)(settings);
325330
}
326331

327332
#endif

cli/cppcheckexecutorsig.h renamed to cli/signalhandler.h

Lines changed: 9 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -16,18 +16,20 @@
1616
* along with this program. If not, see <http://www.gnu.org/licenses/>.
1717
*/
1818

19-
#ifndef CPPCHECKEXECUTORSIG_H
20-
#define CPPCHECKEXECUTORSIG_H
19+
#ifndef SIGNALHANDLER_H
20+
#define SIGNALHANDLER_H
2121

2222
#include "config.h"
2323

2424
#if defined(USE_UNIX_SIGNAL_HANDLING)
2525

26-
class CppCheckExecutor;
27-
class Settings;
26+
/**
27+
* @param f Output file
28+
*/
29+
void set_signal_handler_output(FILE* f);
2830

29-
int check_wrapper_sig(CppCheckExecutor& executor, int (CppCheckExecutor::*f)(const Settings&) const, const Settings& settings);
31+
void register_signal_handler();
3032

31-
#endif // CPPCHECKEXECUTORSIG_H
33+
#endif // USE_UNIX_SIGNAL_HANDLING
3234

33-
#endif // CPPCHECKEXECUTORSIG_H
35+
#endif // SIGNALHANDLER_H

0 commit comments

Comments
 (0)