Skip to content

gh-132026: Ensure _MIPS_SIM has defined _ABI identifiers for comparison #132027

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 our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Apr 27, 2025

Conversation

sigv
Copy link
Contributor

@sigv sigv commented Apr 2, 2025

When building C code on a MIPS architecture, _MIPS_SIM is used to determine architecture specifics. The value is expected to match either _ABIO32, _ABIN32 or _ABI64.

In gcc config/mips/mips.h these values are defined as compiler builtin_define inside of a switch/case. That means, mips64el and mips64 architectures know about _ABI64 but don't know about _ABIO32 and _ABIN32. In turn, when CPython tries to use them in comparison, they may be undefined identifiers.

In default compiler behavior, the undefined identifier will be evaluated as zero, and it will not match _MIPS_SIM. However, the issues pop up when -Wundef (or, even worse, -Werror=undef) compiler flag is enabled. Then suddenly it's visible as a warning or error.

@ghost
Copy link

ghost commented Apr 2, 2025

All commit authors signed the Contributor License Agreement.
CLA signed

@bedevere-app
Copy link

bedevere-app bot commented Apr 2, 2025

Most changes to Python require a NEWS entry. Add one using the blurb_it web app or the blurb command-line tool.

If this change has little impact on Python users, wait for a maintainer to apply the skip news label instead.

@sigv
Copy link
Contributor Author

sigv commented Apr 2, 2025

Heads up: On Python 3.13 and older, this logic is not yet moved to Misc/platform_triplet.c. Instead it's in configure and configure.ac, which results in the comparison being directly in pyconfig.h. Can provide a backport there, if this is accepted.

@python-cla-bot
Copy link

python-cla-bot bot commented Apr 6, 2025

All commit authors signed the Contributor License Agreement.

CLA signed

@sigv sigv force-pushed the gh-132026 branch 3 times, most recently from f37099c to da82bea Compare April 6, 2025 16:58
…mparison

When built on a MIPS architecture, `_MIPS_SIM` is used to determine
architecture specifics. The value is expected to match either
`_ABIO32`, `_ABIN32` or `_ABI64`.

In `gcc` config/mips/mips.h these values are defined as compiler
`builtin_define` inside of a switch/case. That means, mips64el and
mips64 architectures know about `_ABI64` but don't know about `_ABIO32`
and `_ABIN32`. In turn, when CPython tries to use them in comparison,
they may be undefined identifiers.

In default compiler behavior, the undefined identifier will be
evaluated as zero, and it will not match `_MIPS_SIM`. However, the
issues pop up when `-Wundef` (or, even worse, `-Werror=undef`) compiler
flag is enabled. Then suddenly it's visible as a warning or error.
Copy link
Member

@zware zware left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM with a rewording of the NEWS entry.

@zware zware enabled auto-merge (squash) April 27, 2025 21:33
@zware zware merged commit 6985e2e into python:main Apr 27, 2025
27 checks passed
@sigv sigv deleted the gh-132026 branch April 28, 2025 15:54
@sigv
Copy link
Contributor Author

sigv commented Apr 28, 2025

@zware, thank you!

Would it be possible for me to manually backport this to 3.13's configure?
I must have been looking at the tree wrong previously. This applies cleanly to 3.13 as a cherry-pick.

@zware zware added the needs backport to 3.13 bugs and security fixes label Apr 28, 2025
@miss-islington-app
Copy link

Thanks @sigv for the PR, and @zware for merging it 🌮🎉.. I'm working now to backport this PR to: 3.13.
🐍🍒⛏🤖

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Apr 28, 2025
…mparison (pythonGH-132027)

When built on a MIPS architecture, `_MIPS_SIM` is used to determine
architecture specifics. The value is expected to match either
`_ABIO32`, `_ABIN32` or `_ABI64`.

In `gcc` config/mips/mips.h these values are defined as compiler
`builtin_define` inside of a switch/case. That means, mips64el and
mips64 architectures know about `_ABI64` but don't know about `_ABIO32`
and `_ABIN32`. In turn, when CPython tries to use them in comparison,
they may be undefined identifiers.

In default compiler behavior, the undefined identifier will be
evaluated as zero, and it will not match `_MIPS_SIM`. However, the
issues pop up when `-Wundef` (or, even worse, `-Werror=undef`) compiler
flag is enabled. Then suddenly it's visible as a warning or error.
(cherry picked from commit 6985e2e)

Co-authored-by: Valters Jansons <sigv@users.noreply.github.com>
@bedevere-app
Copy link

bedevere-app bot commented Apr 28, 2025

GH-133092 is a backport of this pull request to the 3.13 branch.

@bedevere-app bedevere-app bot removed the needs backport to 3.13 bugs and security fixes label Apr 28, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants