Skip to content

gh-120834: fix type of *_iframe field in _PyGenObject_HEAD declaration#120835

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
iritkatriel merged 6 commits into from
Jun 24, 2024
Merged

gh-120834: fix type of *_iframe field in _PyGenObject_HEAD declaration #120835

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
3e0689c
gh-120834: fix type of *_iframe field in _PyGenObject_HEAD declaration
iritkatriel Jun 21, 2024
04e18ca
update includes
iritkatriel Jun 21, 2024
a76a5ad
move _PyInterpreterFrame to public header so it can be included in ge…
iritkatriel Jun 21, 2024
dcd8ef2
make _PyGenObject_HEAD private, etc
iritkatriel Jun 21, 2024
6a7c2a9
remove calculation of tp_basicsize
iritkatriel Jun 23, 2024
c9ed63f
Merge branch 'main' into gi_frame-type
iritkatriel Jun 23, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 3 additions & 30 deletions Include/cpython/genobject.h
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -9,29 +9,7 @@ extern "C"{

/* --- Generators --------------------------------------------------------- */

/* _PyGenObject_HEAD defines the initial segment of generator
and coroutine objects. */
#define _PyGenObject_HEAD(prefix) \
PyObject_HEAD \
/* List of weak reference. */ \
PyObject *prefix##_weakreflist; \
/* Name of the generator. */ \
PyObject *prefix##_name; \
/* Qualified name of the generator. */ \
PyObject *prefix##_qualname; \
_PyErr_StackItem prefix##_exc_state; \
PyObject *prefix##_origin_or_finalizer; \
char prefix##_hooks_inited; \
char prefix##_closed; \
char prefix##_running_async; \
/* The frame */ \
int8_t prefix##_frame_state; \
PyObject *prefix##_iframe[1]; \

typedef struct{
/* The gi_ prefix is intended to remind of generator-iterator. */
_PyGenObject_HEAD(gi)
} PyGenObject;
typedef struct _PyGenObject PyGenObject;

PyAPI_DATA(PyTypeObject) PyGen_Type;

Expand All@@ -46,9 +24,7 @@ PyAPI_FUNC(PyCodeObject *) PyGen_GetCode(PyGenObject *gen);

/* --- PyCoroObject ------------------------------------------------------- */

typedef struct{
_PyGenObject_HEAD(cr)
} PyCoroObject;
typedef struct _PyCoroObject PyCoroObject;

PyAPI_DATA(PyTypeObject) PyCoro_Type;

Expand All@@ -59,9 +35,7 @@ PyAPI_FUNC(PyObject *) PyCoro_New(PyFrameObject *,

/* --- Asynchronous Generators -------------------------------------------- */

typedef struct{
_PyGenObject_HEAD(ag)
} PyAsyncGenObject;
typedef struct _PyAsyncGenObject PyAsyncGenObject;

PyAPI_DATA(PyTypeObject) PyAsyncGen_Type;
PyAPI_DATA(PyTypeObject) _PyAsyncGenASend_Type;
Expand All@@ -73,7 +47,6 @@ PyAPI_FUNC(PyObject *) PyAsyncGen_New(PyFrameObject *,

#define PyAsyncGenASend_CheckExact(op) Py_IS_TYPE((op), &_PyAsyncGenASend_Type)


#undef _PyGenObject_HEAD

#ifdef __cplusplus
Expand Down
8 changes: 0 additions & 8 deletions Include/internal/pycore_frame.h
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -307,14 +307,6 @@ _PyFrame_PushTrampolineUnchecked(PyThreadState *tstate, PyCodeObject *code, int
return frame;
}

static inline
PyGenObject *_PyFrame_GetGenerator(_PyInterpreterFrame *frame)
{
assert(frame->owner == FRAME_OWNED_BY_GENERATOR);
size_t offset_in_gen = offsetof(PyGenObject, gi_iframe);
return (PyGenObject *)(((char *)frame) - offset_in_gen);
}

PyAPI_FUNC(_PyInterpreterFrame *)
_PyEvalFramePushAndInit(PyThreadState *tstate, PyFunctionObject *func,
PyObject *locals, PyObject* const* args,
Expand Down
44 changes: 43 additions & 1 deletion Include/internal/pycore_genobject.h
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -8,7 +8,49 @@ extern "C"{
# error "this header requires Py_BUILD_CORE define"
#endif

#include "pycore_freelist.h"
#include "pycore_frame.h"

/* _PyGenObject_HEAD defines the initial segment of generator
and coroutine objects. */
#define _PyGenObject_HEAD(prefix) \
PyObject_HEAD \
/* List of weak reference. */ \
PyObject *prefix##_weakreflist; \
/* Name of the generator. */ \
PyObject *prefix##_name; \
/* Qualified name of the generator. */ \
PyObject *prefix##_qualname; \
_PyErr_StackItem prefix##_exc_state; \
PyObject *prefix##_origin_or_finalizer; \
char prefix##_hooks_inited; \
char prefix##_closed; \
char prefix##_running_async; \
/* The frame */ \
int8_t prefix##_frame_state; \
struct _PyInterpreterFrame prefix##_iframe; \

struct _PyGenObject{
/* The gi_ prefix is intended to remind of generator-iterator. */
_PyGenObject_HEAD(gi)
};

struct _PyCoroObject{
_PyGenObject_HEAD(cr)
};

struct _PyAsyncGenObject{
_PyGenObject_HEAD(ag)
};

#undef _PyGenObject_HEAD

static inline
PyGenObject *_PyGen_GetGeneratorFromFrame(_PyInterpreterFrame *frame)
{
assert(frame->owner == FRAME_OWNED_BY_GENERATOR);
size_t offset_in_gen = offsetof(PyGenObject, gi_iframe);
return (PyGenObject *)(((char *)frame) - offset_in_gen);
}

PyAPI_FUNC(PyObject *)_PyGen_yf(PyGenObject *);
extern void _PyGen_Finalize(PyObject *self);
Expand Down
2 changes: 1 addition & 1 deletion Include/internal/pycore_interp.h
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -23,7 +23,7 @@ extern "C"{
#include "pycore_floatobject.h" // struct _Py_float_state
#include "pycore_function.h" // FUNC_MAX_WATCHERS
#include "pycore_gc.h" // struct _gc_runtime_state
#include "pycore_genobject.h" // struct _Py_async_gen_state
#include "pycore_genobject.h" // _PyGen_FetchStopIterationValue
#include "pycore_global_objects.h"// struct _Py_interp_cached_objects
#include "pycore_import.h" // struct _import_state
#include "pycore_instruments.h" // _PY_MONITORING_EVENTS
Expand Down
6 changes: 3 additions & 3 deletions Objects/frameobject.c
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -1338,7 +1338,7 @@ static bool frame_is_suspended(PyFrameObject *frame)
{
assert(!_PyFrame_IsIncomplete(frame->f_frame));
if (frame->f_frame->owner == FRAME_OWNED_BY_GENERATOR){
PyGenObject *gen = _PyFrame_GetGenerator(frame->f_frame);
PyGenObject *gen = _PyGen_GetGeneratorFromFrame(frame->f_frame);
return FRAME_STATE_SUSPENDED(gen->gi_frame_state);
}
return false;
Expand DownExpand Up@@ -1665,7 +1665,7 @@ static PyObject *
frame_clear(PyFrameObject *f, PyObject *Py_UNUSED(ignored))
{
if (f->f_frame->owner == FRAME_OWNED_BY_GENERATOR){
PyGenObject *gen = _PyFrame_GetGenerator(f->f_frame);
PyGenObject *gen = _PyGen_GetGeneratorFromFrame(f->f_frame);
if (gen->gi_frame_state == FRAME_EXECUTING){
goto running;
}
Expand DownExpand Up@@ -2097,7 +2097,7 @@ PyFrame_GetGenerator(PyFrameObject *frame)
if (frame->f_frame->owner != FRAME_OWNED_BY_GENERATOR){
return NULL;
}
PyGenObject *gen = _PyFrame_GetGenerator(frame->f_frame);
PyGenObject *gen = _PyGen_GetGeneratorFromFrame(frame->f_frame);
return Py_NewRef(gen);
}

Expand Down
32 changes: 14 additions & 18 deletions Objects/genobject.c
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -6,8 +6,8 @@
#include "pycore_call.h" // _PyObject_CallNoArgs()
#include "pycore_ceval.h" // _PyEval_EvalFrame()
#include "pycore_frame.h" // _PyInterpreterFrame
#include "pycore_freelist.h" // struct _Py_async_gen_freelist
#include "pycore_gc.h" // _PyGC_CLEAR_FINALIZED()
#include "pycore_genobject.h" // struct _Py_async_gen_freelist
#include "pycore_modsupport.h" // _PyArg_CheckPositional()
#include "pycore_object.h" // _PyObject_GC_UNTRACK()
#include "pycore_opcode_utils.h" // RESUME_AFTER_YIELD_FROM
Expand All@@ -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 *
Expand All@@ -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);
Expand DownExpand Up@@ -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);
Expand All@@ -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){
Expand DownExpand Up@@ -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));
Expand DownExpand Up@@ -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
Expand All@@ -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;
}
}
Expand DownExpand Up@@ -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) &&
Expand DownExpand Up@@ -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 *
Expand DownExpand Up@@ -814,8 +813,7 @@ static PyAsyncMethods gen_as_async ={
PyTypeObject PyGen_Type ={
PyVarObject_HEAD_INIT(&PyType_Type, 0)
"generator", /* tp_name */
offsetof(PyGenObject, gi_iframe) +
offsetof(_PyInterpreterFrame, localsplus), /* tp_basicsize */
sizeof(PyGenObject), /* tp_basicsize */
Copy link
Member

@markshannonmarkshannonJun 24, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this should be offsetof(PyGenObject, gi_frame.localsplus) otherwise we are overallocating one PyObject * slot.

Likewise for PyCoroObject and PyAsyncGenObject

Copy link
MemberAuthor

@iritkatrieliritkatrielJun 24, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's try that: #120941

sizeof(PyObject *), /* tp_itemsize */
/* methods */
(destructor)gen_dealloc, /* tp_dealloc */
Expand DownExpand Up@@ -949,7 +947,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);
Expand DownExpand Up@@ -1166,8 +1164,7 @@ static PyAsyncMethods coro_as_async ={
PyTypeObject PyCoro_Type ={
PyVarObject_HEAD_INIT(&PyType_Type, 0)
"coroutine", /* tp_name */
offsetof(PyCoroObject, cr_iframe) +
offsetof(_PyInterpreterFrame, localsplus), /* tp_basicsize */
sizeof(PyCoroObject), /* tp_basicsize */
sizeof(PyObject *), /* tp_itemsize */
/* methods */
(destructor)gen_dealloc, /* tp_dealloc */
Expand DownExpand Up@@ -1582,8 +1579,7 @@ static PyAsyncMethods async_gen_as_async ={
PyTypeObject PyAsyncGen_Type ={
PyVarObject_HEAD_INIT(&PyType_Type, 0)
"async_generator", /* tp_name */
offsetof(PyAsyncGenObject, ag_iframe) +
offsetof(_PyInterpreterFrame, localsplus), /* tp_basicsize */
sizeof(PyAsyncGenObject), /* tp_basicsize */
sizeof(PyObject *), /* tp_itemsize */
/* methods */
(destructor)gen_dealloc, /* tp_dealloc */
Expand Down
12 changes: 6 additions & 6 deletions Python/bytecodes.c
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -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;
Expand DownExpand Up@@ -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;
Expand All@@ -1069,7 +1069,7 @@ dummy_func(
inst(INSTRUMENTED_YIELD_VALUE, (retval -- unused)){
assert(frame != &entry_frame);
frame->instr_ptr = next_instr;
PyGenObject *gen = _PyFrame_GetGenerator(frame);
PyGenObject *gen = _PyGen_GetGeneratorFromFrame(frame);
assert(FRAME_SUSPENDED_YIELD_FROM == FRAME_SUSPENDED + 1);
assert(oparg == 0 || oparg == 1);
gen->gi_frame_state = FRAME_SUSPENDED + oparg;
Expand DownExpand Up@@ -1099,7 +1099,7 @@ dummy_func(
assert(frame != &entry_frame);
#endif
frame->instr_ptr++;
PyGenObject *gen = _PyFrame_GetGenerator(frame);
PyGenObject *gen = _PyGen_GetGeneratorFromFrame(frame);
assert(FRAME_SUSPENDED_YIELD_FROM == FRAME_SUSPENDED + 1);
assert(oparg == 0 || oparg == 1);
gen->gi_frame_state = FRAME_SUSPENDED + oparg;
Expand DownExpand Up@@ -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;
Expand DownExpand Up@@ -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);
Expand Down
2 changes: 1 addition & 1 deletion Python/ceval.c
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -1711,7 +1711,7 @@ static void
clear_gen_frame(PyThreadState *tstate, _PyInterpreterFrame * frame)
{
assert(frame->owner == FRAME_OWNED_BY_GENERATOR);
PyGenObject *gen = _PyFrame_GetGenerator(frame);
PyGenObject *gen = _PyGen_GetGeneratorFromFrame(frame);
gen->gi_frame_state = FRAME_CLEARED;
assert(tstate->exc_info == &gen->gi_exc_state);
tstate->exc_info = gen->gi_exc_state.previous_item;
Expand Down
6 changes: 3 additions & 3 deletions Python/executor_cases.c.h
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion Python/frame.c
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -112,7 +112,7 @@ _PyFrame_ClearExceptCode(_PyInterpreterFrame *frame)
/* It is the responsibility of the owning generator/coroutine
* to have cleared the enclosing generator, if any. */
assert(frame->owner != FRAME_OWNED_BY_GENERATOR ||
_PyFrame_GetGenerator(frame)->gi_frame_state == FRAME_CLEARED);
_PyGen_GetGeneratorFromFrame(frame)->gi_frame_state == FRAME_CLEARED);
// GH-99729: Clearing this frame can expose the stack (via finalizers). It's
// crucial that this frame has been unlinked, and is no longer visible:
assert(_PyThreadState_GET()->current_frame != frame);
Expand Down
Loading