Add variadic support

Message ID 20110222154022.GA29862@davesworkthinkpad
State New
Headers show

Commit Message

Dr. David Alan Gilbert Feb. 22, 2011, 3:40 p.m.
Hi,
  As discussed a couple of months ago, I've added explicit support
for calling variadic functions to libffi.  This work is required
for ARM hard float (aka VFP) systems to call variadic functions
(by switching away from the vfp abi).

  I'm assuming this is too late for the 3.0.10 cycle but I thought
I'd get the patch out for discussion anyway.

  The patch below adds a 'ffi_prep_cif_var' function that
must be used to call variadic functions and it is passed
both the total number of arguments and the number of fixed
arguments.  Code that uses ffi_prep_cif to call variadic functions
will still work on architectures where that works at the moment (most
of them).

  Notes:
   1) This patch is against the top of git ( cbb062cc35c518004f1ab45c847f8ec4f66069ad )

   2) I've tested it on i386 (1659 expected passes, 15 unsupported),
                        armel (1654 expected passes, 5 unexpecteed passes (cls_longdouble.c), 15 unsupported)
                        armhf (1654 expected passes, 5 unexpecteed passes (cls_longdouble.c), 15 unsupported)
                        s390x (1639 expected passes, 5 unexpected failures - err_bad_abi.c)
		Those cls_longdouble unexpected passes and err_bad_abi.c
		failure are there in the current head.
	powerpc64 seems to be broken on current head, but I tested this patch
	on powerpc64 on the .0.9 release and it was OK.

   3) I've not included the changed .info file in the patch, even though it's
      in the git repo, it'll just regen from the .texi I guess.

   4) The only place it touches another architecture is CRIS which has it's own
      ffi_prep_cif implementation; I think I've fixed that up, but someone
      who can test CRIS probably should.

All comments welcome,

Dave

commit 9ac312697816b20896ca9c237afa908c9875fa3c
Author: Dr. David Alan Gilbert <david.gilbert@linaro.org>
Date:   Tue Feb 22 12:30:23 2011 +0000

    Add ffi_prep_cif_var variadic api and test it.

Patch

diff --git a/doc/libffi.texi b/doc/libffi.texi
index 5cdd667..a7c629a 100644
--- a/doc/libffi.texi
+++ b/doc/libffi.texi
@@ -133,8 +133,6 @@  This initializes @var{cif} according to the given parameters.
 you want.  @ref{Multiple ABIs} for more information.
 
 @var{nargs} is the number of arguments that this function accepts.
-@samp{libffi} does not yet handle varargs functions; see @ref{Missing
-Features} for more information.
 
 @var{rtype} is a pointer to an @code{ffi_type} structure that
 describes the return type of the function.  @xref{Types}.
@@ -150,6 +148,28 @@  objects is incorrect; or @code{FFI_BAD_ABI} if the @var{abi} parameter
 is invalid.
 @end defun
 
+If the function being called is variadic (varargs) then @code{ffi_prep_cif_var}
+must be used instead of @code{ffi_prep_cif}.
+
+@findex ffi_prep_cif_var
+@defun ffi_status ffi_prep_cif_var (ffi_cif *@var{cif}, ffi_abi @var{abi}, unsigned int @var{nfixedargs}, unsigned int @var{ntotalargs}, ffi_type *@var{rtype}, ffi_type **@var{argtypes})
+This initializes @var{cif} according to the given parameters for
+a call to a variadic function.  In general it's operation is the
+same as for @code{ffi_prep_cif} except that:
+
+@var{nfixedargs} is the number of fixed arguments, prior to any
+variadic arguments.  It must be greater than zero.
+
+@var{ntotalargs} the total number of arguments, including variadic
+and fixed arguments.
+
+Note that, different cif's must be prepped for calls to the same
+function when different numbers of arguments are passed.
+
+Also note that a call to @code{ffi_prep_cif_var} with @var{nfixedargs}=@var{nototalargs}
+is NOT equivalent to a call to @code{ffi_prep_cif}.
+
+@end defun
 
 To call a function using an initialized @code{ffi_cif}, use the
 @code{ffi_call} function:
