classification
Title: rangeiter_new fails when creating a range of step 0
Type: behavior Stage: resolved
Components: Interpreter Core Versions: Python 3.7, Python 3.6, Python 3.5
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: serhiy.storchaka Nosy List: Oren Milman, abarry, inada.naoki, python-dev, serhiy.storchaka
Priority: normal Keywords: patch

Created on 2016-10-06 12:00 by Oren Milman, last changed 2017-03-31 16:36 by dstufft. This issue is now closed.

Files
File name Uploaded Description Edit
CPythonTestOutput.txt Oren Milman, 2016-10-06 12:00 test output of CPython without my patches (tested on my PC)
patchedCPythonTestOutput_ver1.txt Oren Milman, 2016-10-06 12:00 test output of CPython with my patches (tested on my PC) - ver1
issue28376_ver1.diff Oren Milman, 2016-10-06 12:02 proposed patches diff file - ver1 review
issue28376_CPython35_ver2.diff Oren Milman, 2016-10-07 08:51 proposed patches diff file for 3.5 - ver2 review
issue28376_CPython36_ver2.diff Oren Milman, 2016-10-07 08:53 proposed patches diff file for 3.6 - ver2 review
issue28376_CPython37_ver2.diff Oren Milman, 2016-10-07 08:53 proposed patches diff file for 3.7 - ver2 review
patchedCPython35TestOutput_ver2.txt Oren Milman, 2016-10-07 08:54 test output of CPython3.5 with my patches (tested on my PC) - ver2
patchedCPython36TestOutput_ver2.txt Oren Milman, 2016-10-07 08:54 test output of CPython3.6 with my patches (tested on my PC) - ver2
patchedCPython37TestOutput_ver2.txt Oren Milman, 2016-10-07 08:55 test output of CPython3.7 with my patches (tested on my PC) - ver2
issue28376_CPython36_ver3.diff Oren Milman, 2016-10-08 16:25 proposed patches diff file for 3.6 - ver3 review
issue28376_CPython37_ver3.diff Oren Milman, 2016-10-08 16:25 proposed patches diff file for 3.7 - ver3 review
patchedCPython36TestOutput_ver3.txt Oren Milman, 2016-10-08 16:26 test output of CPython3.6 with my patches (tested on my PC) - ver3
patchedCPython37TestOutput_ver3.txt Oren Milman, 2016-10-08 16:26 test output of CPython3.7 with my patches (tested on my PC) - ver3
issue28376_CPython36_ver4.diff Oren Milman, 2016-10-08 17:51 proposed patches diff file for 3.6 - ver4 review
Pull Requests
URL Status Linked Edit
PR 552 closed dstufft, 2017-03-31 16:36
Messages (14)
msg278187 - (view) Author: Oren Milman (Oren Milman) * Date: 2016-10-06 12:00
------------ current state ------------
An assertion in Objects/rangeobject.c might fail:
    >>> type(iter(range(0)))
    <class 'range_iterator'>
    >>> type(iter(range(0)))(1, 1, 0)
    Assertion failed: step != 0, file ..\Objects\rangeobject.c, line 895

This is caused by the lack of a check whether 'step' is zero, during the creation of a range_iterator object, in rangeiter_new.

Note that during the creation of a range object, the function range_new does check that, by calling validate_step, which leads to the following behavior:
    >>> range(1, 1, 0)
    Traceback (most recent call last):
      File "<stdin>", line 1, in <module>
    ValueError: range() arg 3 must not be zero
    >>>


------------ proposed changes ------------
    1. In Objects/rangeobject.c in rangeiter_new:
        - in case 'step' is zero, raise a ValueError
        - in error messages, replace 'rangeiter' with 'range_iterator', as the latter is the name of the type in Python code

    2. In Lib/test/test_range.py, add tests for calling the range_iterator type (i.e. creating a range_iterator object)


------------ diff ------------
The proposed patches diff file is attached.


