Skip to content

Commit

Permalink
[CPyCppyy] Fix regression in passing buffers through void *
Browse files Browse the repository at this point in the history
Credit goes to Wim Lavrijsen (@wlav).

See also: wlav/cppyy#272.
  • Loading branch information
guitargeek committed Nov 8, 2024
1 parent 6543e55 commit 8b48528
Show file tree
Hide file tree
Showing 9 changed files with 133 additions and 123 deletions.
4 changes: 2 additions & 2 deletions bindings/pyroot/cppyy/CPyCppyy/src/CPyCppyy.h
Original file line number Diff line number Diff line change
Expand Up @@ -352,7 +352,7 @@ inline PyObject* CPyCppyy_tp_call(PyObject* cb, PyObject* args, size_t, PyObject
#endif

// weakref forced strong reference
#if PY_VERSION_HEX < 0x30d00f0
#if PY_VERSION_HEX < 0x30d0000
static inline PyObject* CPyCppyy_GetWeakRef(PyObject* ref) {
PyObject* pyobject = PyWeakref_GetObject(ref);
if (!pyobject || pyobject == Py_None)
Expand All @@ -370,7 +370,7 @@ static inline PyObject* CPyCppyy_GetWeakRef(PyObject* ref) {
#endif

// Py_TYPE as inline function
#if PY_VERSION_HEX < 0x030900A4 && !defined(Py_SET_TYPE)
#if PY_VERSION_HEX < 0x03090000 && !defined(Py_SET_TYPE)
static inline
void _Py_SET_TYPE(PyObject *ob, PyTypeObject *type) { ob->ob_type = type; }
#define Py_SET_TYPE(ob, type) _Py_SET_TYPE((PyObject*)(ob), type)
Expand Down
71 changes: 71 additions & 0 deletions bindings/pyroot/cppyy/CPyCppyy/src/Converters.cxx
Original file line number Diff line number Diff line change
Expand Up @@ -203,6 +203,62 @@ static bool IsPyCArgObject(PyObject* pyobject)
return Py_TYPE(pyobject) == pycarg_type;
}

#if PY_VERSION_HEX < 0x30d0000
static bool IsCTypesArrayOrPointer(PyObject* pyobject)
{
static PyTypeObject* cstgdict_type = nullptr;
if (!cstgdict_type) {
// get any pointer type to initialize the extended dictionary type
PyTypeObject* ct_int = GetCTypesType(ct_c_int);
if (ct_int && ct_int->tp_dict) {
cstgdict_type = Py_TYPE(ct_int->tp_dict);
}
}

PyTypeObject* pytype = Py_TYPE(pyobject);
if (pytype->tp_dict && Py_TYPE(pytype->tp_dict) == cstgdict_type)
return true;
return false;
}
#else
// the internals of ctypes have been redone, requiring a more complex checking
namespace {

typedef struct {
PyTypeObject *DictRemover_Type;
PyTypeObject *PyCArg_Type;
PyTypeObject *PyCField_Type;
PyTypeObject *PyCThunk_Type;
PyTypeObject *StructParam_Type;
PyTypeObject *PyCType_Type;
PyTypeObject *PyCStructType_Type;
PyTypeObject *UnionType_Type;
PyTypeObject *PyCPointerType_Type;
// ... unused fields omitted ...
} _cppyy_ctypes_state;

} // unnamed namespace

static bool IsCTypesArrayOrPointer(PyObject* pyobject)
{
static _cppyy_ctypes_state* state = nullptr;
if (!state) {
PyObject* ctmod = PyImport_AddModule("_ctypes"); // the extension module, not the Python one
if (ctmod)
state = (_cppyy_ctypes_state*)PyModule_GetState(ctmod);
}

// verify for object types that have a C payload
if (state && (PyObject_IsInstance((PyObject*)Py_TYPE(pyobject), (PyObject*)state->PyCType_Type) ||
PyObject_IsInstance((PyObject*)Py_TYPE(pyobject), (PyObject*)state->PyCPointerType_Type))) {
return true;
}

return false;
}
#endif


//- helper to establish life lines -------------------------------------------
static inline bool SetLifeLine(PyObject* holder, PyObject* target, intptr_t ref)
{
Expand Down Expand Up @@ -1488,6 +1544,16 @@ bool CPyCppyy::VoidArrayConverter::SetArg(
return true;
}

// allow any other ctypes pointer type
if (IsCTypesArrayOrPointer(pyobject)) {
void** payload = (void**)((CPyCppyy_tagCDataObject*)pyobject)->b_ptr;
if (payload) {
para.fValue.fVoidp = *payload;
para.fTypeCode = 'p';
return true;
}
}

// final try: attempt to get buffer
Py_ssize_t buflen = Utility::GetBuffer(pyobject, '*', 1, para.fValue.fVoidp, false);

Expand Down Expand Up @@ -1600,6 +1666,11 @@ bool CPyCppyy::name##ArrayConverter::SetArg( \
para.fValue.fVoidp = (void*)((CPyCppyy_tagCDataObject*)pyobject)->b_ptr;\
para.fTypeCode = 'p'; \
convOk = true; \
} else if (Py_TYPE(pyobject) == GetCTypesType(ct_c_void_p)) { \
/* special case: pass address of c_void_p buffer to return the address */\
para.fValue.fVoidp = (void*)((CPyCppyy_tagCDataObject*)pyobject)->b_ptr;\
para.fTypeCode = 'p'; \
convOk = true; \
} else if (LowLevelView_Check(pyobject) && \
((LowLevelView*)pyobject)->fBufInfo.ndim == 2 && \
strchr(((LowLevelView*)pyobject)->fBufInfo.format, code)) { \
Expand Down
66 changes: 45 additions & 21 deletions bindings/pyroot/cppyy/CPyCppyy/src/LowLevelViews.cxx
Original file line number Diff line number Diff line change
Expand Up @@ -817,38 +817,62 @@ static PyObject* ll_reshape(CPyCppyy::LowLevelView* self, PyObject* shape)


//---------------------------------------------------------------------------
static PyObject* ll_array(CPyCppyy::LowLevelView* self, PyObject* args, PyObject* /* kwds */)
static PyObject* ll_array(CPyCppyy::LowLevelView* self, PyObject* args, PyObject* kwds)
{
// Construct a numpy array from the lowlevelview (w/o copy if possible); this
// uses the Python methods to avoid depending on numpy directly

// Expect as most a dtype from the arguments;
static PyObject* ctmod = PyImport_ImportModule("numpy"); // ref-count kept
if (!ctmod)
static PyObject* npmod = PyImport_ImportModule("numpy"); // ref-count kept
if (!npmod)
return nullptr;

// expect possible dtype from the arguments, otherwie take it from the type code
PyObject* dtype;
if (!args || PyTuple_GET_SIZE(args) != 1) {
PyObject* npdtype = PyObject_GetAttr(ctmod, CPyCppyy::PyStrings::gDType);
PyObject* typecode = ll_typecode(self, nullptr);
dtype = PyObject_CallFunctionObjArgs(npdtype, typecode, nullptr);
Py_DECREF(typecode);
Py_DECREF(npdtype);
} else {
dtype = PyTuple_GET_ITEM(args, 0);
Py_INCREF(dtype);
bool docopy = false;
if (kwds) {
PyObject* pycp = PyObject_GetItem(kwds, CPyCppyy::PyStrings::gCopy);
if (!pycp) {
PyErr_SetString(PyExc_TypeError, "__array__ only supports the \"copy\" keyword");
return nullptr;
}

docopy = PyObject_IsTrue(pycp);
Py_DECREF(pycp);
}

if (!dtype)
return nullptr;
if (!docopy) { // view requested
// expect possible dtype from the arguments, otherwise take it from the type code
PyObject* dtype;
if (!args || PyTuple_GET_SIZE(args) != 1) {
PyObject* npdtype = PyObject_GetAttr(npmod, CPyCppyy::PyStrings::gDType);
PyObject* typecode = ll_typecode(self, nullptr);
dtype = PyObject_CallFunctionObjArgs(npdtype, typecode, nullptr);
Py_DECREF(typecode);
Py_DECREF(npdtype);
} else {
dtype = PyTuple_GET_ITEM(args, 0);
Py_INCREF(dtype);
}

PyObject* npfrombuf = PyObject_GetAttr(ctmod, CPyCppyy::PyStrings::gFromBuffer);
PyObject* view = PyObject_CallFunctionObjArgs(npfrombuf, (PyObject*)self, dtype, nullptr);
Py_DECREF(dtype);
Py_DECREF(npfrombuf);
if (!dtype)
return nullptr;

PyObject* npfrombuf = PyObject_GetAttr(npmod, CPyCppyy::PyStrings::gFromBuffer);
PyObject* view = PyObject_CallFunctionObjArgs(npfrombuf, (PyObject*)self, dtype, nullptr);
Py_DECREF(dtype);
Py_DECREF(npfrombuf);

return view;

} else { // copy requested
PyObject* npcopy = PyObject_GetAttr(npmod, CPyCppyy::PyStrings::gCopy);
PyObject* newarr = PyObject_CallFunctionObjArgs(npcopy, (PyObject*)self, nullptr);
Py_DECREF(npcopy);

return view;
return newarr;
}

// never get here
return nullptr;
}


Expand Down
3 changes: 3 additions & 0 deletions bindings/pyroot/cppyy/CPyCppyy/src/PyStrings.cxx
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
PyObject* CPyCppyy::PyStrings::gAssign = nullptr;
PyObject* CPyCppyy::PyStrings::gBases = nullptr;
PyObject* CPyCppyy::PyStrings::gBase = nullptr;
PyObject* CPyCppyy::PyStrings::gCopy = nullptr;
PyObject* CPyCppyy::PyStrings::gCppBool = nullptr;
PyObject* CPyCppyy::PyStrings::gCppName = nullptr;
PyObject* CPyCppyy::PyStrings::gAnnotations = nullptr;
Expand Down Expand Up @@ -87,6 +88,7 @@ bool CPyCppyy::CreatePyStrings() {
CPPYY_INITIALIZE_STRING(gAssign, __assign__);
CPPYY_INITIALIZE_STRING(gBases, __bases__);
CPPYY_INITIALIZE_STRING(gBase, __base__);
CPPYY_INITIALIZE_STRING(gCopy, copy);
#if PY_VERSION_HEX < 0x03000000
CPPYY_INITIALIZE_STRING(gCppBool, __cpp_nonzero__);
#else
Expand Down Expand Up @@ -169,6 +171,7 @@ PyObject* CPyCppyy::DestroyPyStrings() {
// Remove all cached python strings.
Py_DECREF(PyStrings::gBases); PyStrings::gBases = nullptr;
Py_DECREF(PyStrings::gBase); PyStrings::gBase = nullptr;
Py_DECREF(PyStrings::gCopy); PyStrings::gCopy = nullptr;
Py_DECREF(PyStrings::gCppBool); PyStrings::gCppBool = nullptr;
Py_DECREF(PyStrings::gCppName); PyStrings::gCppName = nullptr;
Py_DECREF(PyStrings::gAnnotations); PyStrings::gAnnotations = nullptr;
Expand Down
1 change: 1 addition & 0 deletions bindings/pyroot/cppyy/CPyCppyy/src/PyStrings.h
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ namespace PyStrings {
extern PyObject* gAssign;
extern PyObject* gBases;
extern PyObject* gBase;
extern PyObject* gCopy;
extern PyObject* gCppBool;
extern PyObject* gCppName;
extern PyObject* gAnnotations;
Expand Down
9 changes: 5 additions & 4 deletions bindings/pyroot/cppyy/CPyCppyy/src/Pythonize.cxx
Original file line number Diff line number Diff line change
Expand Up @@ -528,13 +528,14 @@ PyObject* VectorData(PyObject* self, PyObject*)


//---------------------------------------------------------------------------
PyObject* VectorArray(PyObject* self, PyObject* /* args */, PyObject* /* kwargs */)
PyObject* VectorArray(PyObject* self, PyObject* args, PyObject* kwargs)
{
// TODO: kwargs are ignored as the only known use is NumPy passing copy=False
PyObject* pydata = VectorData(self, nullptr);
PyObject* view = PyObject_CallMethodNoArgs(pydata, PyStrings::gArray);
PyObject* arrcall = PyObject_GetAttr(pydata, PyStrings::gArray);
PyObject* newarr = PyObject_Call(arrcall, args, kwargs);
Py_DECREF(arrcall);
Py_DECREF(pydata);
return view;
return newarr;
}


Expand Down
8 changes: 6 additions & 2 deletions bindings/pyroot/cppyy/cppyy/test/test_regression.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,12 @@ class TestREGRESSION:
def setup_class(cls):
import cppyy

def stringpager(text, cls=cls):
cls.helpout.append(text)
if sys.hexversion < 0x30d0000:
def stringpager(text, cls=cls):
cls.helpout.append(text)
else:
def stringpager(text, title='', cls=cls):
cls.helpout.append(text)

import pydoc
pydoc.pager = stringpager
Expand Down

This file was deleted.

1 change: 0 additions & 1 deletion bindings/pyroot/cppyy/sync-upstream
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,6 @@ git apply patches/CPyCppyy-Adapt-to-no-std-in-ROOT.patch
git apply patches/CPyCppyy-Always-convert-returned-std-string.patch
git apply patches/CPyCppyy-Disable-implicit-conversion-to-smart-ptr.patch
git apply patches/CPyCppyy-TString_converter.patch
git apply patches/CPyCppyy-Remove-implicit-conversion-of-any-ctypes-ptr.patch
git apply patches/CPyCppyy-Prevent-construction-of-agg-init-for-tuple.patch
git apply patches/cppyy-No-CppyyLegacy-namespace.patch
git apply patches/cppyy-Remove-Windows-workaround.patch
Expand Down

0 comments on commit 8b48528

Please sign in to comment.