classification
Title: Segfaults on win-amd64 due to corrupt pointer to Tkapp_Interp
Type: crash Stage: patch review
Components: Versions: Python 3.4, Python 3.3, Python 2.7, Python 2.6
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: Nosy List: cgohlke, haypo, python-dev, serhiy.storchaka
Priority: high Keywords: patch

Created on 2013-09-03 01:25 by cgohlke, last changed 2013-09-05 20:17 by cgohlke. This issue is now closed.

Files
File name Uploaded Description Edit
tkapp_interpaddr_crash_win-amd64.py cgohlke, 2013-09-03 01:28
fix_tkapp_interpaddr-win-amd64-py2.7.diff cgohlke, 2013-09-03 01:28 review
fix_tkapp_interpaddr-win-amd64-py3.3.diff cgohlke, 2013-09-03 01:28 review
Messages (9)
msg196819 - (view) Author: Christoph Gohlke (cgohlke) Date: 2013-09-03 01:25
Using 64 bit CPython 2.6.6, 2.7.5, 3.2.5 or 3.3.2, numpy 1.7.1 and matplotlib 1.3.0 on Windows 8 64 bit, the following script segfaults most of the times: 

```
# allocate ~4GB fragmented data
import numpy
a = [numpy.zeros(2**i, 'uint8') for i in range(1, 31)]
b = [numpy.zeros(131072, 'float64') for i in range(2048)]

# plot using TkAgg
import matplotlib
matplotlib.use('TkAgg')
from matplotlib import pyplot
pyplot.plot()
pyplot.show()
```

```
Fatal Python error: Segmentation fault

Current thread 0x00036c5c:
  File "X:\Python33\lib\site-packages\matplotlib\backends\tkagg.py", line 17 in blit
  File "X:\Python33\lib\site-packages\matplotlib\backends\backend_tkagg.py", line 349 in draw
  File "X:\Python33\lib\site-packages\matplotlib\backends\backend_tkagg.py", line 276 in resize
  File "X:\Python33\lib\tkinter\__init__.py", line 1475 in __call__
  File "X:\Python33\lib\tkinter\__init__.py", line 965 in update
  File "X:\Python33\lib\site-packages\matplotlib\backends\backend_tkagg.py", line 574 in show
  File "X:\Python33\lib\site-packages\matplotlib\backend_bases.py", line 87 in __call__
  File "X:\Python33\lib\site-packages\matplotlib\pyplot.py", line 145 in show
  File "tk_crash_win-amd64.py", line 14 in <module>
```

The pointer returned by Python's _tkinter.tkapp.interpaddr() is often wrong because the 64 bit pointer is cast to 32 bit long and returned as PyInt. Instead, the pointer should be cast to Py_ssize_t and returned as PyLong on win-amd64.

The following patches for win-amd64-py2.7.5 and win-amd64-py3.3.2 fix the issue:

```
--- a/Modules/_tkinter.c        Sun Sep 01 19:06:35 2013 -0400
+++ b/Modules/_tkinter.c        Mon Sep 02 17:44:53 2013 -0700
@@ -2814,7 +2814,7 @@
     if (!PyArg_ParseTuple(args, ":interpaddr"))
         return NULL;

-    return PyInt_FromLong((long)Tkapp_Interp(self));
+    return PyInt_FromSsize_t((Py_ssize_t)Tkapp_Interp(self));
 }
```

```
--- a/Modules/_tkinter.c        Sun Sep 01 19:03:41 2013 -0400
+++ b/Modules/_tkinter.c        Mon Sep 02 17:44:02 2013 -0700
@@ -2688,7 +2688,7 @@
     if (!PyArg_ParseTuple(args, ":interpaddr"))
         return NULL;

-    return PyLong_FromLong((long)Tkapp_Interp(self));
+    return PyLong_FromSsize_t((Py_ssize_t)Tkapp_Interp(self));
 }
```

Updated _tkinter.pyd files are available at <http://www.lfd.uci.edu/~cgohlke/pythonlibs/#_tkinter>.
msg196877 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2013-09-03 22:49
There is PyLong_FromVoidPtr() for such cases.
msg196885 - (view) Author: Christoph Gohlke (cgohlke) Date: 2013-09-04 03:54
Is PyLong_FromVoidPtr compatible to PyInt_FromLong on 32 bit Python 2.7? It looks like PyLong_FromVoidPtr returns a PyLong when the address is > 2**31, while PyInt_FromLong returns a PyInt? Not sure it matters. PyLong_FromVoidPtr is certainly cleaner.
msg196921 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2013-09-04 14:28
Indeed, for less surprise we should use PyInt_FromSsize_t() on Python 2.7.
msg196968 - (view) Author: Roundup Robot (python-dev) Date: 2013-09-04 22:28
New changeset f01e06d26b41 by Victor Stinner in branch '3.3':
Issue #18909: Fix _tkinter.tkapp.interpaddr() on Windows 64-bit, don't cast
http://hg.python.org/cpython/rev/f01e06d26b41

