Skip to content
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
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
9 changes: 5 additions & 4 deletions Doc/c-api/module.rst
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
8 changes: 6 additions & 2 deletions Lib/test/test_capi/test_module.py
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand All @@ -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'):
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
In :c:member:`PyModuleDef.m_slots`, allow slots that repeat information
present in :c:type:`PyModuleDef`.
43 changes: 43 additions & 0 deletions Modules/_testcapi/module.c
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
#include "parts.h"
#include "util.h"
#include <stdbool.h>

// Test PyModule_* API

Expand Down Expand Up @@ -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)
{
Expand Down Expand Up @@ -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},
Expand Down
120 changes: 78 additions & 42 deletions Objects/moduleobject.c
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand All @@ -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;
}
Expand All @@ -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);
Expand All @@ -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_NONNULL_SLOT
}

#ifdef Py_GIL_DISABLED
Expand Down Expand Up @@ -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;
}
Expand Down
Loading