Before 2.5 - More signed integer overflows
Hi all, There are more cases of signed integer overflows in the CPython source code base... That's on a 64-bits machine: [GCC 4.1.2 20060715 (prerelease) (Debian 4.1.1-9)] on linux2 abs(-sys.maxint-1) == -sys.maxint-1 I'd expect the same breakage everywhere when GCC 4.2 is used. Note that the above is Python 2.4.4c0 - apparently Python 2.3 compiled with GCC 4.1.2 works, although that doesn't make much sense to me because intobject.c didn't change here - 2.3, 2.4, 2.5, trunk are all the same. Both tested Pythons are Debian packages, not in-house compiled. Humpf! Looks like one person or two need to do a quick last-minute review of all places trying to deal with -sys.maxint-1, and replace them all with the "official" fix from Tim [SF 1545668]. A bientot, Armin
On Saturday 16 September 2006 21:11, Armin Rigo wrote:
Hi all,
There are more cases of signed integer overflows in the CPython source code base...
That's on a 64-bits machine:
[GCC 4.1.2 20060715 (prerelease) (Debian 4.1.1-9)] on linux2 abs(-sys.maxint-1) == -sys.maxint-1
Humpf! Looks like one person or two need to do a quick last-minute review of all places trying to deal with -sys.maxint-1, and replace them all with the "official" fix from Tim [SF 1545668].
Ick. We're now less than 24 hours from the scheduled release date for 2.5 final. There seems to be a couple of approaches here: 1. Someone (it won't be me, I'm flat out with work and paperwriting today) reviews the code and fixes it 2. We leave it for a 2.5.1. I'm expecting (based on the number of bugs found and fixed during the release cycle) that we'll probably need a 2.5.1 in about 3 months. 3. We delay the release until it's fixed. I'm strongly leaning towards (2) at this point. (1) would probably require another release candidate, while (3) would result in another release candidate and massive amount of sobbing from a lot of people (including me). -- Anthony Baxter <anthony@interlink.com.au> It's never too late to have a happy childhood.
[Armin Rigo]
There are more cases of signed integer overflows in the CPython source code base...
That's on a 64-bits machine:
[GCC 4.1.2 20060715 (prerelease) (Debian 4.1.1-9)] on linux2 abs(-sys.maxint-1) == -sys.maxint-1 < Humpf! Looks like one person or two need to do a quick last-minute review of all places trying to deal with -sys.maxint-1, and replace them all with the "official" fix from Tim [SF 1545668].
[Anthony Baxter]
Ick. We're now less than 24 hours from the scheduled release date for 2.5 final. There seems to be a couple of approaches here:
1. Someone (it won't be me, I'm flat out with work and paperwriting today) reviews the code and fixes it 2. We leave it for a 2.5.1. I'm expecting (based on the number of bugs found and fixed during the release cycle) that we'll probably need a 2.5.1 in about 3 months. 3. We delay the release until it's fixed.
I'm strongly leaning towards (2) at this point. (1) would probably require another release candidate, while (3) would result in another release candidate and massive amount of sobbing from a lot of people (including me).
I ignored this since I don't have a box where problems are visible (& nobody responded to my request to check my last flying-blind "fix" on a box where it mattered). Given that these are weird, unlikely-in-real-life endcase bugs specific to a single compiler, #2 is the natural choice. BTW, did anyone try compiling Python with -fwrapv on a box where it matters? I doubt that Python's speed is affected one way or the other, and if adding wrapv makes the problems go away, that would be an easy last-second workaround for all possible such problems (which of course could get fixed "for real" for 2.5.1, provided someone cares enough to dig into it).
BTW, did anyone try compiling Python with -fwrapv on a box where it matters? I doubt that Python's speed is affected one way or the other, and if adding wrapv makes the problems go away, that would be an easy last-second workaround for all possible such problems (which of course could get fixed "for real" for 2.5.1, provided someone cares enough to dig into it).
It's not so easy to add this option: configure needs to be taught to check whether the option is supported first; to test it, you ideally need an installation where it is supported, and one where it isn't. I've added a note to README indicating that GCC 4.2 shouldn't be used to compile Python. I don't consider this a terrible limitation, especially since GCC 4.2 isn't released, yet. OTOH, I get the same problem that Armin gets (abs(-sys.maxint-1) is negative) also on a 32-bit system, with Debian's gcc 4.1.2 (which also isn't released, yet), so it appears that the problem is already with gcc 4.1. On my system, adding -fwrapv indeed solves the problem (tested for abs()). So I added this to the README also. Regards, Martin
I also tested the fix (see patch below) for the abs() issue and it seemed to work for 4.1.1 on 64-bit. I'll apply the patch to head and 2.5 and a test after 2.5 is out. I have no idea how to search for these problems. I know that xrange can't display -sys.maxint-1 properly, but I think it works properly. n -- Index: Objects/intobject.c =================================================================== --- Objects/intobject.c (revision 51886) +++ Objects/intobject.c (working copy) @@ -763,7 +763,7 @@ register long a, x; a = v->ob_ival; x = -a; - if (a < 0 && x < 0) { + if (a < 0 && (unsigned long)x == 0-(unsigned long)x) { PyObject *o = PyLong_FromLong(a); if (o != NULL) { PyObject *result = PyNumber_Negative(o); On 9/17/06, "Martin v. Löwis" <martin@v.loewis.de> wrote:
BTW, did anyone try compiling Python with -fwrapv on a box where it matters? I doubt that Python's speed is affected one way or the other, and if adding wrapv makes the problems go away, that would be an easy last-second workaround for all possible such problems (which of course could get fixed "for real" for 2.5.1, provided someone cares enough to dig into it).
It's not so easy to add this option: configure needs to be taught to check whether the option is supported first; to test it, you ideally need an installation where it is supported, and one where it isn't.
I've added a note to README indicating that GCC 4.2 shouldn't be used to compile Python. I don't consider this a terrible limitation, especially since GCC 4.2 isn't released, yet.
OTOH, I get the same problem that Armin gets (abs(-sys.maxint-1) is negative) also on a 32-bit system, with Debian's gcc 4.1.2 (which also isn't released, yet), so it appears that the problem is already with gcc 4.1.
On my system, adding -fwrapv indeed solves the problem (tested for abs()). So I added this to the README also.
Regards, Martin _______________________________________________ Python-Dev mailing list Python-Dev@python.org http://mail.python.org/mailman/listinfo/python-dev Unsubscribe: http://mail.python.org/mailman/options/python-dev/nnorwitz%40gmail.com
Neal Norwitz schrieb:
I also tested the fix (see patch below) for the abs() issue and it seemed to work for 4.1.1 on 64-bit. I'll apply the patch to head and 2.5 and a test after 2.5 is out.
Please also add it to 2.4.
Index: Objects/intobject.c =================================================================== --- Objects/intobject.c (revision 51886) +++ Objects/intobject.c (working copy) @@ -763,7 +763,7 @@ register long a, x; a = v->ob_ival; x = -a; - if (a < 0 && x < 0) { + if (a < 0 && (unsigned long)x == 0-(unsigned long)x) {
Hmm. Shouldn't this drop 'x' and use 'a' instead? If a is -sys.maxint-1, -a is already undefined. Regards, Martin P.S. As for finding these problems, I would have hoped that -ftrapv could help - unfortunately, gcc breaks with this option (consumes incredible amounts of memory).
On 9/17/06, "Martin v. Löwis" <martin@v.loewis.de> wrote:
Neal Norwitz schrieb:
I also tested the fix (see patch below) for the abs() issue and it seemed to work for 4.1.1 on 64-bit. I'll apply the patch to head and 2.5 and a test after 2.5 is out.
Please also add it to 2.4.
Yes
Index: Objects/intobject.c =================================================================== --- Objects/intobject.c (revision 51886) +++ Objects/intobject.c (working copy) @@ -763,7 +763,7 @@ register long a, x; a = v->ob_ival; x = -a; - if (a < 0 && x < 0) { + if (a < 0 && (unsigned long)x == 0-(unsigned long)x) {
Hmm. Shouldn't this drop 'x' and use 'a' instead? If a is -sys.maxint-1, -a is already undefined.
Yes, probably. I didn't review carefully.
P.S. As for finding these problems, I would have hoped that -ftrapv could help - unfortunately, gcc breaks with this option (consumes incredible amounts of memory).
I'm getting a crash when running test_builtin and test_calendar (at least) with gcc 4.1.1 on amd64. It's happening in pymalloc, though I don't know what the cause is. I thought I tested with gcc 4.1 before, but probably would have been in debug mode. n -- Program received signal SIGSEGV, Segmentation fault. [Switching to Thread 16384 (LWP 22020)] PyObject_Malloc (nbytes=40) at obmalloc.c:746 746 if ((pool->freeblock = *(block **)bp) != NULL) { (gdb) p bp $1 = (block *) 0x2a9558d41800 <Address 0x2a9558d41800 out of bounds> (gdb) l 741 * Pick up the head block of its free list. 742 */ 743 ++pool->ref.count; 744 bp = pool->freeblock; 745 assert(bp != NULL); 746 if ((pool->freeblock = *(block **)bp) != NULL) { 747 UNLOCK(); 748 return (void *)bp; 749 } 750 /* (gdb) p *pool $2 = {ref = {_padding = 0x1a <Address 0x1a out of bounds>, count = 26}, freeblock = 0x2a9558d41800 <Address 0x2a9558d41800 out of bounds>, nextpool = 0x2a95eac000, prevpool = 0x620210, arenaindex = 0, szidx = 4, nextoffset = 4088, maxnextoffset = 4056} (gdb) p size $3 = 4
Neal Norwitz schrieb:
I'm getting a crash when running test_builtin and test_calendar (at least) with gcc 4.1.1 on amd64. It's happening in pymalloc, though I don't know what the cause is. I thought I tested with gcc 4.1 before, but probably would have been in debug mode.
Can't really check right now, but it might be that this is just the limitation that a debug obmalloc doesn't work on 64-bit systems. There is a header at each block with a fixed size of 4 bytes, even though it should be 8 bytes on 64-bit systems. This header is there only in a debug build. Regards, Martin
[Neal Norwitz]
I'm getting a crash when running test_builtin and test_calendar (at least) with gcc 4.1.1 on amd64. It's happening in pymalloc, though I don't know what the cause is. I thought I tested with gcc 4.1 before, but probably would have been in debug mode.
Neil, in context it was unclear whether you were using trapv at the time. Were you? [Martin v. Löwis]
Can't really check right now, but it might be that this is just the limitation that a debug obmalloc doesn't work on 64-bit systems. There is a header at each block with a fixed size of 4 bytes, even though it should be 8 bytes on 64-bit systems. This header is there only in a debug build.
Funny then how all the 64-bit buildbots manage to pass running debug builds ;-) As of revs 46637 + 46638 (3-4 months ago), debug-build obmalloc uses sizeof(size_t) bytes for each of its header and trailer debugging fields. Before then, the debug-build obmalloc was "safe" in this respect: if it /needed/ to store more than 4 bytes in a debug bookkeeping field, it assert-failed in a debug build. That would happen if and only if a call to malloc/realloc requested >= 2**32 bytes, so was never provoked by Python's test suite. As of rev 46638, that limitation should have gone away.
On 9/18/06, Tim Peters <tim.peters@gmail.com> wrote:
[Neal Norwitz]
I'm getting a crash when running test_builtin and test_calendar (at least) with gcc 4.1.1 on amd64. It's happening in pymalloc, though I don't know what the cause is. I thought I tested with gcc 4.1 before, but probably would have been in debug mode.
Neil, in context it was unclear whether you were using trapv at the time. Were you?
No trapv, just ./configure --without-pydebug IIRC. I should have sent a msg last night, but was too tired. I got the same crash (I think) with gcc 3.4.4, so it's almost definitely due to an outstanding change, not python's or gcc's fault. n
participants (5)
-
"Martin v. Löwis"
-
Anthony Baxter
-
Armin Rigo
-
Neal Norwitz
-
Tim Peters