traverseda
/
cpython
1
0
Fork 0
Code Issues Pull Requests Releases Wiki Activity

bpo-39824: module_traverse() don't call m_traverse if md_state=NULL (GH-18738) 

Extension modules: m_traverse, m_clear and m_free functions of
PyModuleDef are no longer called if the module state was requested
but is not allocated yet. This is the case immediately after the
module is created and before the module is executed (Py_mod_exec
function). More precisely, these functions are not called if m_size is
greater than 0 and the module state (as returned by
PyModule_GetState()) is NULL.

Extension modules without module state (m_size <= 0) are not affected.

Co-Authored-By: Petr Viktorin <encukou@gmail.com>
This commit is contained in:
Victor Stinner 2020-03-17 18:09:46 +01:00 committed by GitHub
parent 52268941f3
commit 5b1ef200d3
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
10 changed files with 91 additions and 144 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

44
Doc/c-api/module.rst
View File

@ -196,23 +196,47 @@ or request "multi-phase initialization" by returning the definition struct itsel
.. c:member:: traverseproc m_traverse
A traversal function to call during GC traversal of the module object, or
``NULL`` if not needed. This function may be called before module state
is allocated (:c:func:`PyModule_GetState()` may return `NULL`),
and before the :c:member:`Py_mod_exec` function is executed.
``NULL`` if not needed.
This function is not called if the module state was requested but is not
allocated yet. This is the case immediately after the module is created
and before the module is executed (:c:data:`Py_mod_exec` function). More
precisely, this function is not called if :c:member:`m_size` is greater
than 0 and the module state (as returned by :c:func:`PyModule_GetState`)
is ``NULL``.
.. versionchanged:: 3.9
No longer called before the module state is allocated.
.. c:member:: inquiry m_clear
A clear function to call during GC clearing of the module object, or
``NULL`` if not needed. This function may be called before module state
is allocated (:c:func:`PyModule_GetState()` may return `NULL`),
and before the :c:member:`Py_mod_exec` function is executed.
``NULL`` if not needed.
This function is not called if the module state was requested but is not
allocated yet. This is the case immediately after the module is created
and before the module is executed (:c:data:`Py_mod_exec` function). More
precisely, this function is not called if :c:member:`m_size` is greater
than 0 and the module state (as returned by :c:func:`PyModule_GetState`)
is ``NULL``.
.. versionchanged:: 3.9
No longer called before the module state is allocated.
.. c:member:: freefunc m_free
A function to call during deallocation of the module object, or ``NULL`` if
not needed. This function may be called before module state
is allocated (:c:func:`PyModule_GetState()` may return `NULL`),
and before the :c:member:`Py_mod_exec` function is executed.
A function to call during deallocation of the module object, or ``NULL``
if not needed.
This function is not called if the module state was requested but is not
allocated yet. This is the case immediately after the module is created
and before the module is executed (:c:data:`Py_mod_exec` function). More
precisely, this function is not called if :c:member:`m_size` is greater
than 0 and the module state (as returned by :c:func:`PyModule_GetState`)
is ``NULL``.
.. versionchanged:: 3.9
No longer called before the module state is allocated.
Single-phase initialization
...........................

11
Doc/whatsnew/3.9.rst
View File

@ -503,6 +503,17 @@ Build and C API Changes
*tstate* parameter (``PyThreadState*``).
(Contributed by Victor Stinner in :issue:`38500`.)
* Extension modules: :c:member:`~PyModuleDef.m_traverse`,
:c:member:`~PyModuleDef.m_clear` and :c:member:`~PyModuleDef.m_free`
functions of :c:type:`PyModuleDef` are no longer called if the module state
was requested but is not allocated yet. This is the case immediately after
the module is created and before the module is executed
(:c:data:`Py_mod_exec` function). More precisely, these functions are not called
if :c:member:`~PyModuleDef.m_size` is greater than 0 and the module state (as
returned by :c:func:`PyModule_GetState`) is ``NULL``.
Extension modules without module state (``m_size <= 0``) are not affected.
Deprecated
==========

23
Lib/test/test_importlib/extension/test_loader.py
View File

