Display Python backtrace on SIGSEGV, SIGFPE and fatal error

[Antoine Pitrou, 2010-10-13 15:02:24]

A quick review. Nice patch overall!

http://codereview.appspot.com/2477041/diff/1/Python/pythonrun.c
File Python/pythonrun.c (right):

http://codereview.appspot.com/2477041/diff/1/Python/pythonrun.c#newcode2365
Python/pythonrun.c:2365: fputs(msg, stderr);
Why do you print msg a second time? (it's already printed ten lines higher).

http://codereview.appspot.com/2477041/diff/1/Python/segfault.c
File Python/segfault.c (right):

http://codereview.appspot.com/2477041/diff/1/Python/segfault.c#newcode2
Python/segfault.c:2: * Python SIGSEGV and SIGFPE signal handlers: display the
Python backtrace and
Is there any use for catching SIGFPE?
What happens if Python code already has a signal handler for SIGFPE (is it
possible?)?

http://codereview.appspot.com/2477041/diff/1/Python/segfault.c#newcode45
Python/segfault.c:45: if (*u < 65535)
I think you want "*u < 65536".

http://codereview.appspot.com/2477041/diff/1/Python/segfault.c#newcode88
Python/segfault.c:88: _Py_DumpBacktrace(void)
Do you plan to add unit tests for all this?
You could launch a subprocess which runs a function in _testcapi which would
deliberatly provoke a segfault, or a floating-point exception, and check that
stderr contains a stacktrace.

http://codereview.appspot.com/2477041/diff/1/Python/segfault.c#newcode104
Python/segfault.c:104: dump_frame(frame);
Perhaps you want to call PyFrame_Check() first.

http://codereview.appspot.com/2477041/diff/1/Python/segfault.c#newcode120
Python/segfault.c:120: fflush(stderr);
According to the POSIX standard (and the Linux man pages), buffered IO functions
(fwrite, fflush, fputs...) are not safe for use inside signal handlers. Perhaps
you want to use write() instead, which is safe.

(see the table at the end of
http://www.opengroup.org/onlinepubs/000095399/functions/xsh_chap02_04.html )

http://codereview.appspot.com/2477041/diff/1/Python/segfault.c#newcode145
Python/segfault.c:145: stack.ss_sp = PyMem_Malloc(stack.ss_size);
It might be better to use malloc() here, so that a bug in the Python memory
allocator doesn't prevent proper stack trace printing :)

http://codereview.appspot.com/2477041/diff/1/Python/segfault.c#newcode187
Python/segfault.c:187: PyMem_Free(stack.ss_sp);
Same comment as PyMem_Malloc above.

[haypo, 2010-10-19 22:25:35]

http://codereview.appspot.com/2477041/diff/1/Python/pythonrun.c
File Python/pythonrun.c (right):

http://codereview.appspot.com/2477041/diff/1/Python/pythonrun.c#newcode2365
Python/pythonrun.c:2365: fputs(msg, stderr);
On 2010/10/13 15:02:25, Antoine Pitrou wrote:
> Why do you print msg a second time? (it's already printed ten lines higher).

It's to keep the same output format than a classic Python exception.

http://codereview.appspot.com/2477041/diff/1/Python/segfault.c
File Python/segfault.c (right):

http://codereview.appspot.com/2477041/diff/1/Python/segfault.c#newcode2
Python/segfault.c:2: * Python SIGSEGV and SIGFPE signal handlers: display the
Python backtrace and
On 2010/10/13 15:02:25, Antoine Pitrou wrote:
> Is there any use for catching SIGFPE?
> What happens if Python code already has a signal handler for SIGFPE (is it
> possible?)?

Catch SIGFPE is Python is not possible because Python notes that SIGFPE was
raised and then continue the execution, but then the SIGFPE is raised again
because the bug is not fixed yet...

In C, it's possible to set another custom SIGFPE which will replace my "dump
Python backtrace" handler.

http://codereview.appspot.com/2477041/diff/1/Python/segfault.c#newcode45
Python/segfault.c:45: if (*u < 65535)
On 2010/10/13 15:02:25, Antoine Pitrou wrote:
> I think you want "*u < 65536".

Oh yes, fixed in patch version 6.

http://codereview.appspot.com/2477041/diff/1/Python/segfault.c#newcode88
Python/segfault.c:88: _Py_DumpBacktrace(void)
On 2010/10/13 15:02:25, Antoine Pitrou wrote:
> Do you plan to add unit tests for all this?
> You could launch a subprocess which runs a function in _testcapi which would
> deliberatly provoke a segfault, or a floating-point exception, and check that
> stderr contains a stacktrace.

I added some tests to patch version 6.

http://codereview.appspot.com/2477041/diff/1/Python/segfault.c#newcode104
Python/segfault.c:104: dump_frame(frame);
On 2010/10/13 15:02:25, Antoine Pitrou wrote:
> Perhaps you want to call PyFrame_Check() first.

PyFrame_Check() doesn't exist, but I will add a check in my next patch (version
7).

http://codereview.appspot.com/2477041/diff/1/Python/segfault.c#newcode120
Python/segfault.c:120: fflush(stderr);
On 2010/10/13 15:02:25, Antoine Pitrou wrote:
> According to the POSIX standard (and the Linux man pages), buffered IO
functions
> (fwrite, fflush, fputs...) are not safe for use inside signal handlers.
Perhaps
> you want to use write() instead, which is safe.
> 
> (see the table at the end of
> http://www.opengroup.org/onlinepubs/000095399/functions/xsh_chap02_04.html )

Patch version 6 uses snprintf() + write(), next patch (version 7) will only use
write() syscall.

http://codereview.appspot.com/2477041/diff/1/Python/segfault.c#newcode145
Python/segfault.c:145: stack.ss_sp = PyMem_Malloc(stack.ss_size);
On 2010/10/13 15:02:25, Antoine Pitrou wrote:
> It might be better to use malloc() here, so that a bug in the Python memory
> allocator doesn't prevent proper stack trace printing :)

PyMem_Malloc() is called outside the signal handler (before the bug). I don't
see how memory allocated by malloc() has a less risk to be corrupted, than
memory allocated by PyMem_Malloc().

It looks like PyMem_Malloc() just calls malloc(), except if PYMALLOC_DEBUG is
defined.