Skip to content
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

gh-111178: fix USAN failures for partialobject #124733

Merged

Conversation

picnixz
Copy link
Member

@picnixz picnixz commented Sep 28, 2024

Before reviewing this one, I'll add a suppression file with all the incriminating functions. And whenever I make a new PR for fixing USAN failures, I'll just remove the corresponding entries so that we can really check the changes by running the build bot.

@picnixz picnixz force-pushed the chore/fix-usan-failures-partialobject-111178 branch from 080d875 to 287594e Compare September 29, 2024 11:01
@vstinner
Copy link
Member

vstinner commented Oct 9, 2024

What's the status of this PR? Why is it a draft?

@picnixz
Copy link
Member Author

picnixz commented Oct 11, 2024

What's the status of this PR?

Forgotten :D

Why is it a draft?

I wanted to add the suppression before but since it wouldn't have worked anyway, I can mark it as ready (I'll just check that the warnings for partialobjects indeed disappeared).

@picnixz picnixz marked this pull request as ready for review October 11, 2024 10:16
@picnixz picnixz requested a review from rhettinger as a code owner October 11, 2024 10:16
@picnixz picnixz requested a review from vstinner October 11, 2024 10:16
@picnixz
Copy link
Member Author

picnixz commented Oct 11, 2024

@vstinner Do you want me to add some _PyPartialObject_CAST?

Copy link
Member

@vstinner vstinner left a comment

Choose a reason for hiding this comment

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

If you want to fix UBSan issues for partial(), you should patch more methods like: partial_dealloc, partial_repr, partial_traverse, etc.

@vstinner
Copy link
Member

@vstinner Do you want me to add some _PyPartialObject_CAST?

I have mixed feelings about these macros :-) You can add a macro which just do the cast and don't check the type, or don't add the macro. As you want.

@picnixz picnixz marked this pull request as draft October 11, 2024 15:56
@picnixz
Copy link
Member Author

picnixz commented Oct 11, 2024

If you want to fix UBSan issues for partial(), you should patch more methods like: partial_dealloc, partial_repr, partial_traverse, etc.

Oups, I forgot to do it. I think I only checked the warnings thinking that the rest would be reported. I'll do it tomorrow (hence reconverting into a draft).

@picnixz picnixz marked this pull request as ready for review October 12, 2024 09:10
@picnixz
Copy link
Member Author

picnixz commented Oct 12, 2024

I haven't touched the utilities functions since they are not used as callbacks and shouldn't lead to undefined behaviours (e.g., partial_setvectorcall).

@picnixz picnixz requested a review from vstinner October 12, 2024 09:11
Copy link
Member

@vstinner vstinner left a comment

Choose a reason for hiding this comment

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

LGTM

@vstinner vstinner merged commit c77121e into python:main Oct 14, 2024
38 checks passed
@vstinner
Copy link
Member

Merged, thanks.

@picnixz picnixz deleted the chore/fix-usan-failures-partialobject-111178 branch October 14, 2024 14:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants