Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(1098)

Issue 2477041: Display Python backtrace on SIGSEGV, SIGFPE and fatal error

Can't Edit
Can't Publish+Mail
Start Review
Created:
7 years, 1 month ago by haypo
Modified:
7 years, 1 month ago
Reviewers:
Antoine Pitrou
Base URL:
http://svn.python.org/view/*checkout*/python/branches/py3k/
Visibility:
Public.

Patch Set 1 #

Total comments: 15
Unified diffs Side-by-side diffs Delta from patch set Stats (+224 lines, -5 lines) Patch
M Makefile.pre.in View 1 chunk +1 line, -0 lines 0 comments Download
M Misc/NEWS View 1 chunk +3 lines, -0 lines 0 comments Download
M PCbuild/pythoncore.vcproj View 1 chunk +4 lines, -0 lines 0 comments Download
M Python/pythonrun.c View 4 chunks +21 lines, -3 lines 2 comments Download
A Python/segfault.c View 1 chunk +190 lines, -0 lines 13 comments Download
M configure View 1 chunk +1 line, -1 line 0 comments Download
M configure.in View 1 chunk +1 line, -1 line 0 comments Download
M pyconfig.h.in View 1 chunk +3 lines, -0 lines 0 comments Download

Messages

Total messages: 2
Antoine Pitrou
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 ...
7 years, 1 month ago (2010-10-13 15:02:24 UTC) #1
haypo
7 years, 1 month ago (2010-10-19 22:25:35 UTC) #2
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.
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld c59f3c4