From c011b5b47370481e789fd170c8763e412783e052 Mon Sep 17 00:00:00 2001 From: Mark Shannon Date: Fri, 6 Jun 2025 16:26:31 +0100 Subject: [PATCH 1/6] Add NULL check to FOR_ITER --- Python/bytecodes.c | 27 ++++++++++++++++++++++++--- Python/executor_cases.c.h | 11 ++++++++++- Python/generated_cases.c.h | 22 ++++++++++++++++++++-- 3 files changed, 54 insertions(+), 6 deletions(-) diff --git a/Python/bytecodes.c b/Python/bytecodes.c index 6a0766626402b0..568f61a9f5da1b 100644 --- a/Python/bytecodes.c +++ b/Python/bytecodes.c @@ -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); diff --git a/Python/executor_cases.c.h b/Python/executor_cases.c.h index 3e51ac41fa3a2e..af4c39d03afa86 100644 --- a/Python/executor_cases.c.h +++ b/Python/executor_cases.c.h @@ -4273,8 +4273,17 @@ _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)) { diff --git a/Python/generated_cases.c.h b/Python/generated_cases.c.h index 7e8b05b747ed8f..9124035fc9e34d 100644 --- a/Python/generated_cases.c.h +++ b/Python/generated_cases.c.h @@ -5724,8 +5724,17 @@ // _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)) { @@ -7037,8 +7046,17 @@ /* 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); From 225e36abcd87214c8114ce3d348363612514ffd7 Mon Sep 17 00:00:00 2001 From: Mark Shannon Date: Fri, 6 Jun 2025 17:59:13 +0100 Subject: [PATCH 2/6] Move GET_ITER back to genexpr creation --- Python/codegen.c | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/Python/codegen.c b/Python/codegen.c index 1672b655fa9423..93c3a21266cb7d 100644 --- a/Python/codegen.c +++ b/Python/codegen.c @@ -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) { From 4bdd75e6ab1b881587757f1639e44460d43219e1 Mon Sep 17 00:00:00 2001 From: Mark Shannon Date: Fri, 6 Jun 2025 17:59:53 +0100 Subject: [PATCH 3/6] Fix up test generators to handle different behavior of gen exprs and gen functions --- Lib/test/test_generators.py | 21 +++++++++++++-------- 1 file changed, 13 insertions(+), 8 deletions(-) diff --git a/Lib/test/test_generators.py b/Lib/test/test_generators.py index 3e41c7b9663491..6c056b63f88480 100644 --- a/Lib/test/test_generators.py +++ b/Lib/test/test_generators.py @@ -318,21 +318,26 @@ def gen(it): 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 @@ def get_generator_genexpr(obj): 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 @@ def get_generator_genexpr(obj): 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): From fe8ca4b87a819c129552cc580de497707c5385de Mon Sep 17 00:00:00 2001 From: Mark Shannon Date: Fri, 6 Jun 2025 18:03:50 +0100 Subject: [PATCH 4/6] Fix up test_dis --- Lib/test/test_dis.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/Lib/test/test_dis.py b/Lib/test/test_dis.py index dbf90472ca34b6..48df7e530de2c1 100644 --- a/Lib/test/test_dis.py +++ b/Lib/test/test_dis.py @@ -204,6 +204,7 @@ def bug1333982(x=[]): LOAD_CONST 1 ( 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 @@ def foo(x): MAKE_FUNCTION SET_FUNCTION_ATTRIBUTE 8 (closure) LOAD_DEREF 1 (y) + GET_ITER CALL 0 CALL 1 RETURN_VALUE @@ -851,8 +853,7 @@ def foo(x): %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) From a96a192bd8263b6c159bd26dadbd7a9afb0479f3 Mon Sep 17 00:00:00 2001 From: Mark Shannon Date: Fri, 6 Jun 2025 18:58:06 +0100 Subject: [PATCH 5/6] Add news --- .../2025-06-06-18-57-30.gh-issue-135171.0YtLq6.rst | 6 ++++++ 1 file changed, 6 insertions(+) create mode 100644 Misc/NEWS.d/next/Core_and_Builtins/2025-06-06-18-57-30.gh-issue-135171.0YtLq6.rst 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 index 00000000000000..e46cb2b455f87e --- /dev/null +++ b/Misc/NEWS.d/next/Core_and_Builtins/2025-06-06-18-57-30.gh-issue-135171.0YtLq6.rst @@ -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``. From 0d47bea07f23fbb91c01d119aa2745c6460a2584 Mon Sep 17 00:00:00 2001 From: Mark Shannon Date: Fri, 6 Jun 2025 19:06:31 +0100 Subject: [PATCH 6/6] Add back genexpr test --- Lib/test/test_genexps.py | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/Lib/test/test_genexps.py b/Lib/test/test_genexps.py index fe5f18fa3f88a0..d90097dabea0c0 100644 --- a/Lib/test/test_genexps.py +++ b/Lib/test/test_genexps.py @@ -131,6 +131,14 @@ >>> 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 "", 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))