gh-146636: Improve ABI/feature selection, add new header for it#148302
gh-146636: Improve ABI/feature selection, add new header for it#148302encukou wants to merge 7 commits intopython:mainfrom
Conversation
…ABLED Add a test that Python headers themselves don't use Py_GIL_DISABLED in abi3t: abi3 and abi3t ought to be the same except the _Py_OPAQUE_PYOBJECT differences. This is done using the GCC-only poison pragma.
encukou
requested review from
a team,
AA-Turner,
emmatyping and
erlend-aasland
as code owners
| // whether they need extra synchronization. | ||
| # define Py_GIL_DISABLED | ||
| # endif | ||
| # if defined(_Py_IS_TESTCEXT) |
There was a problem hiding this comment.
You may move #ifdef __GNUC__ here:
| # if defined(_Py_IS_TESTCEXT) | |
| # if defined(_Py_IS_TESTCEXT) && defined(__GNUC__) |
There was a problem hiding this comment.
This subtle style detail is intentional: I'm inviting #elifs for other compilers.
|
|
||
| /* The internal C API must not be used with the limited C API: make sure | ||
| * that Py_BUILD_CORE* macros are not defined in this case. | ||
| * But, keep the "original" values, under different names, for "exports.h" |
There was a problem hiding this comment.
It's surprising that _PyEXPORTS_CORE and _PyEXPORTS_CORE_MODULE are defined even if Py_LIMITED_API is defined. I tried to undefine Py_BUILD_CORE if Py_LIMITED_API is defined before _PyEXPORTS_CORE code, but I got linker errors on Windows. Examples:
LINK : warning LNK4217: symbol 'PyType_GetFlags' defined in 'typeobject.obj' is imported by '_stat.obj' in fu
nction '_PyLong_AsMode_t' [C:\victor\python\main\PCbuild\pythoncore.vcxproj]
LINK : warning LNK4217: symbol '_Py_DecRef' defined in 'object.obj' is imported by '_stat.obj' in function '_
PyLong_AsMode_t' [C:\victor\python\main\PCbuild\pythoncore.vcxproj]
LINK : warning LNK4286: symbol '_Py_DecRef' defined in 'object.obj' is imported by 'errnomodule.obj' [C:\vict
or\python\main\PCbuild\pythoncore.vcxproj]
LINK : warning LNK4217: symbol 'PyUnicode_FromStringAndSize' defined in 'unicodeobject.obj' is imported by '_
stat.obj' in function 'stat_filemode' [C:\victor\python\main\PCbuild\pythoncore.vcxproj]
There was a problem hiding this comment.
Yes; these select whether PyAPI_FUNC sumbols are exported or imported. Anything compiled into the main binary needs to be exported, regardless of the API it uses.
(This is currently done by including "exports.h" in the middle of pyport.h, right before Py_BUILD_CORE is undefined -- which is IMO confusing, as it makes the meaning of Py_BUILD_CORE depend on where you are in the header.)
Co-authored-by: Victor Stinner <vstinner@python.org>
encukou
added
the
🔨 test-with-buildbots
|
🤖 New build scheduled with the buildbot fleet by @encukou for commit 7c9d920 🤖 Results will be shown at: https://buildbot.python.org/all/#/grid?branch=refs%2Fpull%2F148302%2Fmerge If you want to schedule another build, you need to add the 🔨 test-with-buildbots label again. |
bedevere-bot
removed
the
🔨 test-with-buildbots
|
I ran into an issue similar to gh-148267 while working on CFFI (see https://github.com/python-cffi/cffi/actions/runs/24516133542/job/71660551119?pr=232) and was curious if this PR fixes it. It doesn't. The compiler error I see on this branch is: I think this is happening because my CFFI branch was using |
|
Looking again, maybe it's because CFFI's headers define Py_LIMITED_API but don't set a value: |
test_cext.test_build_abi3t() failed. All of these buildbot workers use I suppose that the issue can be reproduced with Example on "buildbot/AMD64 FreeBSD PR": I can reproduce the issue on Fedora with Clang on this short program: #define Py_TARGET_ABI3T 0x30f00a8
#pragma GCC poison Py_GIL_DISABLED
#ifdef Py_TARGET_ABI3T
# define _Py_OPAQUE_PYOBJECT
#endif
int main()
{
#ifdef _Py_OPAQUE_PYOBJECT
// All good
return 0;
#elif defined(Py_GIL_DISABLED)
// Not good
return 1;
#endif
return 0;
}Clang output: The workaround is to write: #ifdef _Py_OPAQUE_PYOBJECT
// All good
return 0;
#else
# if defined(Py_GIL_DISABLED)
// Not good
return 1;
# endif
#endif
test.test_io.test_fileio failed: waiting for #148648 fix. |
Oh, maybe we can set the #if Py_LIMITED_API+0 < 3
# warning "Py_LIMITED_API macro value must be at least 3"
# undef Py_LIMITED_API
# define Py_LIMITED_API 3
#endif |
|
@mgorny and I were also looking at the stable-abi-testing repo and got into a situation where "weird" things happen if you define |
4 participants
Feature/ABI selection headers (Py_BUILD_CORE, Py_LIMITED_API, Py_GIL_DISABLED) need to be defined early (so they affect all definitions), but after the version (
patchlevel.h) and configuration (pyconfig.h).Put them in their own header to make this more obvious, and easier for contributors to keep them in the proper place.
Add a test that Python headers themselves don't use Py_GIL_DISABLED, since abi3 and abi3t ought to be identical except for the _Py_OPAQUE_PYOBJECT differences.
This is done using the GCC-only poison pragma, which tells the compiler/preprocessor to error if it ever sees the identifier after the pragma.
This needs some rewriting:
#if defined(Py_LIMITED_API) && defined(Py_GIL_DISABLED)needs to be split into two lines, as the poisoning doesn't honour short-circuiting.This should fix gh-148267, but I'm attaching the PR to the abi3t issue, gh-146636, as it continues that effort (GH-148142 specifically).