Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

gh-127604: Add C stack dumps to faulthandler #128159

Open
wants to merge 27 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 22 commits
Commits
Show all changes
27 commits
Select commit Hold shift + click to select a range
0ebdc69
Initial implementation for glibc
ZeroIntensity Dec 21, 2024
0ccf9fb
Make it cross-platform buildable.
ZeroIntensity Dec 21, 2024
52945e5
Call it on Windows.
ZeroIntensity Dec 21, 2024
9bdd802
Add a whatsnew entry
ZeroIntensity Dec 21, 2024
66f2641
Add dump_c_stack
ZeroIntensity Dec 21, 2024
0591013
Reverse the loop.
ZeroIntensity Dec 21, 2024
64707a2
Add documentation entries.
ZeroIntensity Dec 21, 2024
8a5b784
Make the whatsnew entry more specific.
ZeroIntensity Dec 21, 2024
4f09358
Add tests.
ZeroIntensity Dec 21, 2024
e1aa619
Add blurb from whatsnew.
ZeroIntensity Dec 21, 2024
6b515d3
Fix failing Sphinx build.
ZeroIntensity Dec 21, 2024
c69ee9d
Fix PyArg_ParseTupleAndKeywords format string.
ZeroIntensity Dec 21, 2024
f08b1dd
Fix Sphinx warnings.
ZeroIntensity Dec 21, 2024
dbb6d25
Add ignore to test_inspect
ZeroIntensity Dec 21, 2024
faf1a3e
Fix name.
ZeroIntensity Dec 21, 2024
ec832aa
Ignore the reentrant variable in the C analyzer.
ZeroIntensity Dec 21, 2024
524f167
Apply suggestions from code review
ZeroIntensity Dec 21, 2024
dd08bcb
Use manpage references.
ZeroIntensity Dec 21, 2024
bda3dcd
Add issue number to whatsnew.
ZeroIntensity Dec 21, 2024
a3564a5
Merge branch 'c-faulthandler' of https://github.com/ZeroIntensity/cpy…
ZeroIntensity Dec 21, 2024
f3fcea1
Rename macros.
ZeroIntensity Dec 21, 2024
db97dd2
Reduce number of printed calls.
ZeroIntensity Dec 21, 2024
c17457f
Apply suggestions from code review
ZeroIntensity Dec 21, 2024
d5f7d4b
Address code review.
ZeroIntensity Dec 21, 2024
e79e661
Merge branch 'c-faulthandler' of https://github.com/ZeroIntensity/cpy…
ZeroIntensity Dec 21, 2024
0c84f8a
Explicitly check for backtrace() and backtrace_symbols()
ZeroIntensity Dec 22, 2024
8198997
Handle no frames returned.
ZeroIntensity Dec 22, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 20 additions & 1 deletion Doc/library/faulthandler.rst
Original file line number Diff line number Diff line change
Expand Up @@ -66,10 +66,22 @@ Dumping the traceback
Added support for passing file descriptor to this function.


Dumping the C stack
-------------------

.. versionadded:: next

.. function:: dump_c_stack(file=sys.stderr)
ZeroIntensity marked this conversation as resolved.
Show resolved Hide resolved

Dump the C stack trace of the current thread into *file*.

If the system doesn't support the C-level :manpage:`backtrace` or :manpage:`backtrace_symbols` functions,
then an error message is displayed instead of the C stack.
ZeroIntensity marked this conversation as resolved.
Show resolved Hide resolved

Fault handler state
-------------------

.. function:: enable(file=sys.stderr, all_threads=True)
.. function:: enable(file=sys.stderr, all_threads=True, c_stack=True)

Enable the fault handler: install handlers for the :const:`~signal.SIGSEGV`,
:const:`~signal.SIGFPE`, :const:`~signal.SIGABRT`, :const:`~signal.SIGBUS`
Expand All @@ -81,6 +93,10 @@ Fault handler state
The *file* must be kept open until the fault handler is disabled: see
:ref:`issue with file descriptors <faulthandler-fd>`.

If *c_stack* is ``True``, then the C stack trace is printed after the Python
traceback, unless the system doesn't support it. See :func:`dump_c_stack` for
ZeroIntensity marked this conversation as resolved.
Show resolved Hide resolved
more information on compatibility.

.. versionchanged:: 3.5
Added support for passing file descriptor to this function.

Expand All @@ -91,6 +107,9 @@ Fault handler state
The dump now mentions if a garbage collector collection is running
if *all_threads* is true.