------------ tests ------------
I ran 'python_d.exe -m test -j3' (on my 64-bit Windows 10) with and without the patches, and got quite the same output. (That also means my new tests in test_range passed.)
The outputs of both runs are attached.
msg278188 - (view) Author: Anilyka Barry (abarry) * (Python triager) Date: 2016-10-06 12:31
I'm not able to trigger an assertion error when running the latest trunk in debug mode. I get a "valid" range_iterator object though, and using __reduce__ gives me direct access to `range(0, 0, 0)` (which is completely worthless).

Error or not, this should be fixed, and your patch LGTM. Thanks!

(I changed the type to 'behaviour' since I can't reproduce the assertion, but it doesn't make much of a difference)
msg278189 - (view) Author: Inada Naoki (inada.naoki) * (Python committer) Date: 2016-10-06 12:46
patch is LGTM.

But there is one hidden inconsistency:

>>> r = range(2**100)
>>> type(iter(r))
<class 'longrange_iterator'>
>>> type(iter(r))(1, 1, 0)
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
TypeError: cannot create 'longrange_iterator' instances

Should we have same tp_new method for longrange_iterator?
msg278197 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2016-10-06 17:08
Good point Naoki. I think we can remove tp_new method from range_iterator in 3.7. Seems it is never used.

The patch LGTM for 3.5-3.6, but the test should be marked as CPython implementation detail (@support.cpython_only).
msg278232 - (view) Author: Oren Milman (Oren Milman) * Date: 2016-10-07 08:51
The diff files for 3.5 and 3.6 are attached (I only added @test.support.cpython_only, as you suggested).

The diff file for 3.7 is also attached. It contains:
    - removing rangeiter_new
    - tests to verify one cannot create instances of range_iterator or longrange_iterator (in CPython)
    - added 'Iterator.register(longrange_iterator)' in Lib/_collections_abc.py

The output of 'python_d.exe -m test -j3' for each version is also attached (they were all successful).

P.S. If we remove the ability to create instances of range_iterator in 3.7, shouldn't we raise a DeprecationWarning in rangeiter_new in 3.5 and 3.6?
msg278234 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2016-10-07 11:49
Raising a DeprecationWarning in 3.6 doesn't hurt.
msg278237 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2016-10-07 12:22
rangeiter_new() was added in r55167.
msg278238 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2016-10-07 12:23
And merged in a6eb6acfe04a.
msg278307 - (view) Author: Oren Milman (Oren Milman) * Date: 2016-10-08 16:25
The diff files for 3.6 and 3.7 are attached (named '*ver3.diff').
Changes:
    - in 3.6, added:
        * raising a DeprecationWarning in rangeiter_new
        * a test to verify the DeprecationWarning is raised
    - in 3.7:
        * changed the tests so they would only verify a TypeError is raised when calling either range_iterator or longrange_iterator
        * removed the test.support.cpython_only decorator of the test

The output of 'python_d.exe -m test -j3' for each version is also attached (they were both successful).
msg278309 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2016-10-08 16:44
Thanks Oren. Patches for 3.5 and 3.7 LGTM, the patch for 3.6 should be fixed. Tests should pass with -Wa and -We.

No need to attach test output. Just check that your patch doesn't add a regression.
msg278313 - (view) Author: Oren Milman (Oren Milman) * Date: 2016-10-08 17:51
Alright, just added 'with test.support.check_warnings(('', DeprecationWarning)):'.
*ver4.diff is attached.
I ran the tests again (this time using 'python_d.exe -We -m test -j3'), and they were successful :)

That was totally my bad, of course, but anyway, ISTM that https://docs.python.org/devguide/index.html should say './python -We -m test -j3' in section 4 (Run the tests).
msg278315 - (view) Author: Roundup Robot (python-dev) (Python triager) Date: 2016-10-08 19:08
New changeset ee049edc3fff by Serhiy Storchaka in branch '3.5':
Issue #28376: Fixed typos.
https://hg.python.org/cpython/rev/ee049edc3fff

