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.

classification
Title: gcc trunk (4.2) exposes a signed integer overflows
Type: Stage:
Components: Interpreter Core Versions: Python 2.4
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: arigo Nosy List: arigo, dhopwood, jackjansen, jwhowarth, nnorwitz, sjoerd, tim.peters
Priority: critical Keywords:

Created on 2006-08-24 03:14 by jwhowarth, last changed 2022-04-11 14:56 by admin. This issue is now closed.

Messages (16)
msg29642 - (view) Author: Jack Howarth (jwhowarth) Date: 2006-08-24 03:14
While building python 2.4.3 with the current gcc trunk (soon to be 4.2), 
I uncovered a signed integer overflows bug in Python with the help of 
one of the gcc developers. The bug I observed is documented in this 
gcc mailing list message...

http://gcc.gnu.org/ml/gcc/2006-08/msg00436.html

The gcc developer comments about its origin are in the messages...

http://gcc.gnu.org/ml/gcc/2006-08/msg00434.html
http://gcc.gnu.org/ml/gcc/2006-08/msg00442.html

which in short says...

It *is* a bug in python, here is the proof:
https://codespeak.net/viewvc/vendor/cpython/Python-r243/dist/src/
Objects/intobject.c?revision=25647&view=markup
Function

* i_divmod*(*register* *long* x, *register* *long* y,

the following lines:

/        /* (-sys.maxint-1)/-1 is the only overflow case. *//
	*if* (y == -1 && x < 0 && x == -x)
		*return* DIVMOD_OVERFLOW;

If overflow is barred then x==-x may happen only when x==0.
This conflicts with x<0, which means that the compiler may assume
that
  x<0 && x==-x
always yields false. This may allow the compiler to eliminate the whole 
if
statement. Hence, clearly python is at fault.
msg29643 - (view) Author: Jack Howarth (jwhowarth) Date: 2006-08-24 04:13
Logged In: YES 
user_id=403009

As suggested by another gcc developer in...

http://gcc.gnu.org/ml/gcc/2006-08/msg00446.html

...the following patch eliminates the error when python is built with gcc 
trunk...

--- Python-2.4.3/Objects/intobject.c.org        2006-08-23 
23:49:33.000000000 -0400
+++ Python-2.4.3/Objects/intobject.c    2006-08-23 23:52:01.000000000 
-0400
@@ -479,7 +479,7 @@
                return DIVMOD_ERROR;
        }
        /* (-sys.maxint-1)/-1 is the only overflow case. */
-       if (y == -1 && x < 0 && x == -x)
+       if (y == -1 && x < 0 && ((unsigned)x) == -(unsigned)x)
                return DIVMOD_OVERFLOW;
        xdivy = x / y;
        xmody = x - xdivy * y;

This change allows python to completely pass its make check now when built 
with gcc trunk.
msg29644 - (view) Author: Jack Jansen (jackjansen) * (Python committer) Date: 2006-08-24 09:14
Logged In: YES 
user_id=45365

I don't think this is a mac-specific bug, and I'm also not really the right person 
to look into this...
msg29645 - (view) Author: Jack Howarth (jwhowarth) Date: 2006-08-24 11:22
Logged In: YES 
user_id=403009

The was a few other comments from the gcc developers on the proposed 
fix...

http://gcc.gnu.org/ml/gcc/2006-08/msg00448.html
http://gcc.gnu.org/ml/gcc/2006-08/msg00449.html

...so that since x is a long the more correct fix is...

--- Python-2.4.3/Objects/intobject.c.org        2006-08-24 
07:06:51.000000000 -0400
+++ Python-2.4.3/Objects/intobject.c    2006-08-24 07:08:06.000000000 
-0400
@@ -479,7 +479,7 @@
                return DIVMOD_ERROR;
        }
        /* (-sys.maxint-1)/-1 is the only overflow case. */
-       if (y == -1 && x < 0 && x == -x)
+       if (y == -1 && x < 0 && ((unsigned long)x) == -(unsigned long)x)
                return DIVMOD_OVERFLOW;
        xdivy = x / y;
        xmody = x - xdivy * y;

I have tested this form of the patch and it works as well. My main concern is 
that we get this fix in python 2.5 before release. Jack, could you reassign this 
to the person you think might be most appropriate out of the list of python 
developers? I really should be a pretty simple review for the patch.
msg29646 - (view) Author: Jack Jansen (jackjansen) * (Python committer) Date: 2006-08-24 14:37
Logged In: YES 
user_id=45365

Hmm... intobject.c doesn't really have a "champion"...

