Skip to content

Commit 249e2ab

Browse files
author
armin.rigo
committed
Forward-port of r52136,52138: 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... git-svn-id: http://svn.python.org/projects/python/trunk@52139 6015fed2-1504-0410-9fe1-9d1591cc4771
1 parent 48e2e82 commit 249e2ab

19 files changed

Lines changed: 186 additions & 106 deletions

‎Lib/test/list_tests.py‎

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -269,7 +269,6 @@ def test_insert(self):
269269
self.assertRaises(TypeError, a.insert)
270270

271271
def test_pop(self):
272-
from decimal import Decimal
273272
a = self.type2test([-1, 0, 1])
274273
a.pop()
275274
self.assertEqual(a, [-1, 0])
@@ -281,8 +280,6 @@ def test_pop(self):
281280
self.assertRaises(IndexError, a.pop)
282281
self.assertRaises(TypeError, a.pop, 42, 42)
283282
a = self.type2test([0, 10, 20, 30, 40])
284-
self.assertEqual(a.pop(Decimal(2)), 20)
285-
self.assertRaises(IndexError, a.pop, Decimal(25))
286283

287284
def test_remove(self):
288285
a = self.type2test([0, 0, 1])

‎Lib/test/test_builtin.py‎

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -156,6 +156,11 @@ def test_any(self):
156156
S = [10, 20, 30]
157157
self.assertEqual(any(x > 42 for x in S), False)
158158

159+
def test_neg(self):
160+
x = -sys.maxint-1
161+
self.assert_(isinstance(x, int))
162+
self.assertEqual(-x, sys.maxint+1)
163+
159164
def test_apply(self):
160165
def f0(*args):
161166
self.assertEqual(args, ())
@@ -702,9 +707,11 @@ def test_int(self):
702707
pass
703708

704709
s = repr(-1-sys.maxint)
705-
self.assertEqual(int(s)+1, -sys.maxint)
710+
x = int(s)
711+
self.assertEqual(x+1, -sys.maxint)
712+
self.assert_(isinstance(x, int))
706713
# should return long
707-
int(s[1:])
714+
self.assertEqual(int(s[1:]), sys.maxint+1)
708715

709716
# should return long
710717
x = int(1e100)

‎Lib/test/test_long.py‎

Lines changed: 20 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -247,17 +247,23 @@ def test_misc(self):
247247
"long(-sys.maxint-1) != -sys.maxint-1")
248248

