Forward-port of r52136: a review of overflow-detecting code.

* unified the way intobject, longobject and mystrtoul handle
  values around -sys.maxint-1.

* in general, trying to entierely avoid overflows in any computation
  involving signed ints or longs is extremely involved.  Fixed a few
  simple cases where a compiler might be too clever (but that's all
  guesswork).

* more overflow checks against bad data in marshal.c.

* 2.5 specific: fixed a number of places that were still confusing int
  and Py_ssize_t.  Some of them could potentially have caused
  "real-world" breakage.

* list.pop(x): fixing overflow issues on x was messy.  I just reverted
  to PyArg_ParseTuple("n"), which does the right thing.  (An obscure
  test was trying to give a Decimal to list.pop()... doesn't make
  sense any more IMHO)

* trying to write a few tests...

Lib/test/list_tests.py

@@ -269,7 +269,6 @@ def test_insert(self):
         self.assertRaises(TypeError, a.insert)
 
     def test_pop(self):
-        from decimal import Decimal
         a = self.type2test([-1, 0, 1])
         a.pop()
         self.assertEqual(a, [-1, 0])
@@ -281,8 +280,6 @@ def test_pop(self):
         self.assertRaises(IndexError, a.pop)
         self.assertRaises(TypeError, a.pop, 42, 42)
         a = self.type2test([0, 10, 20, 30, 40])
-        self.assertEqual(a.pop(Decimal(2)), 20)
-        self.assertRaises(IndexError, a.pop, Decimal(25))
 
     def test_remove(self):
         a = self.type2test([0, 0, 1])

Lib/test/test_builtin.py

@@ -156,6 +156,11 @@ def test_any(self):
         S = [10, 20, 30]
         self.assertEqual(any(x > 42 for x in S), False)
 
+    def test_neg(self):
+        x = -sys.maxint-1
+        self.assert_(isinstance(x, int))
+        self.assertEqual(-x, sys.maxint+1)
+
     def test_apply(self):
         def f0(*args):
             self.assertEqual(args, ())
@@ -702,9 +707,11 @@ def test_int(self):
                         pass
 
         s = repr(-1-sys.maxint)
-        self.assertEqual(int(s)+1, -sys.maxint)
+        x = int(s)
+        self.assertEqual(x+1, -sys.maxint)
+        self.assert_(isinstance(x, int))
         # should return long
-        int(s[1:])
+        self.assertEqual(int(s[1:]), sys.maxint+1)
 
         # should return long
         x = int(1e100)

Lib/test/test_long.py

@@ -247,17 +247,23 @@ def test_misc(self):
             "long(-sys.maxint-1) != -sys.maxint-1")
 
         # long -> int should not fail for hugepos_aslong or hugeneg_aslong
+        x = int(hugepos_aslong)
         try:
