Skip to content

Commit

Permalink
added separate program to test signal handler / cleaned up and improv…
Browse files Browse the repository at this point in the history
…ed stacktrace generation
  • Loading branch information
firewave committed Jan 5, 2024
1 parent e82776f commit a678b81
Show file tree
Hide file tree
Showing 8 changed files with 220 additions and 60 deletions.
7 changes: 7 additions & 0 deletions .github/workflows/CI-unixish.yml
Original file line number Diff line number Diff line change
Expand Up @@ -418,6 +418,13 @@ jobs:
run: |
make -j$(nproc) checkCWEEntries validateXML
- name: Test Signalhandler
run: |
cmake -S . -B cmake.output.signal -G "Unix Makefiles" -DBUILD_TESTS=On
cmake --build cmake.output.signal --target test-signalhandler -- -j$(nproc)
cp cmake.output.signal/bin/test-s* .
python3 -m pytest -Werror --strict-markers -vv test/signal/test-*.py
# TODO: move to scriptcheck.yml so these are tested with all Python versions?
- name: Test addons
run: |
Expand Down
22 changes: 12 additions & 10 deletions cli/signalhandler.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -125,9 +125,6 @@ static void CppcheckSignalHandler(int signo, siginfo_t * info, void * context)

const Signalmap_t::const_iterator it=listofsignals.find(signo);
const char * const signame = (it==listofsignals.end()) ? "unknown" : it->second.c_str();
#ifdef USE_UNIX_BACKTRACE_SUPPORT
bool lowMem=false; // was low-memory condition detected? Be careful then! Avoid allocating much more memory then.
#endif
bool unexpectedSignal=true; // unexpected indicates program failure
bool terminate=true; // exit process/thread
const bool isAddressOnStack = IsAddressOnStack(info->si_addr);
Expand All @@ -138,14 +135,11 @@ static void CppcheckSignalHandler(int signo, siginfo_t * info, void * context)
fputs(signame, output);
fputs(
#ifdef NDEBUG
" - out of memory?\n",
" - abort\n",
#else
" - out of memory or assertion?\n",
" - abort or assertion\n",
#endif
output);
#ifdef USE_UNIX_BACKTRACE_SUPPORT
lowMem=true; // educated guess
#endif
break;
case SIGBUS:
fputs("Internal error: cppcheck received signal ", output);
Expand Down Expand Up @@ -285,7 +279,9 @@ static void CppcheckSignalHandler(int signo, siginfo_t * info, void * context)
break;
}
#ifdef USE_UNIX_BACKTRACE_SUPPORT
print_stacktrace(output, true, -1, lowMem);
// flush otherwise the trace might be printed earlier
fflush(output);
print_stacktrace(output, 1, true, -1);
#endif
if (unexpectedSignal) {
fputs("\nPlease report this to the cppcheck developers!\n", output);
Expand All @@ -304,6 +300,8 @@ static void CppcheckSignalHandler(int signo, siginfo_t * info, void * context)

void register_signal_handler()
{
FILE * const output = signalOutput;

// determine stack vs. heap
char stackVariable;
char *heapVariable=static_cast<char*>(malloc(1));
Expand All @@ -315,7 +313,11 @@ void register_signal_handler()
segv_stack.ss_sp = mytstack;
segv_stack.ss_flags = 0;
segv_stack.ss_size = MYSTACKSIZE;
sigaltstack(&segv_stack, nullptr);
if (sigaltstack(&segv_stack, nullptr) != 0) {
// TODO: log errno
fputs("could not set alternate signal stack context.\n", output);
std::exit(EXIT_FAILURE);
}

// install signal handler
struct sigaction act;
Expand Down
100 changes: 51 additions & 49 deletions cli/stacktrace.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -28,68 +28,70 @@
#include <cxxabi.h>
#include <execinfo.h>

void print_stacktrace(FILE* output, bool demangling, int maxdepth, bool lowMem)
void print_stacktrace(FILE* output, int start_idx, bool demangling, int maxdepth)
{
// 32 vs. 64bit
#define ADDRESSDISPLAYLENGTH ((sizeof(long)==8)?12:8)
const int fd = fileno(output);
void *callstackArray[32]= {nullptr}; // the less resources the better...
const int currentdepth = backtrace(callstackArray, (int)getArrayLength(callstackArray));
constexpr int offset=2; // some entries on top are within our own exception handling code or libc
// set offset to 1 to omit the printing function itself
const int offset=start_idx+1; // some entries on top are within our own exception handling code or libc
if (maxdepth<0)
maxdepth=currentdepth-offset;
else
maxdepth = std::min(maxdepth, currentdepth);
if (lowMem) {
fputs("Callstack (symbols only):\n", output);
backtrace_symbols_fd(callstackArray+offset, maxdepth, fd);
} else {
char **symbolStringList = backtrace_symbols(callstackArray, currentdepth);
if (symbolStringList) {
fputs("Callstack:\n", output);
char demangle_buffer[2048]= {0};
for (int i = offset; i < maxdepth; ++i) {
const char * const symbolString = symbolStringList[i];
char * realnameString = nullptr;
const char * const firstBracketName = strchr(symbolString, '(');
const char * const firstBracketAddress = strchr(symbolString, '[');
const char * const secondBracketAddress = strchr(firstBracketAddress, ']');
const char * const beginAddress = firstBracketAddress+3;
const int addressLen = int(secondBracketAddress-beginAddress);
const int padLen = int(ADDRESSDISPLAYLENGTH-addressLen);
if (demangling && firstBracketName) {
const char * const plus = strchr(firstBracketName, '+');
if (plus && (plus>(firstBracketName+1))) {
char input_buffer[1024]= {0};
strncpy(input_buffer, firstBracketName+1, plus-firstBracketName-1);
size_t length = getArrayLength(demangle_buffer);
int status=0;
// We're violating the specification - passing stack address instead of malloc'ed heap.
// Benefit is that no further heap is required, while there is sufficient stack...
realnameString = abi::__cxa_demangle(input_buffer, demangle_buffer, &length, &status); // non-NULL on success
}
}
const int ordinal=i-offset;
fprintf(output, "#%-2d 0x",
ordinal);
if (padLen>0)
fprintf(output, "%0*d",
padLen, 0);
if (realnameString) {
fprintf(output, "%.*s in %s\n",
(int)(secondBracketAddress-firstBracketAddress-3), firstBracketAddress+3,
realnameString);
} else {
fprintf(output, "%.*s in %.*s\n",
(int)(secondBracketAddress-firstBracketAddress-3), firstBracketAddress+3,
(int)(firstBracketAddress-symbolString), symbolString);
}

char **symbolStringList = backtrace_symbols(callstackArray, currentdepth);
if (!symbolStringList) {
fputs("Callstack could not be obtained\n", output);
return;
}

fputs("Callstack:\n", output);
bool own_code = false;
char demangle_buffer[2048]= {0};
for (int i = offset; i < maxdepth; ++i) {
const char * const symbolString = symbolStringList[i];
// skip all leading libc symbols so the first symbol is our code
if (!own_code && strstr(symbolString, "/libc.so.6") != nullptr)
continue;
own_code = true;
char * realnameString = nullptr;
const char * const firstBracketName = strchr(symbolString, '(');
const char * const firstBracketAddress = strchr(symbolString, '[');
const char * const secondBracketAddress = strchr(firstBracketAddress, ']');
const char * const beginAddress = firstBracketAddress+3;
const int addressLen = int(secondBracketAddress-beginAddress);
const int padLen = int(ADDRESSDISPLAYLENGTH-addressLen);
if (demangling && firstBracketName) {
const char * const plus = strchr(firstBracketName, '+');
if (plus && (plus>(firstBracketName+1))) {
char input_buffer[1024]= {0};
strncpy(input_buffer, firstBracketName+1, plus-firstBracketName-1);
size_t length = getArrayLength(demangle_buffer);
int status=0;
// We're violating the specification - passing stack address instead of malloc'ed heap.
// Benefit is that no further heap is required, while there is sufficient stack...
realnameString = abi::__cxa_demangle(input_buffer, demangle_buffer, &length, &status); // non-NULL on success
}
free(symbolStringList);
}
const int ordinal=i-offset;
fprintf(output, "#%-2d 0x",
ordinal);
if (padLen>0)
fprintf(output, "%0*d",
padLen, 0);
if (realnameString) {
fprintf(output, "%.*s in %s\n",
(int)(secondBracketAddress-firstBracketAddress-3), firstBracketAddress+3,
realnameString);
} else {
fputs("Callstack could not be obtained\n", output);
fprintf(output, "%.*s in %.*s\n",
(int)(secondBracketAddress-firstBracketAddress-3), firstBracketAddress+3,
(int)(firstBracketAddress-symbolString), symbolString);
}
}
free(symbolStringList);
#undef ADDRESSDISPLAYLENGTH
}

Expand Down
2 changes: 1 addition & 1 deletion cli/stacktrace.h
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@
* The code is not meant for production environment!
* One reason is named first: it's using functions not whitelisted for usage in a signal handler function.
*/
void print_stacktrace(FILE* output, bool demangling, int maxdepth, bool lowMem);
void print_stacktrace(FILE* output, int start_idx, bool demangling, int maxdepth);

#endif

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

file(GLOB hdrs "*.h")
file(GLOB srcs "*.cpp")
list(APPEND testrunner_SOURCES ${hdrs} ${srcs} $<TARGET_OBJECTS:cli_objs>)
Expand Down
10 changes: 10 additions & 0 deletions test/signal/CMakeLists.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
add_executable(test-signalhandler
test-signalhandler.cpp
${PROJECT_SOURCE_DIR}/cli/signalhandler.cpp
${PROJECT_SOURCE_DIR}/cli/stacktrace.cpp)
target_include_directories(test-signalhandler PRIVATE ${PROJECT_SOURCE_DIR}/cli ${PROJECT_SOURCE_DIR}/lib)
# names for static functions are omitted from trace
target_compile_options_safe(test-signalhandler -Wno-missing-declarations)
target_compile_options_safe(test-signalhandler -Wno-missing-prototypes)
# required for backtrace() to produce function names
target_link_options(test-signalhandler PRIVATE -rdynamic)
65 changes: 65 additions & 0 deletions test/signal/test-signalhandler.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,65 @@
/*
* 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 "signalhandler.h"

#include <cassert>
#include <cfenv>
#include <cstring>

NORETURN void my_assert()
{
assert(false);
}

NORETURN void my_abort()
{
abort();
}

void my_segv()
{
// cppcheck-suppress nullPointer
++*(int*)nullptr;
}

void my_fpe()
{
feenableexcept(FE_ALL_EXCEPT); // TODO: check result
std::feraiseexcept(FE_UNDERFLOW | FE_DIVBYZERO); // TODO: check result
// TODO: to generate this via code
}

int main(int argc, const char * const argv[])
{
if (argc != 2)
return 1;

register_signal_handler();

if (strcmp(argv[1], "assert") == 0)
my_assert();
else if (strcmp(argv[1], "abort") == 0)
my_abort();
else if (strcmp(argv[1], "fpe") == 0)
my_fpe();
else if (strcmp(argv[1], "segv") == 0)
my_segv();

return 0;
}
72 changes: 72 additions & 0 deletions test/signal/test-signalhandler.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,72 @@
import subprocess
import os
import sys
import pytest

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-signalhandler')
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 stderr.endswith("test-signalhandler.cpp:27: void my_assert(): Assertion `false' failed.\n"), stderr
lines = stdout.splitlines()
assert lines[0] == 'Internal error: cppcheck received signal SIGABRT - abort or assertion'
assert lines[1] == 'Callstack:'
assert lines[2].endswith('my_abort()'), lines[2] # TODO: wrong function
assert lines[len(lines)-1] == 'Please report this to the cppcheck developers!'


def test_abort():
exitcode, stdout, stderr = _call_process('abort')
lines = stdout.splitlines()
assert lines[0] == 'Internal error: cppcheck received signal SIGABRT - abort or assertion'
assert lines[1] == 'Callstack:'
assert lines[2].endswith('my_segv()'), lines[2] # TODO: wrong function
assert lines[len(lines)-1] == 'Please report this to the cppcheck developers!'


def test_segv():
exitcode, stdout, stderr = _call_process('segv')
assert stderr == ''
lines = stdout.splitlines()
assert lines[0] == 'Internal error: cppcheck received signal SIGSEGV - SEGV_MAPERR (reading at 0x0).'
assert lines[1] == 'Callstack:'
assert lines[2].endswith('my_segv()'), lines[2] # TODO: wrong function
assert lines[len(lines)-1] == 'Please report this to the cppcheck developers!'


# 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!'

0 comments on commit a678b81

Please sign in to comment.