From 3fdf7eca1234035062e3e3594bede533d8a391a8 Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Thu, 10 Nov 2022 17:24:47 -0700 Subject: [PATCH 1/5] Add _PyRuntimeState.imports.inittab. --- Include/internal/pycore_import.h | 2 ++ Include/internal/pycore_pylifecycle.h | 1 + Python/import.c | 20 ++++++++++++++++---- Python/pylifecycle.c | 5 +++++ Python/sysmodule.c | 5 +++-- 5 files changed, 27 insertions(+), 6 deletions(-) diff --git a/Include/internal/pycore_import.h b/Include/internal/pycore_import.h index 7f1240f7c9db3c..7e8d73be5b9ac1 100644 --- a/Include/internal/pycore_import.h +++ b/Include/internal/pycore_import.h @@ -7,6 +7,8 @@ extern "C" { struct _import_runtime_state { + /* The builtin modules (defined in config.c). */ + struct _inittab *inittab; /* The most recent value assigned to a PyModuleDef.m_base.m_index. This is incremented each time PyModuleDef_Init() is called, which is just about every time an extension module is imported. diff --git a/Include/internal/pycore_pylifecycle.h b/Include/internal/pycore_pylifecycle.h index b4718b8ade2354..85bf166d92fd47 100644 --- a/Include/internal/pycore_pylifecycle.h +++ b/Include/internal/pycore_pylifecycle.h @@ -33,6 +33,7 @@ PyAPI_FUNC(int) _Py_IsLocaleCoercionTarget(const char *ctype_loc); /* Various one-time initializers */ +extern PyStatus _PyImport_Init(void); extern PyStatus _PyFaulthandler_Init(int enable); extern int _PyTraceMalloc_Init(int enable); extern PyObject * _PyBuiltin_Init(PyInterpreterState *interp); diff --git a/Python/import.c b/Python/import.c index d16161381a97a8..11503a4ab4fd11 100644 --- a/Python/import.c +++ b/Python/import.c @@ -218,6 +218,16 @@ _imp_release_lock_impl(PyObject *module) Py_RETURN_NONE; } +PyStatus +_PyImport_Init(void) +{ + if (_PyRuntime.imports.inittab != NULL) { + return _PyStatus_ERR("global import state already initialized"); + } + _PyRuntime.imports.inittab = PyImport_Inittab; + return _PyStatus_OK(); +} + static inline void _extensions_cache_clear(void); void @@ -228,6 +238,7 @@ _PyImport_Fini(void) PyThread_free_lock(import_lock); import_lock = NULL; } + _PyRuntime.imports.inittab = NULL; } void @@ -889,9 +900,10 @@ static int is_builtin(PyObject *name) { int i; - for (i = 0; PyImport_Inittab[i].name != NULL; i++) { - if (_PyUnicode_EqualToASCIIString(name, PyImport_Inittab[i].name)) { - if (PyImport_Inittab[i].initfunc == NULL) + struct _inittab *inittab = _PyRuntime.imports.inittab; + for (i = 0; inittab[i].name != NULL; i++) { + if (_PyUnicode_EqualToASCIIString(name, inittab[i].name)) { + if (inittab[i].initfunc == NULL) return -1; else return 1; @@ -984,7 +996,7 @@ create_builtin(PyThreadState *tstate, PyObject *name, PyObject *spec) } PyObject *modules = tstate->interp->modules; - for (struct _inittab *p = PyImport_Inittab; p->name != NULL; p++) { + for (struct _inittab *p = _PyRuntime.imports.inittab; p->name != NULL; p++) { if (_PyUnicode_EqualToASCIIString(name, p->name)) { if (p->initfunc == NULL) { /* Cannot re-init internal module ("sys" or "builtins") */ diff --git a/Python/pylifecycle.c b/Python/pylifecycle.c index 3991089b38c451..a025b879cc813e 100644 --- a/Python/pylifecycle.c +++ b/Python/pylifecycle.c @@ -605,6 +605,11 @@ pycore_init_runtime(_PyRuntimeState *runtime, return status; } + status = _PyImport_Init(); + if (_PyStatus_EXCEPTION(status)) { + return status; + } + status = _PyInterpreterState_Enable(runtime); if (_PyStatus_EXCEPTION(status)) { return status; diff --git a/Python/sysmodule.c b/Python/sysmodule.c index 5de684e7195bff..1090b124feab26 100644 --- a/Python/sysmodule.c +++ b/Python/sysmodule.c @@ -2252,8 +2252,9 @@ list_builtin_module_names(void) if (list == NULL) { return NULL; } - for (Py_ssize_t i = 0; PyImport_Inittab[i].name != NULL; i++) { - PyObject *name = PyUnicode_FromString(PyImport_Inittab[i].name); + struct _inittab *inittab = _PyRuntime.imports.inittab; + for (Py_ssize_t i = 0; inittab[i].name != NULL; i++) { + PyObject *name = PyUnicode_FromString(inittab[i].name); if (name == NULL) { goto error; } From 492ffdf0499788f0afe410461b59816bfe094d67 Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Thu, 10 Nov 2022 17:25:09 -0700 Subject: [PATCH 2/5] Use a copy of PyImport_Inittab. --- Include/cpython/import.h | 1 + Python/import.c | 46 ++++++++++++++++++++- Tools/c-analyzer/cpython/globals-to-fix.tsv | 1 - Tools/c-analyzer/cpython/ignored.tsv | 1 + 4 files changed, 46 insertions(+), 3 deletions(-) diff --git a/Include/cpython/import.h b/Include/cpython/import.h index a69b4f34def342..a58801b47f1bec 100644 --- a/Include/cpython/import.h +++ b/Include/cpython/import.h @@ -25,6 +25,7 @@ struct _inittab { const char *name; /* ASCII encoded string */ PyObject* (*initfunc)(void); }; +// This is not used after Py_Initialize() is called. PyAPI_DATA(struct _inittab *) PyImport_Inittab; PyAPI_FUNC(int) PyImport_ExtendInittab(struct _inittab *newtab); diff --git a/Python/import.c b/Python/import.c index 11503a4ab4fd11..a2e477ca87c9c3 100644 --- a/Python/import.c +++ b/Python/import.c @@ -30,6 +30,8 @@ static PyObject *import_add_module(PyThreadState *tstate, PyObject *name); /* This table is defined in config.c: */ extern struct _inittab _PyImport_Inittab[]; +// This is not used after Py_Initialize() is called. +// (See _PyRuntimeState.imports.inittab.) struct _inittab *PyImport_Inittab = _PyImport_Inittab; static struct _inittab *inittab_copy = NULL; @@ -224,8 +226,30 @@ _PyImport_Init(void) if (_PyRuntime.imports.inittab != NULL) { return _PyStatus_ERR("global import state already initialized"); } - _PyRuntime.imports.inittab = PyImport_Inittab; - return _PyStatus_OK(); + PyStatus status = _PyStatus_OK(); + + size_t size; + for (size = 0; PyImport_Inittab[size].name != NULL; size++) + ; + size++; + + /* Force default raw memory allocator to get a known allocator to be able + to release the memory in _PyImport_Fini() */ + PyMemAllocatorEx old_alloc; + _PyMem_SetDefaultAllocator(PYMEM_DOMAIN_RAW, &old_alloc); + + /* Make the copy. */ + struct _inittab *copied = PyMem_RawMalloc(size * sizeof(struct _inittab)); + if (copied == NULL) { + status = PyStatus_NoMemory(); + goto done; + } + memcpy(copied, PyImport_Inittab, size * sizeof(struct _inittab)); + _PyRuntime.imports.inittab = copied; + +done: + PyMem_SetAllocator(PYMEM_DOMAIN_RAW, &old_alloc); + return status; } static inline void _extensions_cache_clear(void); @@ -238,7 +262,17 @@ _PyImport_Fini(void) PyThread_free_lock(import_lock); import_lock = NULL; } + + /* Use the same memory allocator as _PyImport_Init(). */ + PyMemAllocatorEx old_alloc; + _PyMem_SetDefaultAllocator(PYMEM_DOMAIN_RAW, &old_alloc); + + /* Free memory allocated by _PyImport_Init() */ + struct _inittab *inittab = _PyRuntime.imports.inittab; _PyRuntime.imports.inittab = NULL; + PyMem_RawFree(inittab); + + PyMem_SetAllocator(PYMEM_DOMAIN_RAW, &old_alloc); } void @@ -2604,6 +2638,10 @@ PyImport_ExtendInittab(struct _inittab *newtab) size_t i, n; int res = 0; + if (_PyRuntime.imports.inittab != NULL) { + Py_FatalError("PyImport_ExtendInittab() may be be called after Py_Initialize()"); + } + /* Count the number of entries in both tables */ for (n = 0; newtab[n].name != NULL; n++) ; @@ -2648,6 +2686,10 @@ PyImport_AppendInittab(const char *name, PyObject* (*initfunc)(void)) { struct _inittab newtab[2]; + if (_PyRuntime.imports.inittab != NULL) { + Py_FatalError("PyImport_AppendInittab() may be be called after Py_Initialize()"); + } + memset(newtab, '\0', sizeof newtab); newtab[0].name = name; diff --git a/Tools/c-analyzer/cpython/globals-to-fix.tsv b/Tools/c-analyzer/cpython/globals-to-fix.tsv index bb05a2c469bd9a..d2e58ccace2977 100644 --- a/Tools/c-analyzer/cpython/globals-to-fix.tsv +++ b/Tools/c-analyzer/cpython/globals-to-fix.tsv @@ -393,7 +393,6 @@ Python/frozen.c - _PyImport_FrozenBootstrap - Python/frozen.c - _PyImport_FrozenStdlib - Python/frozen.c - _PyImport_FrozenTest - Python/import.c - inittab_copy - -Python/import.c - PyImport_Inittab - Python/preconfig.c - Py_FileSystemDefaultEncoding - Python/preconfig.c - Py_HasFileSystemDefaultEncoding - Python/preconfig.c - Py_FileSystemDefaultEncodeErrors - diff --git a/Tools/c-analyzer/cpython/ignored.tsv b/Tools/c-analyzer/cpython/ignored.tsv index 6be61c94d87a3d..897b0db08fdd54 100644 --- a/Tools/c-analyzer/cpython/ignored.tsv +++ b/Tools/c-analyzer/cpython/ignored.tsv @@ -4,6 +4,7 @@ filename funcname name reason ################################## # mutable but known to be safe +Python/import.c - PyImport_Inittab - Python/pylifecycle.c - _PyRuntime - # All uses of _PyArg_Parser are handled in c-analyzr/cpython/_analyzer.py. From a4c2ea128619d3917a3ac9b210c7f739de4185e3 Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Thu, 10 Nov 2022 17:38:48 -0700 Subject: [PATCH 3/5] Deal with inittab_copy. --- Python/import.c | 2 ++ Tools/c-analyzer/cpython/globals-to-fix.tsv | 1 - Tools/c-analyzer/cpython/ignored.tsv | 1 + 3 files changed, 3 insertions(+), 1 deletion(-) diff --git a/Python/import.c b/Python/import.c index a2e477ca87c9c3..daee16ec4ddddf 100644 --- a/Python/import.c +++ b/Python/import.c @@ -33,6 +33,8 @@ extern struct _inittab _PyImport_Inittab[]; // This is not used after Py_Initialize() is called. // (See _PyRuntimeState.imports.inittab.) struct _inittab *PyImport_Inittab = _PyImport_Inittab; +// When we dynamically allocate a larger table for PyImport_ExtendInittab(), +// we track the pointer here so we can deallocate it during finalization. static struct _inittab *inittab_copy = NULL; /*[clinic input] diff --git a/Tools/c-analyzer/cpython/globals-to-fix.tsv b/Tools/c-analyzer/cpython/globals-to-fix.tsv index d2e58ccace2977..ab5c299c19a6d6 100644 --- a/Tools/c-analyzer/cpython/globals-to-fix.tsv +++ b/Tools/c-analyzer/cpython/globals-to-fix.tsv @@ -392,7 +392,6 @@ Python/frozen.c - _PyImport_FrozenAliases - Python/frozen.c - _PyImport_FrozenBootstrap - Python/frozen.c - _PyImport_FrozenStdlib - Python/frozen.c - _PyImport_FrozenTest - -Python/import.c - inittab_copy - Python/preconfig.c - Py_FileSystemDefaultEncoding - Python/preconfig.c - Py_HasFileSystemDefaultEncoding - Python/preconfig.c - Py_FileSystemDefaultEncodeErrors - diff --git a/Tools/c-analyzer/cpython/ignored.tsv b/Tools/c-analyzer/cpython/ignored.tsv index 897b0db08fdd54..e657fa77be0145 100644 --- a/Tools/c-analyzer/cpython/ignored.tsv +++ b/Tools/c-analyzer/cpython/ignored.tsv @@ -4,6 +4,7 @@ filename funcname name reason ################################## # mutable but known to be safe +Python/import.c - inittab_copy - Python/import.c - PyImport_Inittab - Python/pylifecycle.c - _PyRuntime - From fa85845a278ceb64613812189471d85f35b77532 Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Fri, 11 Nov 2022 14:48:27 -0700 Subject: [PATCH 4/5] Add a NEWS entry. --- .../2022-11-11-14-48-17.gh-issue-81057.ik4iOv.rst | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 Misc/NEWS.d/next/Core and Builtins/2022-11-11-14-48-17.gh-issue-81057.ik4iOv.rst diff --git a/Misc/NEWS.d/next/Core and Builtins/2022-11-11-14-48-17.gh-issue-81057.ik4iOv.rst b/Misc/NEWS.d/next/Core and Builtins/2022-11-11-14-48-17.gh-issue-81057.ik4iOv.rst new file mode 100644 index 00000000000000..1ca2ee9e028db5 --- /dev/null +++ b/Misc/NEWS.d/next/Core and Builtins/2022-11-11-14-48-17.gh-issue-81057.ik4iOv.rst @@ -0,0 +1,5 @@ +The docs clearly say that ``PyImport_Inittab``, +:c:api:`PyImport_AppendInittab`, and :c:api:`PyImport_ExtendInittab` should +not be used after :c:api:`Py_Initialize` has been called. We now enforce +this for the two functions. Additionally, the runtime now uses an internal +copy of ``PyImport_Inittab``, to guard against modification. From e373f89b28d6661321fd9f6112bb5cdb18cf1184 Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Fri, 11 Nov 2022 15:53:23 -0700 Subject: [PATCH 5/5] Fix the NEWS entry. --- .../2022-11-11-14-48-17.gh-issue-81057.ik4iOv.rst | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/Misc/NEWS.d/next/Core and Builtins/2022-11-11-14-48-17.gh-issue-81057.ik4iOv.rst b/Misc/NEWS.d/next/Core and Builtins/2022-11-11-14-48-17.gh-issue-81057.ik4iOv.rst index 1ca2ee9e028db5..7474cee0c87213 100644 --- a/Misc/NEWS.d/next/Core and Builtins/2022-11-11-14-48-17.gh-issue-81057.ik4iOv.rst +++ b/Misc/NEWS.d/next/Core and Builtins/2022-11-11-14-48-17.gh-issue-81057.ik4iOv.rst @@ -1,5 +1,6 @@ The docs clearly say that ``PyImport_Inittab``, -:c:api:`PyImport_AppendInittab`, and :c:api:`PyImport_ExtendInittab` should -not be used after :c:api:`Py_Initialize` has been called. We now enforce -this for the two functions. Additionally, the runtime now uses an internal -copy of ``PyImport_Inittab``, to guard against modification. +:c:func:`PyImport_AppendInittab`, and :c:func:`PyImport_ExtendInittab` +should not be used after :c:func:`Py_Initialize` has been called. +We now enforce this for the two functions. Additionally, the runtime +now uses an internal copy of ``PyImport_Inittab``, +to guard against modification.