Skip to content

Disable unsafe identical code folding in BOLT #622

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

geofft
Copy link
Collaborator

@geofft geofft commented May 23, 2025

astral-sh/uv#13610 reported a misbehavior that is the result of a subclass of str incorrectly having its ->tp_as_number->nb_add slot filled in with the value of PyUnicode_Type->tp_as_sequence->sq_concat. There are some times when this is an appropriate thing to do iwhen subclassing, but this is not one of them. The logic to prevent it in this case relies on two helper functions in the file, wrap_binaryfunc and wrap_binaryfunc_l, having different addresses, even though they contain identical code.

For some reason BOLT does not do this optimization in the shared library (even though those are static functions and not exported), so we only started seeing this in the static build.

@geofft geofft added the platform:linux Specific to the Linux platform label May 23, 2025
@geofft geofft requested review from zanieb and indygreg May 23, 2025 19:22
@geofft
Copy link
Collaborator Author

geofft commented May 23, 2025

We very much need to upstream this change, btw, but let's get this shipped.

@andersk
Copy link

andersk commented May 24, 2025

@geofft
Copy link
Collaborator Author

geofft commented May 25, 2025

Ah, thanks for the links, Anders! list + tuple subclass and list + str subclass presumably trigger the same behavior, yeah. And indeed that second link also confirms that this only happens with static libpython.

I opened python/cpython#134642 which is a more thorough version of this change. (It doesn't matter for us since we know we're on LLVM 20, but I'll update the patch in this PR next week.)

astral-sh/uv#13610 reported a misbehavior that is the result of a subclass of
str incorrectly having its ->tp_as_number->nb_add slot filled in with the value
of PyUnicode_Type->tp_as_sequence->sq_concat. There are some times when this is
an appropriate thing to do iwhen subclassing, but this is not one of them. The
logic to prevent it in this case relies on two helper functions in the file,
wrap_binaryfunc and wrap_binaryfunc_l, having different addresses, even though
they contain identical code.

For some reason BOLT does not do this optimization in the shared library
(even though those are static functions and not exported), so we only
started seeing this in the static build.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
platform:linux Specific to the Linux platform
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants