[issue7358] cStringIO not 64-bit safe

[Serhiy Storchaka]

Changes by Serhiy Storchaka :


--
assignee:  -> serhiy.storchaka
components: +Extension Modules, IO -Library (Lib)
nosy: +serhiy.storchaka
stage:  -> needs patch
versions:  -Python 2.6

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue7358] cStringIO not 64-bit safe

[Serhiy Storchaka]

Serhiy Storchaka added the comment:

Here is a patch.

--
keywords: +patch
stage: needs patch -> patch review
Added file: http://bugs.python.org/file28880/cStringIO64.patch

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue7358] cStringIO not 64-bit safe

[Gregory P. Smith]

Gregory P. Smith added the comment:

Suggested workaround: use io.BytesIO instead of cStringIO.

comments on the patch... in short: your patch is changing an ABI and isn't 
suitable for a maintenance release.

I'm not sure how much we can usefully do to cStringIO in a maintenance release. 
 I'd be inclined to close as wontfix as io.BytesIO is available.

--
nosy: +gregory.p.smith

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue7358] cStringIO not 64-bit safe

[Antoine Pitrou]

Antoine Pitrou added the comment:

Not making cStringIO 64-bit compatible is one thing. But we can still fix the 
crashes by detecting an overflow before it happens ;)

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue7358] cStringIO not 64-bit safe

[Serhiy Storchaka]

Serhiy Storchaka added the comment:

> Suggested workaround: use io.BytesIO instead of cStringIO.

Well, maybe we can replace cStringIO by io.BytesIO in cPickle (and other 
places).

> comments on the patch... in short: your patch is changing an ABI and isn't 
> suitable for a maintenance release.

Isn't this a private API?

> I'm not sure how much we can usefully do to cStringIO in a maintenance 
>release.  I'd be inclined to close as wontfix as io.BytesIO is available.

We can't just close this issue. cStringIO crashes Python. We can use another 
approach. Change cStringIO so that user can't write or read more than INT_MAX 
bytes at a time (an exception will be raised instead of crash), but can write 
more than INT_MAX bytes in sum and get all string with getvalue(). This will 
preserve an ABI.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue7358] cStringIO not 64-bit safe

[Gregory P. Smith]

Gregory P. Smith added the comment:

Include/cStringIO.h is public and the name of the structure "PycStringIO" lacks 
an _ which implies that it is public.

There appear to be a few references to it in external projects doing some 
searching.  A (very old) version of twisted/protocols/_c_urlarg.c uses it for 
example.

Doing the latter and limiting writes to INT_MAX but allowing greater to 
accumulate makes sense.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue7358] cStringIO not 64-bit safe

[Serhiy Storchaka]

Serhiy Storchaka added the comment:

Thank you for review and enlightenment Gregory. Here is an updated patch which 
doesn't change an ABI.

--
Added file: http://bugs.python.org/file28951/cStringIO64_2.patch

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue7358] cStringIO not 64-bit safe

[Roundup Robot]

Roundup Robot added the comment:

New changeset a025b04332fe by Serhiy Storchaka in branch '2.7':
Issue #7358: cStringIO.StringIO now supports writing to and reading from
http://hg.python.org/cpython/rev/a025b04332fe

--
nosy: +python-dev

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue7358] cStringIO not 64-bit safe

[Serhiy Storchaka]

Changes by Serhiy Storchaka :


--
resolution:  -> fixed
stage: patch review -> committed/rejected
status: open -> closed

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue7358] cStringIO not 64-bit safe

[Jesús Cea Avión]

Changes by Jesús Cea Avión :


--
nosy: +jcea

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue7358] cStringIO not 64-bit safe

[Antoine Pitrou]

Antoine Pitrou  added the comment:

The issue looks quite clear: cStringIO.write() asserts that the required 
storage size is less than INT_MAX. Therefore, in all likelihood, the pickle 
dump is simply larger than 2GB.

Now, the cStringIO structures seem 64-bit safe, so the comparison to INT_MAX 
seems useless. Also, in non-debug builds, the assert is skipped but the 
Py_ssize_t values are first cast to int before being saved into a Py_ssize_t 
struct member! Which would lead to data corruption and/or crashes.

The INT_MAX asserts were added in r42382 which also made cStringIO internals 
64-bit safe. Martin, do you remember why you did this?

The asserts don't even protect against the following crash:

$ ./python -c "from cStringIO import StringIO; b=b'x'*(2**31+1); s=StringIO(); 
s.write(b)"
Erreur de segmentation

gdb shows the following stack excerpt:

#0  0x7724e8c2 in memcpy () from /lib64/libc.so.6
#1  0x76b47a46 in O_cwrite (self=, 
c=0x7fff76b44054 'x' ..., 
l=-2147483647) at /home/antoine/cpython/27/Modules/cStringIO.c:415


Now, onto the problem of reproducing, here's another interesting thing: while 
some internal structures of cStringIO are 64-bit safe, the Python-facing API 
isn't:

>>> from cStringIO import StringIO
>>> b = b"x" * (2**32+1)
>>> s = StringIO()
>>> s.write(b)
>>> s.tell()
1

(the module doesn't use Py_SSIZE_T_CLEAN)

--
nosy: +loewis, pitrou
title: cPickle crash on failed assertion -> cStringIO not 64-bit safe
versions: +Python 2.7

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue7358] cStringIO not 64-bit safe

[Gilles Louppe]

Gilles Louppe added the comment:

Hi,

Any solution regarding that issue? We are currently encountering the exact same 
bug when pickling too large objects. 

Best,

Gilles

--
nosy: +Gilles.Louppe

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue7358] cStringIO not 64-bit safe

[Martin v . Löwis]

Martin v. Löwis added the comment:

Gilles: it's really puzzling that you run into the issue. This is an assert, so 
in a regular build of Python, it should never trigger. Instead, it should 
trigger only in a debug build. Why are you running a debug build?

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue7358] cStringIO not 64-bit safe

[Martin v . Löwis]

Martin v. Löwis added the comment:

Antoine: if you look at

http://svn.python.org/view/python/branches/ssize_t/Modules/cStringIO.c?revision=41731&view=markup&pathrev=42382

you'll notice that IOobject and Iobject have pos and stringsize as int, whereas 
Oobject has it as Py_ssize_t. This must have been a bug; I think the structures 
were meant as compatible. If they all should have been defined in terms of int, 
the assert would make sense.

That said, what actually got merged included all the fields as Py_ssize_t, in 
which case the assert makes no sense. OTOH, it prevents the cast below (of 
(int)l) to produce bogus results, so it actually helped to detect the bugs :-)

What is now needed (from people running into the issue) is a patch that 
resolves all aspects of >2GB handling in these modules. I believe the issue is 
resolved in Python 3 (by both StringIO and BytesIO properly using Py_ssize_t 
throughout there), so unless somebody interested in this problem actually 
contributes a patch, the issue is unlikely to advance.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com