]> git.ipfire.org Git - thirdparty/Python/cpython.git/commitdiff
[3.14] GH-135171: Fix generator expressions one last time (hopefully) (GH-135225)
authorMark Shannon <mark@hotpy.org>
Mon, 9 Jun 2025 10:04:23 +0000 (11:04 +0100)
committerGitHub <noreply@github.com>
Mon, 9 Jun 2025 10:04:23 +0000 (11:04 +0100)
* Add NULL check to FOR_ITER

* Move GET_ITER back to genexpr creation

Lib/test/test_dis.py
Lib/test/test_generators.py
Lib/test/test_genexps.py
Misc/NEWS.d/next/Core_and_Builtins/2025-06-06-18-57-30.gh-issue-135171.0YtLq6.rst [new file with mode: 0644]
Python/bytecodes.c
Python/codegen.c
Python/executor_cases.c.h
Python/generated_cases.c.h

index dbf90472ca34b6b71e0d347ab244addb36709bf5..48df7e530de2c1efbcd80ad50cdf7aad96674c50 100644 (file)
@@ -204,6 +204,7 @@ dis_bug1333982 = """\
               LOAD_CONST               1 (<code object <genexpr> at 0x..., file "%s", line %d>)
               MAKE_FUNCTION
               LOAD_FAST_BORROW         0 (x)
+              GET_ITER
               CALL                     0
 
 %3d           LOAD_SMALL_INT           1
@@ -832,6 +833,7 @@ Disassembly of <code object foo at 0x..., file "%s", line %d>:
                MAKE_FUNCTION
                SET_FUNCTION_ATTRIBUTE   8 (closure)
                LOAD_DEREF               1 (y)
+               GET_ITER
                CALL                     0
                CALL                     1
                RETURN_VALUE
@@ -851,8 +853,7 @@ Disassembly of <code object <genexpr> at 0x..., file "%s", line %d>:
 %4d           RETURN_GENERATOR
                POP_TOP
        L1:     RESUME                   0
-               LOAD_FAST_BORROW         0 (.0)
-               GET_ITER
+               LOAD_FAST                0 (.0)
        L2:     FOR_ITER                14 (to L3)
                STORE_FAST               1 (z)
                LOAD_DEREF               2 (x)
index 3e41c7b9663491a05508d0b0e1f3a968b8b510d0..6c056b63f88480b354fab126156e8d0eb9cfaed6 100644 (file)
@@ -318,21 +318,26 @@ class ModifyUnderlyingIterableTest(unittest.TestCase):
                 yield x
         return gen(range(10))
 
-    def process_tests(self, get_generator):
+    def process_tests(self, get_generator, is_expr):
+        err_iterator = "'.*' object is not an iterator"
+        err_iterable = "'.*' object is not iterable"
         for obj in self.iterables:
             g_obj = get_generator(obj)
             with self.subTest(g_obj=g_obj, obj=obj):
-                self.assertListEqual(list(g_obj), list(obj))
+                if is_expr:
+                    self.assertRaisesRegex(TypeError, err_iterator, list, g_obj)
+                else:
+                    self.assertListEqual(list(g_obj), list(obj))
 
             g_iter = get_generator(iter(obj))
             with self.subTest(g_iter=g_iter, obj=obj):
                 self.assertListEqual(list(g_iter), list(obj))
 
-        err_regex = "'.*' object is not iterable"
         for obj in self.non_iterables:
             g_obj = get_generator(obj)
             with self.subTest(g_obj=g_obj):
-                self.assertRaisesRegex(TypeError, err_regex, list, g_obj)
+                err = err_iterator if is_expr else err_iterable
+                self.assertRaisesRegex(TypeError, err, list, g_obj)
 
     def test_modify_f_locals(self):
         def modify_f_locals(g, local, obj):
@@ -345,8 +350,8 @@ class ModifyUnderlyingIterableTest(unittest.TestCase):
         def get_generator_genfunc(obj):
             return modify_f_locals(self.genfunc(), 'it', obj)
 
-        self.process_tests(get_generator_genexpr)
-        self.process_tests(get_generator_genfunc)
+        self.process_tests(get_generator_genexpr, True)
+        self.process_tests(get_generator_genfunc, False)
 
     def test_new_gen_from_gi_code(self):
         def new_gen_from_gi_code(g, obj):
@@ -359,8 +364,8 @@ class ModifyUnderlyingIterableTest(unittest.TestCase):
         def get_generator_genfunc(obj):
             return new_gen_from_gi_code(self.genfunc(), obj)
 
-        self.process_tests(get_generator_genexpr)
-        self.process_tests(get_generator_genfunc)
+        self.process_tests(get_generator_genexpr, True)
+        self.process_tests(get_generator_genfunc, False)
 
 
 class ExceptionTest(unittest.TestCase):
index fe5f18fa3f88a016b65a6711a45c2439b1490c61..d90097dabea0c01b6f3d3166a4efb4e317aeed9b 100644 (file)
@@ -131,6 +131,14 @@ Verify late binding for the outermost if-expression
     >>> list(g)
     [1, 9, 25, 49, 81]
 
+Verify that the outermost for-expression makes an immediate check
+for iterability
+    >>> (i for i in 6)
+    Traceback (most recent call last):
+      File "<pyshell#4>", line 1, in -toplevel-
+        (i for i in 6)
+    TypeError: 'int' object is not iterable
+
 Verify late binding for the innermost for-expression
 
     >>> g = ((i,j) for i in range(3) for j in range(x))
diff --git a/Misc/NEWS.d/next/Core_and_Builtins/2025-06-06-18-57-30.gh-issue-135171.0YtLq6.rst b/Misc/NEWS.d/next/Core_and_Builtins/2025-06-06-18-57-30.gh-issue-135171.0YtLq6.rst
new file mode 100644 (file)
index 0000000..e46cb2b
--- /dev/null
@@ -0,0 +1,6 @@
+Reverts the behavior of generator expressions when created with a
+non-iterable to the pre-3.13 behavior of raising a TypeError. It is no
+longer possible to cause a crash in the debugger by altering the generator
+expression's local variables. This is achieved by moving the ``GET_ITER``
+instruction back to the creation of the generator expression and adding an
+additional check to ``FOR_ITER``.
index 6a0766626402b09c7cb69c40b17e4db577d3ef79..568f61a9f5da1b33304ec4fbf24f9d1f0a77baa8 100644 (file)
@@ -3155,7 +3155,14 @@ dummy_func(
         replaced op(_FOR_ITER, (iter -- iter, next)) {
             /* before: [iter]; after: [iter, iter()] *or* [] (and jump over END_FOR.) */
             PyObject *iter_o = PyStackRef_AsPyObjectBorrow(iter);
-            PyObject *next_o = (*Py_TYPE(iter_o)->tp_iternext)(iter_o);
+            iternextfunc func = Py_TYPE(iter_o)->tp_iternext;
+            if (func == NULL) {
+                _PyErr_Format(tstate, PyExc_TypeError,
+                              "'%.100s' object is not an iterator",
+                              Py_TYPE(iter_o)->tp_name);
+                ERROR_NO_POP();
+            }
+            PyObject *next_o = func(iter_o);
             if (next_o == NULL) {
                 if (_PyErr_Occurred(tstate)) {
                     int matches = _PyErr_ExceptionMatches(tstate, PyExc_StopIteration);
@@ -3179,7 +3186,14 @@ dummy_func(
         op(_FOR_ITER_TIER_TWO, (iter -- iter, next)) {
             /* before: [iter]; after: [iter, iter()] *or* [] (and jump over END_FOR.) */
             PyObject *iter_o = PyStackRef_AsPyObjectBorrow(iter);
-            PyObject *next_o = (*Py_TYPE(iter_o)->tp_iternext)(iter_o);
+            iternextfunc func = Py_TYPE(iter_o)->tp_iternext;
+            if (func == NULL) {
+                _PyErr_Format(tstate, PyExc_TypeError,
+                              "'%.100s' object is not an iterator",
+                              Py_TYPE(iter_o)->tp_name);
+                ERROR_NO_POP();
+            }
+            PyObject *next_o = func(iter_o);
             if (next_o == NULL) {
                 if (_PyErr_Occurred(tstate)) {
                     int matches = _PyErr_ExceptionMatches(tstate, PyExc_StopIteration);
@@ -3202,7 +3216,14 @@ dummy_func(
 
         inst(INSTRUMENTED_FOR_ITER, (unused/1, iter -- iter, next)) {
             PyObject *iter_o = PyStackRef_AsPyObjectBorrow(iter);
-            PyObject *next_o = (*Py_TYPE(iter_o)->tp_iternext)(iter_o);
+            iternextfunc func = Py_TYPE(iter_o)->tp_iternext;
+            if (func == NULL) {
+                _PyErr_Format(tstate, PyExc_TypeError,
+                              "'%.100s' object is not an iterator",
+                              Py_TYPE(iter_o)->tp_name);
+                ERROR_NO_POP();
+            }
+            PyObject *next_o = func(iter_o);
             if (next_o != NULL) {
                 next = PyStackRef_FromPyObjectSteal(next_o);
                 INSTRUMENTED_JUMP(this_instr, next_instr, PY_MONITORING_EVENT_BRANCH_LEFT);
index 1672b655fa9423150958001511af1b51887e0b97..93c3a21266cb7d7f85ad4ffb98ebe7d3b17a71fa 100644 (file)
@@ -4411,7 +4411,7 @@ codegen_sync_comprehension_generator(compiler *c, location loc,
 
     comprehension_ty gen = (comprehension_ty)asdl_seq_GET(generators,
                                                           gen_index);
-
+    int is_outer_genexpr = gen_index == 0 && type == COMP_GENEXP;
     if (!iter_on_stack) {
         if (gen_index == 0) {
             assert(METADATA(c)->u_argcount == 1);
@@ -4442,14 +4442,15 @@ codegen_sync_comprehension_generator(compiler *c, location loc,
             }
             if (IS_JUMP_TARGET_LABEL(start)) {
                 VISIT(c, expr, gen->iter);
-                ADDOP(c, LOC(gen->iter), GET_ITER);
             }
         }
     }
 
     if (IS_JUMP_TARGET_LABEL(start)) {
         depth++;
-        ADDOP(c, LOC(gen->iter), GET_ITER);
+        if (!is_outer_genexpr) {
+            ADDOP(c, LOC(gen->iter), GET_ITER);
+        }
         USE_LABEL(c, start);
         ADDOP_JUMP(c, LOC(gen->iter), FOR_ITER, anchor);
     }
@@ -4775,6 +4776,7 @@ codegen_comprehension(compiler *c, expr_ty e, int type,
     location loc = LOC(e);
 
     outermost = (comprehension_ty) asdl_seq_GET(generators, 0);
+    int is_sync_genexpr = type == COMP_GENEXP && !outermost->is_async;
     if (is_inlined) {
         VISIT(c, expr, outermost->iter);
         if (push_inlined_comprehension_state(c, loc, entry, &inline_state)) {
@@ -4851,6 +4853,9 @@ codegen_comprehension(compiler *c, expr_ty e, int type,
     Py_CLEAR(co);
 
     VISIT(c, expr, outermost->iter);
+    if (is_sync_genexpr) {
+        ADDOP(c, loc, GET_ITER);
+    }
     ADDOP_I(c, loc, CALL, 0);
 
     if (is_async_comprehension && type != COMP_GENEXP) {
index 3e51ac41fa3a2ed9b271e30123c26b1985537fb7..af4c39d03afa8632b55d204ee713242e22e64dcd 100644 (file)
             _PyStackRef next;
             iter = stack_pointer[-1];
             PyObject *iter_o = PyStackRef_AsPyObjectBorrow(iter);
+            iternextfunc func = Py_TYPE(iter_o)->tp_iternext;
+            if (func == NULL) {
+                _PyFrame_SetStackPointer(frame, stack_pointer);
+                _PyErr_Format(tstate, PyExc_TypeError,
+                              "'%.100s' object is not an iterator",
+                              Py_TYPE(iter_o)->tp_name);
+                stack_pointer = _PyFrame_GetStackPointer(frame);
+                JUMP_TO_ERROR();
+            }
             _PyFrame_SetStackPointer(frame, stack_pointer);
-            PyObject *next_o = (*Py_TYPE(iter_o)->tp_iternext)(iter_o);
+            PyObject *next_o = func(iter_o);
             stack_pointer = _PyFrame_GetStackPointer(frame);
             if (next_o == NULL) {
                 if (_PyErr_Occurred(tstate)) {
index 7e8b05b747ed8f0764c05cc4ef92631e6f578c33..9124035fc9e34dac2c2312ed2e40f10165da512b 100644 (file)
             // _FOR_ITER
             {
                 PyObject *iter_o = PyStackRef_AsPyObjectBorrow(iter);
+                iternextfunc func = Py_TYPE(iter_o)->tp_iternext;
+                if (func == NULL) {
+                    _PyFrame_SetStackPointer(frame, stack_pointer);
+                    _PyErr_Format(tstate, PyExc_TypeError,
+                              "'%.100s' object is not an iterator",
+                              Py_TYPE(iter_o)->tp_name);
+                    stack_pointer = _PyFrame_GetStackPointer(frame);
+                    JUMP_TO_LABEL(error);
+                }
                 _PyFrame_SetStackPointer(frame, stack_pointer);
-                PyObject *next_o = (*Py_TYPE(iter_o)->tp_iternext)(iter_o);
+                PyObject *next_o = func(iter_o);
                 stack_pointer = _PyFrame_GetStackPointer(frame);
                 if (next_o == NULL) {
                     if (_PyErr_Occurred(tstate)) {
             /* Skip 1 cache entry */
             iter = stack_pointer[-1];
             PyObject *iter_o = PyStackRef_AsPyObjectBorrow(iter);
+            iternextfunc func = Py_TYPE(iter_o)->tp_iternext;
+            if (func == NULL) {
+                _PyFrame_SetStackPointer(frame, stack_pointer);
+                _PyErr_Format(tstate, PyExc_TypeError,
+                              "'%.100s' object is not an iterator",
+                              Py_TYPE(iter_o)->tp_name);
+                stack_pointer = _PyFrame_GetStackPointer(frame);
+                JUMP_TO_LABEL(error);
+            }
             _PyFrame_SetStackPointer(frame, stack_pointer);
-            PyObject *next_o = (*Py_TYPE(iter_o)->tp_iternext)(iter_o);
+            PyObject *next_o = func(iter_o);
             stack_pointer = _PyFrame_GetStackPointer(frame);
             if (next_o != NULL) {
                 next = PyStackRef_FromPyObjectSteal(next_o);