bpo-1635741: Convert _imp to multi-phase init (GH-23378)

Convert the _imp extension module to the multi-phase initialization
API (PEP 489).

* Add _PyImport_BootstrapImp() which fix a bootstrap issue: import
  the _imp module before importlib is initialized.
* Add create_builtin() sub-function, used by _imp_create_builtin().
* Initialize PyInterpreterState.import_func earlier, in
  pycore_init_builtins().
* Remove references to _PyImport_Cleanup(). This function has been
  renamed to finalize_modules() and moved to pylifecycle.c.

4 files changed, 122 insertions, 81 deletions

diff --git a/Include/internal/pycore_import.h b/Include/internal/pycore_import.h
index 35a67ab..fd9fa5a 100644
--- a/Include/internal/pycore_import.h
+++ b/Include/internal/pycore_import.h
@@ -13,7 +13,7 @@ PyAPI_FUNC(PyObject *) _PyImport_FindBuiltin(
 #ifdef HAVE_FORK
 extern PyStatus _PyImport_ReInitLock(void);
 #endif
-extern void _PyImport_Cleanup(PyThreadState *tstate);
+extern PyObject* _PyImport_BootstrapImp(PyThreadState *tstate);
 
 #ifdef __cplusplus
 }
diff --git a/Modules/posixmodule.c b/Modules/posixmodule.c
index 703309f..efa9653 100644
--- a/Modules/posixmodule.c
+++ b/Modules/posixmodule.c
@@ -14501,7 +14501,7 @@ os__remove_dll_directory_impl(PyObject *module, PyObject *cookie)
 
    os.waitstatus_to_exitcode() is implemented in C and not in Python, so
    subprocess can safely call it during late Python finalization without
-   risking that used os attributes were set to None by _PyImport_Cleanup(). */
+   risking that used os attributes were set to None by finalize_modules(). */
 #if defined(WIFEXITED) || defined(MS_WINDOWS)
 /*[clinic input]
 os.waitstatus_to_exitcode
diff --git a/Python/import.c b/Python/import.c
index 51630c3..1522abc 100644
--- a/Python/import.c
+++ b/Python/import.c
@@ -4,6 +4,7 @@
 
 #include "Python-ast.h"
 #undef Yield   /* undefine macro conflicting with <winbase.h> */
+#include "pycore_import.h"        // _PyImport_BootstrapImp()
 #include "pycore_initconfig.h"
 #include "pycore_pyerrors.h"
 #include "pycore_pyhash.h"
@@ -978,84 +979,80 @@ PyImport_GetImporter(PyObject *path)
     return importer;
 }
 
