[Patches] [ python-Patches-1038854 ] Fix struct.pack on 64-bit archs (broken on 2.*)

[SourceForge.net]

Patches item #1038854, was opened at 2004-10-02 07:07
Message generated for change (Comment added) made by sgatwasetde
You can respond by visiting: 
https://sourceforge.net/tracker/?func=detail&atid=305470&aid=1038854&group_id=5470

Category: Modules
Group: Python 2.4
Status: Open
Resolution: None
Priority: 5
Submitted By: Stefan Grundmann (sgatwasetde)
Assigned to: Nobody/Anonymous (nobody)
Summary: Fix struct.pack on 64-bit archs (broken on 2.*)

Initial Comment:

Description:

The code in the struct module assumes that sizeof(long)
== sizeof(int)
which is wrong on (most) 64-bit architectures (linux on
amd64 with a 32-bit userland is an exception). 


How To Repeat:

on a 32-bit platform struct.pack behaves as expected:

$  uname -m  -r  -s
FreeBSD 4.10-STABLE i386
$ python -c "import struct; print repr(struct.pack('I',
4294967296))"
Traceback (most recent call last):
  File "<string>", line 1, in ?
OverflowError: long int too large to convert

on a 64-bit platform it treats integers as longs 
(it does not check for over/underflows and returns the
lower 4 byte): 

$ uname -m  -r  -s
FreeBSD 5.2-CURRENT sparc64
$ python -c "import struct; print repr(struct.pack('I',
4294967296))" 
'\x00\x00\x00\x00'


Fix:

in python/python/dist/src/Modules/structmodule.c:
np_uint() and np_int() have to check for 
over/underflows using MAX_UINT, MAX_INT, MIN_INT  as
np_ushort() and np_short() already do for MAX_USHORT, ...

the attached patch does this
(diff was generated using diff -rNu and  Revision 2.62
of python/python/dist/src/Modules/structmodule.c)






----------------------------------------------------------------------

>Comment By: Stefan Grundmann (sgatwasetde)
Date: 2004-11-10 09:36

Message:
Logged In: YES 
user_id=1131807

The patch breaks test_binhex on 32 and 64 bit architectures 
because Lib/binhex.py is using struct.pack('>h' ...) to pack 
an unsigned short (which is  wrong but does work with the 
current version of Modules/structmodule.c because of the 
lack of range checking).  
 
The patch breaks test_gzip (and test_tarfile) on 64 bit 
architectures because Lib/gzip.py is using write32 (which 
uses to pack('<l',...) ) to write an unsigned 32 bit checksum.  
 
This should be fixed in Lib/gzip.py and Lib/binhex.py. 
   

----------------------------------------------------------------------

Comment By: Neal Norwitz (nnorwitz)
Date: 2004-11-08 04:15

Message:
Logged In: YES 
user_id=33168

This patch breaks 3 tests:  test_binhex test_gzip test_tarfile.
tarfile breaks because of gzip.

Run on opterons.  Can you update the patch so these tests pass?

----------------------------------------------------------------------

Comment By: Stefan Grundmann (sgatwasetde)
Date: 2004-10-08 08:07

Message:
Logged In: YES 
user_id=1131807

The attached patch will fix the range-checking-code of the
integer pack functions for 64 and 32 bit architectures
(tested on i386 and Sparc64, 64-bit little-endian was not
tested 'cause of lack of hardware). All test cases worked as
expected, there is no more need for BUGGY_RANGE_CHECK.

I'm a bit unsure about the used method to get an unsigned
long from a Python_Long object with overflow checking... 
PyLong_AsUnsignedLong(PyLong_FromLong(PyInt_AS_LONG(v))) 
looks a rather excessive.


----------------------------------------------------------------------

Comment By: Stefan Grundmann (sgatwasetde)
Date: 2004-10-06 08:31

Message:
Logged In: YES 
user_id=1131807

I removed the attached patch since it only handles overflows
for np_int/np_uint. bp_int/bp_uint and lp_int/lp_unit have
different issues (no overflow checking at all, unneccessary
loops - that will not be optimized by 
the  compiler). I'm working on a new patch that fixes these
problems 
(and the rest of  BUGGY_RANGE_CHECK ;) ).

----------------------------------------------------------------------

Comment By: Armin Rigo (arigo)
Date: 2004-10-05 15:31

Message:
Logged In: YES 
user_id=4771

At a first glance, it appears that there is the same problem
in bp_int/bp_uint and lp_int/lp_unit.  Can you check that
pack('<I',...) and pack('>I',...) fail to detect the
overflow in the same way?  If so, can you also provide the
patch for these 4 other routines?  Finally, it would be nice
to add a test case in Lib/test/test_struct.py.

----------------------------------------------------------------------

You can respond by visiting: 
https://sourceforge.net/tracker/?func=detail&atid=305470&aid=1038854&group_id=5470