[BUG] Using types.CodeType in Python 3.11 fails with ValueError: types.CodeType size changed, may indicate binary incompatibility. Expected 168 from C header, got 160 from PyObject

@hroncok

Versions:

Cython 0.29 branch: commit c0dfb9c2f592b7e0ece90ec83fbadcf6e6da3d47 (May 17, 2022)
Python 3.11.0b3+: commit a848a9894d (Jun 6 2022)

Reproducer, myext.pyx:

# cython: language_level=3
cdef extern from "Python.h":
    ctypedef class types.CodeType [object PyCodeObject]:
        pass

Build the extension (I make the assumption that cythonize uses Python 3.11):

cythonize -i myext.pyx

Importing the C extension fails:

$ python3.11 -c 'import myext'
Traceback (most recent call last):
  File "<string>", line 1, in <module>
  File "myext.pyx", line 1, in init myext
    # cython: language_level=3
                               
ValueError: types.CodeType size changed, may indicate binary incompatibility. Expected 168 from C header, got 160 from PyObject

To get Python 3.11 with Cython, you can use for example:

wget https://www.python.org/ftp/python/3.11.0/Python-3.11.0b3.tar.xz
tar -xf Python-3.11.0b3.tar.xz
cd Python-3.11.0b3
./configure --prefix /opt/py311
make
make install
cd ..
/opt/py311/bin/python3.11 -m pip install Cython

Then use /opt/py311/bin/cythonize. You can use a different prefix.

The problem is that tp_basicsize of &PyCode_Type is 160 bytes, whereas and sizeof(PyCodeObject) is 168 bytes.

Issue discovered when on my PR to update gevent to Python 3.11: gevent/gevent#1872 (comment)

@hroncok proposed a Cython pull request in Fedora: https://src.fedoraproject.org/rpms/Cython/pull-request/35

I am working on reworking @hroncok's PR to propose a PR to the master branch of Cython.

cc @encukou who helped to debug this tedious issue.

@brandtbucher

