From 59bc36aacddd5a3acd32c80c0dfd0726135a7817 Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Mon, 1 May 2023 15:08:34 -0600 Subject: gh-84436: Immortalize in _PyStructSequence_InitBuiltinWithFlags() (gh-104054) This also does some cleanup. --- Include/internal/pycore_structseq.h | 4 +- Objects/floatobject.c | 4 +- Objects/longobject.c | 4 +- Objects/structseq.c | 102 ++++++++++++++++++++---------------- Objects/typeobject.c | 1 + Python/errors.c | 4 +- Python/sysmodule.c | 10 ++-- Python/thread.c | 4 +- 8 files changed, 74 insertions(+), 59 deletions(-) diff --git a/Include/internal/pycore_structseq.h b/Include/internal/pycore_structseq.h index d10a921..bd1e85c 100644 --- a/Include/internal/pycore_structseq.h +++ b/Include/internal/pycore_structseq.h @@ -15,7 +15,7 @@ PyAPI_FUNC(PyTypeObject *) _PyStructSequence_NewType( PyStructSequence_Desc *desc, unsigned long tp_flags); -PyAPI_FUNC(int) _PyStructSequence_InitBuiltinWithFlags( +extern int _PyStructSequence_InitBuiltinWithFlags( PyTypeObject *type, PyStructSequence_Desc *desc, unsigned long tp_flags); @@ -27,7 +27,7 @@ _PyStructSequence_InitBuiltin(PyTypeObject *type, return _PyStructSequence_InitBuiltinWithFlags(type, desc, 0); } -extern void _PyStructSequence_FiniType(PyTypeObject *type); +extern void _PyStructSequence_FiniBuiltin(PyTypeObject *type); #ifdef __cplusplus } diff --git a/Objects/floatobject.c b/Objects/floatobject.c index 9c23157..a694ddc 100644 --- a/Objects/floatobject.c +++ b/Objects/floatobject.c @@ -12,7 +12,7 @@ #include "pycore_object.h" // _PyObject_Init() #include "pycore_pymath.h" // _PY_SHORT_FLOAT_REPR #include "pycore_pystate.h" // _PyInterpreterState_GET() -#include "pycore_structseq.h" // _PyStructSequence_FiniType() +#include "pycore_structseq.h" // _PyStructSequence_FiniBuiltin() #include #include @@ -2029,7 +2029,7 @@ void _PyFloat_FiniType(PyInterpreterState *interp) { if (_Py_IsMainInterpreter(interp)) { - _PyStructSequence_FiniType(&FloatInfoType); + _PyStructSequence_FiniBuiltin(&FloatInfoType); } } diff --git a/Objects/longobject.c b/Objects/longobject.c index f84809b..de04348 100644 --- a/Objects/longobject.c +++ b/Objects/longobject.c @@ -9,7 +9,7 @@ #include "pycore_object.h" // _PyObject_Init() #include "pycore_pystate.h" // _Py_IsMainInterpreter() #include "pycore_runtime.h" // _PY_NSMALLPOSINTS -#include "pycore_structseq.h" // _PyStructSequence_FiniType() +#include "pycore_structseq.h" // _PyStructSequence_FiniBuiltin() #include #include @@ -6367,5 +6367,5 @@ _PyLong_FiniTypes(PyInterpreterState *interp) return; } - _PyStructSequence_FiniType(&Int_InfoType); + _PyStructSequence_FiniBuiltin(&Int_InfoType); } diff --git a/Objects/structseq.c b/Objects/structseq.c index 88a71bc..d8f55dc 100644 --- a/Objects/structseq.c +++ b/Objects/structseq.c @@ -433,12 +433,10 @@ error: static PyMemberDef * initialize_members(PyStructSequence_Desc *desc, - Py_ssize_t *pn_members, Py_ssize_t *pn_unnamed_members) + Py_ssize_t n_members, Py_ssize_t n_unnamed_members) { PyMemberDef *members; - Py_ssize_t n_members, n_unnamed_members; - n_members = count_members(desc, &n_unnamed_members); members = PyMem_NEW(PyMemberDef, n_members - n_unnamed_members + 1); if (members == NULL) { PyErr_NoMemory(); @@ -463,8 +461,6 @@ initialize_members(PyStructSequence_Desc *desc, } members[k].name = NULL; - *pn_members = n_members; - *pn_unnamed_members = n_unnamed_members; return members; } @@ -510,39 +506,58 @@ _PyStructSequence_InitBuiltinWithFlags(PyTypeObject *type, PyStructSequence_Desc *desc, unsigned long tp_flags) { - if (type->tp_flags & Py_TPFLAGS_READY) { - if (_PyStaticType_InitBuiltin(type) < 0) { - goto failed_init_builtin; + Py_ssize_t n_unnamed_members; + Py_ssize_t n_members = count_members(desc, &n_unnamed_members); + PyMemberDef *members = NULL; + + int initialized = 1; + if ((type->tp_flags & Py_TPFLAGS_READY) == 0) { + assert(type->tp_name == NULL); + assert(type->tp_members == NULL); + assert(type->tp_base == NULL); + + members = initialize_members(desc, n_members, n_unnamed_members); + if (members == NULL) { + goto error; } - return 0; - } + initialize_static_fields(type, desc, members, tp_flags); - PyMemberDef *members; - Py_ssize_t n_members, n_unnamed_members; - - members = initialize_members(desc, &n_members, &n_unnamed_members); - if (members == NULL) { - return -1; + _Py_SetImmortal(type); + initialized = 0; + } +#ifndef NDEBUG + else { + // Ensure that the type was initialized. + assert(type->tp_name != NULL); + assert(type->tp_members != NULL); + assert(type->tp_base == &PyTuple_Type); + assert((type->tp_flags & _Py_TPFLAGS_STATIC_BUILTIN)); + assert(_Py_IsImmortal(type)); } - initialize_static_fields(type, desc, members, tp_flags); +#endif - Py_INCREF(type); // XXX It should be immortal. if (_PyStaticType_InitBuiltin(type) < 0) { - PyMem_Free(members); - goto failed_init_builtin; + PyErr_Format(PyExc_RuntimeError, + "Can't initialize builtin type %s", + desc->name); + goto error; + } + // This should be dropped if tp_dict is made per-interpreter. + if (initialized) { + return 0; } if (initialize_structseq_dict( desc, type->tp_dict, n_members, n_unnamed_members) < 0) { - PyMem_Free(members); - return -1; + goto error; } + return 0; -failed_init_builtin: - PyErr_Format(PyExc_RuntimeError, - "Can't initialize builtin type %s", - desc->name); +error: + if (members != NULL) { + PyMem_Free(members); + } return -1; } @@ -566,7 +581,8 @@ PyStructSequence_InitType2(PyTypeObject *type, PyStructSequence_Desc *desc) return -1; } - members = initialize_members(desc, &n_members, &n_unnamed_members); + n_members = count_members(desc, &n_unnamed_members); + members = initialize_members(desc, n_members, n_unnamed_members); if (members == NULL) { return -1; } @@ -585,35 +601,32 @@ PyStructSequence_InitType(PyTypeObject *type, PyStructSequence_Desc *desc) } +/* This is exposed in the internal API, not the public API. + It is only called on builtin static types, which are all + initialized via _PyStructSequence_InitBuiltinWithFlags(). */ + void -_PyStructSequence_FiniType(PyTypeObject *type) +_PyStructSequence_FiniBuiltin(PyTypeObject *type) { // Ensure that the type is initialized assert(type->tp_name != NULL); assert(type->tp_base == &PyTuple_Type); + assert((type->tp_flags & _Py_TPFLAGS_STATIC_BUILTIN)); + assert(_Py_IsImmortal(type)); // Cannot delete a type if it still has subclasses if (_PyType_HasSubclasses(type)) { + // XXX Shouldn't this be an error? return; } - // Undo PyStructSequence_NewType() - type->tp_name = NULL; - PyMem_Free(type->tp_members); - _PyStaticType_Dealloc(type); - assert(Py_REFCNT(type) == 1); - // Undo Py_INCREF(type) of _PyStructSequence_InitType(). - // Don't use Py_DECREF(): static type must not be deallocated - Py_SET_REFCNT(type, 0); -#ifdef Py_REF_DEBUG - _Py_DecRefTotal(_PyInterpreterState_GET()); -#endif - // Make sure that _PyStructSequence_InitType() will initialize - // the type again - assert(Py_REFCNT(type) == 0); - assert(type->tp_name == NULL); + // Undo _PyStructSequence_InitBuiltinWithFlags(). + type->tp_name = NULL; + PyMem_Free(type->tp_members); + type->tp_members = NULL; + type->tp_base = NULL; } @@ -627,7 +640,8 @@ _PyStructSequence_NewType(PyStructSequence_Desc *desc, unsigned long tp_flags) Py_ssize_t n_members, n_unnamed_members; /* Initialize MemberDefs */ - members = initialize_members(desc, &n_members, &n_unnamed_members); + n_members = count_members(desc, &n_unnamed_members); + members = initialize_members(desc, n_members, n_unnamed_members); if (members == NULL) { return NULL; } diff --git a/Objects/typeobject.c b/Objects/typeobject.c index e807cc9..060d14e 100644 --- a/Objects/typeobject.c +++ b/Objects/typeobject.c @@ -7031,6 +7031,7 @@ PyType_Ready(PyTypeObject *type) int _PyStaticType_InitBuiltin(PyTypeObject *self) { + assert(_Py_IsImmortal((PyObject *)self)); assert(!(self->tp_flags & Py_TPFLAGS_HEAPTYPE)); if (self->tp_flags & Py_TPFLAGS_READY) { diff --git a/Python/errors.c b/Python/errors.c index 7fc2673..ce72049 100644 --- a/Python/errors.c +++ b/Python/errors.c @@ -6,7 +6,7 @@ #include "pycore_initconfig.h" // _PyStatus_ERR() #include "pycore_pyerrors.h" // _PyErr_Format() #include "pycore_pystate.h" // _PyThreadState_GET() -#include "pycore_structseq.h" // _PyStructSequence_FiniType() +#include "pycore_structseq.h" // _PyStructSequence_FiniBuiltin() #include "pycore_sysmodule.h" // _PySys_Audit() #include "pycore_traceback.h" // _PyTraceBack_FromFrame() @@ -1357,7 +1357,7 @@ _PyErr_FiniTypes(PyInterpreterState *interp) return; } - _PyStructSequence_FiniType(&UnraisableHookArgsType); + _PyStructSequence_FiniBuiltin(&UnraisableHookArgsType); } diff --git a/Python/sysmodule.c b/Python/sysmodule.c index d673e40..81dabe6 100644 --- a/Python/sysmodule.c +++ b/Python/sysmodule.c @@ -3492,13 +3492,13 @@ void _PySys_Fini(PyInterpreterState *interp) { if (_Py_IsMainInterpreter(interp)) { - _PyStructSequence_FiniType(&VersionInfoType); - _PyStructSequence_FiniType(&FlagsType); + _PyStructSequence_FiniBuiltin(&VersionInfoType); + _PyStructSequence_FiniBuiltin(&FlagsType); #if defined(MS_WINDOWS) - _PyStructSequence_FiniType(&WindowsVersionType); + _PyStructSequence_FiniBuiltin(&WindowsVersionType); #endif - _PyStructSequence_FiniType(&Hash_InfoType); - _PyStructSequence_FiniType(&AsyncGenHooksType); + _PyStructSequence_FiniBuiltin(&Hash_InfoType); + _PyStructSequence_FiniBuiltin(&AsyncGenHooksType); #ifdef __EMSCRIPTEN__ Py_CLEAR(EmscriptenInfoType); #endif diff --git a/Python/thread.c b/Python/thread.c index 7fdedb0..f90cd34 100644 --- a/Python/thread.c +++ b/Python/thread.c @@ -7,7 +7,7 @@ #include "Python.h" #include "pycore_pystate.h" // _PyInterpreterState_GET() -#include "pycore_structseq.h" // _PyStructSequence_FiniType() +#include "pycore_structseq.h" // _PyStructSequence_FiniBuiltin() #include "pycore_pythread.h" #ifndef DONT_HAVE_STDIO_H @@ -195,5 +195,5 @@ _PyThread_FiniType(PyInterpreterState *interp) return; } - _PyStructSequence_FiniType(&ThreadInfoType); + _PyStructSequence_FiniBuiltin(&ThreadInfoType); } -- cgit v0.12