@ -267,29 +267,6 @@ class MultiPhaseExtensionModuleTests(abc.LoaderTests):
self.assertEqual(module.__name__, name)
self.assertEqual(module.__doc__, "Module named in %s" % lang)
@unittest.skipIf(not hasattr(sys, 'gettotalrefcount'),
'--with-pydebug has to be enabled for this test')
def test_bad_traverse(self):
''' Issue #32374: Test that traverse fails when accessing per-module
state before Py_mod_exec was executed.
(Multiphase initialization modules only)
'''
script = """if True:
try:
from test import support
import importlib.util as util
spec = util.find_spec('_testmultiphase')
spec.name = '_testmultiphase_with_bad_traverse'
with support.SuppressCrashReport():
m = spec.loader.create_module(spec)
except:
# Prevent Python-level exceptions from
# ending the process with non-zero status
# (We are testing for a crash in C-code)
pass"""
assert_python_failure("-c", script)
(Frozen_MultiPhaseExtensionModuleTests,
Source_MultiPhaseExtensionModuleTests

10
Misc/NEWS.d/next/C API/2020-03-02-11-29-45.bpo-39824.71_ZMn.rst Normal file
View File

@ -0,0 +1,10 @@
Extension modules: :c:member:`~PyModuleDef.m_traverse`,
:c:member:`~PyModuleDef.m_clear` and :c:member:`~PyModuleDef.m_free` functions
of :c:type:`PyModuleDef` are no longer called if the module state was requested
but is not allocated yet. This is the case immediately after the module is
created and before the module is executed (:c:data:`Py_mod_exec` function). More
precisely, these functions are not called if :c:member:`~PyModuleDef.m_size` is
greater than 0 and the module state (as returned by
:c:func:`PyModule_GetState`) is ``NULL``.
Extension modules without module state (``m_size <= 0``) are not affected.

8
Modules/_localemodule.c
View File

@ -778,9 +778,7 @@ static int
locale_traverse(PyObject *m, visitproc visit, void *arg)
{
_locale_state *state = (_locale_state*)PyModule_GetState(m);
if (state) {
Py_VISIT(state->Error);
}
Py_VISIT(state->Error);
return 0;
}
@ -788,9 +786,7 @@ static int
locale_clear(PyObject *m)
{
_locale_state *state = (_locale_state*)PyModule_GetState(m);
if (state) {
Py_CLEAR(state->Error);
}
Py_CLEAR(state->Error);
return 0;
}

54
Modules/_testmultiphase.c
View File

@ -225,20 +225,18 @@ static int execfunc(PyObject *m)
/* Helper for module definitions; there'll be a lot of them */
#define TEST_MODULE_DEF_EX(name, slots, methods, statesize, traversefunc) { \
#define TEST_MODULE_DEF(name, slots, methods) { \
PyModuleDef_HEAD_INIT, /* m_base */ \
name, /* m_name */ \
PyDoc_STR("Test module " name), /* m_doc */ \
statesize, /* m_size */ \
0, /* m_size */ \
methods, /* m_methods */ \
slots, /* m_slots */ \
traversefunc, /* m_traverse */ \
NULL, /* m_traverse */ \
NULL, /* m_clear */ \
NULL, /* m_free */ \
}
#define TEST_MODULE_DEF(name, slots, methods) TEST_MODULE_DEF_EX(name, slots, methods, 0, NULL)
static PyModuleDef_Slot main_slots[] = {
{Py_mod_exec, execfunc},
{0, NULL},
@ -622,52 +620,6 @@ PyInit__testmultiphase_exec_unreported_exception(PyObject *spec)
return PyModuleDef_Init(&def_exec_unreported_exception);
}
static int
bad_traverse(PyObject *self, visitproc visit, void *arg) {
testmultiphase_state *m_state;
m_state = PyModule_GetState(self);
/* The following assertion mimics any traversal function that doesn't correctly handle
* the case during module creation where the module state hasn't been created yet.
*
* The check that it is used to test only runs in debug mode, so it is OK that the
* assert() will get compiled out in fully optimised release builds.
*/
assert(m_state != NULL);
Py_VISIT(m_state->integer);
return 0;
}
static int
execfunc_with_bad_traverse(PyObject *mod) {
testmultiphase_state *m_state;
m_state = PyModule_GetState(mod);
if (m_state == NULL) {
return -1;
}
m_state->integer = PyLong_FromLong(0x7fffffff);
Py_INCREF(m_state->integer);
return 0;
}
static PyModuleDef_Slot slots_with_bad_traverse[] = {
{Py_mod_exec, execfunc_with_bad_traverse},
{0, NULL}
};
static PyModuleDef def_with_bad_traverse = TEST_MODULE_DEF_EX(
"_testmultiphase_with_bad_traverse", slots_with_bad_traverse, NULL,
sizeof(testmultiphase_state), bad_traverse);
PyMODINIT_FUNC
PyInit__testmultiphase_with_bad_traverse(PyObject *spec) {
return PyModuleDef_Init(&def_with_bad_traverse);
}
/*** Helper for imp test ***/
static PyModuleDef imp_dummy_def = TEST_MODULE_DEF("imp_dummy", main_slots, testexport_methods);

27
Modules/atexitmodule.c
View File

