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 #7064.

Signed-off-by: Damien George <damien@micropython.org>

3 files changed, 50 insertions, 45 deletions

diff --git a/ports/unix/modffi.c b/ports/unix/modffi.c
index 598a28cd5..5c2595ec5 100644
--- a/ports/unix/modffi.c
+++ b/ports/unix/modffi.c
@@ -56,6 +56,13 @@
  * but may be later.
  */
 
+// This union is large enough to hold any supported argument/return value.
+typedef union _ffi_union_t {
+    ffi_arg ffi;
+    float flt;
+    double dbl;
+} ffi_union_t;
+
 typedef struct _mp_obj_opaque_t {
     mp_obj_base_t base;
     void *val;
@@ -151,10 +158,10 @@ STATIC ffi_type *get_ffi_type(mp_obj_t o_in) {
     mp_raise_TypeError(MP_ERROR_TEXT("unknown type"));
 }
 
-STATIC mp_obj_t return_ffi_value(ffi_arg val, char type) {
+STATIC mp_obj_t return_ffi_value(ffi_union_t *val, char type) {
     switch (type) {
         case 's': {
-            const char *s = (const char *)(intptr_t)val;
+            const char *s = (const char *)(intptr_t)val->ffi;
             if (!s) {
                 return mp_const_none;
             }
@@ -164,20 +171,16 @@ STATIC mp_obj_t return_ffi_value(ffi_arg val, char type) {
             return mp_const_none;
         #if MICROPY_PY_BUILTINS_FLOAT
         case 'f': {
-            union { ffi_arg ffi;
-                    float flt;
-            } val_union = { .ffi = val };
-            return mp_obj_new_float_from_f(val_union.flt);
+            return mp_obj_new_float_from_f(val->flt);
         }
         case 'd': {
-            double *p = (double *)&val;
-            return mp_obj_new_float_from_d(*p);
+            return mp_obj_new_float_from_d(val->dbl);
         }
         #endif
         case 'O':
-            return (mp_obj_t)(intptr_t)val;
+            return (mp_obj_t)(intptr_t)val->ffi;
         default:
-            return mp_obj_new_int(val);
+            return mp_obj_new_int(val->ffi);
     }
 }
 
@@ -368,28 +371,26 @@ STATIC mp_obj_t ffifunc_call(mp_obj_t self_in, size_t n_args, size_t n_kw, const
     assert(n_kw == 0);
     assert(n_args == self->cif.nargs);
 
-    ffi_arg values[n_args];
+    ffi_union_t values[n_args];
     void *valueptrs[n_args];
     const char *argtype = self->argtypes;
     for (uint i = 0; i < n_args; i++, argtype++) {
         mp_obj_t a = args[i];
         if (*argtype == 'O') {
-            values[i] = (ffi_arg)(intptr_t)a;
+            values[i].ffi = (ffi_arg)(intptr_t)a;
         #if MICROPY_PY_BUILTINS_FLOAT
         } else if (*argtype == 'f') {
-            float *p = (float *)&values[i];
-            *p = mp_obj_get_float_to_f(a);
+            values[i].flt = mp_obj_get_float_to_f(a);
         } else if (*argtype == 'd') {
-            double *p = (double *)&values[i];
-            *p = mp_obj_get_float_to_d(a);
+            values[i].dbl = mp_obj_get_float_to_d(a);
         #endif
         } else if (a == mp_const_none) {
-            values[i] = 0;
+            values[i].ffi = 0;
         } else if (mp_obj_is_int(a)) {
-            values[i] = mp_obj_int_get_truncated(a);
+            values[i].ffi = mp_obj_int_get_truncated(a);
         } else if (mp_obj_is_str(a)) {
             const char *s = mp_obj_str_get_str(a);
-            values[i] = (ffi_arg)(intptr_t)s;
+            values[i].ffi = (ffi_arg)(intptr_t)s;
         } else if (((mp_obj_base_t *)MP_OBJ_TO_PTR(a))->type->buffer_p.get_buffer != NULL) {
             mp_obj_base_t *o = (mp_obj_base_t *)MP_OBJ_TO_PTR(a);
             mp_buffer_info_t bufinfo;
@@ -397,32 +398,19 @@ STATIC mp_obj_t ffifunc_call(mp_obj_t self_in, size_t n_args, size_t n_kw, const
             if (ret != 0) {
                 goto error;
             }
-            values[i] = (ffi_arg)(intptr_t)bufinfo.buf;
+            values[i].ffi = (ffi_arg)(intptr_t)bufinfo.buf;
         } else if (mp_obj_is_type(a, &fficallback_type)) {
             mp_obj_fficallback_t *p = MP_OBJ_TO_PTR(a);
-            values[i] = (ffi_arg)(intptr_t)p->func;
+            values[i].ffi = (ffi_arg)(intptr_t)p->func;
         } else {
             goto error;
         }
         valueptrs[i] = &values[i];
     }
 
-    // If ffi_arg is not big enough to hold a double, then we must pass along a
-    // pointer to a memory location of the correct size.
-    // TODO check if this needs to be done for other types which don't fit into
-    // ffi_arg.
-    #if MICROPY_PY_BUILTINS_FLOAT
-    if (sizeof(ffi_arg) == 4 && self->rettype == 'd') {
-        double retval;
-        ffi_call(&self->cif, self->func, &retval, valueptrs);
-        return mp_obj_new_float_from_d(retval);
-    } else
-    #endif
-    {
-        ffi_arg retval;
-        ffi_call(&self->cif, self->func, &retval, valueptrs);
-        return return_ffi_value(retval, self->rettype);
-    }
+    ffi_union_t retval;
+    ffi_call(&self->cif, self->func, &retval, valueptrs);
+    return return_ffi_value(&retval, self->rettype);
 
 error:
     mp_raise_TypeError(MP_ERROR_TEXT("don't know how to pass object to native function"));
diff --git a/tests/unix/ffi_float.py b/tests/unix/ffi_float.py
index d03939896..03bd9f7f1 100644
--- a/tests/unix/ffi_float.py
+++ b/tests/unix/ffi_float.py
@@ -35,6 +35,15 @@ print("%.6f" % strtod("1.23", None))
 # test passing double and float args
 libm = ffi_open(("libm.so", "libm.so.6", "libc.so.0", "libc.so.6", "libc.dylib"))
 tgamma = libm.func("d", "tgamma", "d")
-for fun in (tgamma,):
+for fun_name in ("tgamma",):
+    fun = globals()[fun_name]
     for val in (0.5, 1, 1.0, 1.5, 4, 4.0):
-        print("%.6f" % fun(val))
+        print(fun_name, "%.5f" % fun(val))
+
+# test passing 2x float/double args
+powf = libm.func("f", "powf", "ff")
+pow = libm.func("d", "pow", "dd")
+for fun_name in ("powf", "pow"):
+    fun = globals()[fun_name]
+    for args in ((0, 1), (1, 0), (2, 0.5), (3, 4)):
+        print(fun_name, "%.5f" % fun(*args))
diff --git a/tests/unix/ffi_float.py.exp b/tests/unix/ffi_float.py.exp
index b9d7da2bd..3d9091431 100644
--- a/tests/unix/ffi_float.py.exp
+++ b/tests/unix/ffi_float.py.exp
@@ -1,8 +1,16 @@
 1.230000
 1.230000
-1.772454
-1.000000
-1.000000
-0.886227
-6.000000
-6.000000
+tgamma 1.77245
+tgamma 1.00000
+tgamma 1.00000
+tgamma 0.88623
+tgamma 6.00000
+tgamma 6.00000
+powf 0.00000
+powf 1.00000
+powf 1.41421
+powf 81.00000
+pow 0.00000
+pow 1.00000
+pow 1.41421
+pow 81.00000