Skip to content

Commit 009ae86

Browse files
committed
Avoid signed overflow in some xrange calculations, and extend
xrange tests to cover some special cases that caused problems in py3k. This is a partial backport of r76292-76293 (see issue #7298.)
1 parent 20eb4f0 commit 009ae86

File tree

2 files changed

+112
-34
lines changed

2 files changed

+112
-34
lines changed

Lib/test/test_xrange.py

Lines changed: 68 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,12 +3,49 @@
33
import test.test_support, unittest
44
import sys
55
import pickle
6+
import itertools
67

78
import warnings
89
warnings.filterwarnings("ignore", "integer argument expected",
910
DeprecationWarning, "unittest")
1011

12+
# pure Python implementations (3 args only), for comparison
13+
def pyrange(start, stop, step):
14+
if (start - stop) // step < 0:
15+
# replace stop with next element in the sequence of integers
16+
# that are congruent to start modulo step.
17+
stop += (start - stop) % step
18+
while start != stop:
19+
yield start
20+
start += step
21+
22+
def pyrange_reversed(start, stop, step):
23+
stop += (start - stop) % step
24+
return pyrange(stop - step, start - step, -step)
25+
26+
1127
class XrangeTest(unittest.TestCase):
28+
def assert_iterators_equal(self, xs, ys, test_id, limit=None):
29+
# check that an iterator xs matches the expected results ys,
30+
# up to a given limit.
31+
if limit is not None:
32+
xs = itertools.islice(xs, limit)
33+
ys = itertools.islice(ys, limit)
34+
sentinel = object()
35+
pairs = itertools.izip_longest(xs, ys, fillvalue=sentinel)
36+
for i, (x, y) in enumerate(pairs):
37+
if x == y:
38+
continue
39+
elif x == sentinel:
40+
self.fail('{}: iterator ended unexpectedly '
41+
'at position {}; expected {}'.format(test_id, i, y))
42+
elif y == sentinel:
43+
self.fail('{}: unexpected excess element {} at '
44+
'position {}'.format(test_id, x, i))
45+
else:
46+
self.fail('{}: wrong element at position {};'
47+
'expected {}, got {}'.format(test_id, i, y, x))
48+
1249
def test_xrange(self):
1350
self.assertEqual(list(xrange(3)), [0, 1, 2])
1451
self.assertEqual(list(xrange(1, 5)), [1, 2, 3, 4])
@@ -67,6 +104,37 @@ def test_pickling(self):
67104
self.assertEquals(list(pickle.loads(pickle.dumps(r, proto))),
68105
list(r))
69106

107+
def test_range_iterators(self):
108+
# see issue 7298
109+
limits = [base + jiggle
110+
for M in (2**32, 2**64)
111+
for base in (-M, -M//2, 0, M//2, M)
112+
for jiggle in (-2, -1, 0, 1, 2)]
113+
test_ranges = [(start, end, step)
114+
for start in limits
115+
for end in limits
116+
for step in (-2**63, -2**31, -2, -1, 1, 2)]
117+
118+
for start, end, step in test_ranges:
119+
try:
120+
iter1 = xrange(start, end, step)
121+
except OverflowError:
122+
pass
123+
else:
124+
iter2 = pyrange(start, end, step)
125+
test_id = "xrange({}, {}, {})".format(start, end, step)
126+
# check first 100 entries
127+
self.assert_iterators_equal(iter1, iter2, test_id, limit=100)
128+
129+
try:
130+
iter1 = reversed(xrange(start, end, step))
131+
except OverflowError:
132+
pass
133+
else:
134+
iter2 = pyrange_reversed(start, end, step)
135+
test_id = "reversed(xrange({}, {}, {}))".format(start, end, step)
136+
self.assert_iterators_equal(iter1, iter2, test_id, limit=100)
137+
70138

71139
def test_main():
72140
test.test_support.run_unittest(XrangeTest)

Objects/rangeobject.c

Lines changed: 44 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -9,41 +9,40 @@ typedef struct {
99
long len;
1010
} rangeobject;
1111

12-
/* Return number of items in range/xrange (lo, hi, step). step > 0
13-
* required. Return a value < 0 if & only if the true value is too
14-
* large to fit in a signed long.
12+
/* Return number of items in range (lo, hi, step). step != 0
13+
* required. The result always fits in an unsigned long.
1514
*/
16-
static long
15+
static unsigned long
1716
get_len_of_range(long lo, long hi, long step)
1817
{
19-
/* -------------------------------------------------------------
20-
If lo >= hi, the range is empty.
21-
Else if n values are in the range, the last one is
22-
lo + (n-1)*step, which must be <= hi-1. Rearranging,
23-
n <= (hi - lo - 1)/step + 1, so taking the floor of the RHS gives
24-
the proper value. Since lo < hi in this case, hi-lo-1 >= 0, so
25-
the RHS is non-negative and so truncation is the same as the
26-
floor. Letting M be the largest positive long, the worst case
27-
for the RHS numerator is hi=M, lo=-M-1, and then
28-
hi-lo-1 = M-(-M-1)-1 = 2*M. Therefore unsigned long has enough
29-
precision to compute the RHS exactly.
30-
---------------------------------------------------------------*/
31-
long n = 0;
32-
if (lo < hi) {
33-
unsigned long uhi = (unsigned long)hi;
34-
unsigned long ulo = (unsigned long)lo;
35-
unsigned long diff = uhi - ulo - 1;
36-
n = (long)(diff / (unsigned long)step + 1);
37-
}
38-
return n;
18+
/* -------------------------------------------------------------
19+
If step > 0 and lo >= hi, or step < 0 and lo <= hi, the range is empty.
20+
Else for step > 0, if n values are in the range, the last one is
21+
lo + (n-1)*step, which must be <= hi-1. Rearranging,
22+
n <= (hi - lo - 1)/step + 1, so taking the floor of the RHS gives
23+
the proper value. Since lo < hi in this case, hi-lo-1 >= 0, so
24+
the RHS is non-negative and so truncation is the same as the
25+
floor. Letting M be the largest positive long, the worst case
26+
for the RHS numerator is hi=M, lo=-M-1, and then
27+
hi-lo-1 = M-(-M-1)-1 = 2*M. Therefore unsigned long has enough
28+
precision to compute the RHS exactly. The analysis for step < 0
29+
is similar.
30+
---------------------------------------------------------------*/
31+
assert(step != 0);
32+
if (step > 0 && lo < hi)
33+
return 1UL + (hi - 1UL - lo) / step;
34+
else if (step < 0 && lo > hi)
35+
return 1UL + (lo - 1UL - hi) / (0UL - step);
36+
else
37+
return 0UL;
3938
}
4039

4140
static PyObject *
4241
range_new(PyTypeObject *type, PyObject *args, PyObject *kw)
4342
{
4443
rangeobject *obj;
4544
long ilow = 0, ihigh = 0, istep = 1;
46-
long n;
45+
unsigned long n;
4746

4847
if (!_PyArg_NoKeywords("xrange()", kw))
4948
return NULL;
@@ -64,11 +63,8 @@ range_new(PyTypeObject *type, PyObject *args, PyObject *kw)
6463
PyErr_SetString(PyExc_ValueError, "xrange() arg 3 must not be zero");
6564
return NULL;
6665
}
67-
if (istep > 0)
68-
n = get_len_of_range(ilow, ihigh, istep);
69-
else
70-
n = get_len_of_range(ihigh, ilow, -istep);
71-
if (n < 0) {
66+
n = get_len_of_range(ilow, ihigh, istep);
67+
if (n > (unsigned long)LONG_MAX || (long)n > PY_SSIZE_T_MAX) {
7268
PyErr_SetString(PyExc_OverflowError,
7369
"xrange() result has too many items");
7470
return NULL;
@@ -78,7 +74,7 @@ range_new(PyTypeObject *type, PyObject *args, PyObject *kw)
7874
if (obj == NULL)
7975
return NULL;
8076
obj->start = ilow;
81-
obj->len = n;
77+
obj->len = (long)n;
8278
obj->step = istep;
8379
return (PyObject *) obj;
8480
}
@@ -98,7 +94,9 @@ range_item(rangeobject *r, Py_ssize_t i)
9894
"xrange object index out of range");
9995
return NULL;
10096
}
101-
return PyInt_FromSsize_t(r->start + i * r->step);
97+
/* do calculation entirely using unsigned longs, to avoid
98+
undefined behaviour due to signed overflow. */
99+
return PyInt_FromLong((long)(r->start + (unsigned long)i * r->step));
102100
}
103101

104102
static Py_ssize_t
@@ -304,9 +302,21 @@ range_reverse(PyObject *seq)
304302
len = ((rangeobject *)seq)->len;
305303

306304
it->index = 0;
307-
it->start = start + (len-1) * step;
308-
it->step = -step;
309305
it->len = len;
306+
/* the casts below guard against signed overflow by turning it
307+
into unsigned overflow instead. The correctness of this
308+
code still depends on conversion from unsigned long to long
309+
wrapping modulo ULONG_MAX+1, which isn't guaranteed (see
310+
C99 6.3.1.3p3) but seems to hold in practice for all
311+
platforms we're likely to meet.
312+
313+
If step == LONG_MIN then we still end up with LONG_MIN
314+
after negation; but this works out, since we've still got
315+
the correct value modulo ULONG_MAX+1, and the range_item
316+
calculation is also done modulo ULONG_MAX+1.
317+
*/
318+
it->start = (long)(start + (unsigned long)(len-1) * step);
319+
it->step = (long)(-(unsigned long)step);
310320

311321
return (PyObject *)it;
312322
}

0 commit comments

Comments
 (0)