Repository: python/cpython
Pull request: gh-120834: fix type of *_iframe field in _PyGenObject_HEAD declaration
Fixes #120834 .

<!-- gh-issue-number: gh-120834 -->
* Issue: gh-120834
<!-- /gh-issue-number -->


Problem statement:
generator frame type should not be PyObject*[]

For historical reasons, the `*_iframe` field in `_PyGenObject_HEAD` is declared as ` PyObject*[]`, and later cast to `_PyInterpreterFrame *` (which is not a PyObject*). 



<!-- gh-linked-prs -->
### Linked PRs
* gh-120835
* gh-120941
* gh-120976
<!-- /gh-linked-prs -->

Hints from issue discussion:

Commit message: gh-120834: fix type of *_iframe field in _PyGenObject_HEAD declaration

Patch to review:
diff --git a/Include/cpython/genobject.h b/Include/cpython/genobject.h
index 49e46c277d75ae..18cf8b59a4d56c 100644
--- a/Include/cpython/genobject.h
+++ b/Include/cpython/genobject.h
@@ -7,6 +7,8 @@
 extern "C" {
 #endif
 
+struct _PyInterpreterFrame;
+
 /* --- Generators --------------------------------------------------------- */
 
 /* _PyGenObject_HEAD defines the initial segment of generator
@@ -26,7 +28,7 @@ extern "C" {
     char prefix##_running_async;                                            \
     /* The frame */                                                         \
     int8_t prefix##_frame_state;                                            \
-    PyObject *prefix##_iframe[1];                                           \
+    struct _PyInterpreterFrame *prefix##_iframe[1];                         \
 
 typedef struct {
     /* The gi_ prefix is intended to remind of generator-iterator. */
diff --git a/Objects/genobject.c b/Objects/genobject.c
index 662e644f8aeddd..4fce370e90c823 100644
--- a/Objects/genobject.c
+++ b/Objects/genobject.c
@@ -30,8 +30,7 @@ static const char *ASYNC_GEN_IGNORED_EXIT_MSG =
 /* Returns a borrowed reference */
 static inline PyCodeObject *
 _PyGen_GetCode(PyGenObject *gen) {
-    _PyInterpreterFrame *frame = (_PyInterpreterFrame *)(gen->gi_iframe);
-    return _PyFrame_GetCode(frame);
+    return _PyFrame_GetCode(gen->gi_iframe);
 }
 
 PyCodeObject *
@@ -48,7 +47,7 @@ gen_traverse(PyGenObject *gen, visitproc visit, void *arg)
     Py_VISIT(gen->gi_name);
     Py_VISIT(gen->gi_qualname);
     if (gen->gi_frame_state != FRAME_CLEARED) {
-        _PyInterpreterFrame *frame = (_PyInterpreterFrame *)(gen->gi_iframe);
+        _PyInterpreterFrame *frame = gen->gi_iframe;
         assert(frame->frame_obj == NULL ||
                frame->frame_obj->f_frame->owner == FRAME_OWNED_BY_GENERATOR);
         int err = _PyFrame_Traverse(frame, visit, arg);
@@ -141,7 +140,7 @@ gen_dealloc(PyGenObject *gen)
         Py_CLEAR(((PyAsyncGenObject*)gen)->ag_origin_or_finalizer);
     }
     if (gen->gi_frame_state != FRAME_CLEARED) {
-        _PyInterpreterFrame *frame = (_PyInterpreterFrame *)gen->gi_iframe;
+        _PyInterpreterFrame *frame = gen->gi_iframe;
         gen->gi_frame_state = FRAME_CLEARED;
         frame->previous = NULL;
         _PyFrame_ClearExceptCode(frame);
@@ -163,7 +162,7 @@ gen_send_ex2(PyGenObject *gen, PyObject *arg, PyObject **presult,
              int exc, int closing)
 {
     PyThreadState *tstate = _PyThreadState_GET();
-    _PyInterpreterFrame *frame = (_PyInterpreterFrame *)gen->gi_iframe;
+    _PyInterpreterFrame *frame = gen->gi_iframe;
 
     *presult = NULL;
     if (gen->gi_frame_state == FRAME_CREATED && arg && arg != Py_None) {
@@ -342,7 +341,7 @@ PyObject *
 _PyGen_yf(PyGenObject *gen)
 {
     if (gen->gi_frame_state == FRAME_SUSPENDED_YIELD_FROM) {
-        _PyInterpreterFrame *frame = (_PyInterpreterFrame *)gen->gi_iframe;
+        _PyInterpreterFrame *frame = gen->gi_iframe;
         assert(is_resume(frame->instr_ptr));
         assert((frame->instr_ptr->op.arg & RESUME_OPARG_LOCATION_MASK) >= RESUME_AFTER_YIELD_FROM);
         return Py_NewRef(_PyFrame_StackPeek(frame));
@@ -372,7 +371,7 @@ gen_close(PyGenObject *gen, PyObject *args)
         gen->gi_frame_state = state;
         Py_DECREF(yf);
     }
-    _PyInterpreterFrame *frame = (_PyInterpreterFrame *)gen->gi_iframe;
+    _PyInterpreterFrame *frame = gen->gi_iframe;
     if (is_resume(frame->instr_ptr)) {
         /* We can safely ignore the outermost try block
          * as it is automatically generated to handle
@@ -382,7 +381,7 @@ gen_close(PyGenObject *gen, PyObject *args)
             // RESUME after YIELD_VALUE and exception depth is 1
             assert((oparg & RESUME_OPARG_LOCATION_MASK) != RESUME_AT_FUNC_START);
             gen->gi_frame_state = FRAME_COMPLETED;
-            _PyFrame_ClearLocals((_PyInterpreterFrame *)gen->gi_iframe);
+            _PyFrame_ClearLocals(gen->gi_iframe);
             Py_RETURN_NONE;
         }
     }
@@ -431,7 +430,7 @@ _gen_throw(PyGenObject *gen, int close_on_genexit,
     PyObject *yf = _PyGen_yf(gen);
 
     if (yf) {
-        _PyInterpreterFrame *frame = (_PyInterpreterFrame *)gen->gi_iframe;
+        _PyInterpreterFrame *frame = gen->gi_iframe;
         PyObject *ret;
         int err;
         if (PyErr_GivenExceptionMatches(typ, PyExc_GeneratorExit) &&
@@ -739,7 +738,7 @@ _gen_getframe(PyGenObject *gen, const char *const name)
     if (FRAME_STATE_FINISHED(gen->gi_frame_state)) {
         Py_RETURN_NONE;
     }
-    return _Py_XNewRef((PyObject *)_PyFrame_GetFrameObject((_PyInterpreterFrame *)gen->gi_iframe));
+    return _Py_XNewRef((PyObject *)_PyFrame_GetFrameObject(gen->gi_iframe));
 }
 
 static PyObject *
@@ -949,7 +948,7 @@ gen_new_with_qualname(PyTypeObject *type, PyFrameObject *f,
     /* Copy the frame */
     assert(f->f_frame->frame_obj == NULL);
     assert(f->f_frame->owner == FRAME_OWNED_BY_FRAME_OBJECT);
-    _PyInterpreterFrame *frame = (_PyInterpreterFrame *)gen->gi_iframe;
+    _PyInterpreterFrame *frame = gen->gi_iframe;
     _PyFrame_Copy((_PyInterpreterFrame *)f->_f_frame_data, frame);
     gen->gi_frame_state = FRAME_CREATED;
     assert(frame->frame_obj == f);
diff --git a/Python/bytecodes.c b/Python/bytecodes.c
index 50444bcc0d200c..f8f22435faa9f2 100644
--- a/Python/bytecodes.c
+++ b/Python/bytecodes.c
@@ -1015,7 +1015,7 @@ dummy_func(
                 ((PyGenObject *)receiver)->gi_frame_state < FRAME_EXECUTING)
             {
                 PyGenObject *gen = (PyGenObject *)receiver;
-                _PyInterpreterFrame *gen_frame = (_PyInterpreterFrame *)gen->gi_iframe;
+                _PyInterpreterFrame *gen_frame = gen->gi_iframe;
                 STACK_SHRINK(1);
                 _PyFrame_StackPush(gen_frame, v);
                 gen->gi_frame_state = FRAME_EXECUTING;
@@ -1055,7 +1055,7 @@ dummy_func(
             DEOPT_IF(Py_TYPE(gen) != &PyGen_Type && Py_TYPE(gen) != &PyCoro_Type);
             DEOPT_IF(gen->gi_frame_state >= FRAME_EXECUTING);
             STAT_INC(SEND, hit);
-            _PyInterpreterFrame *gen_frame = (_PyInterpreterFrame *)gen->gi_iframe;
+            _PyInterpreterFrame *gen_frame = gen->gi_iframe;
             STACK_SHRINK(1);
             _PyFrame_StackPush(gen_frame, v);
             gen->gi_frame_state = FRAME_EXECUTING;
@@ -2799,7 +2799,7 @@ dummy_func(
             DEOPT_IF(Py_TYPE(gen) != &PyGen_Type);
             DEOPT_IF(gen->gi_frame_state >= FRAME_EXECUTING);
             STAT_INC(FOR_ITER, hit);
-            gen_frame = (_PyInterpreterFrame *)gen->gi_iframe;
+            gen_frame = gen->gi_iframe;
             _PyFrame_StackPush(gen_frame, Py_None);
             gen->gi_frame_state = FRAME_EXECUTING;
             gen->gi_exc_state.previous_item = tstate->exc_info;
@@ -3940,7 +3940,7 @@ dummy_func(
             }
             assert(EMPTY());
             _PyFrame_SetStackPointer(frame, stack_pointer);
-            _PyInterpreterFrame *gen_frame = (_PyInterpreterFrame *)gen->gi_iframe;
+            _PyInterpreterFrame *gen_frame = gen->gi_iframe;
             frame->instr_ptr++;
             _PyFrame_Copy(frame, gen_frame);
             assert(frame->frame_obj == NULL);
diff --git a/Python/executor_cases.c.h b/Python/executor_cases.c.h
index b4e5261f57ab8b..1f8e70be7cce56 100644
--- a/Python/executor_cases.c.h
+++ b/Python/executor_cases.c.h
@@ -2871,7 +2871,7 @@
                 JUMP_TO_JUMP_TARGET();
             }
             STAT_INC(FOR_ITER, hit);
-            gen_frame = (_PyInterpreterFrame *)gen->gi_iframe;
+            gen_frame = gen->gi_iframe;
             _PyFrame_StackPush(gen_frame, Py_None);
             gen->gi_frame_state = FRAME_EXECUTING;
             gen->gi_exc_state.previous_item = tstate->exc_info;
@@ -4110,7 +4110,7 @@
             }
             assert(EMPTY());
             _PyFrame_SetStackPointer(frame, stack_pointer);
-            _PyInterpreterFrame *gen_frame = (_PyInterpreterFrame *)gen->gi_iframe;
+            _PyInterpreterFrame *gen_frame = gen->gi_iframe;
             frame->instr_ptr++;
             _PyFrame_Copy(frame, gen_frame);
             assert(frame->frame_obj == NULL);
diff --git a/Python/generated_cases.c.h b/Python/generated_cases.c.h
index 3980f9852e6396..7792649fbba600 100644
--- a/Python/generated_cases.c.h
+++ b/Python/generated_cases.c.h
@@ -2747,7 +2747,7 @@
                 DEOPT_IF(Py_TYPE(gen) != &PyGen_Type, FOR_ITER);
                 DEOPT_IF(gen->gi_frame_state >= FRAME_EXECUTING, FOR_ITER);
                 STAT_INC(FOR_ITER, hit);
-                gen_frame = (_PyInterpreterFrame *)gen->gi_iframe;
+                gen_frame = gen->gi_iframe;
                 _PyFrame_StackPush(gen_frame, Py_None);
                 gen->gi_frame_state = FRAME_EXECUTING;
                 gen->gi_exc_state.previous_item = tstate->exc_info;
@@ -5180,7 +5180,7 @@
             }
             assert(EMPTY());
             _PyFrame_SetStackPointer(frame, stack_pointer);
-            _PyInterpreterFrame *gen_frame = (_PyInterpreterFrame *)gen->gi_iframe;
+            _PyInterpreterFrame *gen_frame = gen->gi_iframe;
             frame->instr_ptr++;
             _PyFrame_Copy(frame, gen_frame);
             assert(frame->frame_obj == NULL);
@@ -5260,7 +5260,7 @@
                     ((PyGenObject *)receiver)->gi_frame_state < FRAME_EXECUTING)
                 {
                     PyGenObject *gen = (PyGenObject *)receiver;
-                    _PyInterpreterFrame *gen_frame = (_PyInterpreterFrame *)gen->gi_iframe;
+                    _PyInterpreterFrame *gen_frame = gen->gi_iframe;
                     STACK_SHRINK(1);
                     _PyFrame_StackPush(gen_frame, v);
                     gen->gi_frame_state = FRAME_EXECUTING;
@@ -5310,7 +5310,7 @@
             DEOPT_IF(Py_TYPE(gen) != &PyGen_Type && Py_TYPE(gen) != &PyCoro_Type, SEND);
             DEOPT_IF(gen->gi_frame_state >= FRAME_EXECUTING, SEND);
             STAT_INC(SEND, hit);
-            _PyInterpreterFrame *gen_frame = (_PyInterpreterFrame *)gen->gi_iframe;
+            _PyInterpreterFrame *gen_frame = gen->gi_iframe;
             STACK_SHRINK(1);
             _PyFrame_StackPush(gen_frame, v);
             gen->gi_frame_state = FRAME_EXECUTING;


Code review comments:
diff_hunk: @@ -26,7 +28,7 @@ extern "C" {
     char prefix##_running_async;                                            \
     /* The frame */                                                         \
     int8_t prefix##_frame_state;                                            \
-    PyObject *prefix##_iframe[1];                                           \
+    struct _PyInterpreterFrame *prefix##_iframe[1];                         \
path: Include/cpython/genobject.h
text: @author:
I tried to remove the [1] but then it crashes during the build. Not sure why.

@user1:
```suggestion
    struct _PyInterpreterFrame prefix##_iframe;                         \
```
It's not a pointer to a frame, it is a frame.


@user2:
That's pretty subtle, so it's nice to see this being made more explicit by this PR.

Given that, maybe the object size calculation could be simplified to not need the explicit addition (not tested or 100% sure on that, though): https://github.com/python/cpython/blob/main/Objects/genobject.c#L818


@author:
> Given that, maybe the object size calculation could be simplified to not need the explicit addition (not tested or 100% sure on that, though): https://github.com/python/cpython/blob/main/Objects/genobject.c#L818

That seems to work.

diff_hunk: @@ -48,7 +47,7 @@ gen_traverse(PyGenObject *gen, visitproc visit, void *arg)
     Py_VISIT(gen->gi_name);
     Py_VISIT(gen->gi_qualname);
     if (gen->gi_frame_state != FRAME_CLEARED) {
-        _PyInterpreterFrame *frame = (_PyInterpreterFrame *)(gen->gi_iframe);
+        _PyInterpreterFrame *frame = gen->gi_iframe;
path: Objects/genobject.c
text: @user1:
```suggestion
        _PyInterpreterFrame *frame = &gen->gi_iframe;
```

diff_hunk: @@ -141,7 +140,7 @@ gen_dealloc(PyGenObject *gen)
         Py_CLEAR(((PyAsyncGenObject*)gen)->ag_origin_or_finalizer);
     }
     if (gen->gi_frame_state != FRAME_CLEARED) {
-        _PyInterpreterFrame *frame = (_PyInterpreterFrame *)gen->gi_iframe;
+        _PyInterpreterFrame *frame = gen->gi_iframe;
path: Objects/genobject.c
text: @user1:
```suggestion
        _PyInterpreterFrame *frame = &gen->gi_iframe;
```

diff_hunk: @@ -342,7 +341,7 @@ PyObject *
 _PyGen_yf(PyGenObject *gen)
 {
     if (gen->gi_frame_state == FRAME_SUSPENDED_YIELD_FROM) {
-        _PyInterpreterFrame *frame = (_PyInterpreterFrame *)gen->gi_iframe;
+        _PyInterpreterFrame *frame = gen->gi_iframe;
path: Objects/genobject.c
text: @user1:
```suggestion
        _PyInterpreterFrame *frame = &gen->gi_iframe;
```

diff_hunk: @@ -163,7 +162,7 @@ gen_send_ex2(PyGenObject *gen, PyObject *arg, PyObject **presult,
              int exc, int closing)
 {
     PyThreadState *tstate = _PyThreadState_GET();
-    _PyInterpreterFrame *frame = (_PyInterpreterFrame *)gen->gi_iframe;
+    _PyInterpreterFrame *frame = gen->gi_iframe;
path: Objects/genobject.c
text: @user1:
```suggestion
    _PyInterpreterFrame *frame = &gen->gi_iframe;
```