@@ -572,9 +592,7 @@  support for these.
 
 @itemize @bullet
 @item
-There is no support for calling varargs functions.  This may work on
-some platforms, depending on how the ABI is defined, but it is not
-reliable.
+Variadic closures.
 
 @item
 There is no support for bit fields in structures.
@@ -591,6 +609,7 @@  The ``raw'' API is undocumented.
 @c anything else?
 @end itemize
 
+Note that variadic support is very new and tested on a relatively small number of platforms.
 
 @node Index
 @unnumbered Index
diff --git a/include/ffi.h.in b/include/ffi.h.in
index 96b8fd3..95f1471 100644
--- a/include/ffi.h.in
+++ b/include/ffi.h.in
@@ -198,11 +198,22 @@  typedef struct {
   ffi_type *rtype;
   unsigned bytes;
   unsigned flags;
+#ifdef FFI_TARGET_SPECIFIC_VARIADIC
+  /* 0 is used as a flag to indicate a non-variadic function */
+  unsigned int nfixedargs;
+#endif
 #ifdef FFI_EXTRA_CIF_FIELDS
   FFI_EXTRA_CIF_FIELDS;
 #endif
 } ffi_cif;
 
+/* Used internally, but overridden by some architectures */
+ffi_status ffi_prep_cif_core(ffi_cif *cif,
+			     ffi_abi abi,
+			     unsigned int nargs,
+			     ffi_type *rtype,
+			     ffi_type **atypes);
+
 /* ---- Definitions for the raw API -------------------------------------- */
 
 #ifndef FFI_SIZEOF_ARG
@@ -380,6 +391,13 @@  ffi_status ffi_prep_cif(ffi_cif *cif,
 			ffi_type *rtype,
 			ffi_type **atypes);
 