-            self.assertEqual(int(hugepos_aslong), hugepos,
+            self.assertEqual(x, hugepos,
                   "converting sys.maxint to long and back to int fails")
         except OverflowError:
             self.fail("int(long(sys.maxint)) overflowed!")
+        if not isinstance(x, int):
+            raise TestFailed("int(long(sys.maxint)) should have returned int")
+        x = int(hugeneg_aslong)
         try:
-            self.assertEqual(int(hugeneg_aslong), hugeneg,
+            self.assertEqual(x, hugeneg,
                   "converting -sys.maxint-1 to long and back to int fails")
         except OverflowError:
             self.fail("int(long(-sys.maxint-1)) overflowed!")
-
+        if not isinstance(x, int):
+            raise TestFailed("int(long(-sys.maxint-1)) should have "
+                             "returned int")
         # but long -> int should overflow for hugepos+1 and hugeneg-1
         x = hugepos_aslong + 1
         try:
@@ -282,6 +288,17 @@ class long2(long):
         self.assert_(type(y) is long,
             "overflowing int conversion must return long not long subtype")
 
+        # long -> Py_ssize_t conversion
+        class X(object):
+            def __getslice__(self, i, j):
+                return i, j
+
+        self.assertEqual(X()[-5L:7L], (-5, 7))
+        # use the clamping effect to test the smallest and largest longs
+        # that fit a Py_ssize_t
+        slicemin, slicemax = X()[-2L**100:2L**100]
+        self.assertEqual(X()[slicemin:slicemax], (slicemin, slicemax))
+
 # ----------------------------------- tests of auto int->long conversion
 
     def test_auto_overflow(self):

Misc/NEWS

@@ -12,8 +12,14 @@ What's New in Python 2.5.1c1?
 Core and builtins
 -----------------
 
-- Integer negation and absolute value were fixed to not rely
-  on undefined behaviour of the C compiler anymore.
+- list.pop(x) accepts any object x following the __index__ protocol.
+
+- Fix some leftovers from the conversion from int to Py_ssize_t
+  (relevant to strings and sequences of more than 2**31 items).
+
+- A number of places, including integer negation and absolute value,
+  were fixed to not rely on undefined behaviour of the C compiler
+  anymore.
 
 - Bug #1566800: make sure that EnvironmentError can be called with any
   number of arguments, as was the case in Python 2.4.

Modules/cPickle.c

@@ -1024,7 +1024,7 @@ save_int(Picklerobject *self, PyObject *args)
 static int
 save_long(Picklerobject *self, PyObject *args)
 {
-	int size;
+	Py_ssize_t size;
 	int res = -1;
 	PyObject *repr = NULL;
 
@@ -1066,7 +1066,7 @@ save_long(Picklerobject *self, PyObject *args)
 		 * byte at the start, and cut it back later if possible.
 		 */
 		nbytes = (nbits >> 3) + 1;
-		if ((int)nbytes < 0 || (size_t)(int)nbytes != nbytes) {
+		if (nbytes > INT_MAX) {
 			PyErr_SetString(PyExc_OverflowError, "long too large "
 				"to pickle");
 			goto finally;
@@ -1208,12 +1208,14 @@ save_string(Picklerobject *self, PyObject *args, int doput)
 			c_str[1] = size;
 			len = 2;
 		}
-		else {
+		else if (size <= INT_MAX) {
 			c_str[0] = BINSTRING;
 			for (i = 1; i < 5; i++)
 				c_str[i] = (int)(size >> ((i - 1) * 8));
 			len = 5;
 		}
+		else
+			return -1;    /* string too large */
 
 		if (self->write_func(self, c_str, len) < 0)
 			return -1;
@@ -1286,7 +1288,7 @@ modified_EncodeRawUnicodeEscape(const Py_UNICODE *s, int size)
 static int
 save_unicode(Picklerobject *self, PyObject *args, int doput)
 {
-	int size, len;
+	Py_ssize_t size, len;
 	PyObject *repr=0;
 
 	if (!PyUnicode_Check(args))
@@ -1325,6 +1327,8 @@ save_unicode(Picklerobject *self, PyObject *args, int doput)
 
 		if ((size = PyString_Size(repr)) < 0)
 			goto err;
+		if (size > INT_MAX)
+			return -1;   /* string too large */
 
 		c_str[0] = BINUNICODE;
 		for (i = 1; i < 5; i++)

Objects/abstract.c

@@ -1652,20 +1652,18 @@ _PySequence_IterSearch(PyObject *seq, PyObject *obj, int operation)
 		if (cmp > 0) {
 			switch (operation) {
 			case PY_ITERSEARCH_COUNT:
-				++n;
-				if (n <= 0) {
-					/* XXX(nnorwitz): int means ssize_t */
+				if (n == PY_SSIZE_T_MAX) {
 					PyErr_SetString(PyExc_OverflowError,
-				                "count exceeds C int size");
+					       "count exceeds C integer size");
 					goto Fail;
 				}
+				++n;
 				break;
 
 			case PY_ITERSEARCH_INDEX:
 				if (wrapped) {
-					/* XXX(nnorwitz): int means ssize_t */
 					PyErr_SetString(PyExc_OverflowError,
-			                	"index exceeds C int size");
+					       "index exceeds C integer size");
 					goto Fail;
 				}
 				goto Done;
@@ -1680,9 +1678,9 @@ _PySequence_IterSearch(PyObject *seq, PyObject *obj, int operation)
 		}
 
 		if (operation == PY_ITERSEARCH_INDEX) {
-			++n;
-			if (n <= 0)
+			if (n == PY_SSIZE_T_MAX)
 				wrapped = 1;
+			++n;
 		}
 	}
 

Objects/fileobject.c

@@ -1001,6 +1001,7 @@ getline_via_fgets(FILE *fp)
 	size_t nfree;	/* # of free buffer slots; pvend-pvfree */
 	size_t total_v_size;  /* total # of slots in buffer */
 	size_t increment;	/* amount to increment the buffer */
+	size_t prev_v_size;
 
 	/* Optimize for normal case:  avoid _PyString_Resize if at all
 	 * possible via first reading into stack buffer "buf".
@@ -1115,8 +1116,11 @@ getline_via_fgets(FILE *fp)
 		/* expand buffer and try again */
 		assert(*(pvend-1) == '\0');
 		increment = total_v_size >> 2;	/* mild exponential growth */
+		prev_v_size = total_v_size;
 		total_v_size += increment;
-		if (total_v_size > PY_SSIZE_T_MAX) {
+		/* check for overflow */
+		if (total_v_size <= prev_v_size ||
+		    total_v_size > PY_SSIZE_T_MAX) {
 			PyErr_SetString(PyExc_OverflowError,
 			    "line is longer than a Python string can hold");
 			Py_DECREF(v);
@@ -1125,7 +1129,7 @@ getline_via_fgets(FILE *fp)
 		if (_PyString_Resize(&v, (int)total_v_size) < 0)
 			return NULL;
 		/* overwrite the trailing null byte */
-		pvfree = BUF(v) + (total_v_size - increment - 1);
+		pvfree = BUF(v) + (prev_v_size - 1);
 	}
 	if (BUF(v) + total_v_size != p)
 		_PyString_Resize(&v, p - BUF(v));

Objects/intobject.c

@@ -546,6 +546,17 @@ int_mul(PyObject *v, PyObject *w)
 	}
 }
 
+/* Integer overflow checking for unary negation: on a 2's-complement
+ * box, -x overflows iff x is the most negative long.  In this case we
+ * get -x == x.  However, -x is undefined (by C) if x /is/ the most
+ * negative long (it's a signed overflow case), and some compilers care.
+ * So we cast x to unsigned long first.  However, then other compilers
+ * warn about applying unary minus to an unsigned operand.  Hence the
+ * weird "0-".
+ */
+#define UNARY_NEG_WOULD_OVERFLOW(x)	\
+	((x) < 0 && (unsigned long)(x) == 0-(unsigned long)(x))
+
 /* Return type of i_divmod */
 enum divmod_result {
 	DIVMOD_OK,		/* Correct result */
@@ -564,14 +575,8 @@ i_divmod(register long x, register long y,
 				"integer division or modulo by zero");
 		return DIVMOD_ERROR;
 	}
-	/* (-sys.maxint-1)/-1 is the only overflow case.  x is the most
-	 * negative long iff x < 0 and, on a 2's-complement box, x == -x.
-	 * However, -x is undefined (by C) if x /is/ the most negative long
-	 * (it's a signed overflow case), and some compilers care.  So we cast
-	 * x to unsigned long first.  However, then other compilers warn about
-	 * applying unary minus to an unsigned operand.  Hence the weird "0-".
-	 */
-	if (y == -1 && x < 0 && (unsigned long)x == 0-(unsigned long)x)
+	/* (-sys.maxint-1)/-1 is the only overflow case. */
+	if (y == -1 && UNARY_NEG_WOULD_OVERFLOW(x))
 		return DIVMOD_OVERFLOW;
 	xdivy = x / y;
 	xmody = x - xdivy * y;
@@ -762,7 +767,8 @@ int_neg(PyIntObject *v)
 {
 	register long a;
 	a = v->ob_ival;
-	if (a < 0 && (unsigned long)a == 0-(unsigned long)a) {
+        /* check for overflow */
+	if (UNARY_NEG_WOULD_OVERFLOW(a)) {
 		PyObject *o = PyLong_FromLong(a);
 		if (o != NULL) {
 			PyObject *result = PyNumber_Negative(o);

Objects/listobject.c

@@ -863,17 +863,12 @@ static PyObject *
 listpop(PyListObject *self, PyObject *args)
 {
 	Py_ssize_t i = -1;
-	PyObject *v, *arg = NULL;
+	PyObject *v;
 	int status;
 
-	if (!PyArg_UnpackTuple(args, "pop", 0, 1, &arg))
+	if (!PyArg_ParseTuple(args, "|n:pop", &i))
 		return NULL;
-	if (arg != NULL) {
-		if (PyInt_Check(arg))
-			i = PyInt_AS_LONG((PyIntObject*) arg);
-		else if (!PyArg_ParseTuple(args, "|n:pop", &i))
-   			return NULL;
-	}
+
 	if (self->ob_size == 0) {
 		/* Special-case most common failure cause */
 		PyErr_SetString(PyExc_IndexError, "pop from empty list");