C-API: Allocating "PyTypeObject-extension"

Question

I have found some code in PyCXX that may be buggy.

Is it indeed a bug, and if so, what is the right way to fix it?

Here is the problem:

struct PythonClassInstance
{
    PyObject_HEAD
    ExtObjBase* m_pycxx_object;
}
:
{
    :
    table->tp_new = extension_object_new; // PyTypeObject
    :
}
:
static PyObject* extension_object_new( 
                 PyTypeObject* subtype, PyObject* args, PyObject* kwds )
{
    PythonClassInstance* o = reinterpret_cast<PythonClassInstance *>
                                       ( subtype->tp_alloc(subtype,0) );
    if( ! o )
        return nullptr;

    o->m_pycxx_object = nullptr;

    PyObject* self = reinterpret_cast<PyObject* >( o );

    return self;
}

Now PyObject_HEAD expands to "PyObject ob_base;", so clearly PythonClassInstance trivially extends PyObject to contain an extra pointer (which will point to PyCXX's representation for this PyObject)

tp_alloc allocates memory for storing a PyObject

The code then typecasts this pointer to a PythonClassInstance, laying claim to an extra 4(or 8?) bytes that it does not own!

And then it sets this extra memory to 0.

This looks very dangerous, and I'm surprised the bug has gone unnoticed. The risk is that some future object will get placed in this location (that is meant to be storing the ExtObjBase*).

How to fix it?

PythonClassInstance foo{};

PyObject* tmp = subtype->tp_alloc(subtype,0);

// !!! memcpy sizeof(PyObject) bytes starting from location tmp into location (void*)foo

But I think now maybe I need to release tmp, and I don't think I should be playing with memory directly like this. I feel like it could be jeopardising Python's memory management/garbage collection inbuilt machinery.

The other option is maybe I can persuade tp_alloc to allocate 4 extra bytes (or is it 8 now; enough for a pointer) bypassing in 1 instead of 0.

Documentation says this second parameter is "Py_ssize_t nitems" and:

If the type’s tp_itemsize is non-zero, the object’s ob_size field should be initialized to nitems and the length of the allocated memory block should be tp_basicsize + nitemstp_itemsize, rounded up to a multiple of sizeof(void); otherwise, nitems is not used and the length of the block should be tp_basicsize.

So it looks like I should be setting:

table->tp_itemsize = sizeof(void*);
:
PyObject* tmp = subtype->tp_alloc(subtype,1);

EDIT: just tried this and it causes a crash

But then the documentation goes on to say:

Do not use this function to do any other instance initialization, not even to allocate additional memory; that should be done by tp_new.

Now I'm not sure whether this code belongs in tp_new or tp_init.

Related:

Passing arguments to tp_new and tp_init from subtypes in Python C API

Python C-API Object Allocation‏

Answer 1

Actually this is a (minor/harmless) bug in PyCXX

SO would like to convert this answer to a comment, which makes no sense I can't awarded the green tick of completion so I comment. So I have to ramble in order to qualify it. blerh.

Answer 2

The code is correct.

As long as the PyTypeObject for the extension object is properly initialized it should work.

The base class tp_alloc receives subtype so it should know how much memory to allocate by checking the tp_basicsize member.

This is a common Python C/API pattern as demonstrated int the tutorial.