.. versionchanged:: next
The dump now displays the C stack trace if *c_stack* is true.

.. function:: disable()

Disable the fault handler: uninstall the signal handlers installed by
Expand Down
8 changes: 8 additions & 0 deletions Doc/whatsnew/3.14.rst
Original file line number Diff line number Diff line change
Expand Up @@ -367,6 +367,14 @@ errno
(Contributed by James Roy in :gh:`126585`.)


faulthandler
------------

* Add support for printing the C stack trace on systems that support it via
:func:`faulthandler.dump_c_stack` or via the *c_stack* argument in :func:`faulthandler.enable`.
ZeroIntensity marked this conversation as resolved.
Show resolved Hide resolved
(Contributed by Peter Bierma in :gh:`127604`.)


fractions
---------

Expand Down
1 change: 1 addition & 0 deletions Include/internal/pycore_faulthandler.h
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@ struct _faulthandler_runtime_state {
#ifdef MS_WINDOWS
void *exc_handler;
#endif
int c_stack;
} fatal_error;

struct {
Expand Down
9 changes: 7 additions & 2 deletions Lib/test/test_faulthandler.py
Original file line number Diff line number Diff line change
Expand Up @@ -93,18 +93,20 @@ def check_error(self, code, lineno, fatal_error, *,
fd=None, know_current_thread=True,
py_fatal_error=False,
garbage_collecting=False,
c_stack=True,
ZeroIntensity marked this conversation as resolved.
Show resolved Hide resolved
function='<module>'):
"""
Check that the fault handler for fatal errors is enabled and check the
traceback from the child process output.

Raise an error if the output doesn't match the expected format.
"""
address_expr = "0x[0-9a-f]+"
if all_threads:
if know_current_thread:
header = 'Current thread 0x[0-9a-f]+'
header = f'Current thread {address_expr}'
else:
header = 'Thread 0x[0-9a-f]+'
header = f'Thread {address_expr}'
else:
header = 'Stack'
regex = [f'^{fatal_error}']
Expand All @@ -115,6 +117,9 @@ def check_error(self, code, lineno, fatal_error, *,
if garbage_collecting:
regex.append(' Garbage-collecting')
regex.append(fr' File "<string>", line {lineno} in {function}')
if c_stack:
regex.append("Current thread's C stack (most recent call first):")
regex.append(r" (\/.+\(\+.+\) \[0x[0-9a-f]+\])|(<.+>)")
regex = '\n'.join(regex)

if other_regex:
Expand Down
2 changes: 1 addition & 1 deletion Lib/test/test_inspect/test_inspect.py
Original file line number Diff line number Diff line change
Expand Up @@ -5726,7 +5726,7 @@ def test_errno_module_has_signatures(self):

def test_faulthandler_module_has_signatures(self):
import faulthandler
unsupported_signature = {'dump_traceback', 'dump_traceback_later', 'enable'}
unsupported_signature = {'dump_traceback', 'dump_traceback_later', 'enable', 'dump_c_stack'}
unsupported_signature |= {name for name in ['register']
if hasattr(faulthandler, name)}
self._test_module_has_signatures(faulthandler, unsupported_signature=unsupported_signature)
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
Add support for printing the C stack trace on systems that support it via
:func:`faulthandler.dump_c_stack` or via the *c_stack* argument in
:func:`faulthandler.enable`.
116 changes: 114 additions & 2 deletions Modules/faulthandler.c
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,10 @@
#ifdef HAVE_UNISTD_H
# include <unistd.h> // _exit()
#endif
#ifdef HAVE_EXECINFO_H
# include <execinfo.h> // backtrace(), backtrace_symbols()
#endif

#include <signal.h> // sigaction()
#include <stdlib.h> // abort()
#if defined(HAVE_PTHREAD_SIGMASK) && !defined(HAVE_BROKEN_PTHREAD_SIGMASK) && defined(HAVE_PTHREAD_H)
Expand Down Expand Up @@ -212,6 +216,80 @@ faulthandler_dump_traceback(int fd, int all_threads,
reentrant = 0;
}

#ifdef HAVE_EXECINFO_H
static void
faulthandler_stack_dump_impl(int fd)
{
#define BACKTRACE_SIZE 16
#define TRACEBACK_ENTRY_MAX_SIZE 256
void *callstack[BACKTRACE_SIZE];
int frames = backtrace(callstack, BACKTRACE_SIZE);
char **strings = backtrace_symbols(callstack, BACKTRACE_SIZE);
ZeroIntensity marked this conversation as resolved.
Show resolved Hide resolved
if (strings == NULL) {
PUTS(fd, " <failed to get stack trace>\n");
}
else {
for (int i = 0; i < frames; ++i) {
char entry_str[TRACEBACK_ENTRY_MAX_SIZE];
snprintf(entry_str, TRACEBACK_ENTRY_MAX_SIZE, " %s\n", strings[i]);
size_t length = strlen(entry_str) + 1;
if (length == TRACEBACK_ENTRY_MAX_SIZE) {
/* We exceeded the size, make it look prettier */

// Add ellipsis to last 3 characters
entry_str[TRACEBACK_ENTRY_MAX_SIZE - 5] = '.';
entry_str[TRACEBACK_ENTRY_MAX_SIZE - 4] = '.';
entry_str[TRACEBACK_ENTRY_MAX_SIZE - 3] = '.';

ZeroIntensity marked this conversation as resolved.
Show resolved Hide resolved
// Ensure trailing newline
entry_str[TRACEBACK_ENTRY_MAX_SIZE - 2] = '\n';

// Ensure that it's null-terminated
entry_str[TRACEBACK_ENTRY_MAX_SIZE - 1] = '\0';
}
_Py_write_noraise(fd, entry_str, length);
}

if (frames == BACKTRACE_SIZE) {
PUTS(fd, " <truncated rest of calls>\n");
}
}
#undef BACKTRACE_SIZE
#undef TRACEBACK_ENTRY_MAX_SIZE
}
#else
static void
faulthandler_stack_dump_impl(int fd)
{
PUTS(fd, " <cannot get C stack on this system>\n");
}
ZeroIntensity marked this conversation as resolved.
Show resolved Hide resolved
#endif

static void
faulthandler_dump_c_stack_nocheck(int fd)
{
PUTS(fd, "Current thread's C stack trace (most recent call first):\n");
faulthandler_stack_dump_impl(fd);
}

static void
faulthandler_dump_c_stack(int fd)
{
static volatile int reentrant = 0;

if (reentrant)
return;
ZeroIntensity marked this conversation as resolved.
Show resolved Hide resolved

reentrant = 1;

if (fatal_error.c_stack) {
PUTS(fd, "\n");
faulthandler_dump_c_stack_nocheck(fd);
}

reentrant = 0;
}

static PyObject*
faulthandler_dump_traceback_py(PyObject *self,
PyObject *args, PyObject *kwargs)
Expand Down Expand Up @@ -253,6 +331,32 @@ faulthandler_dump_traceback_py(PyObject *self,
Py_RETURN_NONE;
}

static PyObject *
faulthandler_dump_c_stack_py(PyObject *self,
PyObject *args, PyObject *kwargs)
{
static char *kwlist[] = {"file", NULL};
PyObject *file = NULL;

if (!PyArg_ParseTupleAndKeywords(args, kwargs,
"|p:dump_c_stack", kwlist,
&file))
return NULL;
ZeroIntensity marked this conversation as resolved.
Show resolved Hide resolved

int fd = faulthandler_get_fileno(&file);
if (fd < 0) {
return NULL;
}

faulthandler_dump_c_stack_nocheck(fd);

if (PyErr_CheckSignals()) {
return NULL;
}

Py_RETURN_NONE;
}

static void
faulthandler_disable_fatal_handler(fault_handler_t *handler)
{
Expand Down Expand Up @@ -322,6 +426,7 @@ faulthandler_fatal_error(int signum)

faulthandler_dump_traceback(fd, fatal_error.all_threads,
fatal_error.interp);
faulthandler_dump_c_stack(fd);

_Py_DumpExtensionModules(fd, fatal_error.interp);

Expand Down Expand Up @@ -398,6 +503,7 @@ faulthandler_exc_handler(struct _EXCEPTION_POINTERS *exc_info)

faulthandler_dump_traceback(fd, fatal_error.all_threads,
fatal_error.interp);
faulthandler_dump_c_stack(fd);

/* call the next exception handler */
return EXCEPTION_CONTINUE_SEARCH;
Expand Down Expand Up @@ -492,14 +598,15 @@ faulthandler_enable(void)
static PyObject*
faulthandler_py_enable(PyObject *self, PyObject *args, PyObject *kwargs)
{
static char *kwlist[] = {"file", "all_threads", NULL};
static char *kwlist[] = {"file", "all_threads", "c_stack", NULL};
PyObject *file = NULL;
int all_threads = 1;
int fd;
int c_stack = 1;
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think it should be defaulted to True, especially when only available on a subset of systems.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Eh, I don't think it matters. People don't use faulthandler for compatibility reasons, they just enable it for debugging. I feel like not enabling the C stack trace by default will just add an extra, annoying step for users.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm also in favor of keeping the stack trace.

especially when only available on a subset of systems

Well.. I'd say it's available on many popular systems except FreeBSD/CentoS. Ubuntu and Fedora being supported are engouh IMO, at least for many users.

Is it available on macOS by the way?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it available on macOS by the way?

Probably, but I haven't checked.

PyThreadState *tstate;

if (!PyArg_ParseTupleAndKeywords(args, kwargs,
"|Op:enable", kwlist, &file, &all_threads))
"|Opp:enable", kwlist, &file, &all_threads, &c_stack))
return NULL;

fd = faulthandler_get_fileno(&file);
Expand All @@ -515,6 +622,7 @@ faulthandler_py_enable(PyObject *self, PyObject *args, PyObject *kwargs)
fatal_error.fd = fd;
fatal_error.all_threads = all_threads;
fatal_error.interp = PyThreadState_GetInterpreter(tstate);
fatal_error.c_stack = c_stack;

if (faulthandler_enable() < 0) {
return NULL;
Expand Down Expand Up @@ -1205,6 +1313,10 @@ static PyMethodDef module_methods[] = {
PyDoc_STR("dump_traceback($module, /, file=sys.stderr, all_threads=True)\n--\n\n"
"Dump the traceback of the current thread, or of all threads "
"if all_threads is True, into file.")},
{"dump_c_stack",
_PyCFunction_CAST(faulthandler_dump_c_stack_py), METH_VARARGS|METH_KEYWORDS,
PyDoc_STR("dump_c_stack($module, /, file=sys.stderr)\n--\n\n"
"Dump the C stack of the current thread.")},
{"dump_traceback_later",
_PyCFunction_CAST(faulthandler_dump_traceback_later), METH_VARARGS|METH_KEYWORDS,
PyDoc_STR("dump_traceback_later($module, /, timeout, repeat=False, file=sys.stderr, exit=False)\n--\n\n"
Expand Down
1 change: 1 addition & 0 deletions Tools/c-analyzer/cpython/ignored.tsv
Original file line number Diff line number Diff line change
Expand Up @@ -161,6 +161,7 @@ Python/sysmodule.c - _preinit_xoptions -
# thread-safety
# XXX need race protection?
Modules/faulthandler.c faulthandler_dump_traceback reentrant -
Modules/faulthandler.c faulthandler_dump_c_stack reentrant -
Python/pylifecycle.c _Py_FatalErrorFormat reentrant -
Python/pylifecycle.c fatal_error reentrant -

Expand Down
6 changes: 6 additions & 0 deletions configure

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion configure.ac
Original file line number Diff line number Diff line change
Expand Up @@ -2929,7 +2929,7 @@ AC_DEFINE([STDC_HEADERS], [1],

# checks for header files
AC_CHECK_HEADERS([ \
alloca.h asm/types.h bluetooth.h conio.h direct.h dlfcn.h endian.h errno.h fcntl.h grp.h \
alloca.h asm/types.h bluetooth.h conio.h direct.h dlfcn.h endian.h errno.h execinfo.h fcntl.h grp.h \
ZeroIntensity marked this conversation as resolved.
Show resolved Hide resolved
io.h langinfo.h libintl.h libutil.h linux/auxvec.h sys/auxv.h linux/fs.h linux/limits.h linux/memfd.h \
linux/netfilter_ipv4.h linux/random.h linux/soundcard.h linux/sched.h \
linux/tipc.h linux/wait.h netdb.h net/ethernet.h netinet/in.h netpacket/packet.h poll.h process.h pthread.h pty.h \
Expand Down
3 changes: 3 additions & 0 deletions pyconfig.h.in
Original file line number Diff line number Diff line change
Expand Up @@ -331,6 +331,9 @@
/* Define if you have the 'eventfd' function. */
#undef HAVE_EVENTFD

/* Define to 1 if you have the <execinfo.h> header file. */
#undef HAVE_EXECINFO_H

/* Define to 1 if you have the `execv' function. */
#undef HAVE_EXECV

Expand Down
Loading