249249
# long -> int should not fail for hugepos_aslong or hugeneg_aslong
250+
x = int(hugepos_aslong)
250251
try:
251-
self.assertEqual(int(hugepos_aslong), hugepos,
252+
self.assertEqual(x, hugepos,
252253
"converting sys.maxint to long and back to int fails")
253254
except OverflowError:
254255
self.fail("int(long(sys.maxint)) overflowed!")
256+
if not isinstance(x, int):
257+
raise TestFailed("int(long(sys.maxint)) should have returned int")
258+
x = int(hugeneg_aslong)
255259
try:
256-
self.assertEqual(int(hugeneg_aslong), hugeneg,
260+
self.assertEqual(x, hugeneg,
257261
"converting -sys.maxint-1 to long and back to int fails")
258262
except OverflowError:
259263
self.fail("int(long(-sys.maxint-1)) overflowed!")
260-
264+
if not isinstance(x, int):
265+
raise TestFailed("int(long(-sys.maxint-1)) should have "
266+
"returned int")
261267
# but long -> int should overflow for hugepos+1 and hugeneg-1
262268
x = hugepos_aslong + 1
263269
try:
@@ -282,6 +288,17 @@ class long2(long):
282288
self.assert_(type(y) is long,
283289
"overflowing int conversion must return long not long subtype")
284290

291+
# long -> Py_ssize_t conversion
292+
class X(object):
293+
def __getslice__(self, i, j):
294+
return i, j
295+
296+
self.assertEqual(X()[-5L:7L], (-5, 7))
297+
# use the clamping effect to test the smallest and largest longs
298+
# that fit a Py_ssize_t
299+
slicemin, slicemax = X()[-2L**100:2L**100]
300+
self.assertEqual(X()[slicemin:slicemax], (slicemin, slicemax))
301+
285302
# ----------------------------------- tests of auto int->long conversion
286303

287304
def test_auto_overflow(self):

‎Misc/NEWS‎

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,8 +12,14 @@ What's New in Python 2.6 alpha 1?
1212
Core and builtins
1313
-----------------
1414

15-
- Integer negation and absolute value were fixed to not rely
16-
on undefined behaviour of the C compiler anymore.
15+
- list.pop(x) accepts any object x following the __index__ protocol.
16+
17+
- Fix some leftovers from the conversion from int to Py_ssize_t
18+
(relevant to strings and sequences of more than 2**31 items).
19+
20+
- A number of places, including integer negation and absolute value,
21+
were fixed to not rely on undefined behaviour of the C compiler
22+
anymore.
1723

1824
- Bug #1566800: make sure that EnvironmentError can be called with any
1925
number of arguments, as was the case in Python 2.4.

‎Modules/cPickle.c‎

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1024,7 +1024,7 @@ save_int(Picklerobject *self, PyObject *args)
10241024
static int
10251025
save_long(Picklerobject *self, PyObject *args)
10261026
{
1027-
int size;
1027+
Py_ssize_t size;
10281028
int res = -1;
10291029
PyObject *repr = NULL;
10301030

@@ -1066,7 +1066,7 @@ save_long(Picklerobject *self, PyObject *args)
10661066
* byte at the start, and cut it back later if possible.
10671067
*/
10681068
nbytes = (nbits >> 3) + 1;
1069-
if ((int)nbytes < 0 || (size_t)(int)nbytes != nbytes) {
1069+
if (nbytes > INT_MAX) {
10701070
PyErr_SetString(PyExc_OverflowError, "long too large "
10711071
"to pickle");
10721072
goto finally;
@@ -1208,12 +1208,14 @@ save_string(Picklerobject *self, PyObject *args, int doput)
12081208
c_str[1] = size;
12091209
len = 2;
12101210
}
1211-
else {
1211+
else if (size <= INT_MAX) {
12121212
c_str[0] = BINSTRING;
12131213
for (i = 1; i < 5; i++)
12141214
c_str[i] = (int)(size >> ((i - 1) * 8));
12151215
len = 5;
12161216
}
1217+
else
1218+
return -1; /* string too large */
12171219

12181220
if (self->write_func(self, c_str, len) < 0)
12191221
return -1;
@@ -1286,7 +1288,7 @@ modified_EncodeRawUnicodeEscape(const Py_UNICODE *s, int size)
12861288
static int
12871289
save_unicode(Picklerobject *self, PyObject *args, int doput)
12881290
{
1289-
int size, len;
1291+
Py_ssize_t size, len;
12901292
PyObject *repr=0;
12911293

12921294
if (!PyUnicode_Check(args))
@@ -1325,6 +1327,8 @@ save_unicode(Picklerobject *self, PyObject *args, int doput)
13251327

13261328
if ((size = PyString_Size(repr)) < 0)
13271329
goto err;
1330+
if (size > INT_MAX)
1331+
return -1; /* string too large */
13281332

13291333
c_str[0] = BINUNICODE;
13301334
for (i = 1; i < 5; i++)

‎Objects/abstract.c‎

Lines changed: 6 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1652,20 +1652,18 @@ _PySequence_IterSearch(PyObject *seq, PyObject *obj, int operation)
16521652
if (cmp > 0) {
16531653
switch (operation) {
16541654
case PY_ITERSEARCH_COUNT:
1655-
++n;
1656-
if (n <= 0) {
1657-
/* XXX(nnorwitz): int means ssize_t */
1655+
if (n == PY_SSIZE_T_MAX) {
16581656
PyErr_SetString(PyExc_OverflowError,
1659-
"count exceeds C int size");
1657+
"count exceeds C integer size");
16601658
goto Fail;
16611659
}
1660+
++n;
16621661
break;
16631662

16641663
case PY_ITERSEARCH_INDEX:
16651664
if (wrapped) {
1666-
/* XXX(nnorwitz): int means ssize_t */
16671665
PyErr_SetString(PyExc_OverflowError,
1668-
"index exceeds C int size");
1666+
"index exceeds C integer size");
16691667
goto Fail;
16701668
}
16711669
goto Done;
@@ -1680,9 +1678,9 @@ _PySequence_IterSearch(PyObject *seq, PyObject *obj, int operation)
16801678
}
16811679

16821680
if (operation == PY_ITERSEARCH_INDEX) {
1683-
++n;
1684-
if (n <= 0)
1681+
if (n == PY_SSIZE_T_MAX)
16851682
wrapped = 1;
1683+
++n;
16861684
}
16871685
}
16881686

‎Objects/fileobject.c‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1001,6 +1001,7 @@ getline_via_fgets(FILE *fp)
10011001
size_t nfree; /* # of free buffer slots; pvend-pvfree */
10021002
size_t total_v_size; /* total # of slots in buffer */
10031003
size_t increment; /* amount to increment the buffer */
1004+
size_t prev_v_size;
10041005

10051006
/* Optimize for normal case: avoid _PyString_Resize if at all
10061007
* possible via first reading into stack buffer "buf".
@@ -1115,8 +1116,11 @@ getline_via_fgets(FILE *fp)
11151116
/* expand buffer and try again */
11161117
assert(*(pvend-1) == '\0');
11171118
increment = total_v_size >> 2; /* mild exponential growth */
1119+
prev_v_size = total_v_size;
11181120
total_v_size += increment;
1119-
if (total_v_size > PY_SSIZE_T_MAX) {
1121+
/* check for overflow */
1122+
if (total_v_size <= prev_v_size ||
1123+
total_v_size > PY_SSIZE_T_MAX) {
11201124
PyErr_SetString(PyExc_OverflowError,
11211125
"line is longer than a Python string can hold");
11221126
Py_DECREF(v);
@@ -1125,7 +1129,7 @@ getline_via_fgets(FILE *fp)
11251129
if (_PyString_Resize(&v, (int)total_v_size) < 0)
11261130
return NULL;
11271131
/* overwrite the trailing null byte */
1128-
pvfree = BUF(v) + (total_v_size - increment - 1);
1132+
pvfree = BUF(v) + (prev_v_size - 1);
11291133
}
11301134
if (BUF(v) + total_v_size != p)
11311135
_PyString_Resize(&v, p - BUF(v));

‎Objects/intobject.c‎

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -546,6 +546,17 @@ int_mul(PyObject *v, PyObject *w)
546546
}
547547
}
548548

549+
/* Integer overflow checking for unary negation: on a 2's-complement
550+
* box, -x overflows iff x is the most negative long. In this case we
551+
* get -x == x. However, -x is undefined (by C) if x /is/ the most
552+
* negative long (it's a signed overflow case), and some compilers care.
553+
* So we cast x to unsigned long first. However, then other compilers
554+
* warn about applying unary minus to an unsigned operand. Hence the
555+
* weird "0-".
556+
*/
557+
#define UNARY_NEG_WOULD_OVERFLOW(x) \
558+
((x) < 0 && (unsigned long)(x) == 0-(unsigned long)(x))
559+
549560
/* Return type of i_divmod */
550561
enum divmod_result {
551562
DIVMOD_OK, /* Correct result */
@@ -565,7 +576,7 @@ i_divmod(register long x, register long y,
565576
return DIVMOD_ERROR;
566577
}
567578
/* (-sys.maxint-1)/-1 is the only overflow case. */
568-
if (y == -1 && x == LONG_MIN)
579+
if (y == -1 && UNARY_NEG_WOULD_OVERFLOW(x))
569580
return DIVMOD_OVERFLOW;
570581
xdivy = x / y;
571582
xmody = x - xdivy * y;
@@ -756,7 +767,8 @@ int_neg(PyIntObject *v)
756767
{
757768
register long a;
758769
a = v->ob_ival;
759-
if (a < 0 && (unsigned long)a == 0-(unsigned long)a) {
770+
/* check for overflow */
771+
if (UNARY_NEG_WOULD_OVERFLOW(a)) {
760772
PyObject *o = PyLong_FromLong(a);
761773
if (o != NULL) {
762774
PyObject *result = PyNumber_Negative(o);

‎Objects/listobject.c‎

Lines changed: 3 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -863,17 +863,12 @@ static PyObject *
863863
listpop(PyListObject *self, PyObject *args)
864864
{
865865
Py_ssize_t i = -1;
866-
PyObject *v, *arg = NULL;
866+
PyObject *v;
867867
int status;
868868

869-
if (!PyArg_UnpackTuple(args, "pop", 0, 1, &arg))
869+
if (!PyArg_ParseTuple(args, "|n:pop", &i))
870870
return NULL;
871-
if (arg != NULL) {
872-
if (PyInt_Check(arg))
873-
i = PyInt_AS_LONG((PyIntObject*) arg);
874-
else if (!PyArg_ParseTuple(args, "|n:pop", &i))
875-
return NULL;
876-
}
871+
877872
if (self->ob_size == 0) {
878873
/* Special-case most common failure cause */
879874
PyErr_SetString(PyExc_IndexError, "pop from empty list");

0 commit comments

Comments
 (0)