8000 Type of vectorcall nargs inconsistent · Issue #134457 · python/cpython · GitHub
[go: up one dir, main page]
Skip to content

Type of vectorcall nargs inconsistent #134457

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
Closed
rogerbinns opened this issue May 21, 2025 · 4 comments
Closed

Type of vectorcall nargs inconsistent #134457

rogerbinns opened this issue May 21, 2025 · 4 comments
Labels
topic-C-API type-bug An unexpected behavior, bug, or error

Comments

@rogerbinns
Copy link
rogerbinns commented May 21, 2025 •
edited by github-actions bot
Loading

Bug report

Bug description:

The documentation is clear that it is size_t

However if you search the Python source for nargs, it is often Py_ssize_t

One is signed and the other is unsigned. It looks like argument clinic is one culprit.

Ordinarily this is no big deal, because it would take too many arguments to overflow. However the clang undefined behaviour sanitizer gets upset when the sign on that parameter is wrong. See #111178 for why that matters.

I discovered this because I had copied argument clinic using Py_ssize_t and then hit ubsan issues.

CPython versions tested on:

3.14

Operating systems tested on:

Linux
@rogerbinns rogerbinns added the type-bug An unexpected behavior, bug, or error label May 21, 2025
@ZeroIntensity
Copy link
Member

cc @picnixz for UBSan

@rogerbinns
Copy link
Author

Another problematic location is methodobject.c:466 where Py_ssize_t is used for nargs instead of size_t.

That is because methodobject.h defines the callbacks with nargs as Py_ssize_t and should be size_t.

This is an example of the sanitizer output:

Objects/methodobject.c:466:24: runtime error: call to function APSWCursor_execute through pointer to incorrect function type 'struct _object *(*)(struct _object *, struct _object *const *, long, struct _object *)'
/space/apsw/src/cursor.c:1033: note: APSWCursor_execute defined here
SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior Objects/methodobject.c:466:24

Switching nargs back to signed in my code makes that go away, but CPython being so inconsistent is what needs to be fized.

@encukou
Copy link
Member
encukou commented May 22, 2025

Don't know how consistent it's been kept over the years, but the intended convention for types and names was:

  • size_t nargsf: number of arguments plus an optional PY_VECTORCALL_ARGUMENTS_OFFSET flag. (The f stands for flag.) This is used in vectorcall function definitions & calls.
  • Py_ssize_t nargs: the number of arguments. This is used “outside” vectorcall (and Py_ssize_t is also used for sizes pretty much everywhere else in the C API).

In particular, METH_FASTCALL uses Py_ssize_t nargs. It's an older and easier-to-use API where you don't need to worry about the flag.

That seems to be your case: if APSWCursor_execute is used with METH_FASTCALL | METH_KEYWORDS, its signature should be PyCFunctionFastWithKeywords, with Py_ssize_t nargs.

Do you have an example where the convention isn't followed?

@encukou encukou added the pending The issue will be closed if no feedback is provided label May 22, 2025
@rogerbinns
Copy link
Author

@encukou you are correct, and there are no CPython problems.

That vector and fast calls have the same signatures, except for the signedness of nargs turned out to be a benefit.

I've been converting over 300 C method callbacks so that the first parameter is a PyObject * and not my custom types, to avoid the sanitizer warnings. The methods include vector and fast calls. The signedness caught one vector call I had wrong, while the overall calling conventions caught an incorrect METH_ flag.

@ZeroIntensity ZeroIntensity removed the pending The issue will be closed if no feedback is provided label May 22, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
topic-C-API type-bug An unexpected behavior, bug, or error
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@encukou @rogerbinns @ZeroIntensity
0