Neal, I'm assigning this to you purely on the ground that you're the last person 
to have edited this file. Feel free to pass on (but not back:-).
msg29647 - (view) Author: Neal Norwitz (nnorwitz) * (Python committer) Date: 2006-08-24 14:54
Logged In: YES 
user_id=33168

Assigning to Tim, he likes these problems. :-)
He recently fixed a similar problem in another piece of
code.  I'm going to try to grep and see if I can find more
occurrences of this.
msg29648 - (view) Author: Sjoerd Mullender (sjoerd) * (Python committer) Date: 2006-08-24 14:54
Logged In: YES 
user_id=43607

Just a comment, I'm not claiming this bug.

The test (x < 0 && x == -x) tests whether x is equal to the
smallest negative number.  If x is equal to -2147483648,
then -x is also equal to -2147483648 due to overflow.

What does this version of gcc do with this code when x =
-2147483648?
msg29649 - (view) Author: Neal Norwitz (nnorwitz) * (Python committer) Date: 2006-08-24 15:00
Logged In: YES 
user_id=33168

I grepped for ' == .*-[^1>]' and ' != .*-[^1>]' and didn't
spot any other occurrences.
msg29650 - (view) Author: Jack Howarth (jwhowarth) Date: 2006-08-24 15:22
Logged In: YES 
user_id=403009

Everyone involved in reviewing this patch should definitely
read the following sequence of gcc mailing list messages
which show the process by which this patch was arrived at...

http://gcc.gnu.org/ml/gcc/2006-08/msg00434.html
http://gcc.gnu.org/ml/gcc/2006-08/msg00436.html
http://gcc.gnu.org/ml/gcc/2006-08/msg00437.html
http://gcc.gnu.org/ml/gcc/2006-08/msg00443.html
http://gcc.gnu.org/ml/gcc/2006-08/msg00446.html
http://gcc.gnu.org/ml/gcc/2006-08/msg00447.html
http://gcc.gnu.org/ml/gcc/2006-08/msg00449.html

So we have had lots of gcc developer eyes on this
problem and they all agree on the flaw and the fix
as posted. It's unfortunate that I had to abuse their
mailing list to get this addressed before python 2.5
gets released.
msg29651 - (view) Author: Tim Peters (tim.peters) * (Python committer) Date: 2006-08-26 20:25
Logged In: YES 
user_id=31435

Looks like the same deal as bug 1521947 (which was about
similar code in PyOS_strtol()).
msg29652 - (view) Author: Tim Peters (tim.peters) * (Python committer) Date: 2006-08-26 20:33
Logged In: YES 
user_id=31435

Boosted priority to 8 since it was brought up on python-dev
as a suggested 2.5 release-blocker.  The patch in the first
comment looks fine, if a release manager wants to apply it.

Python 2.4 surely has the same "issue".
msg29653 - (view) Author: David Hopwood (dhopwood) Date: 2006-08-26 23:24
Logged In: YES 
user_id=634468

The correct patch is the one that uses

if (y == -1 && x < 0 && (unsigned long)x == -(unsigned long)x)

The one that uses (unsigned int)x will break some 64-bit
platforms where int != long.
msg29654 - (view) Author: Neal Norwitz (nnorwitz) * (Python committer) Date: 2006-09-05 04:04
Logged In: YES 
user_id=33168

Tim checked in fixes for 2.6 (r51716), 2.5 (r51711), and 2.4.

msg29655 - (view) Author: Armin Rigo (arigo) * (Python committer) Date: 2006-09-16 11:28
Logged In: YES 
user_id=4771

More of the same kind of problem:
abs(-sys.maxint-1) sometimes gives -sys.maxint-1.
It would be a good idea to review all places that need
to special-case -sys.maxint-1 for overflow detection.
(It would be a still better idea to review all overflow
detection code, but that may have to wait after the
2.5 release).
msg29656 - (view) Author: Armin Rigo (arigo) * (Python committer) Date: 2006-10-03 10:17
Logged In: YES 
user_id=4771

I'd like to review this in 2.4/2.5/trunk before the 2.4.4
release.

Debian "testing" ships with everything compiled with the
faulty gcc -- even though this gcc is not released yet!  I
hate Debian's policies.  "Fixing" 2.4.4 would help me a bit
to get a reasonable Python installation on Debian machines
where I have to log to (assuming the sysadmin knew he had
to fish 72 small packages to get just a complete stdlib,
that is, but that's another matter).
msg29657 - (view) Author: Armin Rigo (arigo) * (Python committer) Date: 2006-10-04 12:22
Logged In: YES 
user_id=4771

Checked in: r52136 (2.4)
            r52138 (2.5)
            r52139 (2.6)
History
Date User Action Args
2022-04-11 14:56:19adminsetgithub: 43886
2006-08-24 03:14:43jwhowarthcreate