Skip to content

Commit b6f9a50

Browse files
authored
gh-156933: Widen narrow integer results in ctypes callbacks (GH-157045)
_CallPythonObject() only wrote restype->size bytes into the closure's result buffer, leaving the unused high-order bits of the ffi_arg-sized register untouched. libffi's ffi_prep_closure_loc() documents that integral types narrower than a machine register must be widened to fill it, sign-extending signed types. On architectures that always read the full register for narrow return values (s390x), this leaves garbage in the high bits, which broke libclang callbacks used by cindex.py.
1 parent baec764 commit b6f9a50

4 files changed

Lines changed: 81 additions & 9 deletions

File tree

‎Lib/test/test_ctypes/test_callbacks.py‎

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@
1111
c_short, c_ushort, c_int, c_uint,
1212
c_long, c_longlong, c_ulonglong, c_ulong,
1313
c_float, c_double, c_longdouble, py_object)
14-
from ctypes.util import find_library
14+
from ctypes.util import find_library, wrap_dll_function
1515
from test import support
1616
from test.support import import_helper
1717
_ctypes_test = import_helper.import_module("_ctypes_test")
@@ -328,6 +328,20 @@ def func():
328328
f"of ctypes callback function {func!r}")
329329
self.assertIsNone(cm.unraisable.object)
330330

331+
def test_narrow_int_return_widened(self):
332+
# gh-156933: Narrow integers were not widened on s390x
333+
CALLBACK = CFUNCTYPE(c_int)
334+
335+
@wrap_dll_function(CDLL(_ctypes_test.__file__))
336+
def _testfunc_callback_int_to_longlong(func: CALLBACK) -> c_longlong:
337+
pass
338+
339+
@CALLBACK
340+
def cb():
341+
return -1
342+
343+
self.assertEqual(_testfunc_callback_int_to_longlong(cb), -1)
344+
331345

332346
if __name__ == '__main__':
333347
unittest.main()
Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
Fix incorrect integer return values from :mod:`ctypes` callbacks on some
2+
platforms, such as s390x.

‎Modules/_ctypes/_ctypes_test.c‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -638,6 +638,11 @@ EXPORT(long long) _testfunc_callback_q_qf(long long value,
638638
return sum;
639639
}
640640

641+
EXPORT(long long) _testfunc_callback_int_to_longlong(int (*func)(void))
642+
{
643+
return func();
644+
}
645+
641646
typedef struct {
642647
char *name;
643648
char *value;

‎Modules/_ctypes/callbacks.c‎

Lines changed: 59 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -101,6 +101,22 @@ TryAddRef(PyObject *cnv, CDataObject *obj)
101101
}
102102
#endif
103103

104+
static int
105+
is_narrow_int_ffi_type(int type)
106+
{
107+
switch (type) {
108+
case FFI_TYPE_SINT8:
109+
case FFI_TYPE_UINT8:
110+
case FFI_TYPE_SINT16:
111+
case FFI_TYPE_UINT16:
112+
case FFI_TYPE_SINT32:
113+
case FFI_TYPE_UINT32:
114+
return 1;
115+
default:
116+
return 0;
117+
}
118+
}
119+
104120
/******************************************************************************
105121
*
106122
* Call the python object with all arguments
@@ -222,13 +238,21 @@ static void _CallPythonObject(ctypes_state *st,
222238
if (restype != &ffi_type_void && result) {
223239
assert(setfunc);
224240

225-
#ifdef WORDS_BIGENDIAN
226-
/* See the corresponding code in _ctypes_callproc():
227-
in callproc.c, around line 1219. */
228-
if (restype->type != FFI_TYPE_FLOAT && restype->size < sizeof(ffi_arg)) {
229-
mem = (char *)mem + sizeof(ffi_arg) - restype->size;
230-
}
231-
#endif
241+
/* libffi's closure contract requires integral results narrower
242+
than ffi_arg to fill a whole register, sign-extended if signed;
243+
setfunc() only writes restype->size bytes. */
244+
union {
245+
ffi_arg arg;
246+
int8_t s8;
247+
uint8_t u8;
248+
int16_t s16;
249+
uint16_t u16;
250+
int32_t s32;
251+
uint32_t u32;
252+
} narrow_res = {0};
253+
int narrow = restype->size < sizeof(ffi_arg) &&
254+
is_narrow_int_ffi_type(restype->type);
255+
void *resmem = narrow ? (void *)&narrow_res : mem;
232256

233257
/* keep is an object we have to keep alive so that the result
234258
stays valid. If there is no such object, the setfunc will
@@ -239,7 +263,34 @@ static void _CallPythonObject(ctypes_state *st,
239263
be the result. EXCEPT when restype is py_object - Python
240264
itself knows how to manage the refcount of these objects.
241265
*/
242-
PyObject *keep = setfunc(mem, result, restype->size);
266+
PyObject *keep = setfunc(resmem, result, restype->size);
267+
268+
if (narrow && keep != NULL) {
269+
ffi_arg widened;
270+
switch (restype->type) {
271+
case FFI_TYPE_SINT8:
272+
widened = (ffi_arg)(ffi_sarg)narrow_res.s8;
273+
break;
274+
case FFI_TYPE_SINT16:
275+
widened = (ffi_arg)(ffi_sarg)narrow_res.s16;
276+
break;
277+
case FFI_TYPE_SINT32:
278+
widened = (ffi_arg)(ffi_sarg)narrow_res.s32;
279+
break;
280+
case FFI_TYPE_UINT8:
281+
widened = narrow_res.u8;
282+
break;
283+
case FFI_TYPE_UINT16:
284+
widened = narrow_res.u16;
285+
break;
286+
case FFI_TYPE_UINT32:
287+
widened = narrow_res.u32;
288+
break;
289+
default:
290+
Py_UNREACHABLE();
291+
}
292+
memcpy(mem, &widened, sizeof(ffi_arg));
293+
}
243294

244295
if (keep == NULL) {
245296
/* Could not convert callback result. */

0 commit comments

Comments
 (0)