+ffi_status ffi_prep_cif_var(ffi_cif *cif,
+			    ffi_abi abi,
+			    unsigned int nfixedargs,
+			    unsigned int ntotalargs,
+			    ffi_type *rtype,
+			    ffi_type **atypes);
+
 void ffi_call(ffi_cif *cif,
 	      void (*fn)(void),
 	      void *rvalue,
diff --git a/man/Makefile.am b/man/Makefile.am
index 2519277..afcbfb6 100644
--- a/man/Makefile.am
+++ b/man/Makefile.am
@@ -2,7 +2,7 @@ 
 
 AUTOMAKE_OPTIONS=foreign
 
-EXTRA_DIST = ffi.3 ffi_call.3 ffi_prep_cif.3
+EXTRA_DIST = ffi.3 ffi_call.3 ffi_prep_cif.3 ffi_prep_cif_var.3
 
-man_MANS = ffi.3 ffi_call.3 ffi_prep_cif.3
+man_MANS = ffi.3 ffi_call.3 ffi_prep_cif.3 ffi_prep_cif_var.3
 
diff --git a/man/ffi.3 b/man/ffi.3
index 18b5d5d..1f1d303 100644
--- a/man/ffi.3
+++ b/man/ffi.3
@@ -16,6 +16,15 @@  libffi, -lffi
 .Fa "ffi_type **atypes"
 .Fc
 .Ft void
+.Fo ffi_prep_cif_var
+.Fa "ffi_cif *cif"
+.Fa "ffi_abi abi"
+.Fa "unsigned int nfixedargs"
+.Fa "unsigned int ntotalargs"
+.Fa "ffi_type *rtype"
+.Fa "ffi_type **atypes"
+.Fc
+.Ft void
 .Fo ffi_call
 .Fa "ffi_cif *cif"
 .Fa "void (*fn)(void)"
@@ -28,4 +37,5 @@  generate a call to another function at runtime without requiring knowledge of
 the called function's interface at compile time.
 .Sh SEE ALSO
 .Xr ffi_prep_cif 3 ,
+.Xr ffi_prep_cif_var 3 ,
 .Xr ffi_call 3
diff --git a/man/ffi_prep_cif.3 b/man/ffi_prep_cif.3
index 9436b31..fe66452 100644
--- a/man/ffi_prep_cif.3
+++ b/man/ffi_prep_cif.3
@@ -37,7 +37,9 @@  structs that describe the data type, size and alignment of each argument.
 points to an
 .Nm ffi_type
 that describes the data type, size and alignment of the
-return value.
+return value. Note that to call a variadic function
+.Nm ffi_prep_cif_var
+must be used instead.
 .Sh RETURN VALUES
 Upon successful completion,
 .Nm ffi_prep_cif
@@ -63,4 +65,5 @@  defined in
 .
 .Sh SEE ALSO
 .Xr ffi 3 ,
-.Xr ffi_call 3 
+.Xr ffi_call 3 ,
+.Xr ffi_prep_cif_var 3
diff --git a/man/ffi_prep_cif_var.3 b/man/ffi_prep_cif_var.3
new file mode 100644
index 0000000..1b39c03
--- /dev/null
+++ b/man/ffi_prep_cif_var.3
@@ -0,0 +1,73 @@ 
+.Dd January 25, 2011
+.Dt ffi_prep_cif_var 3
+.Sh NAME
+.Nm ffi_prep_cif_var
+.Nd Prepare a
+.Nm ffi_cif
+structure for use with
+.Nm ffi_call 
+for variadic functions.
+.Sh SYNOPSIS
+.In ffi.h
+.Ft ffi_status
+.Fo ffi_prep_cif_var
+.Fa "ffi_cif *cif"
+.Fa "ffi_abi abi"
+.Fa "unsigned int nfixedargs"
+.Fa "unsigned int ntotalargs"
+.Fa "ffi_type *rtype"
+.Fa "ffi_type **atypes"
+.Fc
+.Sh DESCRIPTION
+The
+.Nm ffi_prep_cif_var
+function prepares a
+.Nm ffi_cif
+structure for use with 
+.Nm ffi_call
+for variadic functions.
+.Fa abi
+specifies a set of calling conventions to use.
+.Fa atypes
+is an array of
+.Fa ntotalargs
+pointers to
+.Nm ffi_type
+structs that describe the data type, size and alignment of each argument.
+.Fa rtype
+points to an
+.Nm ffi_type
+that describes the data type, size and alignment of the
+return value. 
+.Fa nfixedargs
+must contain the number of fixed (non-variadic) arguments.
+Note that to call a non-variadic function
+.Nm ffi_prep_cif
+must be used.
+.Sh RETURN VALUES
+Upon successful completion,
+.Nm ffi_prep_cif_var
+returns
+.Nm FFI_OK .
+It will return
+.Nm FFI_BAD_TYPEDEF
+if
+.Fa cif
+is
+.Nm NULL
+or
+.Fa atypes
+or
+.Fa rtype
+is malformed. If
+.Fa abi
+does not refer to a valid ABI,
+.Nm FFI_BAD_ABI
+will be returned. Available ABIs are
+defined in
+.Nm <ffitarget.h>
+.
+.Sh SEE ALSO
+.Xr ffi 3 ,
+.Xr ffi_call 3 ,
+.Xr ffi_prep_cif 3
diff --git a/src/arm/ffi.c b/src/arm/ffi.c
index 885a9cb..b1bf5a8 100644
--- a/src/arm/ffi.c
+++ b/src/arm/ffi.c
@@ -143,6 +143,12 @@  ffi_status ffi_prep_cif_machdep(ffi_cif *cif)
      when it isn't needed.  */
   cif->bytes = (cif->bytes + 7) & ~7;
 
+#ifdef __ARM_PCS_VFP
+  /* VFP uses the standard ABI for variadic functions */
+  if ((cif->abi == FFI_VFP) && (cif->nfixedargs != 0))
+    cif->abi = FFI_SYSV;
+#endif
+
   /* Set the return type flag */
   switch (cif->rtype->type)
     {
diff --git a/src/arm/ffitarget.h b/src/arm/ffitarget.h
index ce25b23..3f70acb 100644
--- a/src/arm/ffitarget.h
+++ b/src/arm/ffitarget.h
@@ -55,6 +55,10 @@  typedef enum ffi_abi {
 #define FFI_TYPE_STRUCT_VFP_FLOAT  (FFI_TYPE_LAST + 1)
 #define FFI_TYPE_STRUCT_VFP_DOUBLE (FFI_TYPE_LAST + 2)
 
+#ifdef __ARM_PCS_VFP
+#define FFI_TARGET_SPECIFIC_VARIADIC
+#endif
+
 /* ---- Definitions for closures ----------------------------------------- */
 
 #define FFI_CLOSURES 1
@@ -62,4 +66,3 @@  typedef enum ffi_abi {
 #define FFI_NATIVE_RAW_API 0
 
 #endif
-
diff --git a/src/cris/ffi.c b/src/cris/ffi.c
index f25d7b4..a114c0b 100644
--- a/src/cris/ffi.c
+++ b/src/cris/ffi.c
@@ -153,10 +153,10 @@  ffi_prep_args (char *stack, extended_cif * ecif)
   return (struct_count);
 }
 
-ffi_status
-ffi_prep_cif (ffi_cif * cif,
-	      ffi_abi abi, unsigned int nargs,
-	      ffi_type * rtype, ffi_type ** atypes)
+ffi_status FFI_HIDDEN
+ffi_prep_cif_core (ffi_cif * cif,
+	           ffi_abi abi, unsigned int nargs,
+	           ffi_type * rtype, ffi_type ** atypes)
 {
   unsigned bytes = 0;
   unsigned int i;
diff --git a/src/prep_cif.c b/src/prep_cif.c
index 8548cfd..b1ce954 100644
--- a/src/prep_cif.c
+++ b/src/prep_cif.c
@@ -90,8 +90,8 @@  static ffi_status initialize_aggregate(ffi_type *arg)
 /* Perform machine independent ffi_cif preparation, then call
    machine dependent routine. */
 
-ffi_status ffi_prep_cif(ffi_cif *cif, ffi_abi abi, unsigned int nargs,
-			ffi_type *rtype, ffi_type **atypes)
+ffi_status FFI_HIDDEN ffi_prep_cif_core(ffi_cif *cif, ffi_abi abi, unsigned int nargs,
+			     ffi_type *rtype, ffi_type **atypes)
 {
   unsigned bytes = 0;
   unsigned int i;
@@ -163,6 +163,33 @@  ffi_status ffi_prep_cif(ffi_cif *cif, ffi_abi abi, unsigned int nargs,
 }
 #endif /* not __CRIS__ */
 
+ffi_status ffi_prep_cif(ffi_cif *cif, ffi_abi abi, unsigned int nargs,
+			     ffi_type *rtype, ffi_type **atypes)
+{
+#ifdef FFI_TARGET_SPECIFIC_VARIADIC
+  cif->nfixedargs = 0;
+#endif
+
+  return ffi_prep_cif_core(cif, abi, nargs, rtype, atypes);
+}
+
+ffi_status ffi_prep_cif_var(ffi_cif *cif,
+                            ffi_abi abi,
+                            unsigned int nfixedargs,
+                            unsigned int ntotalargs,
+                            ffi_type *rtype,
+                            ffi_type **atypes)
+{
+  FFI_ASSERT(cif != NULL);
+  /* There should always be at least one fixed arg */
+  FFI_ASSERT(nfixedargs >= 1);
+  FFI_ASSERT(nfixedargs <= ntotalargs);
+#ifdef FFI_TARGET_SPECIFIC_VARIADIC
+  cif->nfixedargs = nfixedargs;
+#endif
+  return ffi_prep_cif_core(cif, abi, ntotalargs, rtype, atypes);
+}
+
 #if FFI_CLOSURES
 
 ffi_status
diff --git a/testsuite/libffi.call/cls_double_va.c b/testsuite/libffi.call/cls_double_va.c
index 62bebbd..1a2706e 100644
--- a/testsuite/libffi.call/cls_double_va.c
+++ b/testsuite/libffi.call/cls_double_va.c
@@ -6,7 +6,6 @@ 
 
 /* { dg-do run { xfail strongarm*-*-* xscale*-*-* } } */
 /* { dg-output "" { xfail avr32*-*-* } } */
-/* { dg-skip-if "" arm*-*-* { "-mfloat-abi=hard" } { "" } } */
 
 #include "ffitest.h"
 
@@ -36,7 +35,8 @@  int main (void)
 	arg_types[1] = &ffi_type_double;
 	arg_types[2] = NULL;
 
-	CHECK(ffi_prep_cif(&cif, FFI_DEFAULT_ABI, 2, &ffi_type_sint,
+	/* This printf call is variadic */
+	CHECK(ffi_prep_cif_var(&cif, FFI_DEFAULT_ABI, 1, 2, &ffi_type_sint,
 		arg_types) == FFI_OK);
 
 	args[0] = &format;
@@ -48,6 +48,10 @@  int main (void)
 	printf("res: %d\n", (int) res);
 	// { dg-output "\nres: 4" }
 
+	/* The call to cls_double_va_fn is static, so have to use a normal prep_cif */
+	CHECK(ffi_prep_cif(&cif, FFI_DEFAULT_ABI, 2, &ffi_type_sint,
+		arg_types) == FFI_OK);
+
 	CHECK(ffi_prep_closure_loc(pcl, &cif, cls_double_va_fn, NULL, code) == FFI_OK);
 
 	res	= ((int(*)(char*, double))(code))(format, doubleArg);
diff --git a/testsuite/libffi.call/cls_longdouble_va.c b/testsuite/libffi.call/cls_longdouble_va.c
index b33b2b7..95f9ff4 100644
--- a/testsuite/libffi.call/cls_longdouble_va.c
+++ b/testsuite/libffi.call/cls_longdouble_va.c
@@ -6,7 +6,6 @@ 
 
 /* { dg-do run { xfail strongarm*-*-* xscale*-*-* } } */
 /* { dg-output "" { xfail avr32*-*-* x86_64-*-mingw* } } */
-/* { dg-skip-if "" arm*-*-* { "-mfloat-abi=hard" } { "" } } */
 
 #include "ffitest.h"
 
@@ -36,7 +35,8 @@  int main (void)
 	arg_types[1] = &ffi_type_longdouble;
 	arg_types[2] = NULL;
 
-	CHECK(ffi_prep_cif(&cif, FFI_DEFAULT_ABI, 2, &ffi_type_sint,
+	/* This printf call is variadic */
+	CHECK(ffi_prep_cif_var(&cif, FFI_DEFAULT_ABI, 1, 2, &ffi_type_sint,
 		arg_types) == FFI_OK);
 
 	args[0] = &format;
@@ -48,6 +48,10 @@  int main (void)
 	printf("res: %d\n", (int) res);
 	// { dg-output "\nres: 4" }
 
+	/* The call to cls_longdouble_va_fn is static, so have to use a normal prep_cif */
+	CHECK(ffi_prep_cif(&cif, FFI_DEFAULT_ABI, 2, &ffi_type_sint,
+		arg_types) == FFI_OK);
+
 	CHECK(ffi_prep_closure_loc(pcl, &cif, cls_longdouble_va_fn, NULL, code) == FFI_OK);
 
 	res	= ((int(*)(char*, long double))(code))(format, ldArg);
diff --git a/testsuite/libffi.call/float_va.c b/testsuite/libffi.call/float_va.c
new file mode 100644
index 0000000..4611969
--- /dev/null
+++ b/testsuite/libffi.call/float_va.c
@@ -0,0 +1,107 @@ 
+/* Area:        fp and variadics
+   Purpose:     check fp inputs and returns work on variadics, even the fixed params
+   Limitations: None
+   PR:          none
+   Originator:  <david.gilbert@linaro.org> 2011-01-25
+
+   Intended to stress the difference in ABI on ARM vfp
+*/
+
+/* { dg-do run } */
+
+#include <stdarg.h>
+
+#include "ffitest.h"
+
+/* prints out all the parameters, and returns the sum of them all.
+ * 'x' is the number of variadic parameters all of which are double in this test
+ */
+double float_va_fn(unsigned int x, double y,...)
+{
+  double total=0.0;
+  va_list ap;
+  unsigned int i;
+
+  total+=(double)x;
+  total+=y;
+
+  printf("%u: %.1lf :", x, y);
+
+  va_start(ap, y);
+  for(i=0;i<x;i++)
+  {
+    double arg=va_arg(ap, double);
+    total+=arg;
+    printf(" %d:%.1lf ", i, arg);
+  }
+  va_end(ap);
+  
+  printf(" total: %.1lf\n", total);
+
+  return total;
+}
+
+int main (void)
+{
+  ffi_cif    cif;
+
+  ffi_type    *arg_types[5];
+  void        *values[5];
+  double        doubles[5];
+  unsigned int firstarg;
+  double        resfp;
+
+  /* First test, pass float_va_fn(0,2.0) - note there are no actual
+   * variadic parameters, but it's declared variadic so the ABI may be
+   * different. */
+  /* Call it statically and then via ffi */
+  resfp=float_va_fn(0,2.0);
+  // { dg-output "0: 2.0 : total: 2.0" }
+  printf("compiled: %.1lf\n", resfp);
+  // { dg-output "\ncompiled: 2.0" }
+
+  arg_types[0] = &ffi_type_uint;
+  arg_types[1] = &ffi_type_double;
+  arg_types[2] = NULL;
+  CHECK(ffi_prep_cif_var(&cif, FFI_DEFAULT_ABI, 2, 2,
+        &ffi_type_double, arg_types) == FFI_OK);
+
+  firstarg = 0;
+  doubles[0] = 2.0;
+  values[0] = &firstarg;
+  values[1] = &doubles[0];
+  ffi_call(&cif, FFI_FN(float_va_fn), &resfp, values); 
+  // { dg-output "\n0: 2.0 : total: 2.0" }
+  printf("ffi: %.1lf\n", resfp);
+  // { dg-output "\nffi: 2.0" }
+
+  /* Second test, float_va_fn(2,2.0,3.0,4.0), now with variadic params */
+  /* Call it statically and then via ffi */
+  resfp=float_va_fn(2,2.0,3.0,4.0);
+  // { dg-output "\n2: 2.0 : 0:3.0  1:4.0  total: 11.0" }
+  printf("compiled: %.1lf\n", resfp);
+  // { dg-output "\ncompiled: 11.0" }
+
+  arg_types[0] = &ffi_type_uint;
+  arg_types[1] = &ffi_type_double;
+  arg_types[2] = &ffi_type_double;
+  arg_types[3] = &ffi_type_double;
+  arg_types[4] = NULL;
+  CHECK(ffi_prep_cif_var(&cif, FFI_DEFAULT_ABI, 2, 4,
+        &ffi_type_double, arg_types) == FFI_OK);
+
+  firstarg = 2;
+  doubles[0] = 2.0;
+  doubles[1] = 3.0;
+  doubles[2] = 4.0;
+  values[0] = &firstarg;
+  values[1] = &doubles[0];
+  values[2] = &doubles[1];
+  values[3] = &doubles[2];
+  ffi_call(&cif, FFI_FN(float_va_fn), &resfp, values); 
+  // { dg-output "\n2: 2.0 : 0:3.0  1:4.0  total: 11.0" }
+  printf("ffi: %.1lf\n", resfp);
+  // { dg-output "\nffi: 11.0" }
+
+  exit(0);
+}