From 051293227ea2416fe2c01a5429eade406609dfaa Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Wed, 15 Mar 2023 10:28:31 -0600 Subject: [PATCH 1/6] match_mod_name() -> get_core_module_dict(). --- Python/import.c | 26 +++++++++++++++----------- 1 file changed, 15 insertions(+), 11 deletions(-) diff --git a/Python/import.c b/Python/import.c index 612fee243bd9e7..ca2234d708bb9b 100644 --- a/Python/import.c +++ b/Python/import.c @@ -978,11 +978,20 @@ _PyImport_CheckSubinterpIncompatibleExtensionAllowed(const char *name) return 0; } -static inline int -match_mod_name(PyObject *actual, const char *expected) +static PyObject * +get_core_module_dict(PyInterpreterState *interp, + PyObject *name, PyObject *filename) { - if (PyUnicode_CompareWithASCIIString(actual, expected) == 0) { - return 1; + if (filename != name && filename != NULL) { + /* It isn't a builtin module, which means it isn't core. */ + return 0; + } + if (PyUnicode_CompareWithASCIIString(name, "sys") == 0) { + return interp->sysdict_copy; + } + assert(!PyErr_Occurred()); + if (PyUnicode_CompareWithASCIIString(name, "builtins") == 0) { + return interp->builtins_copy; } assert(!PyErr_Occurred()); return 0; @@ -1072,13 +1081,8 @@ import_find_extension(PyThreadState *tstate, PyObject *name, return NULL; } else if (m_copy == Py_None) { - if (match_mod_name(name, "sys")) { - m_copy = tstate->interp->sysdict_copy; - } - else if (match_mod_name(name, "builtins")) { - m_copy = tstate->interp->builtins_copy; - } - else { + m_copy = get_core_module_dict(tstate->interp, name, filename); + if (m_copy == NULL) { _PyErr_SetString(tstate, PyExc_ImportError, "missing m_copy"); return NULL; } From 16348ebbee3c27aa06c4943498cf5fa7c8cb0732 Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Wed, 15 Mar 2023 10:41:22 -0600 Subject: [PATCH 2/6] Stop using None as the m_copy sentinel. --- Python/bltinmodule.c | 3 --- Python/import.c | 39 +++++++++++++++++++++------------------ Python/sysmodule.c | 3 --- 3 files changed, 21 insertions(+), 24 deletions(-) diff --git a/Python/bltinmodule.c b/Python/bltinmodule.c index a8a34620b9bcdf..12ca0ba6c4873c 100644 --- a/Python/bltinmodule.c +++ b/Python/bltinmodule.c @@ -3098,9 +3098,6 @@ _PyBuiltin_Init(PyInterpreterState *interp) } Py_DECREF(debug); - /* m_copy of Py_None means it is copied some other way. */ - builtinsmodule.m_base.m_copy = Py_NewRef(Py_None); - return mod; #undef ADD_TO_ALL #undef SETBUILTIN diff --git a/Python/import.c b/Python/import.c index ca2234d708bb9b..30dc5903e84cf2 100644 --- a/Python/import.c +++ b/Python/import.c @@ -982,21 +982,27 @@ static PyObject * get_core_module_dict(PyInterpreterState *interp, PyObject *name, PyObject *filename) { - if (filename != name && filename != NULL) { - /* It isn't a builtin module, which means it isn't core. */ - return 0; - } - if (PyUnicode_CompareWithASCIIString(name, "sys") == 0) { - return interp->sysdict_copy; - } - assert(!PyErr_Occurred()); - if (PyUnicode_CompareWithASCIIString(name, "builtins") == 0) { - return interp->builtins_copy; + /* Only builtin modules are core. */ + if (filename == name) { + assert(!PyErr_Occurred()); + if (PyUnicode_CompareWithASCIIString(name, "sys") == 0) { + return interp->sysdict_copy; + } + assert(!PyErr_Occurred()); + if (PyUnicode_CompareWithASCIIString(name, "builtins") == 0) { + return interp->builtins_copy; + } + assert(!PyErr_Occurred()); } - assert(!PyErr_Occurred()); return 0; } +static inline int +is_core_module(PyInterpreterState *interp, PyObject *name, PyObject *filename) +{ + return get_core_module_dict(interp, name, filename) == NULL; +} + static int fix_up_extension(PyObject *mod, PyObject *name, PyObject *filename) { @@ -1019,9 +1025,8 @@ fix_up_extension(PyObject *mod, PyObject *name, PyObject *filename) // bpo-44050: Extensions and def->m_base.m_copy can be updated // when the extension module doesn't support sub-interpreters. // XXX Why special-case the main interpreter? - if (_Py_IsMainInterpreter(tstate->interp) || def->m_size == -1) { - /* m_copy of Py_None means it is copied some other way. */ - if (def->m_size == -1 && def->m_base.m_copy != Py_None) { + if (def->m_size == -1) { + if (!is_core_module(tstate->interp, name, filename)) { if (def->m_base.m_copy) { /* Somebody already imported the module, likely under a different name. @@ -1037,7 +1042,9 @@ fix_up_extension(PyObject *mod, PyObject *name, PyObject *filename) return -1; } } + } + if (_Py_IsMainInterpreter(tstate->interp) || def->m_size == -1) { if (_extensions_cache_set(filename, name, def) < 0) { return -1; } @@ -1078,12 +1085,8 @@ import_find_extension(PyThreadState *tstate, PyObject *name, PyObject *m_copy = def->m_base.m_copy; /* Module does not support repeated initialization */ if (m_copy == NULL) { - return NULL; - } - else if (m_copy == Py_None) { m_copy = get_core_module_dict(tstate->interp, name, filename); if (m_copy == NULL) { - _PyErr_SetString(tstate, PyExc_ImportError, "missing m_copy"); return NULL; } } diff --git a/Python/sysmodule.c b/Python/sysmodule.c index fc0550266bf1af..d282104dcad414 100644 --- a/Python/sysmodule.c +++ b/Python/sysmodule.c @@ -3425,9 +3425,6 @@ _PySys_Create(PyThreadState *tstate, PyObject **sysmod_p) return _PyStatus_ERR("failed to create a module object"); } - /* m_copy of Py_None means it is copied some other way. */ - sysmodule.m_base.m_copy = Py_NewRef(Py_None); - PyObject *sysdict = PyModule_GetDict(sysmod); if (sysdict == NULL) { goto error; From d91bd096532a6e6a4d8c4c7ad71806429ef5f2d0 Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Wed, 15 Mar 2023 17:53:59 -0600 Subject: [PATCH 3/6] Fix a comment. --- Python/import.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Python/import.c b/Python/import.c index 30dc5903e84cf2..241f22178d0d53 100644 --- a/Python/import.c +++ b/Python/import.c @@ -1024,7 +1024,6 @@ fix_up_extension(PyObject *mod, PyObject *name, PyObject *filename) // bpo-44050: Extensions and def->m_base.m_copy can be updated // when the extension module doesn't support sub-interpreters. - // XXX Why special-case the main interpreter? if (def->m_size == -1) { if (!is_core_module(tstate->interp, name, filename)) { if (def->m_base.m_copy) { @@ -1044,6 +1043,7 @@ fix_up_extension(PyObject *mod, PyObject *name, PyObject *filename) } } + // XXX Why special-case the main interpreter? if (_Py_IsMainInterpreter(tstate->interp) || def->m_size == -1) { if (_extensions_cache_set(filename, name, def) < 0) { return -1; From d6046c37cc7db3a0381cfe57e16b068123107b85 Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Wed, 15 Mar 2023 17:55:33 -0600 Subject: [PATCH 4/6] Drop an outdated comment. --- Python/import.c | 1 - 1 file changed, 1 deletion(-) diff --git a/Python/import.c b/Python/import.c index 241f22178d0d53..0735d8c0d3a912 100644 --- a/Python/import.c +++ b/Python/import.c @@ -1090,7 +1090,6 @@ import_find_extension(PyThreadState *tstate, PyObject *name, return NULL; } } - /* m_copy of Py_None means it is copied some other way. */ mod = import_add_module(tstate, name); if (mod == NULL) { return NULL; From a2e5ab842a18b37439b791d17abb962b220a4262 Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Wed, 15 Mar 2023 17:57:37 -0600 Subject: [PATCH 5/6] Fix a typo. --- Python/import.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Python/import.c b/Python/import.c index 0735d8c0d3a912..24fcdb8e5b2718 100644 --- a/Python/import.c +++ b/Python/import.c @@ -994,7 +994,7 @@ get_core_module_dict(PyInterpreterState *interp, } assert(!PyErr_Occurred()); } - return 0; + return NULL; } static inline int From 8da964f06b54988777ee24eccd36af7f00cae654 Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Wed, 15 Mar 2023 18:06:25 -0600 Subject: [PATCH 6/6] Flip a conditional check. --- Python/import.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Python/import.c b/Python/import.c index 24fcdb8e5b2718..9f80c6d8dd49a8 100644 --- a/Python/import.c +++ b/Python/import.c @@ -1000,7 +1000,7 @@ get_core_module_dict(PyInterpreterState *interp, static inline int is_core_module(PyInterpreterState *interp, PyObject *name, PyObject *filename) { - return get_core_module_dict(interp, name, filename) == NULL; + return get_core_module_dict(interp, name, filename) != NULL; } static int