gh-153711: Guard dup3/pipe2 with __builtin_available on Apple platforms#154718
gh-153711: Guard dup3/pipe2 with __builtin_available on Apple platforms#154718Vamsi-klu wants to merge 2 commits into
Conversation
…latforms Add HAVE_DUP3_RUNTIME / HAVE_PIPE2_RUNTIME weak-linking so Xcode 27 SDKs do not crash on macOS < 27 when the symbols are weak_import NULL.
|
All commit authors signed the Contributor License Agreement. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e5fbe3bb60
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
|
||
| .. availability:: Unix, not WASI, not macOS, not iOS. | ||
| .. availability:: Unix, not WASI. Availability on macOS requires macOS 27.0 | ||
| or later; availability on iOS requires iOS 27.0 or later. |
There was a problem hiding this comment.
The new iOS availability claim is inaccurate even on iOS 27: configure.ac:5526-5532 unconditionally skips the pipe2 feature check whenever ac_sys_system is iOS, so HAVE_PIPE2 remains undefined and the module never exports os.pipe2. Either retain not iOS here or change the iOS configure policy to enable the function where supported.
Useful? React with 👍 / 👎.
| #endif | ||
|
|
||
| #if defined(HAVE_PIPE2) | ||
| if (HAVE_PIPE2_RUNTIME) {} else { |
There was a problem hiding this comment.
When building with an Xcode 27 SDK for a macOS deployment target below 27, removing pipe2 from the module dictionary happens too late to make the build succeed: os_pipe2_impl still calls the availability-annotated pipe2 unguarded at Modules/posixmodule.c:12861, while configure.ac:2797-2802 promotes Clang's unguarded-availability diagnostic to an error. The call needs a lexical availability guard or the same targeted diagnostic suppression used for the runtime-removed preadv/pwritev methods.
Useful? React with 👍 / 👎.
The availability extension only accepts comma-separated known platforms (optionally with 'not ' / '>= version'). Free-form prose after 'not WASI.' was parsed as an unknown platform and failed Docs EPUB/doctest with warnings-as-errors.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 548b366f3b
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| respectively. | ||
|
|
||
| .. availability:: Unix, not WASI, not macOS, not iOS. | ||
| .. availability:: Unix, not WASI, macOS >= 27.0, iOS >= 27.0. |
There was a problem hiding this comment.
The documentation promises os.pipe2 on iOS 27+, but configure.ac deliberately skips the AC_CHECK_FUNCS invocation containing pipe2 whenever ac_sys_system is iOS (lines 5526–5532). Consequently HAVE_PIPE2 is never defined and the method is not compiled or exported on any iOS version; either enable the configure check for supported iOS versions or continue documenting this as unavailable on iOS.
Useful? React with 👍 / 👎.
Documentation build overview
|
|
Closing this PR. Thank you for the review attention. |
1 participant
Summary
Fix gh-153711 (deferred-blocker): when CPython is built with an Xcode 27 SDK,
dup3/pipe2are detected at configure time but only exist at runtime on macOS / iOS 27+. On older OSes they bind asweak_importNULL and the firstos.pipe()/ non-inheritableos.dup2()jumps to address 0.What the issue is
dup3/pipe2with__API_AVAILABLE(macos(27.0), iOS(27.0), …).AC_CHECK_FUNCSsynthesizes its ownchar dup3();prototype, ignores availability, links againstlibSystem.tbd, and definesHAVE_DUP3/HAVE_PIPE2.Modules/posixmodule.cthen calls them unconditionally insideos_dup2_impl/os_pipe_impl/os_pipe2_impl.MACOSX_DEPLOYMENT_TARGET < 27, the symbols are weak; on macOS 26 they are NULL →EXC_BAD_ACCESS.os.pipe()is on the hot path ofsubprocess,multiprocessing, andasyncio— effectively a total-loss crash for python.org-style builds.iOS was already excluded from the configure check (gh-150443 / #150444). macOS was not. The issue body already points at the gh-97897 (
mkfifoat/mknodat) weak-linking pattern.Why I solved it that way
__builtin_availableguards, not configure hacks — configure cannot see availability attributes, and we still want the symbols on macOS 27+.HAVE_*_RUNTIMEmachinery inposixmodule.c(modern / Xcode≤8 / non-Apple) documented inMac/README.rst. Omitting any arm breaks the build.if (HAVE_X_RUNTIME)— Apple's compiler rejectsif (!HAVE_X_RUNTIME).dup3_works = 0when unavailable so the existingdup2+FD_CLOEXECfallback runs this call and every later call without re-probing.res = -1; errno = ENOSYSbefore the pipe2 probe so the existing ENOSYS →pipe()fallback still runs when the symbol is missing (uninitializedres/errnowould be a footgun).os.pipe2from the module dict when unavailable (same idiom aspwritev/preadv) becauseos_pipe2_implhas no fallback and would crash on direct call. Existing tests alreadyhasattr(os, "pipe2").__builtin_available— no other macro in the block names it; pre-Xcode-15 clang rejects the spelling.How I did it
Modules/posixmodule.cHAVE_DUP3_RUNTIME/HAVE_PIPE2_RUNTIMEin all 3 arms; guarddup3/pipe2call sites; poppipe2at execLib/test/test_os/test_posix.pyTestPosixWeaklinking.test_pipe2/test_dup3Doc/library/os.rstos.pipe2availability notes macOS/iOS ≥ 27Misc/NEWS.d/next/macOS/Impact
1— zero codegen change (constant-folded).Testing plan
./python -m test test_os.test_posix— PASS (weaklinking class skipped on non-macOS, as designed).os.pipe()/os.dup2(..., inheritable=False)on Linux — OK.os.pipe()/os.pipe2/ non-inheritabledup2.os.pipe()works via fallback;hasattr(os, "pipe2")is False;TestPosixWeaklinkingassertions hold.pipe2present, weaklinking tests assert attrs exist.Everything else
Requested reviewers (subject-matter experts)
Could the following SMEs take a look when convenient (I cannot formally request reviews from this fork account):
Thank you!