Skip to content

Commit 9e29217

Browse files
dpgeorgedpgeorge
committed
unix/modffi: Use a union for passing/returning FFI values.
This fixes a bug where double arguments on a 32-bit architecture would not be passed correctly because they only had 4 bytes of storage (not 8). It also fixes a compiler warning/error in return_ffi_value on certian architectures: array subscript 'double[0]' is partly outside array bounds of 'ffi_arg[1]' {aka 'long unsigned int[1]'}. Fixes issue micropython#7064. Signed-off-by: Damien George <damien@micropython.org>
1 parent 8172c2e commit 9e29217

3 files changed

Lines changed: 50 additions & 45 deletions

File tree

‎ports/unix/modffi.c‎

Lines changed: 25 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,13 @@
5656
* but may be later.
5757
*/
5858

59+
// This union is large enough to hold any supported argument/return value.
60+
typedef union _ffi_union_t {
61+
ffi_arg ffi;
62+
float flt;
63+
double dbl;
64+
} ffi_union_t;
65+
5966
typedef struct _mp_obj_opaque_t {
6067
mp_obj_base_t base;
6168
void *val;
@@ -151,10 +158,10 @@ STATIC ffi_type *get_ffi_type(mp_obj_t o_in) {
151158
mp_raise_TypeError(MP_ERROR_TEXT("unknown type"));
152159
}
153160

154-
STATIC mp_obj_t return_ffi_value(ffi_arg val, char type) {
161+
STATIC mp_obj_t return_ffi_value(ffi_union_t *val, char type) {
155162
switch (type) {
156163
case 's': {
157-
const char *s = (const char *)(intptr_t)val;
164+
const char *s = (const char *)(intptr_t)val->ffi;
158165
if (!s) {
159166
return mp_const_none;
160167
}
@@ -164,20 +171,16 @@ STATIC mp_obj_t return_ffi_value(ffi_arg val, char type) {
164171
return mp_const_none;
165172
#if MICROPY_PY_BUILTINS_FLOAT
166173
case 'f': {
167-
union { ffi_arg ffi;
168-
float flt;
169-
} val_union = { .ffi = val };
170-
return mp_obj_new_float_from_f(val_union.flt);
174+
return mp_obj_new_float_from_f(val->flt);
171175
}
172176
case 'd': {
173-
double *p = (double *)&val;
174-
return mp_obj_new_float_from_d(*p);
177+
return mp_obj_new_float_from_d(val->dbl);
175178
}
176179
#endif
177180
case 'O':
178-
return (mp_obj_t)(intptr_t)val;
181+
return (mp_obj_t)(intptr_t)val->ffi;
179182
default:
180-
return mp_obj_new_int(val);
183+
return mp_obj_new_int(val->ffi);
181184
}
182185
}
183186

@@ -368,61 +371,46 @@ STATIC mp_obj_t ffifunc_call(mp_obj_t self_in, size_t n_args, size_t n_kw, const
368371
assert(n_kw == 0);
369372
assert(n_args == self->cif.nargs);
370373

371-
ffi_arg values[n_args];
374+
ffi_union_t values[n_args];
372375
void *valueptrs[n_args];
373376
const char *argtype = self->argtypes;
374377
for (uint i = 0; i < n_args; i++, argtype++) {
375378
mp_obj_t a = args[i];
376379
if (*argtype == 'O') {
377-
values[i] = (ffi_arg)(intptr_t)a;
380+
values[i].ffi = (ffi_arg)(intptr_t)a;
378381
#if MICROPY_PY_BUILTINS_FLOAT
379382
} else if (*argtype == 'f') {
380-
float *p = (float *)&values[i];
381-
*p = mp_obj_get_float_to_f(a);
383+
values[i].flt = mp_obj_get_float_to_f(a);
382384
} else if (*argtype == 'd') {
383-
double *p = (double *)&values[i];
384-
*p = mp_obj_get_float_to_d(a);
385+
values[i].dbl = mp_obj_get_float_to_d(a);
385386
#endif
386387
} else if (a == mp_const_none) {
387-
values[i] = 0;
388+
values[i].ffi = 0;
388389
} else if (mp_obj_is_int(a)) {
389-
values[i] = mp_obj_int_get_truncated(a);
390+
values[i].ffi = mp_obj_int_get_truncated(a);
390391
} else if (mp_obj_is_str(a)) {
391392
const char *s = mp_obj_str_get_str(a);
392-
values[i] = (ffi_arg)(intptr_t)s;
393+
values[i].ffi = (ffi_arg)(intptr_t)s;
393394
} else if (((mp_obj_base_t *)MP_OBJ_TO_PTR(a))->type->buffer_p.get_buffer != NULL) {
394395
mp_obj_base_t *o = (mp_obj_base_t *)MP_OBJ_TO_PTR(a);
395396
mp_buffer_info_t bufinfo;
396397
int ret = o->type->buffer_p.get_buffer(MP_OBJ_FROM_PTR(o), &bufinfo, MP_BUFFER_READ); // TODO: MP_BUFFER_READ?
397398
if (ret != 0) {
398399
goto error;
399400
}
400-
values[i] = (ffi_arg)(intptr_t)bufinfo.buf;
401+
values[i].ffi = (ffi_arg)(intptr_t)bufinfo.buf;
401402
} else if (mp_obj_is_type(a, &fficallback_type)) {
402403
mp_obj_fficallback_t *p = MP_OBJ_TO_PTR(a);
403-
values[i] = (ffi_arg)(intptr_t)p->func;
404+
values[i].ffi = (ffi_arg)(intptr_t)p->func;
404405
} else {
405406
goto error;
406407
}
407408
valueptrs[i] = &values[i];
408409
}
409410

410-
// If ffi_arg is not big enough to hold a double, then we must pass along a
411-
// pointer to a memory location of the correct size.
412-
// TODO check if this needs to be done for other types which don't fit into
413-
// ffi_arg.
414-
#if MICROPY_PY_BUILTINS_FLOAT
415-
if (sizeof(ffi_arg) == 4 && self->rettype == 'd') {
416-
double retval;
417-
ffi_call(&self->cif, self->func, &retval, valueptrs);
418-
return mp_obj_new_float_from_d(retval);
419-
} else
420-
#endif
421-
{
422-
ffi_arg retval;
423-
ffi_call(&self->cif, self->func, &retval, valueptrs);
424-
return return_ffi_value(retval, self->rettype);
425-
}
411+
ffi_union_t retval;
412+
ffi_call(&self->cif, self->func, &retval, valueptrs);
413+
return return_ffi_value(&retval, self->rettype);
426414

427415
error:
428416
mp_raise_TypeError(MP_ERROR_TEXT("don't know how to pass object to native function"));

‎tests/unix/ffi_float.py‎

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,15 @@ def ffi_open(names):
3535
# test passing double and float args
3636
libm = ffi_open(("libm.so", "libm.so.6", "libc.so.0", "libc.so.6", "libc.dylib"))
3737
tgamma = libm.func("d", "tgamma", "d")
38-
for fun in (tgamma,):
38+
for fun_name in ("tgamma",):
39+
fun = globals()[fun_name]
3940
for val in (0.5, 1, 1.0, 1.5, 4, 4.0):
40-
print("%.6f" % fun(val))
41+
print(fun_name, "%.5f" % fun(val))
42+
43+
# test passing 2x float/double args
44+
powf = libm.func("f", "powf", "ff")
45+
pow = libm.func("d", "pow", "dd")
46+
for fun_name in ("powf", "pow"):
47+
fun = globals()[fun_name]
48+
for args in ((0, 1), (1, 0), (2, 0.5), (3, 4)):
49+
print(fun_name, "%.5f" % fun(*args))

‎tests/unix/ffi_float.py.exp‎

Lines changed: 14 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,16 @@
11
1.230000
22
1.230000
3-
1.772454
4-
1.000000
5-
1.000000
6-
0.886227
7-
6.000000
8-
6.000000
3+
tgamma 1.77245
4+
tgamma 1.00000
5+
tgamma 1.00000
6+
tgamma 0.88623
7+
tgamma 6.00000
8+
tgamma 6.00000
9+
powf 0.00000
10+
powf 1.00000
11+
powf 1.41421
12+
powf 81.00000
13+
pow 0.00000
14+
pow 1.00000
15+
pow 1.41421
16+
pow 81.00000

0 commit comments

Comments
 (0)