tp_basicsize of &PyCode_Type was changed in Python 3.11 by the commit python/cpython@2bde682 by @brandtbucher:

 PyTypeObject PyCode_Type = {
     PyVarObject_HEAD_INIT(&PyType_Type, 0)
     "code",
-    sizeof(PyCodeObject),
-    0,
+    offsetof(PyCodeObject, co_code_adaptive),
+    sizeof(_Py_CODEUNIT),
     (destructor)code_dealloc,           /* tp_dealloc */
     0,                                  /* tp_vectorcall_offset */
     0,                                  /* tp_getattr */

A couple of thoughts (not based on an in-depth look):

Try cythonize -if myext.pyx to force recompilation. I've definitely seen similar errors go away with recompilation.
I'm not sure if you can use variable size classes with as cdef classes - you certainly can't inherit from then. It may be that this change just means that you can't use CodeType as a cdef extern class

Documentation of tp_basicsize and tp_itemsize members of PyTypeObject: https://docs.python.org/dev/c-api/typeobj.html#c.PyTypeObject.tp_basicsize

These fields allow calculating the size in bytes of instances of the type.

PyCodeObject is a PyVarObject and PyCode_Type.tp_itemsize is non-zero: it's equal to tp_itemsize = sizeof(_Py_CODEUNIT) (2 bytes).

PyCode_Type.tp_basicsize = offsetof(PyCodeObject, co_code_adaptive) (160 bytes), whereas sizeof(PyCodeObject) is 168 bytes.

PyCode_New() calls PyObject_NewVar(PyCodeObject, &PyCode_Type, size) to allocate memory of a new code object. This function computes the size of the allocated memory block with: type->tp_basicsize + type->tp_itemsize * number_of_items (the full implementation aligns the size to sizeof(void*): round towards infinity).

#define _PyObject_VAR_SIZE(typeobj, nitems)     \
    _Py_SIZE_ROUND_UP((typeobj)->tp_basicsize + \
        (nitems)*(typeobj)->tp_itemsize,        \
        SIZEOF_VOID_P)

By the way, the type defines a __sizeof__() method since "extra" data can be attached dynamically to a code object (PEP 523):

static PyObject *
code_sizeof(PyCodeObject *co, PyObject *Py_UNUSED(args))
{
    Py_ssize_t res = _PyObject_VAR_SIZE(Py_TYPE(co), Py_SIZE(co));

    _PyCodeObjectExtra *co_extra = (_PyCodeObjectExtra*) co->co_extra;
    if (co_extra != NULL) {
        res += sizeof(_PyCodeObjectExtra) +
               (co_extra->ce_size-1) * sizeof(co_extra->ce_extras[0]);
    }

    return PyLong_FromSsize_t(res);
}

I'm not sure if you can use variable size classes with as cdef classes - you certainly can't inherit from then. It may be that this change just means that you can't use CodeType as a cdef extern class

The reproducer was extracted from gevent Cython code which works well on Python 3.10:

https://github.com/gevent/gevent/blob/5858d226045f701deb8e4b49c03db4cab63053d6/src/gevent/_gevent_cgreenlet.pxd#L55-L58

The code fails on Python 3.11 which changed how PyCode_Type.tp_basicsize is defined.

The last member of the PyCodeObject structure is an array of 1 item:

struct PyCodeObject {
   ...
   char co_code_adaptive[1];
}

PyCode_Type.tp_basicsize = offsetof(PyCodeObject, co_code_adaptive) ignores the size of this member, whereas sizeof(PyCodeObject) includes the size of this array of 1 item.

See also the discussion about the undefined behavior related to flexible arrays like this one: python/cpython#84301

Cython/Compiler/ModuleNode.py emits calls to __Pyx_ImportType() with sizeof(type). For example, on the reproducer, Cython generates:

static PyTypeObject *__pyx_ptype_5myext_CodeType = 0;

static int __Pyx_modinit_type_init_code(void) {
  ...
  __pyx_ptype_5myext_CodeType = __Pyx_ImportType(__pyx_t_1, "types", "CodeType", sizeof(PyCodeObject), __Pyx_ImportType_CheckSize_Warn);
  ...
}

Function called by __pyx_pymod_exec_myext() which is registered as a Py_mod_exec slot in PyModuleDef.m.slots.

PyBytes_Type.tp_itemsize is set to sizeof(char) (1 byte) and PyBytes_Type.tp_basicsize is set to PyBytesObject_SIZE which is defined as:

/* PyBytesObject_SIZE gives the basic size of a bytes object; any memory allocation
   for a bytes object of length n should request PyBytesObject_SIZE + n bytes.

   Using PyBytesObject_SIZE instead of sizeof(PyBytesObject) saves
   3 or 7 bytes per bytes object allocation on a typical system.
*/
#define PyBytesObject_SIZE (offsetof(PyBytesObject, ob_sval) + 1)

The PyBytesObject structure also ends with an array of 1 item:

typedef struct {
    PyObject_VAR_HEAD
    Py_hash_t ob_shash;
    char ob_sval[1];
} PyBytesObject;

I created the following issue in the Python bug tracker: python/cpython#93585

In bytes , the extra item is for the terminating NUL, which isn't part of tp_size.

It sounds broadly like there might be two acceptable (and widely used) sizes for the struct of a variably sized PyObject:

tp_basicsize and
tp_basicsize+tp_itemsize

In that was we should probably accept either?

The size check is mainly a sanity check anyway so we can probably afford to be a bit lenient in this case (at the cost of a few false negatives).

The patch to Fedora looks like a good starting point (it isn't quite what I'm proposing, but it looks fairly close)

The other thing is that there is a check_size keyword for extern extension types (https://cython.readthedocs.io/en/latest/src/userguide/extension_types.html#name-specification-clause)

That doesn't help you here because even ignore only allows the struct to be smaller, not larger. But we could always add more options (to be used at the user's own risk)

In this particular case, tp_basicsize+tp_itemsize won't work because PyCodeObject's itemsize is sizeof(_Py_CODEUNIT) while the struct has char[]. I'm not sure why exactly, but it's probably a rare edge case. In most cases, allowing tp_basicsize+tp_itemsize should work.

Another option that comes to mind is adding a directive that would make Cython use offsetof(object_struct_name, items_member) instead of sizeof.

An option to disable the check entirely, or to allow smaller structs, would probably help in dealing with future edge cases.

Actually, it looks like gevent code can be simplified to:

cdef extern from "Python.h":
    ctypedef class types.CodeType [object PyCodeObject]:
        pass
    ctypedef class types.FrameType [object PyFrameObject]:
        pass

    CodeType PyFrame_GetCode(FrameType frame)

i.e. nothing is actually needed from the structs, but the C API deals with PyCodeObject* and PyFrameObject*.
In the future, the structs might even become opaque (just typedef struct PyCodeObject PyCodeObject;), so sizeof() will stop working.
I don't know if Cython has a way to deal with classes backed by an opaque struct?

I don't know if Cython has a way to deal with classes backed by an opaque struct?

Just pass an untyped object? If Cython doesn't need to know or care about direct access to any of the members then there's no value in using a cdef class

That gives compiler warnings when passing it to PyCodeObject *PyFrame_GetCode(PyFrameObject *frame), but I guess those can be removed with casts.

Thank you for your help! This issue no longer blocks our effort to get gevent working with 3.11, though the itemsize/basicsize check could still be improved.

In this particular case, tp_basicsize+tp_itemsize won't work because PyCodeObject's itemsize is sizeof(_Py_CODEUNIT) while the struct has char[]. I'm not sure why exactly, but it's probably a rare edge case. In most cases, allowing tp_basicsize+tp_itemsize should work.

That suggests to me that the check is "between tp_basicsize and tp_basicsize+tp_itemsize (inclusive)".

Another option that comes to mind is adding a directive that would make Cython use offsetof(object_struct_name, items_member) instead of sizeof.

My feeling is that this would be a bit complex to express.

An option to disable the check entirely, or to allow smaller structs, would probably help in dealing with future edge cases.

That's probably the way to go. I think we've resisted it because we can foresee endless bug reports of "I got this error message, so I disabled the check, and now my program is crashing for some reason". But there are clearly cases where the check isn't suitable and causes problems.

Handle extern PyVarObjects

An option to disable the check entirely, or to allow smaller structs, would probably help in dealing with future edge cases.

That's probably the way to go. I think we've resisted it because we can foresee endless bug reports of "I got this error message, so I disabled the check, and now my program is crashing for some reason". But there are clearly cases where the check isn't suitable and causes problems.

Let's wait for feedback on the new size checks from #4869. Adding an option always means admitting defeat regarding a proper solution. In the end, this is a safety check meant to produce error messages instead of crashes. That's what it still does, I think, especially with a strict size check in the case of struct inheritance.

Give better errors on size changes of PyVarObjects and reduce false p…

@vstinner

So, allowing any size for sizeof(PyCodeObject) that is between tp_basicsize and tp_basicsize + tp_itemsize still fails in Py3.11, message being Expected 192 from C header, got 186 from PyObject. Note how sizeof(PyCodeObject) is 6 bytes larger than even tp_basicsize + tp_itemsize (GHA CI, 64 bit Linux). I wonder how that's possible.

The new struct ends with this (SIZE == 1):

    char *_co_linearray;          /* array of line offsets */                  \
    int _co_firsttraceable;       /* index of first traceable instruction */   \
    /* Scratch space for extra data relating to the code object.               \
       Type is a void* to keep the format private in codeobject.c to force     \
       people to go through the proper APIs. */                                \
    void *co_extra;                                                            \
    char co_code_adaptive[(SIZE)];                                             \

So, there's an int between two pointers, which probably induces some alignment space, but then I wonder if sizeof() also adds padding after the last struct field? One code unit has size 2, so 2 item bytes where the aligned struct expected 8 would explain the difference of 6.

@vstinner @encukou Do you think that's what happens here?

I wonder if sizeof() also adds padding after the last struct field?

Yes, it does :(
It seems that the upper bound should be rounded up to the nearest alignof(the struct) boundary. (But alignof is C11. For older C... well, people tend to get away with using sizeof(long long) as maximum alignment, or even hardcoding it to 64 – even on platforms where long long has 128-byte alignment, since long long is rare in complex structs.)

Adding an option always means admitting defeat regarding a proper solution. In the end, this is a safety check meant to produce error messages instead of crashes.

It's good to know there is possibility to admit defeat :)

This has been resolved by weakening the conditions in #4894

Navigation Menu

Repository navigation

[BUG] Using types.CodeType in Python 3.11 fails with ValueError: types.CodeType size changed, may indicate binary incompatibility. Expected 168 from C header, got 160 from PyObject #4827

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

[BUG] Using types.CodeType in Python 3.11 fails with ValueError: types.CodeType size changed, may indicate binary incompatibility. Expected 168 from C header, got 160 from PyObject #4827

Description

Activity

vstinner commented on Jun 7, 2022

da-woods commented on Jun 7, 2022

vstinner commented on Jun 7, 2022

vstinner commented on Jun 7, 2022

vstinner commented on Jun 7, 2022

vstinner commented on Jun 7, 2022

vstinner commented on Jun 7, 2022

vstinner commented on Jun 7, 2022

encukou commented on Jun 7, 2022

da-woods commented on Jun 7, 2022

da-woods commented on Jun 7, 2022

encukou commented on Jun 8, 2022

encukou commented on Jun 8, 2022

da-woods commented on Jun 8, 2022

encukou commented on Jun 8, 2022

da-woods commented on Jun 8, 2022

scoder commented on Jul 16, 2022

scoder commented on Jul 16, 2022

encukou commented on Jul 18, 2022

scoder commented on Jul 31, 2022

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions