From ba08a29dc440d20a54ae0c764bc26e702f1de6e2 Mon Sep 17 00:00:00 2001 From: Armin Rigo Date: Wed, 20 Dec 2023 13:12:28 +0000 Subject: [PATCH 1/3] pass API-mode function wrappers to places that expect a cdata pointer --- src/c/_cffi_backend.c | 31 +++++++++++++++++++-- src/c/lib_obj.c | 47 +++++++++++++++++++++++--------- testing/cffi1/test_recompiler.py | 46 +++++++++++++++++++++++++++++++ 3 files changed, 109 insertions(+), 15 deletions(-) diff --git a/src/c/_cffi_backend.c b/src/c/_cffi_backend.c index 7c72ffe0..54523e32 100644 --- a/src/c/_cffi_backend.c +++ b/src/c/_cffi_backend.c @@ -1613,6 +1613,8 @@ convert_struct_from_object(char *data, CTypeDescrObject *ct, PyObject *init, return _convert_error(init, ct, expected); } +static PyObject* try_extract_directfnptr(PyObject *x); /* forward */ + #ifdef __GNUC__ # if __GNUC__ >= 4 /* Don't go inlining this huge function. Needed because occasionally @@ -1640,9 +1642,15 @@ convert_from_object(char *data, CTypeDescrObject *ct, PyObject *init) CTypeDescrObject *ctinit; if (!CData_Check(init)) { - expected = "cdata pointer"; - goto cannot_convert; + init = try_extract_directfnptr(init); + if (init == NULL || !CData_Check(init)) { + if (PyErr_Occurred()) + return -1; + expected = "cdata pointer"; + goto cannot_convert; + } } + ctinit = ((CDataObject *)init)->c_type; if (!(ctinit->ct_flags & (CT_POINTER|CT_FUNCTIONPTR))) { if (ctinit->ct_flags & CT_ARRAY) @@ -4081,10 +4089,20 @@ static CDataObject *cast_to_integer_or_char(CTypeDescrObject *ct, PyObject *ob) value = res; } else { + if (PyCFunction_Check(ob)) { + PyObject *func_cdata = try_extract_directfnptr(ob); + if (func_cdata != NULL && CData_Check(func_cdata)) { + value = (Py_intptr_t)((CDataObject *)func_cdata)->c_data; + goto got_value; + } + if (PyErr_Occurred()) + return NULL; + } value = _my_PyLong_AsUnsignedLongLong(ob, 0); if (value == (unsigned PY_LONG_LONG)-1 && PyErr_Occurred()) return NULL; } + got_value: if (ct->ct_flags & CT_IS_BOOL) value = !!value; cd = _new_casted_primitive(ct); @@ -4141,6 +4159,15 @@ static PyObject *do_cast(CTypeDescrObject *ct, PyObject *ob) return new_simple_cdata(cdsrc->c_data, ct); } } + if (PyCFunction_Check(ob)) { + PyObject *func_cdata = try_extract_directfnptr(ob); + if (func_cdata != NULL && CData_Check(func_cdata)) { + char *ptr = ((CDataObject *)func_cdata)->c_data; + return new_simple_cdata(ptr, ct); + } + if (PyErr_Occurred()) + return NULL; + } if ((ct->ct_flags & CT_POINTER) && (ct->ct_itemdescr->ct_flags & CT_IS_FILE) && PyFile_Check(ob)) { diff --git a/src/c/lib_obj.c b/src/c/lib_obj.c index 38bf3d55..bd9ba3fc 100644 --- a/src/c/lib_obj.c +++ b/src/c/lib_obj.c @@ -18,6 +18,7 @@ struct CPyExtFunc_s { PyMethodDef md; void *direct_fn; + PyObject *direct_fn_cdata; int type_index; char doc[1]; }; @@ -662,6 +663,31 @@ static LibObject *lib_internal_new(FFIObject *ffi, const char *module_name, return NULL; } +static PyObject* try_extract_directfnptr(PyObject *x) +{ + /* returns: borrowed ref or NULL */ + LibObject *lib; + PyObject *ct; + struct CPyExtFunc_s *exf = _cpyextfunc_get(x); + if (exf == NULL) + return NULL; /* wrong type */ + if (exf->direct_fn_cdata != NULL) + return exf->direct_fn_cdata; /* common case: cached */ + + if (exf->direct_fn == NULL) + return x; /* backward compatibility: no direct_fn */ + + lib = (LibObject *)PyCFunction_GET_SELF(x); + ct = _cpyextfunc_type(lib, exf); + if (ct == NULL) + return NULL; /* error */ + + x = new_simple_cdata(exf->direct_fn, (CTypeDescrObject *)ct); + Py_DECREF(ct); + exf->direct_fn_cdata = x; /* caches x, which becomes immortal like exf */ + return x; +} + static PyObject *address_of_global_var(PyObject *args) { LibObject *lib; @@ -683,20 +709,15 @@ static PyObject *address_of_global_var(PyObject *args) return cg_addressof_global_var((GlobSupportObject *)x); } else { - struct CPyExtFunc_s *exf = _cpyextfunc_get(x); - if (exf != NULL) { /* an OP_CPYTHON_BLTN: '&func' returns a cdata */ - PyObject *ct; - if (exf->direct_fn == NULL) { - Py_INCREF(x); /* backward compatibility */ - return x; - } - ct = _cpyextfunc_type(lib, exf); - if (ct == NULL) - return NULL; - x = new_simple_cdata(exf->direct_fn, (CTypeDescrObject *)ct); - Py_DECREF(ct); - return x; + PyObject *func_cdata = try_extract_directfnptr(x); + if (func_cdata != NULL) { + /* an OP_CPYTHON_BLTN: '&func' returns a cdata */ + Py_INCREF(func_cdata); + return func_cdata; } + if (PyErr_Occurred()) + return NULL; + if (CData_Check(x) && /* a constant functionptr cdata: 'f == &f' */ (((CDataObject *)x)->c_type->ct_flags & CT_FUNCTIONPTR) != 0) { Py_INCREF(x); diff --git a/testing/cffi1/test_recompiler.py b/testing/cffi1/test_recompiler.py index 362a3722..c7f505e4 100644 --- a/testing/cffi1/test_recompiler.py +++ b/testing/cffi1/test_recompiler.py @@ -2493,3 +2493,49 @@ def test_passing_large_list(): arg = list(range(20000000)) lib.passing_large_list(arg) # assert did not segfault + +def test_convert_api_mode_builtin_function_to_cdata(): + ffi = FFI() + ffi.cdef( + """struct s { int x; }; + struct s add1(struct s); struct s add2(struct s); + int mycall(struct s(*)(struct s)); int mycall2(void *);""") + lib = verify(ffi, "test_convert_api_mode_builtin_function_to_cdata", """ + struct s { int x; }; + static struct s add1(struct s a) { + struct s r; r.x = a.x + 1; return r; + } + static struct s add2(struct s a) { + struct s r; r.x = a.x + 2; return r; + } + static int mycall(struct s(*cb)(struct s)) { + struct s a; a.x = 100; + return cb(a).x; + } + static int mycall2(void *cb) { + struct s a; a.x = 200; + return ((struct s(*)(struct s))cb)(a).x; + } + """) + s_ptr = ffi.new("struct s *", [42]) + s = s_ptr[0] + assert lib.add1(s).x == 43 + assert lib.add2(s).x == 44 + assert lib.mycall(lib.add1) == 101 + assert lib.mycall(lib.add2) == 102 + assert lib.mycall2(lib.add1) == 201 + assert lib.mycall2(lib.add2) == 202 + s.x = -42 + my_array = ffi.new("struct s(*[2])(struct s)") + my_array[0] = lib.add1 + my_array[1] = lib.add2 + assert my_array[0](s).x == -41 + assert my_array[1](s).x == -40 + s.x = 84 + p = ffi.cast("void *", lib.add1) + assert ffi.cast("struct s(*)(struct s)", p)(s).x == 85 + q = ffi.cast("intptr_t", lib.add2) + assert ffi.cast("struct s(*)(struct s)", q)(s).x == 86 + s.x = 300 + my_array_2 = ffi.new("void *[]", [lib.add1, lib.add2]) + assert ffi.cast("struct s(*)(struct s)", my_array_2[1])(s).x == 302 From 82e93242ebbfa3d7d9684dc18f0fc2406054c8a1 Mon Sep 17 00:00:00 2001 From: Armin Rigo Date: Wed, 20 Dec 2023 13:25:16 +0000 Subject: [PATCH 2/3] bug fix --- src/c/_cffi_backend.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/c/_cffi_backend.c b/src/c/_cffi_backend.c index 54523e32..5c6fb8bc 100644 --- a/src/c/_cffi_backend.c +++ b/src/c/_cffi_backend.c @@ -1642,8 +1642,11 @@ convert_from_object(char *data, CTypeDescrObject *ct, PyObject *init) CTypeDescrObject *ctinit; if (!CData_Check(init)) { - init = try_extract_directfnptr(init); - if (init == NULL || !CData_Check(init)) { + PyObject *func_cdata = try_extract_directfnptr(init); + if (func_cdata != NULL && CData_Check(func_cdata)) { + init = func_cdata; + } + else { if (PyErr_Occurred()) return -1; expected = "cdata pointer"; From 7aed22fc2bee1f102b3dfbfc43cd8e3c391c2d2e Mon Sep 17 00:00:00 2001 From: Armin Rigo Date: Wed, 20 Dec 2023 14:42:46 +0000 Subject: [PATCH 3/3] update the documentation --- doc/source/ref.rst | 31 +++++++++++++++++++++++++++---- testing/cffi1/test_recompiler.py | 1 + 2 files changed, 28 insertions(+), 4 deletions(-) diff --git a/doc/source/ref.rst b/doc/source/ref.rst index 946e48c7..6c7e017c 100644 --- a/doc/source/ref.rst +++ b/doc/source/ref.rst @@ -377,7 +377,16 @@ and in C, where ``&array[index]`` is just ``array + index``. 3. ``ffi.addressof(, "name")`` returns the address of the named function or global variable from the given library object. For functions, it returns a regular cdata -object containing a pointer to the function. +object containing a pointer to the function. Note that in API +mode, this is not the same as just writing `lib.funcname`: the latter +returns a special object that (before version 1.17) can +mostly only be called. *New in version 1.17:* you can now use +`lib.funcname` in many places where a `` object was required, +so using `ffi.addressof(lib, "funcname")` is generally not needed any +more. For example, you can now pass `lib.funcname` as a callback to +a C function call, or write it inside a C structure field of the +correct pointer-to-function type, or use `ffi.cast()` or +`ffi.typeof()` on it. Note that the case 1. cannot be used to take the address of a primitive or pointer, but only a struct or union. It would be @@ -807,15 +816,15 @@ allowed. +---------------+------------------------+ | | | ``void *`` | another with | | | | | any pointer or array | | | -| | type | | | +| | type, or `[9]` | | | +---------------+------------------------+ +----------------+ | pointers to | same as pointers | | ``[]``, ``+``, | | structure or | | | ``-``, bool(), | | union | | | and read/write | | | | | struct fields | +---------------+------------------------+ +----------------+ -| function | same as pointers | | bool(), | -| pointers | | | call `[2]` | +| function | same as pointers, | | bool(), | +| pointers | or `[9]` | | call `[2]` | +---------------+------------------------+------------------+----------------+ | arrays | a list or tuple of | a |len(), iter(), | | | items | |``[]`` `[4]`, | @@ -946,6 +955,20 @@ allowed. See `Unicode character types`_ below. +`[9]` API-mode function from `lib.myfunc`: + + In API mode, when you get a function from a C library by writing + `fn = lib.myfunc`, you get an object of a special type for performance + reasons, instead of a ``. Before version 1.17 + you could only call such objects. You could write + `ffi.addressof(lib, "myfunc")` in order to get a real `` object, + based on the idea that in these cases in C you'd usually write `&myfunc` + instead of `myfunc`. *New in version 1.17:* the special object + `lib.myfunc` can now be passed in many places where CFFI expects + a regular `` object. For example, you can now pass + it as a callback to a C function call, or write it inside a C + structure field of the correct pointer-to-function type, or use + `ffi.cast()` or `ffi.typeof()` on it. .. _file: diff --git a/testing/cffi1/test_recompiler.py b/testing/cffi1/test_recompiler.py index c7f505e4..e52752ba 100644 --- a/testing/cffi1/test_recompiler.py +++ b/testing/cffi1/test_recompiler.py @@ -2539,3 +2539,4 @@ def test_convert_api_mode_builtin_function_to_cdata(): s.x = 300 my_array_2 = ffi.new("void *[]", [lib.add1, lib.add2]) assert ffi.cast("struct s(*)(struct s)", my_array_2[1])(s).x == 302 + assert ffi.typeof(lib.add1) == ffi.typeof("struct s(*)(struct s)")