diff --git a/Doc/c-api/module.rst b/Doc/c-api/module.rst index e8a6e09f5554ec..9a057e531e1be5 100644 --- a/Doc/c-api/module.rst +++ b/Doc/c-api/module.rst @@ -725,10 +725,11 @@ remove it. An array of additional slots, terminated by a ``{0, NULL}`` entry. - This array may not contain slots corresponding to :c:type:`PyModuleDef` - members. - For example, you cannot use :c:macro:`Py_mod_name` in :c:member:`!m_slots`; - the module name must be given as :c:member:`PyModuleDef.m_name`. + If the array contains slots corresponding to :c:type:`PyModuleDef` + members, the values must match. + For example, if you use :c:macro:`Py_mod_name` in :c:member:`!m_slots`; + :c:member:`PyModuleDef.m_name` must be set to the same pointer + (not just an equal string). .. versionchanged:: 3.5 diff --git a/Lib/test/test_capi/test_module.py b/Lib/test/test_capi/test_module.py index 823e2ab6b2ef0d..053e6709cda42e 100644 --- a/Lib/test/test_capi/test_module.py +++ b/Lib/test/test_capi/test_module.py @@ -122,8 +122,7 @@ def test_create(self): _testcapi.pymodule_get_token(mod) def test_def_slot(self): - """Slots that replace PyModuleDef fields can't be used with PyModuleDef - """ + """Slots cannot contradict PyModuleDef fields""" for name in DEF_SLOTS: with self.subTest(name): spec = FakeSpec() @@ -133,6 +132,11 @@ def test_def_slot(self): self.assertIn(name, str(cm.exception)) self.assertIn("PyModuleDef", str(cm.exception)) + def test_def_slot_parrot(self): + """Slots with same value as PyModuleDef fields are allowed""" + spec = FakeSpec() + _testcapi.module_from_def_slot_parrot(spec) + def test_repeated_def_slot(self): """Slots that replace PyModuleDef fields can't be repeated""" for name in (*DEF_SLOTS, 'Py_mod_exec'): diff --git a/Misc/NEWS.d/next/Core_and_Builtins/2026-01-30-10-38-07.gh-issue-140550.Us9vPD.rst b/Misc/NEWS.d/next/Core_and_Builtins/2026-01-30-10-38-07.gh-issue-140550.Us9vPD.rst new file mode 100644 index 00000000000000..7815176ec85d2d --- /dev/null +++ b/Misc/NEWS.d/next/Core_and_Builtins/2026-01-30-10-38-07.gh-issue-140550.Us9vPD.rst @@ -0,0 +1,2 @@ +In :c:member:`PyModuleDef.m_slots`, allow slots that repeat information +present in :c:type:`PyModuleDef`. diff --git a/Modules/_testcapi/module.c b/Modules/_testcapi/module.c index ef657842e77494..0680eed4f7c239 100644 --- a/Modules/_testcapi/module.c +++ b/Modules/_testcapi/module.c @@ -1,5 +1,6 @@ #include "parts.h" #include "util.h" +#include // Test PyModule_* API @@ -270,6 +271,47 @@ module_from_def_slot(PyObject *self, PyObject *spec) return result; } +static PyModuleDef parrot_def = { + PyModuleDef_HEAD_INIT, + .m_name = "test_capi/parrot", + .m_doc = "created from redundant information", + .m_size = 123, + .m_methods = a_methoddef_array, + .m_traverse = noop_traverse, + .m_clear = noop_clear, + .m_free = (void*)noop_free, + .m_slots = NULL /* set below */, +}; +static PyModuleDef_Slot parrot_slots[] = { + {Py_mod_name, "test_capi/parrot"}, + {Py_mod_doc, "created from redundant information"}, + {Py_mod_state_size, (void*)123}, + {Py_mod_methods, a_methoddef_array}, + {Py_mod_state_traverse, noop_traverse}, + {Py_mod_state_clear, noop_clear}, + {Py_mod_state_free, (void*)noop_free}, + {Py_mod_multiple_interpreters, Py_MOD_PER_INTERPRETER_GIL_SUPPORTED}, + {Py_mod_token, &parrot_def}, + {Py_mod_gil, Py_MOD_GIL_NOT_USED}, + {0}, +}; + +static PyObject * +module_from_def_slot_parrot(PyObject *self, PyObject *spec) +{ + parrot_def.m_slots = parrot_slots; + PyObject *module = PyModule_FromDefAndSpec(&parrot_def, spec); + if (!module || (PyModule_Exec(module) < 0)) { + return NULL; + } + Py_ssize_t size; + assert(PyModule_GetStateSize(module, &size) == 0); + assert(size == 123); + PyModuleDef *def = PyModule_GetDef(module); + assert(def == &parrot_def); + return module; +} + static int another_exec(PyObject *module) { @@ -368,6 +410,7 @@ static PyMethodDef test_methods[] = { {"module_from_slots_null_slot", module_from_slots_null_slot, METH_O}, {"module_from_def_multiple_exec", module_from_def_multiple_exec, METH_O}, {"module_from_def_slot", module_from_def_slot, METH_O}, + {"module_from_def_slot_parrot", module_from_def_slot_parrot, METH_O}, {"pymodule_get_token", pymodule_get_token, METH_O}, {"pymodule_get_def", pymodule_get_def, METH_O}, {"pymodule_get_state_size", pymodule_get_state_size, METH_O}, diff --git a/Objects/moduleobject.c b/Objects/moduleobject.c index 5a0b16ba57242d..c3b30463e48a6a 100644 --- a/Objects/moduleobject.c +++ b/Objects/moduleobject.c @@ -410,35 +410,78 @@ module_from_def_and_spec( goto error; } + bool seen_m_name_slot = false; + bool seen_m_doc_slot = false; + bool seen_m_size_slot = false; + bool seen_m_methods_slot = false; + bool seen_m_traverse_slot = false; + bool seen_m_clear_slot = false; + bool seen_m_free_slot = false; for (cur_slot = def_like->m_slots; cur_slot && cur_slot->slot; cur_slot++) { - // Macro to copy a non-NULL, non-repeatable slot that's unusable with - // PyModuleDef. The destination must be initially NULL. -#define COPY_COMMON_SLOT(SLOT, TYPE, DEST) \ + + // Macro to copy a non-NULL, non-repeatable slot. +#define COPY_NONNULL_SLOT(SLOTNAME, TYPE, DEST) \ do { \ if (!(TYPE)(cur_slot->value)) { \ PyErr_Format( \ PyExc_SystemError, \ - "module %s: " #SLOT " must not be NULL", \ - name); \ + "module %s: %s must not be NULL", \ + name, SLOTNAME); \ goto error; \ } \ - if (original_def) { \ + DEST = (TYPE)(cur_slot->value); \ + } while (0); \ + ///////////////////////////////////////////////////////////////// + + // Macro to copy a non-NULL, non-repeatable slot to def_like. +#define COPY_DEF_SLOT(SLOTNAME, TYPE, MEMBER) \ + do { \ + if (seen_ ## MEMBER ## _slot) { \ PyErr_Format( \ PyExc_SystemError, \ - "module %s: " #SLOT " used with PyModuleDef", \ - name); \ + "module %s has more than one %s slot", \ + name, SLOTNAME); \ goto error; \ } \ + seen_ ## MEMBER ## _slot = true; \ + if (original_def) { \ + TYPE orig_value = (TYPE)original_def->MEMBER; \ + TYPE new_value = (TYPE)cur_slot->value; \ + if (orig_value != new_value) { \ + PyErr_Format( \ + PyExc_SystemError, \ + "module %s: %s conflicts with " \ + "PyModuleDef." #MEMBER, \ + name, SLOTNAME); \ + goto error; \ + } \ + } \ + COPY_NONNULL_SLOT(SLOTNAME, TYPE, (def_like->MEMBER)) \ + } while (0); \ + ///////////////////////////////////////////////////////////////// + + // Macro to copy a non-NULL, non-repeatable slot without a + // corresponding PyModuleDef member. + // DEST must be initially NULL (so we don't need a seen_* flag). +#define COPY_NONDEF_SLOT(SLOTNAME, TYPE, DEST) \ + do { \ if (DEST) { \ PyErr_Format( \ PyExc_SystemError, \ - "module %s has more than one " #SLOT " slot", \ - name); \ + "module %s has more than one %s slot", \ + name, SLOTNAME); \ goto error; \ } \ - DEST = (TYPE)(cur_slot->value); \ + COPY_NONNULL_SLOT(SLOTNAME, TYPE, DEST) \ } while (0); \ ///////////////////////////////////////////////////////////////// + + // Define the whole common case +#define DEF_SLOT_CASE(SLOT, TYPE, MEMBER) \ + case SLOT: \ + COPY_DEF_SLOT(#SLOT, TYPE, MEMBER); \ + break; \ + ///////////////////////////////////////////////////////////////// switch (cur_slot->slot) { case Py_mod_create: if (create) { @@ -453,14 +496,15 @@ module_from_def_and_spec( case Py_mod_exec: has_execution_slots = 1; if (!original_def) { - COPY_COMMON_SLOT(Py_mod_exec, _Py_modexecfunc, m_exec); + COPY_NONDEF_SLOT("Py_mod_exec", _Py_modexecfunc, m_exec); } break; case Py_mod_multiple_interpreters: if (has_multiple_interpreters_slot) { PyErr_Format( PyExc_SystemError, - "module %s has more than one 'multiple interpreters' slots", + "module %s has more than one 'multiple interpreters' " + "slots", name); goto error; } @@ -483,34 +527,23 @@ module_from_def_and_spec( goto error; } break; - case Py_mod_name: - COPY_COMMON_SLOT(Py_mod_name, char*, def_like->m_name); - break; - case Py_mod_doc: - COPY_COMMON_SLOT(Py_mod_doc, char*, def_like->m_doc); - break; - case Py_mod_state_size: - COPY_COMMON_SLOT(Py_mod_state_size, Py_ssize_t, - def_like->m_size); - break; - case Py_mod_methods: - COPY_COMMON_SLOT(Py_mod_methods, PyMethodDef*, - def_like->m_methods); - break; - case Py_mod_state_traverse: - COPY_COMMON_SLOT(Py_mod_state_traverse, traverseproc, - def_like->m_traverse); - break; - case Py_mod_state_clear: - COPY_COMMON_SLOT(Py_mod_state_clear, inquiry, - def_like->m_clear); - break; - case Py_mod_state_free: - COPY_COMMON_SLOT(Py_mod_state_free, freefunc, - def_like->m_free); - break; + DEF_SLOT_CASE(Py_mod_name, char*, m_name) + DEF_SLOT_CASE(Py_mod_doc, char*, m_doc) + DEF_SLOT_CASE(Py_mod_state_size, Py_ssize_t, m_size) + DEF_SLOT_CASE(Py_mod_methods, PyMethodDef*, m_methods) + DEF_SLOT_CASE(Py_mod_state_traverse, traverseproc, m_traverse) + DEF_SLOT_CASE(Py_mod_state_clear, inquiry, m_clear) + DEF_SLOT_CASE(Py_mod_state_free, freefunc, m_free) case Py_mod_token: - COPY_COMMON_SLOT(Py_mod_token, void*, token); + if (original_def && original_def != cur_slot->value) { + PyErr_Format( + PyExc_SystemError, + "module %s: arbitrary Py_mod_token not " + "allowed with PyModuleDef", + name); + goto error; + } + COPY_NONDEF_SLOT("Py_mod_token", void*, token); break; default: assert(cur_slot->slot < 0 || cur_slot->slot > _Py_mod_LAST_SLOT); @@ -520,7 +553,10 @@ module_from_def_and_spec( name, cur_slot->slot); goto error; } -#undef COPY_COMMON_SLOT +#undef DEF_SLOT_CASE +#undef COPY_DEF_SLOT +#undef COPY_NONDEF_SLOT +#undef _COPY_COMMON_SLOT } #ifdef Py_GIL_DISABLED @@ -590,7 +626,7 @@ module_from_def_and_spec( mod->md_state = NULL; module_copy_members_from_deflike(mod, def_like); if (original_def) { - assert (!token); + assert (!token || token == original_def); mod->md_token = original_def; mod->md_token_is_def = 1; }