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

C API: PyType_FromSpec(): __vectorcalloffset__ (and other special offset members) does not support Py_RELATIVE_OFFSET #123465

Closed
wjakob opened this issue Aug 29, 2024 · 3 comments
Assignees
Labels
topic-C-API type-bug An unexpected behavior, bug, or error

Comments

@wjakob
Copy link
Contributor

wjakob commented Aug 29, 2024

Bug report

Bug description:

When declaring members of custom types via PyType_FromSpec(), it is advisable to rely on Py_RELATIVE_OFFSET to declare their position using a relative byte offset.

This is needed to create portable stable API / limited ABI extensions since we don't need to make assumptions about the layout of the base type, and how Python concatenates the two (e.g. is there intermediate padding/alignment?)

In addition, one can specify a special member called __vectorcalloffset__ to declare the byte offset of a pointer that implements a vector call handler.

Unfortunately, these two features don't work together. That is bad news for anyone who wants to extend an opaque type (e.g. PyTypeObject) and add the ability to dispatch vector calls. I ran into this issue while looking for temporary workarounds for #100554.

I created a minified reproducer here: https://github.com/wjakob/vectorcall_issue (installable via pip install https://github.com/wjakob/vectorcall_issue)

This extension creates a metaclass Meta that declares tp_call and vector call, along with a type Class created using this metaclass. It specifies the __vectorcalloffset__ using one of two different ways:

PyMemberDef meta_members [] = {
#if TRIGGER_BUG
    { "__vectorcalloffset__", Py_T_PYSSIZET, 0, Py_READONLY | Py_RELATIVE_OFFSET },
#else
    { "__vectorcalloffset__", Py_T_PYSSIZET, sizeof(PyHeapTypeObject), Py_READONLY },
#endif
    { NULL }
};

Compiling this extension with #define TRIGGER_BUG 1 and instantiating Class shows that the tp_call path is used.

Python 3.12.5 (main, Aug  6 2024, 19:08:49) [Clang 15.0.0 (clang-1500.1.0.2.5)] on darwin
Type "help", "copyright", "credits" or "license" for more information.
>>> from vectorcall_issue import Class
>>> Class()
Constructing using tp_call
<vectorcall_issue.Class object at 0x102faca10>
>>>

If I switch over from a relative to an absolute byte offset (change #define TRIGGER_BUG 1 to #define TRIGGER_BUG 0 at the top), the vector call is used. But that is not portable.

Python 3.12.5 (main, Aug  6 2024, 19:08:49) [Clang 15.0.0 (clang-1500.1.0.2.5)] on darwin
Type "help", "copyright", "credits" or "license" for more information.
>>> from vectorcall_issue import Class
>>> Class()
Constructing using vector call
<vectorcall_issue.Class object at 0x102fb8a10>

CPython versions tested on:

3.12

Operating systems tested on:

macOS

Linked PRs

@wjakob wjakob added the type-bug An unexpected behavior, bug, or error label Aug 29, 2024
@wjakob
Copy link
Contributor Author

wjakob commented Aug 29, 2024

cc @encukou

@encukou encukou self-assigned this Aug 29, 2024
@encukou encukou changed the title C API: PyType_FromSpec(): __vectorcalloffset__ does not support Py_RELATIVE_OFFSET C API: PyType_FromSpec(): __vectorcalloffset__ (and other special offset members) does not support Py_RELATIVE_OFFSET Aug 29, 2024
encukou added a commit to encukou/cpython that referenced this issue Aug 29, 2024
@encukou
Copy link
Member

encukou commented Sep 3, 2024

@wjakob Do you want to check if #123474 fixes the issue?

@wjakob
Copy link
Contributor Author

wjakob commented Sep 3, 2024

It does, thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
topic-C-API type-bug An unexpected behavior, bug, or error
Projects
None yet
Development

No branches or pull requests

3 participants