New changeset e486f3d30e0e by Serhiy Storchaka in branch '3.5':
Issue #28376: The constructor of range_iterator now checks that step is not 0.
https://hg.python.org/cpython/rev/e486f3d30e0e

New changeset 06f065a59751 by Serhiy Storchaka in branch '3.6':
Issue #28376: Creating instances of range_iterator by calling range_iterator
https://hg.python.org/cpython/rev/06f065a59751

New changeset e099583400f3 by Serhiy Storchaka in branch 'default':
Issue #28376: Creating instances of range_iterator by calling range_iterator
https://hg.python.org/cpython/rev/e099583400f3

New changeset ce4af7593e45 by Serhiy Storchaka in branch '3.5':
Issue #28376: The type of long range iterator is now registered as Iterator.
https://hg.python.org/cpython/rev/ce4af7593e45
msg278316 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2016-10-08 19:09
Thank you for your contribution Oren.
msg278352 - (view) Author: Oren Milman (Oren Milman) * Date: 2016-10-09 09:48
Thanks for the reviews and patience :)
History
Date User Action Args
2017-03-31 16:36:37dstufftsetpull_requests: + pull_request1098
2016-10-09 09:48:43Oren Milmansetmessages: + msg278352
2016-10-08 19:09:37serhiy.storchakasetstatus: open -> closed
messages: + msg278316

assignee: serhiy.storchaka
resolution: fixed
stage: patch review -> resolved
2016-10-08 19:08:34python-devsetnosy: + python-dev
messages: + msg278315
2016-10-08 17:51:49Oren Milmansetfiles: + issue28376_CPython36_ver4.diff

messages: + msg278313
2016-10-08 16:44:09serhiy.storchakasetmessages: + msg278309
2016-10-08 16:26:49Oren Milmansetfiles: + patchedCPython37TestOutput_ver3.txt
2016-10-08 16:26:16Oren Milmansetfiles: + patchedCPython36TestOutput_ver3.txt
2016-10-08 16:25:40Oren Milmansetfiles: + issue28376_CPython37_ver3.diff
2016-10-08 16:25:17Oren Milmansetfiles: + issue28376_CPython36_ver3.diff

messages: + msg278307
2016-10-07 12:23:42serhiy.storchakasetmessages: + msg278238
2016-10-07 12:22:26serhiy.storchakasetmessages: + msg278237
2016-10-07 11:49:58serhiy.storchakasetmessages: + msg278234
2016-10-07 08:55:13Oren Milmansetfiles: + patchedCPython37TestOutput_ver2.txt
2016-10-07 08:54:36Oren Milmansetfiles: + patchedCPython36TestOutput_ver2.txt
2016-10-07 08:54:09Oren Milmansetfiles: + patchedCPython35TestOutput_ver2.txt
2016-10-07 08:53:29Oren Milmansetfiles: + issue28376_CPython37_ver2.diff
2016-10-07 08:53:11Oren Milmansetfiles: + issue28376_CPython36_ver2.diff
2016-10-07 08:51:46Oren Milmansetfiles: + issue28376_CPython35_ver2.diff

messages: + msg278232
2016-10-06 17:08:22serhiy.storchakasetmessages: + msg278197
2016-10-06 14:38:28serhiy.storchakasetnosy: + serhiy.storchaka

versions: + Python 3.5, Python 3.6
2016-10-06 12:46:20inada.naokisetnosy: + inada.naoki
messages: + msg278189
2016-10-06 12:31:26abarrysetnosy: + abarry
title: assertion failure in rangeobject.c -> rangeiter_new fails when creating a range of step 0
messages: + msg278188

type: crash -> behavior
stage: patch review
2016-10-06 12:02:33Oren Milmansetfiles: + issue28376_ver1.diff
keywords: + patch
2016-10-06 12:00:51Oren Milmansetfiles: + patchedCPythonTestOutput_ver1.txt
2016-10-06 12:00:27Oren Milmancreate