Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork32k
gh-127545: Specify minimum PyGC_Head alignment to fix build failure#127546
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
base:main
Are you sure you want to change the base?
Conversation
ghost commentedDec 3, 2024 • edited by ghost
Loading Uh oh!
There was an error while loading.Please reload this page.
edited by ghost
Uh oh!
There was an error while loading.Please reload this page.
Most changes to Pythonrequire a NEWS entry. Add one using theblurb_it web app or theblurb command-line tool. If this change has little impact on Python users, wait for a maintainer to apply the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
I think this needs a blurb entry. Could you add one using one of the links the bot gave?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
No need to force push--everything is squashed at the end anyway.
Misc/NEWS.d/next/Build/2024-12-04-10-00-35.gh-issue-127545.t0THjE.rst OutdatedShow resolvedHide resolved
Uh oh!
There was an error while loading.Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Provisionally, this LGTM. (I say "provisionally" because I don't totally understand the issue--there's just nothing wrong with this PR from a triage standpoint.)
I think the patch can made more understandable, by making use of the existing macro definitions. I will update the branch. |
As documented in InternalDocs/garbage_collector.md, the garbage collectorstores flags in the least significant two bits of the _gc_prev pointerin struct PyGC_Head. Consequently, this pointer is only capable of storinga location that's aligned to a 4-byte boundary.This alignment requirement is documented but it's not actually encoded.The code works when python happens to run on a platform that has a largeminimum alignment, but fails otherwise.Since we know that 2 bits are needed, we also know the minimum alignmentthat's needed. Let's make that explicit, so the compiler can then makesure those 2 bits are available.This patch fixes a segfault in _bootstrap_python. It also clarifies thecode by explicitly stating the actual requirement, in terms of_PyGC_PREV_SHIFT.This bug was previously investigated by Adrian Glaubitz here:https://lists.debian.org/debian-68k/2024/11/msg00020.htmlhttps://bugs.debian.org/cgi-bin/bugreport.cgi?bug=1087600Although Adrian's patch isn't really correct (because natural alignmentis not needed), he deserves full credit for finding the root cause.
I just noticed that@markshannon refactored the same headers 2 months ago. So I have now updated the fix to resolve those merge conflicts. Please review. This bug is still present in 3.14.0beta1. |
We're applying this patch in Debian's python3.14 to get it to build on m68k. |
Uh oh!
There was an error while loading.Please reload this page.
#127545