classification
Title: _pickle is not entirely 64-bit safe
Type: behavior Stage: resolved
Components: Extension Modules Versions: Python 3.2, Python 3.3
process
Status: closed Resolution: out of date
Dependencies: Superseder:
Assigned To: Nosy List: alexandre.vassalotti, amaury.forgeotdarc, belopolsky, brian.curtin, janglin, pitrou, rhettinger, santoso.wijaya, tim.golden
Priority: high Keywords: patch

Created on 2010-08-15 18:13 by pitrou, last changed 2011-11-18 19:29 by pitrou. This issue is now closed.

Files
File name Uploaded Description Edit
pickle_warnings_easy.patch amaury.forgeotdarc, 2010-08-17 20:23 review
BuildLog.htm tim.golden, 2011-04-27 08:57
issue9614.diff belopolsky, 2011-04-27 15:21 review
Messages (10)
msg113986 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2010-08-15 18:13
A number of legitimate warnings get emitted under a 64-bit Windows build (in many places, _pickle uses ints or longs instead of "Py_ssize_t" variable to store various lengths and sizes):

1>..\Modules\_pickle.c(284) : warning C4244: '=' : conversion from 'Py_ssize_t' to 'int', possible loss of data
1>..\Modules\_pickle.c(301) : warning C4244: '=' : conversion from 'Py_ssize_t' to 'int', possible loss of data
1>..\Modules\_pickle.c(461) : warning C4244: '+=' : conversion from 'Py_ssize_t' to 'int', possible loss of data
1>..\Modules\_pickle.c(628) : warning C4244: '=' : conversion from 'Py_ssize_t' to 'long', possible loss of data
1>..\Modules\_pickle.c(647) : warning C4267: '=' : conversion from 'size_t' to 'int', possible loss of data
1>..\Modules\_pickle.c(1320) : warning C4244: '=' : conversion from 'Py_ssize_t' to 'int', possible loss of data
1>..\Modules\_pickle.c(1558) : warning C4244: '=' : conversion from 'Py_ssize_t' to 'int', possible loss of data
1>..\Modules\_pickle.c(1806) : warning C4244: '=' : conversion from 'Py_ssize_t' to 'int', possible loss of data
msg114159 - (view) Author: Amaury Forgeot d'Arc (amaury.forgeotdarc) * (Python committer) Date: 2010-08-17 20:23
The warnings at lines 284, 301, 461, 647 are benign. The attached patch fixes them.

The others (lines 628, 1320, 1558, 1806) are real issues: pickle will fail when given a list, a tuple or a dict larger than INT_MAX, or when the memo is too large.

There are other issues in 2.7: pickling a large string fails with a SystemError("Negative size passed to PyString_FromStringAndSize")
msg134483 - (view) Author: Alexander Belopolsky (belopolsky) * (Python committer) Date: 2011-04-26 17:41
See also issue 10640.
msg134504 - (view) Author: Alexander Belopolsky (belopolsky) * (Python committer) Date: 2011-04-26 21:47
> The warnings at lines 284, 301, 461, 647 are benign.

I agree.  There is no loss of data because Py_ssize_t variable bounds are checked before these lines are reached.

> The attached patch fixes them.

I don't like these changes:

-Pdata_poptuple(Pdata *self, Py_ssize_t start)
+Pdata_poptuple(Pdata *self, int start)

-Pdata_poplist(Pdata *self, Py_ssize_t start)
+Pdata_poplist(Pdata *self, int start)

These seem to attempt to fix

    Py_SIZE(self) = start;

assignments, but as far as I can tell, Py_SIZE(self) type is Py_ssize_t.

