Issue1038854
This issue tracker has been migrated to GitHub,
and is currently read-only.
For more information,
see the GitHub FAQs in the Python's Developer Guide.
Created on 2004-10-02 05:07 by sgatwasetde, last changed 2022-04-11 14:56 by admin. This issue is now closed.
Files | ||||
---|---|---|---|---|
File name | Uploaded | Description | Edit | |
structmodule.diff | sgatwasetde, 2004-10-08 06:07 |
Messages (9) | |||
---|---|---|---|
msg46996 - (view) | Author: Stefan Grundmann (sgatwasetde) | Date: 2004-10-02 05:07 | |
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) |
|||
msg46997 - (view) | Author: Armin Rigo (arigo) * | Date: 2004-10-05 13:31 | |
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. |
|||
msg46998 - (view) | Author: Stefan Grundmann (sgatwasetde) | Date: 2004-10-06 06:31 | |
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 ;) ). |
|||
msg46999 - (view) | Author: Stefan Grundmann (sgatwasetde) | Date: 2004-10-08 06:07 | |
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. |
|||
msg47000 - (view) | Author: Neal Norwitz (nnorwitz) * | Date: 2004-11-08 03:15 | |
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? |
|||
msg47001 - (view) | Author: Stefan Grundmann (sgatwasetde) | Date: 2004-11-10 08:36 | |
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. |
|||
msg47002 - (view) | Author: Armin Rigo (arigo) * | Date: 2004-11-10 21:31 | |
Logged In: YES user_id=4771 This indicate that similar breakage will probably occur in user code if we add range checking. Do we want to take the risk? It looks overkill, but what about issuing a warning and turning it into an error in the next version? |
|||
msg47003 - (view) | Author: Stefan Grundmann (sgatwasetde) | Date: 2004-11-11 22:20 | |
Logged In: YES user_id=1131807 Issuing a warning instead of an error might be a good idea (to give the user-base some time adapt). But then we still have to deal with the fact that the some python modules are broken on 64 bit (at least big-endian). The gzip module for example does not work correctly even with the old code (because of stuctmodule). And there is user code out there that relies on correct overflow detection (which was the reason i submitted the patch). Another way would be to omit the overflow detection completely and heave the user take care of it. This will break a lot of applications but is imho more consistent then the old behavior (check some cases on some architectures). Personally i would like to have a structmodule in python 2.4 that works a one would expect (overflow detection) but that is not my decision to make. The fact that code which is under control of the python project uses struct.pack (...) in erroneous ways should be fixed (even if it does work on 32 bit archs atm). |
|||
msg47004 - (view) | Author: Martin v. Löwis (loewis) * | Date: 2006-11-11 13:23 | |
Logged In: YES user_id=21627 I believe this has been fixed in Python 2.5 in a different way (involving a rewrite of the struct module); closing as out-of-date. sgatwasetde, if you think further changes are needed, please submit a new patch. |
History | |||
---|---|---|---|
Date | User | Action | Args |
2022-04-11 14:56:07 | admin | set | github: 40973 |
2004-10-02 05:07:03 | sgatwasetde | create |