classification
Title: undefined behaviour in faulthandler.c, exposed by GCC 5
Type: crash Stage:
Components: Extension Modules Versions: Python 3.5, Python 3.4
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: Nosy List: doko, python-dev, sYnfo, vstinner
Priority: normal Keywords: patch

Created on 2015-02-10 14:14 by doko, last changed 2015-02-11 13:45 by vstinner. This issue is now closed.

Files
File name Uploaded Description Edit
faulthandler_uintptr.patch vstinner, 2015-02-10 14:24 review
Messages (5)
msg235685 - (view) Author: Matthias Klose (doko) * (Python committer) Date: 2015-02-10 14:14
<jakub> richi: https://github.com/nemomobile-packages/python3/blob/master/Modules/faulthandler.c#L903
<polacek> richi: LD_LIBRARY_PATH=/builddir/build/BUILD/Python-3.4.2/build/debug/ /builddir/build/BUILD/Python-3.4.2/build/debug/python -E -c 'import faulthandler; faulthandler.enable(); faulthandler._stack_overflow()'
<polacek> i.e. what Jakub says
<jakub> richi: the function certainly shouldn't return address of a local variable; dunno what would happen if you just cast that to an integer though
<jakub> richi: and it better should do something to avoid tail calls there
<jakub> richi: the if (sp < min_sp || max_sp < sp) is also undefined behavior
<richi> ah, I get python segfaults building some extensions instead (but can't reproduce locally...)
<richi> jakub: so what's your fix?
<jakub> richi: I don't have a fix, we just documented it not to be a gcc fault, we'll leave fixing to the package maintainer
<richi> ah, I see
<jakub> richi: dunno if e.g. uintptr_t x; memcpy (&x, &sp, sizeof (x)); would DTRT and be portable enough
<jakub> richi: and then of course pass uintptr_t min_sp/max_sp, compare the x against that etc.
<richi> well, just (uintptr_t)&buffer should be enough
msg235686 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2015-02-10 14:24
faulthandler._stack_overflow() is written to crash. The comparison on stack pointers is just here to avoid an unlimited loop. A stack of 100 MB is something really large. I never seen an OS where faulthandler._stack_overflow() doesn't crash yet.

Here is a patch using Py_uintptr_t instead of void*.

Can someone test with GCC 5?
msg235743 - (view) Author: Matěj Stuchlík (sYnfo) Date: 2015-02-11 13:12
FWIW with your patch all the tests pass on Fedora rawhide in Koji [1], whereas before the x86_64 build would hang [2]. If you want to do some more testing, you can download the rpms from [1]. :)

[1] http://koji.fedoraproject.org/koji/taskinfo?taskID=8894494
[2] http://koji.fedoraproject.org/koji/taskinfo?taskID=8893808
msg235744 - (view) Author: Roundup Robot (python-dev) Date: 2015-02-11 13:26
New changeset 689092296ad3 by Victor Stinner in branch '3.4':
Issue #23433: Fix faulthandler._stack_overflow()
https://hg.python.org/cpython/rev/689092296ad3
msg235745 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2015-02-11 13:45
Thanks for the report. This issue should now be fixed.
History
Date User Action Args
2015-02-11 13:45:19vstinnersetstatus: open -> closed
resolution: fixed
messages: + msg235745
2015-02-11 13:26:18python-devsetnosy: + python-dev
messages: + msg235744
2015-02-11 13:12:44sYnfosetmessages: + msg235743
2015-02-11 12:08:19sYnfosetnosy: + sYnfo
2015-02-10 14:24:15vstinnersetfiles: + faulthandler_uintptr.patch

nosy: + vstinner
messages: + msg235686

keywords: + patch
2015-02-10 14:14:31dokocreate