What do I miss here?
msg134535 - (view) Author: Amaury Forgeot d'Arc (amaury.forgeotdarc) * (Python committer) Date: 2011-04-27 08:31
The warnings above are a bit old: 027f81579b4a changed Pdata into a PyVarObject, and the "int length" member is now accessed with the Py_SIZE() macro.
Unfortunately, the only win64 buildbot is offline, and I could not find any recent log of compilation warnings.
msg134537 - (view) Author: Tim Golden (tim.golden) * (Python committer) Date: 2011-04-27 08:57
Attached. Looks like they're still there.
msg134538 - (view) Author: Amaury Forgeot d'Arc (amaury.forgeotdarc) * (Python committer) Date: 2011-04-27 09:05
Yes there are still warnings, but in different places; here is an extract of the previous buildlog.html file:

 ..\Modules\_pickle.c(156) : warning C4244:'initializing' : conversion from 'Py_ssize_t' to 'int', possible loss of data
 ..\Modules\_pickle.c(195) : warning C4244: 'initializing' : conversion from 'Py_ssize_t' to 'int', possible loss of data
 ..\Modules\_pickle.c(754) : warning C4244: 'return' : conversion from 'Py_ssize_t' to 'int', possible loss of data
 ..\Modules\_pickle.c(1278) : warning C4244: '=' : conversion from 'Py_ssize_t' to 'long', possible loss of data
 ..\Modules\_pickle.c(1285) : warning C4267: '=' : conversion from 'size_t' to 'int', possible loss of data
 ..\Modules\_pickle.c(1952) : warning C4244: '=' : conversion from 'Py_ssize_t' to 'int', possible loss of data
 ..\Modules\_pickle.c(2233) : warning C4244: '=' : conversion from 'Py_ssize_t' to 'int', possible loss of data
 ..\Modules\_pickle.c(2493) : warning C4244: '=' : conversion from 'Py_ssize_t' to 'int', possible loss of data
 ..\Modules\_pickle.c(4350) : warning C4244: '=' : conversion from 'Py_ssize_t' to 'int', possible loss of data
 ..\Modules\_pickle.c(4615) : warning C4244: 'initializing' : conversion from 'Py_ssize_t' to 'int', possible loss of data
 ..\Modules\_pickle.c(4655) : warning C4244: '=' : conversion from 'Py_ssize_t' to 'int', possible loss of data
 ..\Modules\_pickle.c(4896) : warning C4244: '=' : conversion from 'Py_ssize_t' to 'int', possible loss of data
 ..\Modules\_pickle.c(4943) : warning C4244: 'function' : conversion from 'Py_ssize_t' to 'int', possible loss of data
 ..\Modules\_pickle.c(4960) : warning C4244: '=' : conversion from 'Py_ssize_t' to 'int', possible loss of data
 ..\Modules\_pickle.c(4991) : warning C4244: 'function' : conversion from 'Py_ssize_t' to 'int', possible loss of data
 ..\Modules\_pickle.c(5147) : warning C4244: '=' : conversion from 'Py_ssize_t' to 'int', possible loss of data
msg134574 - (view) Author: Alexander Belopolsky (belopolsky) * (Python committer) Date: 2011-04-27 14:59
On Wed, Apr 27, 2011 at 4:31 AM, Amaury Forgeot d'Arc
<report@bugs.python.org> wrote:
>.. 027f81579b4a changed Pdata into a PyVarObject, and the "int length" member
> is now accessed with the Py_SIZE() macro.

ISTM that with this change Pdata struct is now indistinguishable from
a list.  I wonder if the code could be simplified by using a regular
list instead of a custom object.
msg134578 - (view) Author: Alexander Belopolsky (belopolsky) * (Python committer) Date: 2011-04-27 15:21
I believe attached issue9614.diff should fix the warnings, but I don't have a box to test this on.
msg147917 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2011-11-18 19:29
This issue is obsolete, there are no 64-bit warnings in _pickle.c anymore.
See issue 11564.
History
Date User Action Args
2011-11-18 19:29:03pitrousetstatus: open -> closed
resolution: out of date
messages: + msg147917

stage: patch review -> resolved
2011-11-18 16:23:34ezio.melottisetnosy: + brian.curtin

stage: patch review
2011-06-12 22:25:57terry.reedysetversions: - Python 3.1
2011-04-27 15:21:20belopolskysetfiles: + issue9614.diff

messages: + msg134578
2011-04-27 14:59:19belopolskysetmessages: + msg134574
2011-04-27 09:05:17amaury.forgeotdarcsetmessages: + msg134538
2011-04-27 08:57:24tim.goldensetfiles: + BuildLog.htm

messages: + msg134537
nosy: + tim.golden
2011-04-27 08:31:56amaury.forgeotdarcsetmessages: + msg134535
2011-04-26 21:47:43belopolskysetmessages: + msg134504
2011-04-26 21:22:10rhettingersetpriority: normal -> high
nosy: + rhettinger
2011-04-26 17:41:22belopolskysetnosy: + belopolsky
messages: + msg134483
2011-04-26 17:40:00santoso.wijayasetversions: + Python 3.3
2011-04-26 17:36:45santoso.wijayasetnosy: + santoso.wijaya
2010-09-13 14:12:19janglinsetnosy: + janglin
2010-08-17 20:23:16amaury.forgeotdarcsetfiles: + pickle_warnings_easy.patch

nosy: + amaury.forgeotdarc
messages: + msg114159

keywords: + patch
2010-08-15 18:13:01pitroucreate