Bug #1521947: possible bug in mystrtol.c with recent gcc.

In general, C doesn't define anything about what happens when
an operation on a signed integral type overflows, and PyOS_strtol()
did several formally undefined things of that nature on signed
longs.  Some version of gcc apparently tries to exploit that now,
and PyOS_strtol() could fail to detect overflow then.

Tried to repair all that, although it seems at least as likely to me
that we'll get screwed by bad platform definitions for LONG_MIN
and/or LONG_MAX now.  For that reason, I don't recommend backporting
this.

Note that I have no box on which this makes a lick of difference --
can't really test it, except to note that it didn't break anything
on my boxes.

Silent change:  PyOS_strtol() used to return the hard-coded 0x7fffffff
in case of overflow.  Now it returns LONG_MAX.  They're the same only on
32-bit boxes (although C doesn't guarantee that either ...).

diff --git a/Misc/NEWS b/Misc/NEWS
index dbdb46b28feedcda86e2a80972f97fcc0b7b24a1..c8bfcfd91f3fd7bfe4eea1d927bdc5167252a818 100644 (file)
--- a/Misc/NEWS
+++ b/Misc/NEWS
@@ -12,6 +12,11 @@ What's New in Python 2.5 release candidate 1?
 Core and builtins
 -----------------
 
+- Bug #1521947: When checking for overflow, ``PyOS_strtol()`` used some
+  operations on signed longs that are formally undefined by C.
+  Unfortunately, at least one compiler now cares about that, so complicated
+  the code to make that compiler happy again.
+
 - Bug #1524310: Properly report errors from FindNextFile in os.listdir.
 
 - Patch #1232023: Stop including current directory in search
@@ -37,7 +42,7 @@ Core and builtins
   mapping the faux "thread id" 0 to the current frame.
 
 - Bug #1525447: build on MacOS X on a case-sensitive filesystem.
-  
+
 
 Library
 -------
@@ -88,7 +93,7 @@ Library
 Extension Modules
 -----------------
 
-- Bug #1471938: Fix curses module build problem on Solaris 8; patch by 
+- Bug #1471938: Fix curses module build problem on Solaris 8; patch by
   Paul Eggert.
 
 - Patch #1448199: Release interpreter lock in _winreg.ConnectRegistry.

diff --git a/Python/mystrtoul.c b/Python/mystrtoul.c
index 51553fbe1f34e8c49e81d3a1d31b0cc41f80ca06..0dda4be2ee828b387fda3044f7cf09a520e95ffb 100644 (file)
--- a/Python/mystrtoul.c
+++ b/Python/mystrtoul.c
@@ -195,10 +195,19 @@ overflowed:
        return (unsigned long)-1;
 }
 
+/* Checking for overflow in PyOS_strtol is a PITA since C doesn't define
+ * anything about what happens when a signed integer operation overflows,
+ * and some compilers think they're doing you a favor by being "clever"
+ * then.  Python assumes a 2's-complement representation, so that the bit
+ * pattern for the largest postive signed long is LONG_MAX, and for
+ * the smallest negative signed long is LONG_MAX + 1.
+ */
+
 long
 PyOS_strtol(char *str, char **ptr, int base)
 {
        long result;
+       unsigned long uresult;
        char sign;
 
        while (*str && isspace(Py_CHARMASK(*str)))
@@ -208,17 +217,20 @@ PyOS_strtol(char *str, char **ptr, int base)
        if (sign == '+' || sign == '-')
                str++;
 
-       result = (long) PyOS_strtoul(str, ptr, base);
+       uresult = PyOS_strtoul(str, ptr, base);
 
-       /* Signal overflow if the result appears negative,
-          except for the largest negative integer */
-       if (result < 0 && !(sign == '-' && result == -result)) {
+       if (uresult <= (unsigned long)LONG_MAX) {
+               result = (long)uresult;
+               if (sign == '-')
+                       result = -result;
+       }
+       else if (sign == '-' && uresult == (unsigned long)LONG_MAX + 1) {
+               assert(LONG_MIN == -LONG_MAX-1);
+               result = LONG_MIN;
+       }
+       else {
                errno = ERANGE;
-               result = 0x7fffffff;
+               result = LONG_MAX;
        }
-
-       if (sign == '-')
-               result = -result;
-
        return result;
 }