Skip to content

Commit d550c9a

Browse files
mdickinsonmdickinson
committed
Issue python#7298: Fix a variety of problems leading to wrong results with
the fast versions of range.__reversed__ and range iteration. Also fix wrong results and a refleak for PyLong version of range.__reversed__. Thanks Eric Smith for reviewing, and for suggesting improved tests.
1 parent 4c7eaee commit d550c9a

3 files changed

Lines changed: 205 additions & 55 deletions

File tree

‎Lib/test/test_range.py‎

Lines changed: 61 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,12 +3,49 @@
33
import 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 RangeTest(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.zip_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_range(self):
1350
self.assertEqual(list(range(3)), [0, 1, 2])
1451
self.assertEqual(list(range(1, 5)), [1, 2, 3, 4])
@@ -134,6 +171,30 @@ def test_empty(self):
134171
self.assertFalse(-1 in r)
135172
self.assertFalse(1 in r)
136173

174+
def test_range_iterators(self):
175+
# exercise 'fast' iterators, that use a rangeiterobject internally.
176+
# see issue 7298
177+
limits = [base + jiggle
178+
for M in (2**32, 2**64)
179+
for base in (-M, -M//2, 0, M//2, M)
180+
for jiggle in (-2, -1, 0, 1, 2)]
181+
test_ranges = [(start, end, step)
182+
for start in limits
183+
for end in limits
184+
for step in (-2**63, -2**31, -2, -1, 1, 2)]
185+
186+
for start, end, step in test_ranges:
187+
iter1 = range(start, end, step)
188+
iter2 = pyrange(start, end, step)
189+
test_id = "range({}, {}, {})".format(start, end, step)
190+
# check first 100 entries
191+
self.assert_iterators_equal(iter1, iter2, test_id, limit=100)
192+
193+
iter1 = reversed(range(start, end, step))
194+
iter2 = pyrange_reversed(start, end, step)
195+
test_id = "reversed(range({}, {}, {}))".format(start, end, step)
196+
self.assert_iterators_equal(iter1, iter2, test_id, limit=100)
197+
137198
def test_main():
138199
test.support.run_unittest(RangeTest)
139200

‎Misc/NEWS‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,12 @@ What's New in Python 3.2 Alpha 1?
1212
Core and Builtins
1313
-----------------
1414

15+
- Issue #7298: fixes for range and reversed(range(...)). Iteration
16+
over range(a, b, c) incorrectly gave an empty iterator when a, b and
17+
c fit in C long but the length of the range did not. Also fix
18+
several cases where reversed(range(a, b, c)) gave wrong results, and
19+
fix a refleak for reversed(range(a, b, c)) with large arguments.
20+
1521
- Issue #7244: itertools.izip_longest() no longer ignores exceptions
1622
raised during the formation of an output tuple.
1723

‎Objects/rangeobject.c‎

Lines changed: 138 additions & 55 deletions
Original file line numberDiff line numberDiff line change
@@ -488,47 +488,53 @@ PyTypeObject PyRangeIter_Type = {
488488
rangeiter_new, /* tp_new */
489489
};
490490

491-
/* Return number of items in range (lo, hi, step). step > 0
492-
* required. Return a value < 0 if & only if the true value is too
493-
* large to fit in a signed long.
491+
/* Return number of items in range (lo, hi, step). step != 0
492+
* required. The result always fits in an unsigned long.
494493
*/
495-
static long
494+
static unsigned long
496495
get_len_of_range(long lo, long hi, long step)
497496
{
498497
/* -------------------------------------------------------------
499-
If lo >= hi, the range is empty.
500-
Else if n values are in the range, the last one is
498+
If step > 0 and lo >= hi, or step < 0 and lo <= hi, the range is empty.
499+
Else for step > 0, if n values are in the range, the last one is
501500
lo + (n-1)*step, which must be <= hi-1. Rearranging,
502501
n <= (hi - lo - 1)/step + 1, so taking the floor of the RHS gives
503502
the proper value. Since lo < hi in this case, hi-lo-1 >= 0, so
504503
the RHS is non-negative and so truncation is the same as the
505504
floor. Letting M be the largest positive long, the worst case
506505
for the RHS numerator is hi=M, lo=-M-1, and then
507506
hi-lo-1 = M-(-M-1)-1 = 2*M. Therefore unsigned long has enough
508-
precision to compute the RHS exactly.
507+
precision to compute the RHS exactly. The analysis for step < 0
508+
is similar.
509509
---------------------------------------------------------------*/
510-
long n = 0;
511-
if (lo < hi) {
512-
unsigned long uhi = (unsigned long)hi;
513-
unsigned long ulo = (unsigned long)lo;
514-
unsigned long diff = uhi - ulo - 1;
515-
n = (long)(diff / (unsigned long)step + 1);
516-
}
517-
return n;
510+
assert(step != 0);
511+
if (step > 0 && lo < hi)
512+
return 1UL + (hi - 1UL - lo) / step;
513+
else if (step < 0 && lo > hi)
514+
return 1UL + (lo - 1UL - hi) / (0UL - step);
515+
else
516+
return 0UL;
518517
}
519518

519+
/* Initialize a rangeiter object. If the length of the rangeiter object
520+
is not representable as a C long, OverflowError is raised. */
521+
520522
static PyObject *
521523
int_range_iter(long start, long stop, long step)
522524
{
523525
rangeiterobject *it = PyObject_New(rangeiterobject, &PyRangeIter_Type);
526+
unsigned long ulen;
524527
if (it == NULL)
525528
return NULL;
526529
it->start = start;
527530
it->step = step;
528-
if (step > 0)
529-
it->len = get_len_of_range(start, stop, step);
530-
else
531-
it->len = get_len_of_range(stop, start, -step);
531+
ulen = get_len_of_range(start, stop, step);
532+
if (ulen > (unsigned long)LONG_MAX) {
533+
PyErr_SetString(PyExc_OverflowError,
534+
"range too large to represent as a range_iterator");
535+
return NULL;
536+
}
537+
it->len = (long)ulen;
532538
it->index = 0;
533539
return (PyObject *)it;
534540
}
@@ -637,23 +643,53 @@ range_iter(PyObject *seq)
637643
rangeobject *r = (rangeobject *)seq;
638644
longrangeiterobject *it;
639645
long lstart, lstop, lstep;
646+
PyObject *int_it;
640647

641648
assert(PyRange_Check(seq));
642649

643-
/* If all three fields convert to long, use the int version */
650+
/* If all three fields and the length convert to long, use the int
651+
* version */
644652
lstart = PyLong_AsLong(r->start);
645-
if (lstart != -1 || !PyErr_Occurred()) {
646-
lstop = PyLong_AsLong(r->stop);
647-
if (lstop != -1 || !PyErr_Occurred()) {
648-
lstep = PyLong_AsLong(r->step);
649-
if (lstep != -1 || !PyErr_Occurred())
650-
return int_range_iter(lstart, lstop, lstep);
651-
}
653+
if (lstart == -1 && PyErr_Occurred()) {
654+
PyErr_Clear();
655+
goto long_range;
656+
}
657+
lstop = PyLong_AsLong(r->stop);
658+
if (lstop == -1 && PyErr_Occurred()) {
659+
PyErr_Clear();
660+
goto long_range;
661+
}
662+
lstep = PyLong_AsLong(r->step);
663+
if (lstep == -1 && PyErr_Occurred()) {
664+
PyErr_Clear();
665+
goto long_range;
652666
}
653-
/* Some conversion failed, so there is an error set. Clear it,
654-
and try again with a long range. */
655-
PyErr_Clear();
667+
/* round lstop to the next value congruent to lstart modulo lstep;
668+
if the result would overflow, use PyLong version. */
669+
if (lstep > 0 && lstart < lstop) {
670+
long extra = (lstep - 1) - (long)((lstop - 1UL - lstart) % lstep);
671+
if ((unsigned long)extra > (unsigned long)LONG_MAX - lstop)
672+
goto long_range;
673+
lstop += extra;
674+
}
675+
else if (lstep < 0 && lstart > lstop) {
676+
long extra = (lstep + 1) + (long)((lstart - 1UL - lstop) %
677+
(0UL - lstep));
678+
if ((unsigned long)lstop - LONG_MIN < 0UL - extra)
679+
goto long_range;
680+
lstop += extra;
681+
}
682+
else
683+
lstop = lstart;
684+
685+
int_it = int_range_iter(lstart, lstop, lstep);
686+
if (int_it == NULL && PyErr_ExceptionMatches(PyExc_OverflowError)) {
687+
PyErr_Clear();
688+
goto long_range;
689+
}
690+
return (PyObject *)int_it;
656691

692+
long_range:
657693
it = PyObject_New(longrangeiterobject, &PyLongRangeIter_Type);
658694
if (it == NULL)
659695
return NULL;
@@ -686,34 +722,80 @@ range_reverse(PyObject *seq)
686722
rangeobject *range = (rangeobject*) seq;
687723
longrangeiterobject *it;
688724
PyObject *one, *sum, *diff, *len = NULL, *product;
689-
long lstart, lstop, lstep;
725+
long lstart, lstop, lstep, new_start, new_stop;
726+
unsigned long ulen;
690727

691-
/* XXX(nnorwitz): do the calc for the new start/stop first,
692-
then if they fit, call the proper iter()?
693-
*/
694728
assert(PyRange_Check(seq));
695729

696-
/* If all three fields convert to long, use the int version */
730+
/* reversed(range(start, stop, step)) can be expressed as
731+
range(start+(n-1)*step, start-step, -step), where n is the number of
732+
integers in the range.
733+
734+
If each of start, stop, step, -step, start-step, and the length
735+
of the iterator is representable as a C long, use the int
736+
version. This excludes some cases where the reversed range is
737+
representable as a range_iterator, but it's good enough for
738+
common cases and it makes the checks simple. */
739+
697740
lstart = PyLong_AsLong(range->start);
698-
if (lstart != -1 || !PyErr_Occurred()) {
699-
lstop = PyLong_AsLong(range->stop);
700-
if (lstop != -1 || !PyErr_Occurred()) {
701-
lstep = PyLong_AsLong(range->step);
702-
if (lstep != -1 || !PyErr_Occurred()) {
703-
/* XXX(nnorwitz): need to check for overflow and simplify. */
704-
long len = get_len_of_range(lstart, lstop, lstep);
705-
long new_start = lstart + (len - 1) * lstep;
706-
long new_stop = lstart;
707-
if (lstep > 0)
708-
new_stop -= 1;
709-
else
710-
new_stop += 1;
711-
return int_range_iter(new_start, new_stop, -lstep);
712-
}
713-
}
741+
if (lstart == -1 && PyErr_Occurred()) {
742+
PyErr_Clear();
743+
goto long_range;
714744
}
715-
PyErr_Clear();
745+
lstop = PyLong_AsLong(range->stop);
746+
if (lstop == -1 && PyErr_Occurred()) {
747+
PyErr_Clear();
748+
goto long_range;
749+
}
750+
lstep = PyLong_AsLong(range->step);
751+
if (lstep == -1 && PyErr_Occurred()) {
752+
PyErr_Clear();
753+
goto long_range;
754+
}
755+
/* check for possible overflow of -lstep */
756+
if (lstep == LONG_MIN)
757+
goto long_range;
758+
759+
/* check for overflow of lstart - lstep:
760+
761+
for lstep > 0, need only check whether lstart - lstep < LONG_MIN.
762+
for lstep < 0, need only check whether lstart - lstep > LONG_MAX
716763
764+
Rearrange these inequalities as:
765+
766+
lstart - LONG_MIN < lstep (lstep > 0)
767+
LONG_MAX - lstart < -lstep (lstep < 0)
768+
769+
and compute both sides as unsigned longs, to avoid the
770+
possibility of undefined behaviour due to signed overflow. */
771+
772+
if (lstep > 0) {
773+
if ((unsigned long)lstart - LONG_MIN < (unsigned long)lstep)
774+
goto long_range;
775+
}
776+
else {
777+
if (LONG_MAX - (unsigned long)lstart < 0UL - lstep)
778+
goto long_range;
779+
}
780+
781+
/* set lstop equal to the last element of the range, or to lstart if the
782+
range is empty. */
783+
if (lstep > 0 && lstart < lstop)
784+
lstop += -1 - (long)((lstop - 1UL - lstart) % lstep);
785+
else if (lstep < 0 && lstart > lstop)
786+
lstop += 1 + (long)((lstart - 1UL - lstop) % (0UL - lstep));
787+
else
788+
lstop = lstart;
789+
790+
ulen = get_len_of_range(lstart, lstop, lstep);
791+
if (ulen > (unsigned long)LONG_MAX)
792+
goto long_range;
793+
794+
new_stop = lstart - lstep;
795+
new_start = (long)(new_stop + ulen * lstep);
796+
return int_range_iter(new_start, new_stop, -lstep);
797+
798+
long_range:
717799
it = PyObject_New(longrangeiterobject, &PyLongRangeIter_Type);
718800
if (it == NULL)
719801
return NULL;
@@ -732,7 +814,8 @@ range_reverse(PyObject *seq)
732814
if (!diff)
733815
goto create_failure;
734816

735-
product = PyNumber_Multiply(len, range->step);
817+
product = PyNumber_Multiply(diff, range->step);
818+
Py_DECREF(diff);
736819
if (!product)
737820
goto create_failure;
738821

@@ -741,11 +824,11 @@ range_reverse(PyObject *seq)
741824
it->start = sum;
742825
if (!it->start)
743826
goto create_failure;
827+
744828
it->step = PyNumber_Negative(range->step);
745829
if (!it->step) {
746830
Py_DECREF(it->start);
747-
PyObject_Del(it);
748-
return NULL;
831+
goto create_failure;
749832
}
750833

751834
/* Steal reference to len. */

0 commit comments

Comments
 (0)