-/*[clinic input]
-_imp.create_builtin
-
-    spec: object
-    /
-
-Create an extension module.
-[clinic start generated code]*/
-
-static PyObject *
-_imp_create_builtin(PyObject *module, PyObject *spec)
-/*[clinic end generated code: output=ace7ff22271e6f39 input=37f966f890384e47]*/
+static PyObject*
+create_builtin(PyThreadState *tstate, PyObject *name, PyObject *spec)
 {
-    PyThreadState *tstate = _PyThreadState_GET();
-    struct _inittab *p;
-    PyObject *name;
-    const char *namestr;
-    PyObject *mod;
-
-    name = PyObject_GetAttrString(spec, "name");
-    if (name == NULL) {
-        return NULL;
-    }
-
-    mod = _PyImport_FindExtensionObject(name, name);
+    PyObject *mod = _PyImport_FindExtensionObject(name, name);
     if (mod || _PyErr_Occurred(tstate)) {
-        Py_DECREF(name);
         Py_XINCREF(mod);
         return mod;
     }
 
-    namestr = PyUnicode_AsUTF8(name);
-    if (namestr == NULL) {
-        Py_DECREF(name);
-        return NULL;
-    }
-
     PyObject *modules = tstate->interp->modules;
-    for (p = PyImport_Inittab; p->name != NULL; p++) {
-        PyModuleDef *def;
+    for (struct _inittab *p = PyImport_Inittab; p->name != NULL; p++) {
         if (_PyUnicode_EqualToASCIIString(name, p->name)) {
             if (p->initfunc == NULL) {
                 /* Cannot re-init internal module ("sys" or "builtins") */
-                mod = PyImport_AddModule(namestr);
-                Py_DECREF(name);
-                return mod;
+                return PyImport_AddModuleObject(name);
             }
+
             mod = (*p->initfunc)();
             if (mod == NULL) {
-                Py_DECREF(name);
                 return NULL;
             }
+
             if (PyObject_TypeCheck(mod, &PyModuleDef_Type)) {
-                Py_DECREF(name);
                 return PyModule_FromDefAndSpec((PyModuleDef*)mod, spec);
-            } else {
+            }
+            else {
                 /* Remember pointer to module init function. */
-                def = PyModule_GetDef(mod);
+                PyModuleDef *def = PyModule_GetDef(mod);
                 if (def == NULL) {
-                    Py_DECREF(name);
                     return NULL;
                 }
+
                 def->m_base.m_init = p->initfunc;
                 if (_PyImport_FixupExtensionObject(mod, name, name,
                                                    modules) < 0) {
-                    Py_DECREF(name);
                     return NULL;
                 }
-                Py_DECREF(name);
                 return mod;
             }
         }
     }
-    Py_DECREF(name);
+
+    // not found
     Py_RETURN_NONE;
 }
 
 
+
+/*[clinic input]
+_imp.create_builtin
+
+    spec: object
+    /
+
+Create an extension module.
+[clinic start generated code]*/
+
+static PyObject *
+_imp_create_builtin(PyObject *module, PyObject *spec)
+/*[clinic end generated code: output=ace7ff22271e6f39 input=37f966f890384e47]*/
+{
+    PyThreadState *tstate = _PyThreadState_GET();
+
+    PyObject *name = PyObject_GetAttrString(spec, "name");
+    if (name == NULL) {
+        return NULL;
+    }
+
+    PyObject *mod = create_builtin(tstate, name, spec);
+    Py_DECREF(name);
+    return mod;
+}
+
+
 /* Frozen modules */
 
 static const struct _frozen *
@@ -2127,46 +2124,88 @@ static PyMethodDef imp_methods[] = {
 };
 
 
-static struct PyModuleDef impmodule = {
+static int
+imp_module_exec(PyObject *module)
+{
+    const wchar_t *mode = _Py_GetConfig()->check_hash_pycs_mode;
+    PyObject *pyc_mode = PyUnicode_FromWideChar(mode, -1);
+    if (pyc_mode == NULL) {
+        return -1;
+    }
+    if (PyModule_AddObjectRef(module, "check_hash_based_pycs", pyc_mode) < 0) {
+        Py_DECREF(pyc_mode);
+        return -1;
+    }
+    Py_DECREF(pyc_mode);
+
+    return 0;
+}
+
+
+static PyModuleDef_Slot imp_slots[] = {
+    {Py_mod_exec, imp_module_exec},
+    {0, NULL}
+};
+
+static struct PyModuleDef imp_module = {
     PyModuleDef_HEAD_INIT,
-    "_imp",
-    doc_imp,
-    0,
-    imp_methods,
-    NULL,
-    NULL,
-    NULL,
-    NULL
+    .m_name = "_imp",
+    .m_doc = doc_imp,
+    .m_size = 0,
+    .m_methods = imp_methods,
+    .m_slots = imp_slots,
 };
 
 PyMODINIT_FUNC
 PyInit__imp(void)
 {
-    PyObject *m, *d;
+    return PyModuleDef_Init(&imp_module);
+}
 
-    m = PyModule_Create(&impmodule);
-    if (m == NULL) {
-        goto failure;
+
+// Import the _imp extension by calling manually _imp.create_builtin() and
+// _imp.exec_builtin() since importlib is not initialized yet. Initializing
+// importlib requires the _imp module: this function fix the bootstrap issue.
+PyObject*
+_PyImport_BootstrapImp(PyThreadState *tstate)
+{
+    PyObject *name = PyUnicode_FromString("_imp");
+    if (name == NULL) {
+        return NULL;
     }
-    d = PyModule_GetDict(m);
-    if (d == NULL) {
-        goto failure;
+
+    // Mock a ModuleSpec object just good enough for PyModule_FromDefAndSpec():
+    // an object with just a name attribute.
+    //
+    // _imp.__spec__ is overriden by importlib._bootstrap._instal() anyway.
+    PyObject *attrs = Py_BuildValue("{sO}", "name", name);
+    if (attrs == NULL) {
+        goto error;
+    }
+    PyObject *spec = _PyNamespace_New(attrs);
+    Py_DECREF(attrs);
+    if (spec == NULL) {
+        goto error;
     }
 
-    const wchar_t *mode = _Py_GetConfig()->check_hash_pycs_mode;
-    PyObject *pyc_mode = PyUnicode_FromWideChar(mode, -1);
-    if (pyc_mode == NULL) {
-        goto failure;
+    // Create the _imp module from its definition.
+    PyObject *mod = create_builtin(tstate, name, spec);
+    Py_CLEAR(name);
+    Py_DECREF(spec);
+    if (mod == NULL) {
+        goto error;
     }
-    if (PyDict_SetItemString(d, "check_hash_based_pycs", pyc_mode) < 0) {
-        Py_DECREF(pyc_mode);
-        goto failure;
+    assert(mod != Py_None);  // not found
+
+    // Execute the _imp module: call imp_module_exec().
+    if (exec_builtin_or_dynamic(mod) < 0) {
+        Py_DECREF(mod);
+        goto error;
     }
-    Py_DECREF(pyc_mode);
+    return mod;
 
-    return m;
-  failure:
-    Py_XDECREF(m);
+error:
+    Py_XDECREF(name);
     return NULL;
 }
 
diff --git a/Python/pylifecycle.c b/Python/pylifecycle.c
index 9771951..428c887 100644
--- a/Python/pylifecycle.c
+++ b/Python/pylifecycle.c
@@ -8,6 +8,7 @@
 #include "pycore_ceval.h"         // _PyEval_FiniGIL()
 #include "pycore_context.h"       // _PyContext_Init()
 #include "pycore_fileutils.h"     // _Py_ResetForceASCII()
+#include "pycore_import.h"        // _PyImport_BootstrapImp()
 #include "pycore_initconfig.h"    // _PyStatus_OK()
 #include "pycore_object.h"        // _PyDebug_PrintTotalRefs()
 #include "pycore_pathconfig.h"    // _PyConfig_WritePathConfig()
@@ -155,18 +156,11 @@ init_importlib(PyThreadState *tstate, PyObject *sysmod)
     }
     interp->importlib = Py_NewRef(importlib);
 
-    PyObject *import_func = _PyDict_GetItemStringWithError(interp->builtins,
-                                                           "__import__");
-    if (import_func == NULL) {
-        return -1;
-    }
-    interp->import_func = Py_NewRef(import_func);
-
     // Import the _imp module
     if (verbose) {
         PySys_FormatStderr("import _imp # builtin\n");
     }
-    PyObject *imp_mod = PyInit__imp();
+    PyObject *imp_mod = _PyImport_BootstrapImp(tstate);
     if (imp_mod == NULL) {
         return -1;
     }
@@ -741,6 +735,14 @@ pycore_init_builtins(PyThreadState *tstate)
     }
     Py_DECREF(bimod);
 
+    // Get the __import__ function
+    PyObject *import_func = _PyDict_GetItemStringWithError(interp->builtins,
+                                                           "__import__");
+    if (import_func == NULL) {
+        goto error;
+    }
+    interp->import_func = Py_NewRef(import_func);
+
     assert(!_PyErr_Occurred(tstate));
 
     return _PyStatus_OK();