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.

Sign up for GitHub

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

Jump to bottom

fix vectorcall argument handling #4104

Merged
merged 1 commit into from
Apr 22, 2024
Merged

fix vectorcall argument handling #4104

merged 1 commit into from
Apr 22, 2024

Conversation

birkenfeld
Copy link
Member

Fixes #4093

  • Make PY_VECTORCALL_ARGUMENTS_OFFSET a size_t like on CPython
  • Change the assert in PyVectorcall_NARGS to check the recovered number, not the original value (which is > PY_SSIZE_T_MAX on purpose)

@birkenfeld birkenfeld mentioned this pull request Apr 20, 2024
Closed
@birkenfeld
Copy link
Member Author

For the CPython definitions, see

davidhewitt
davidhewitt reviewed Apr 21, 2024
View reviewed changes
Copy link
Member

@davidhewitt davidhewitt left a comment

Choose a reason for hiding this comment

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

Whoops, my original implementation there makes no sense, thanks for fixing! A bit of a surprise we didn't get this reported before, I guess that we haven't managed to use these vectorcall methods very extensively in PyO3 itself yet...

pyo3-ffi/src/cpython/abstract_.rs Outdated Show resolved Hide resolved
@birkenfeld
fix vectorcall argument handling
2760d74 
Fixes #4093

- Make PY_VECTORCALL_ARGUMENTS_OFFSET a size_t like on CPython
- Remove the assert in PyVectorcall_NARGS and use checked cast
@adamreichold adamreichold enabled auto-merge April 22, 2024 08:09
@adamreichold adamreichold added this pull request to the merge queue Apr 22, 2024
Merged via the queue into main with commit c2ac9a9 Apr 22, 2024
42 of 43 checks passed
@adamreichold adamreichold deleted the fix-4093 branch April 22, 2024 09:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@davidhewitt davidhewitt davidhewitt left review comments

@adamreichold adamreichold adamreichold approved these changes

Assignees
No one assigned
Labels
bugfix
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

PyObject_CallOneArg() usage triggers assert
3 participants
@birkenfeld @davidhewitt @adamreichold