gh-128341: Simplify the definition of _Py_INTERNAL_ABI_SLOT and use it in stdlib modules

[befeleme]

Please consider discussing this in the issue linked, rather than it the PR, since this would ping a huge list of reviewers.

• Issue: Segfault when updating from 3.13.0 to 3.13.1 while Python is running #128341

[bedevere-app]

A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated.

Once you have made the requested changes, please leave a comment on this pull request containing the phrase I have made the requested changes; please review again. I will then notify any core developers who have left a review that you're ready for them to take another look at this pull request.

[vstinner]

I don't understand the (void*) &(PyABIInfo) part. Where is this variable declared? Or does it magically accept the PyABIInfo type here?

I would prefer keeping _Py_INTERNAL_ABI_SLOT name to discourage developers to use it in their project.

Defined here to avoid every stdlib module including pycore_modsupport.h

If it's technically possible to use a macro in a C extension, someone will do it and then it will be hard for us to change the macro (users always complain).

[encukou]

It's a C99 compound literal. Yes, the value “magically” appears here.
This feature safe to use but not part of C++ and therefore not suitable for our public API.

I would prefer keeping _Py_INTERNAL_ABI_SLOT name to discourage developers to use it in their project.
If it's technically possible to use a macro in a C extension, someone will do it and then it will be hard for us to change the macro (users always complain).

I think the underscore is enough of a warning here.
(If it wasn't for the the C++ incompatibility, I'd have made this public and replace the currently recommended two-liner. Also, due to the stable ABI, we can't change much about the macro anyway.)

[vstinner]

I would prefer to keep the #ifdef Py_BUILD_CORE guard, see my comment below.

[vstinner]

"Tests / Ubuntu (bolt) / build and test (ubuntu-24.04)" failed with "llvm-bolt is required": I created issue #146053 about this error.

[vstinner]

I would prefer to not add _Py_ABI_SLOT to the public C API. Here is a short patch to only add it to the internal C API (Py_BUILD_CORE macro):

diff --git a/Include/cpython/modsupport.h b/Include/cpython/modsupport.h
index c54e11a6c08..cfeee6e8ab3 100644
--- a/Include/cpython/modsupport.h
+++ b/Include/cpython/modsupport.h
@@ -38,6 +38,8 @@ typedef struct _PyArg_Parser {
 PyAPI_FUNC(int) _PyArg_ParseTupleAndKeywordsFast(PyObject *, PyObject *,
                                                  struct _PyArg_Parser *, ...);
 
+#ifdef Py_BUILD_CORE
 // For internal use in stdlib. Needs C99 compound literals.
 // Defined here to avoid every stdlib module including pycore_modsupport.h
 #define _Py_ABI_SLOT {Py_mod_abi, (void*) &(PyABIInfo) _PyABIInfo_DEFAULT}
+#endif
diff --git a/Modules/_xxtestfuzz/_xxtestfuzz.c b/Modules/_xxtestfuzz/_xxtestfuzz.c
index 56668e48a95..a2f01eb2490 100644
--- a/Modules/_xxtestfuzz/_xxtestfuzz.c
+++ b/Modules/_xxtestfuzz/_xxtestfuzz.c
@@ -28,8 +28,10 @@ static PyMethodDef module_methods[] = {
     {NULL},
 };
 
+PyABIInfo_VAR(abi_info);
+
 static PyModuleDef_Slot module_slots[] = {
-    _Py_ABI_SLOT,
+    {Py_mod_abi, &abi_info},
     {Py_mod_gil, Py_MOD_GIL_NOT_USED},
     {0, NULL},
 };
diff --git a/Modules/xxsubtype.c b/Modules/xxsubtype.c
index 1d06b0faa0e..7a31ba00b98 100644
--- a/Modules/xxsubtype.c
+++ b/Modules/xxsubtype.c
@@ -301,8 +301,10 @@ xxsubtype_exec(PyObject* m)
     return 0;
 }
 
+PyABIInfo_VAR(abi_info);
+
 static struct PyModuleDef_Slot xxsubtype_slots[] = {
-    _Py_ABI_SLOT,
+    {Py_mod_abi, &abi_info},
     {Py_mod_exec, xxsubtype_exec},
     {Py_mod_multiple_interpreters, Py_MOD_PER_INTERPRETER_GIL_SUPPORTED},
     {Py_mod_gil, Py_MOD_GIL_NOT_USED},

[vstinner]

This PR does different things:

• Replace _Py_INTERNAL_ABI_SLOT macro with _Py_ABI_SLOT in Include/cpython/modsupport.h.
• Add _Py_ABI_SLOT to PyModuleDef_Slot array when it wasn't used before.
• Add PyABIInfo_Check() call to PyInit functions for the 2 extensions (readline, _tkinter) which still use the legacy PyModule_Create() API.

dismiss

[vstinner]

LGTM

[vstinner]

"Tests / All required checks pass" CI failed with a surprising error message: Error: The template is not valid. .github/workflows/build.yml (Line: 679, Col: 24): Error reading JToken from JsonReader. Path '', line 0, position 0.

I clicked on [Update branch] to try to solve the issue.

[vstinner]

Merged, thanks for the fix.