cleaned up signal handler and improved stack trace generation (#5581)

See
#5540 (review)
for related discussion.

.github/workflows/CI-unixish.yml

@@ -418,6 +418,22 @@ 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-signalhandler.py
+
+      # no unix backtrace support on MacOs
+      - name: Test Stacktrace
+        if: contains(matrix.os, 'ubuntu')
+        run: |
+          cmake -S . -B cmake.output.signal -G "Unix Makefiles" -DBUILD_TESTS=On
+          cmake --build cmake.output.signal --target test-stacktrace -- -j$(nproc)
+          cp cmake.output.signal/bin/test-s* .
+          python3 -m pytest -Werror --strict-markers -vv test/signal/test-stacktrace.py
+
       # TODO: move to scriptcheck.yml so these are tested with all Python versions?
       - name: Test addons
         run: |

Makefile

@@ -258,11 +258,11 @@ EXTOBJ =      externals/simplecpp/simplecpp.o \
 CLIOBJ =      cli/cmdlineparser.o \
               cli/cppcheckexecutor.o \
               cli/cppcheckexecutorseh.o \
-              cli/cppcheckexecutorsig.o \
               cli/executor.o \
               cli/filelister.o \
               cli/main.o \
               cli/processexecutor.o \
+              cli/signalhandler.o \
               cli/singleexecutor.o \
               cli/stacktrace.o \
               cli/threadexecutor.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/cppcheckexecutorsig.o cli/stacktrace.o cli/filelister.o
+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
 	$(CXX) $(CPPFLAGS) $(CXXFLAGS) -o $@ $^ $(LIBS) $(LDFLAGS) $(RDYNAMIC)
 
 test:	all
@@ -649,15 +649,12 @@ $(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/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
 	$(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/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
+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
 	$(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/platform.h lib/standards.h lib/utils.h
 	$(CXX) ${INCLUDE_FOR_CLI} $(CPPFLAGS) $(CXXFLAGS) -c -o $@ cli/cppcheckexecutorseh.cpp
 
-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
-	$(CXX) ${INCLUDE_FOR_CLI} $(CPPFLAGS) $(CXXFLAGS) -c -o $@ cli/cppcheckexecutorsig.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
 
@@ -670,6 +667,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/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
 	$(CXX) ${INCLUDE_FOR_CLI} $(CPPFLAGS) $(CXXFLAGS) -c -o $@ cli/processexecutor.cpp
 
+cli/signalhandler.o: cli/signalhandler.cpp cli/signalhandler.h cli/stacktrace.h lib/config.h
+	$(CXX) ${INCLUDE_FOR_CLI} $(CPPFLAGS) $(CXXFLAGS) -c -o $@ cli/signalhandler.cpp
+
 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
 	$(CXX) ${INCLUDE_FOR_CLI} $(CPPFLAGS) $(CXXFLAGS) -c -o $@ cli/singleexecutor.cpp
 

cli/cli.vcxproj

@@ -224,10 +224,10 @@
     <ClInclude Include="cmdlineparser.h" />
     <ClInclude Include="cppcheckexecutor.h" />
     <ClInclude Include="cppcheckexecutorseh.h" />
-    <ClInclude Include="cppcheckexecutorsig.h" />
     <ClInclude Include="executor.h" />
     <ClInclude Include="filelister.h" />
     <ClInclude Include="processexecutor.h" />
+    <ClInclude Include="signalhandler.h" />
     <ClInclude Include="singleexecutor.h" />
     <ClInclude Include="stacktrace.h" />
     <ClInclude Include="threadexecutor.h" />
@@ -241,7 +241,6 @@
     <ClCompile Include="cmdlineparser.cpp" />
     <ClCompile Include="cppcheckexecutor.cpp" />
     <ClCompile Include="cppcheckexecutorseh.cpp" />
-    <ClCompile Include="cppcheckexecutorsig.cpp" />
     <ClCompile Include="executor.cpp">
       <PrecompiledHeader Condition="'$(Configuration)|$(Platform)'=='Release|x64'">Create</PrecompiledHeader>
       <PrecompiledHeader Condition="'$(Configuration)|$(Platform)'=='Debug|x64'">Create</PrecompiledHeader>
@@ -251,6 +250,7 @@
     <ClCompile Include="filelister.cpp" />
     <ClCompile Include="main.cpp" />
     <ClCompile Include="processexecutor.cpp" />
+    <ClCompile Include="signalhandler.cpp" />
     <ClCompile Include="singleexecutor.cpp" />
     <ClCompile Include="stacktrace.cpp" />
     <ClCompile Include="threadexecutor.cpp" />

cli/cli.vcxproj.filters

@@ -26,7 +26,7 @@
     <ClInclude Include="cppcheckexecutorseh.h">
       <Filter>Header Files</Filter>
     </ClInclude>
-    <ClInclude Include="cppcheckexecutorsig.h">
+    <ClInclude Include="signalhandler.h">
       <Filter>Header Files</Filter>
     </ClInclude>
     <ClInclude Include="executor.h">
@@ -58,7 +58,7 @@
     <ClCompile Include="cppcheckexecutorseh.cpp">
       <Filter>Source Files</Filter>
     </ClCompile>
-    <ClCompile Include="cppcheckexecutorsig.cpp">
+    <ClCompile Include="signalhandler.cpp">
       <Filter>Source Files</Filter>
     </ClCompile>
     <ClCompile Include="executor.cpp">

cli/cppcheckexecutor.cpp

@@ -53,7 +53,7 @@
 #include <vector>
 
 #ifdef USE_UNIX_SIGNAL_HANDLING
-#include "cppcheckexecutorsig.h"
+#include "signalhandler.h"
 #endif
 
 #ifdef USE_WINDOWS_SEH
@@ -207,7 +207,7 @@ int CppCheckExecutor::check_wrapper(const Settings& settings)
         return check_wrapper_seh(*this, &CppCheckExecutor::check_internal, settings);
 #elif defined(USE_UNIX_SIGNAL_HANDLING)
     if (settings.exceptionHandling)
-        return check_wrapper_sig(*this, &CppCheckExecutor::check_internal, settings);
+        register_signal_handler();
 #endif
     return check_internal(settings);
 }
@@ -443,8 +443,12 @@ void StdLogger::reportErr(const ErrorMessage &msg)
 void CppCheckExecutor::setExceptionOutput(FILE* exceptionOutput)
 {
     mExceptionOutput = exceptionOutput;
+#if defined(USE_UNIX_SIGNAL_HANDLING)
+    set_signal_handler_output(mExceptionOutput);
+#endif
 }
 
+// cppcheck-suppress unusedFunction - only used by USE_WINDOWS_SEH code
 FILE* CppCheckExecutor::getExceptionOutput()
 {
     return mExceptionOutput;

cli/cppcheckexecutorsig.cpp → cli/signalhandler.cpp

@@ -16,12 +16,10 @@
  * along with this program.  If not, see <http://www.gnu.org/licenses/>.
  */
 
-#include "cppcheckexecutorsig.h"
+#include "signalhandler.h"
 
 #if defined(USE_UNIX_SIGNAL_HANDLING)
 
-#include "cppcheckexecutor.h"
-
 #ifdef USE_UNIX_BACKTRACE_SUPPORT
 #include "stacktrace.h"
 #endif
@@ -57,6 +55,12 @@ static constexpr size_t MYSTACKSIZE = 16*1024+SIGSTKSZ; // wild guess about a re
 #endif
 static char mytstack[MYSTACKSIZE]= {0}; // alternative stack for signal handler
 static bool bStackBelowHeap=false; // lame attempt to locate heap vs. stack address space. See CppCheckExecutor::check_wrapper()
+static FILE* signalOutput = stdout;
+
+void set_signal_handler_output(FILE* f)
+{
+    signalOutput = f;
+}
 
 /**
  * \param[in] ptr address to be examined.
@@ -121,27 +125,21 @@ 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);
-    FILE* output = CppCheckExecutor::getExceptionOutput();
+    FILE * const output = signalOutput;
     switch (signo) {
     case SIGABRT:
         fputs("Internal error: cppcheck received signal ", output);
         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);
@@ -281,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, true);
 #endif
     if (unexpectedSignal) {
         fputs("\nPlease report this to the cppcheck developers!\n", output);
@@ -298,8 +298,10 @@ static void CppcheckSignalHandler(int signo, siginfo_t * info, void * context)
     }
 }
 
-int check_wrapper_sig(CppCheckExecutor& executor, int (CppCheckExecutor::*f)(const Settings&) const, const Settings& settings)
+void register_signal_handler()
 {
+    FILE * const output = signalOutput;
+
     // determine stack vs. heap
     char stackVariable;
     char *heapVariable=static_cast<char*>(malloc(1));
@@ -311,7 +313,11 @@ int check_wrapper_sig(CppCheckExecutor& executor, int (CppCheckExecutor::*f)(con
     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;
@@ -321,7 +327,6 @@ int check_wrapper_sig(CppCheckExecutor& executor, int (CppCheckExecutor::*f)(con
     for (std::map<int, std::string>::const_iterator sig=listofsignals.cbegin(); sig!=listofsignals.cend(); ++sig) {
         sigaction(sig->first, &act, nullptr);
     }
-    return (executor.*f)(settings);
 }
 
 #endif

cli/cppcheckexecutorsig.h → cli/signalhandler.h

@@ -16,18 +16,20 @@
  * along with this program.  If not, see <http://www.gnu.org/licenses/>.
  */
 
-#ifndef CPPCHECKEXECUTORSIG_H
-#define CPPCHECKEXECUTORSIG_H
+#ifndef SIGNALHANDLER_H
+#define SIGNALHANDLER_H
 
 #include "config.h"
 
 #if defined(USE_UNIX_SIGNAL_HANDLING)
 
-class CppCheckExecutor;
-class Settings;
+/**
+ * @param f Output file
+ */
+void set_signal_handler_output(FILE* f);
 
-int check_wrapper_sig(CppCheckExecutor& executor, int (CppCheckExecutor::*f)(const Settings&) const, const Settings& settings);
+void register_signal_handler();
 
-#endif // CPPCHECKEXECUTORSIG_H
+#endif // USE_UNIX_SIGNAL_HANDLING
 
-#endif // CPPCHECKEXECUTORSIG_H
+#endif // SIGNALHANDLER_H