New changeset ac27d979078a by Victor Stinner in branch 'default':
(Merge 3.3) Issue #18909: Fix _tkinter.tkapp.interpaddr() on Windows 64-bit,
http://hg.python.org/cpython/rev/ac27d979078a

New changeset 1a65bb15dedf by Victor Stinner in branch '2.7':
Issue #18909: Fix _tkinter.tkapp.interpaddr() on Windows 64-bit, don't cast
http://hg.python.org/cpython/rev/1a65bb15dedf
msg196969 - (view) Author: STINNER Victor (haypo) * (Python committer) Date: 2013-09-04 22:31
> New changeset 1a65bb15dedf by Victor Stinner in branch '2.7':
> Issue #18909: Fix _tkinter.tkapp.interpaddr() on Windows 64-bit, don't cast
> http://hg.python.org/cpython/rev/1a65bb15dedf

I prefer to use a function taking a void* instead of hoping that Py_ssize_t has exactly the same size than void*. Ok, the type may change, but who cares? Do you really rely on the exact type of a private function (_tkinter.tkapp.interpaddr())? :-)

@Christoph: Thanks for the report! Can you please your usecase on Python 2.7, 3.3 and/or 3.4 on Windows 64 bit?
msg196970 - (view) Author: Christoph Gohlke (cgohlke) Date: 2013-09-04 23:35
@haypo: Thanks for fixing this so fast! Your changes work for me on win-amd64-py2.7 and py3.3.

I am aware of two 3rd party C extensions that use the value of interpaddr(): 

https://github.com/python-imaging/Pillow/blob/master/_imagingtk.c#L40
https://github.com/matplotlib/matplotlib/blob/master/src/_tkagg.cpp#L233

Both parse the PyObject returned by interpaddr() into a Py_ssize_t variable using `PyArg_ParseTuple(PyObject*, "n", &Py_ssize_t)` and then cast the Py_ssize_t variable to a (TkappObject*). Can a PyLong with a value > 3**31 be parsed into a Py_ssize_t on 32 bit platforms? Could this become a problem on systems where 32 bit processes can address more than 2 GB?
msg196982 - (view) Author: STINNER Victor (haypo) * (Python committer) Date: 2013-09-05 07:12
2013/9/5 Christoph Gohlke <report@bugs.python.org>:
> I am aware of two 3rd party C extensions that use the value of interpaddr():
>
> https://github.com/python-imaging/Pillow/blob/master/_imagingtk.c#L40
> https://github.com/matplotlib/matplotlib/blob/master/src/_tkagg.cpp#L233
>
> Both parse the PyObject returned by interpaddr() into a Py_ssize_t variable using `PyArg_ParseTuple(PyObject*, "n", &Py_ssize_t)` and then cast the Py_ssize_t variable to a (TkappObject*).

Ok, the code looks correct. Py_ssize_t is supposed to have the same
size than a void*.

> Can a PyLong with a value > 3**31 be parsed into a Py_ssize_t on 32 bit platforms? Could this become a problem on systems where 32 bit processes can address more than 2 GB?

No, values >= 2^31 will raise an error.

On Linux, addresses with the 32th bit set (range
0x80000000-0xffffffff) are reserved for the Linux kernel. So
interpaddr() should be in the range 0x00000000-0x7fffffff.

If the code worked before my patch, there is not reason for not
working with the patch. My patch only has an impact on one specific
platform: Windows 64 bit, the only platform where sizeof(long) <
sizeof(void*) (on all other platforms supported by Python,
sizeof(void*) == sizeof(long)).
msg197021 - (view) Author: Christoph Gohlke (cgohlke) Date: 2013-09-05 20:17
Sorry, of course I meant > 2**31. Thank you very much for your review!
History
Date User Action Args
2013-09-05 20:17:44cgohlkesetmessages: + msg197021
2013-09-05 07:12:35hayposetmessages: + msg196982
2013-09-04 23:35:54cgohlkesetmessages: + msg196970
2013-09-04 22:31:55hayposetstatus: open -> closed

nosy: + haypo
messages: + msg196969

resolution: fixed
2013-09-04 22:28:26python-devsetnosy: + python-dev
messages: + msg196968
2013-09-04 14:28:55serhiy.storchakasetmessages: + msg196921
2013-09-04 06:37:12pitrousetpriority: normal -> high
stage: patch review
versions: + Python 3.4, - Python 3.2
2013-09-04 03:54:24cgohlkesetmessages: + msg196885
2013-09-03 22:49:58serhiy.storchakasetnosy: + serhiy.storchaka
messages: + msg196877
2013-09-03 01:28:49cgohlkesetfiles: + fix_tkapp_interpaddr-win-amd64-py3.3.diff
2013-09-03 01:28:29cgohlkesetfiles: + fix_tkapp_interpaddr-win-amd64-py2.7.diff
keywords: + patch
2013-09-03 01:28:13cgohlkesetfiles: + tkapp_interpaddr_crash_win-amd64.py
2013-09-03 01:25:03cgohlkecreate