@ -229,15 +229,14 @@ atexit_m_traverse(PyObject *self, visitproc visit, void *arg)
atexitmodule_state *modstate;
modstate = (atexitmodule_state *)PyModule_GetState(self);
if (modstate != NULL) {
for (i = 0; i < modstate->ncallbacks; i++) {
atexit_callback *cb = modstate->atexit_callbacks[i];
if (cb == NULL)
continue;
Py_VISIT(cb->func);
Py_VISIT(cb->args);
Py_VISIT(cb->kwargs);
}
for (i = 0; i < modstate->ncallbacks; i++) {
atexit_callback *cb = modstate->atexit_callbacks[i];
if (cb == NULL)
continue;
Py_VISIT(cb->func);
Py_VISIT(cb->args);
Py_VISIT(cb->kwargs);
}
return 0;
}
@ -247,9 +246,7 @@ atexit_m_clear(PyObject *self)
{
atexitmodule_state *modstate;
modstate = (atexitmodule_state *)PyModule_GetState(self);
if (modstate != NULL) {
atexit_cleanup(modstate);
}
atexit_cleanup(modstate);
return 0;
}
@ -258,10 +255,8 @@ atexit_free(PyObject *m)
{
atexitmodule_state *modstate;
modstate = (atexitmodule_state *)PyModule_GetState(m);
if (modstate != NULL) {
atexit_cleanup(modstate);
PyMem_Free(modstate->atexit_callbacks);
}
atexit_cleanup(modstate);
PyMem_Free(modstate->atexit_callbacks);
}
PyDoc_STRVAR(atexit_unregister__doc__,

8
Modules/audioop.c
View File

@ -1926,9 +1926,7 @@ static int
audioop_traverse(PyObject *module, visitproc visit, void *arg)
{
audioop_state *state = (audioop_state *)PyModule_GetState(module);
if (state) {
Py_VISIT(state->AudioopError);
}
Py_VISIT(state->AudioopError);
return 0;
}
@ -1936,9 +1934,7 @@ static int
audioop_clear(PyObject *module)
{
audioop_state *state = (audioop_state *)PyModule_GetState(module);
if (state) {
Py_CLEAR(state->AudioopError);
}
Py_CLEAR(state->AudioopError);
return 0;
}

14
Modules/binascii.c
View File

@ -1653,10 +1653,8 @@ static int
binascii_traverse(PyObject *module, visitproc visit, void *arg)
{
binascii_state *state = get_binascii_state(module);
if (state) {
Py_VISIT(state->Error);
Py_VISIT(state->Incomplete);
}
Py_VISIT(state->Error);
Py_VISIT(state->Incomplete);
return 0;
}
@ -1664,10 +1662,8 @@ static int
binascii_clear(PyObject *module)
{
binascii_state *state = get_binascii_state(module);
if (state) {
Py_CLEAR(state->Error);
Py_CLEAR(state->Incomplete);
}
Py_CLEAR(state->Error);
Py_CLEAR(state->Incomplete);
return 0;
}
@ -1686,7 +1682,7 @@ static struct PyModuleDef binasciimodule = {
binascii_slots,
binascii_traverse,
binascii_clear,
binascii_free
binascii_free
};
PyMODINIT_FUNC

36
Objects/moduleobject.c
View File

@ -26,16 +26,6 @@ static PyMemberDef module_members[] = {
};
/* Helper for sanity check for traverse not handling m_state == NULL
* Issue #32374 */
#ifdef Py_DEBUG
static int
bad_traverse_test(PyObject *self, void *arg) {
assert(self != NULL);
return 0;
}
#endif
PyTypeObject PyModuleDef_Type = {
PyVarObject_HEAD_INIT(&PyType_Type, 0)
"moduledef", /* tp_name */
@ -360,16 +350,6 @@ PyModule_FromDefAndSpec2(struct PyModuleDef* def, PyObject *spec, int module_api
}
}
/* Sanity check for traverse not handling m_state == NULL
* This doesn't catch all possible cases, but in many cases it should
* make many cases of invalid code crash or raise Valgrind issues
* sooner than they would otherwise.
* Issue #32374 */
#ifdef Py_DEBUG
if (def->m_traverse != NULL) {
def->m_traverse(m, bad_traverse_test, NULL);
}
#endif
Py_DECREF(nameobj);
return m;
@ -687,8 +667,12 @@ module_dealloc(PyModuleObject *m)
}
if (m->md_weaklist != NULL)
PyObject_ClearWeakRefs((PyObject *) m);
if (m->md_def && m->md_def->m_free)
/* bpo-39824: Don't call m_free() if m_size > 0 and md_state=NULL */
if (m->md_def && m->md_def->m_free
&& (m->md_def->m_size <= 0 || m->md_state != NULL))
{
m->md_def->m_free(m);
}
Py_XDECREF(m->md_dict);
Py_XDECREF(m->md_name);
if (m->md_state != NULL)
@ -770,7 +754,10 @@ module_getattro(PyModuleObject *m, PyObject *name)
static int
module_traverse(PyModuleObject *m, visitproc visit, void *arg)
{
if (m->md_def && m->md_def->m_traverse) {
/* bpo-39824: Don't call m_traverse() if m_size > 0 and md_state=NULL */
if (m->md_def && m->md_def->m_traverse
&& (m->md_def->m_size <= 0 || m->md_state != NULL))
{
int res = m->md_def->m_traverse((PyObject*)m, visit, arg);
if (res)
return res;
@ -782,7 +769,10 @@ module_traverse(PyModuleObject *m, visitproc visit, void *arg)
static int
module_clear(PyModuleObject *m)
{
if (m->md_def && m->md_def->m_clear) {
/* bpo-39824: Don't call m_clear() if m_size > 0 and md_state=NULL */
if (m->md_def && m->md_def->m_clear
&& (m->md_def->m_size <= 0 || m->md_state != NULL))
{
int res = m->md_def->m_clear((PyObject*)m);
if (PyErr_Occurred()) {
PySys_FormatStderr("Exception ignored in m_clear of module%s%V\n",