documentation patch, oddities, and proposals

documentation patch, oddities, and proposals

[Tom Tromey]

Hi!

[ I tried to send this once before but I don't know that it went
through, so trying again... ]

I've been working on an FFI:

    https://github.com/tromey/emacs-ffi

While doing so I refreshed my memory of some of the oddities of libffi,
and discovered some I had never run across before.

I found a few things that weren't documented.  I wrote a documentation
patch that I think covers them all.  It is appended.  If you like it I
can write a ChangeLog entry and submit a pull request.

Some questions, observations, and proposals:

* The ffi_arg business for return values I (re-)learned from reading the
  archvies.  I still haven't fixed my own code to deal with this yet.

  I wasn't sure -- is this needed in some way for the closure API as
  well?  If so, and someone can explain where (that is, arguments,
  return values, or both), I will write a documentation patch.

* I wanted to compute the layout of a struct.  libffi does this
  internally but doesn't expose the results.

  I think it would be great to have a new function to do this.  In fact
  I wrote one, but...

* ... this lead me to the discovery that ffi_prep_types modifies some of
  the libffi globals, meaning that libffi is not type-safe when using
  multiple ABIs.

  Currently, AFAICT, this only affects long doubles on PPC.  So, not
  quite as bad as it seems -- but still it is a bug.

* Also due to ffi_prep_types, you can't really use the size and
  alignment fields of an ffi_type unless you know that the ABI hasn't
  changed.  And, because of this, you also can't share structure types
  across ABIs.  (Maybe that is obvious?  It was a bit of a surprise to
  me.)

* As you can see from the docs, I also had to resort to some shenanigans
  to get arrays and unions working.  I think it would be preferable to
  have FFI_TYPE_ARRAY and FFI_TYPE_UNION directly in the library.

* This doesn't impact the docs, but ffi.h.in is a mix of public and
  private stuff.  I think it would be nice to separate these.

* I didn't write docs for the raw or java APIs.  It's probably time for
  those to die.

* Someone who understands what it does should perhaps write
  documentation for the Go closure code.  Or, if this is only for use by
  gccgo, then maybe it should be relegated to the aforementioned
  internal-only header.

Tom

diff --git a/doc/libffi.texi b/doc/libffi.texi
index ff72e58..16c1b53 100644
--- a/doc/libffi.texi
+++ b/doc/libffi.texi
@@ -107,6 +107,7 @@ values passed between the two languages.
 * Multiple ABIs::               Different passing styles on one platform.
 * The Closure API::             Writing a generic function.
 * Closure Example::             A closure example.
+* Thread Safety::               Thread safety.
 @end menu
 
 
@@ -190,12 +191,23 @@ to ensure this.  If @var{cif} declares that the function returns
 @code{void} (using @code{ffi_type_void}), then @var{rvalue} is
 ignored.
 
+For types like @code{char} that are narrower than the system register
+size, @samp{libffi} provides a type, @code{ffi_arg}, that can be used
+as the return type.
+
 @var{avalues} is a vector of @code{void *} pointers that point to the
 memory locations holding the argument values for a call.  If @var{cif}
 declares that the function has no arguments (i.e., @var{nargs} was 0),
 then @var{avalues} is ignored.  Note that argument values may be
 modified by the callee (for instance, structs passed by value); the
 burden of copying pass-by-value arguments is placed on the caller.
+
+Note that while the return value must be register-sized, arguments
+should exactly match their declared type.  For example, if an argument
+is a @code{short}, then the entry is @var{avalues} should point to an
+object declared as @code{short}; but if the return type is
+@code{short}, then @var{rvalue} should point to an object declared as
+a larger type -- usually @code{ffi_arg}.
 @end defun
 
 
@@ -246,6 +258,8 @@ int main()
 @menu
 * Primitive Types::             Built-in types.
 * Structures::                  Structure types.
+* Size and Alignment::          Size and alignment of types.
+* Arrays and Unions::           Arrays and unions.
 * Type Example::                Structure type example.
 * Complex::                     Complex types.
 * Complex Type Example::        Complex type example.
@@ -370,8 +384,7 @@ when passing to @code{ffi_prep_cif}.
 @node Structures
 @subsection Structures
 
-Although @samp{libffi} has no special support for unions or
-bit-fields, it is perfectly happy passing structures back and forth.
+@samp{libffi} is perfectly happy passing structures back and forth.
 You must first describe the structure to @samp{libffi} by creating a
 new @code{ffi_type} object for it.
 
@@ -391,9 +404,132 @@ For a structure, this should be set to @code{FFI_TYPE_STRUCT}.
 @item ffi_type **elements
 This is a @samp{NULL}-terminated array of pointers to @code{ffi_type}
 objects.  There is one element per field of the struct.
+
+Note that @samp{libffi} has no special support for bit-fields.  You
+must manage these manually.
 @end table
 @end deftp
 
+The @code{size} and @code{alignment} fields will be filled in by
+@code{ffi_prep_cif} or @code{ffi_prep_cif_var}, as needed.
+
+@node Size and Alignment
+@subsection Size and Alignment
+
+@code{libffi} will set the @code{size} and @code{alignment} fields of
+an @code{ffi_type} object for you.  It does so using its knowledge of
+the ABI.
+
+You might expect that you can simply read these fields for a type that
+has been laid out by @code{libffi}.  However, there are some caveats.
+
+@itemize @bullet
+@item
+The size or alignment of some of the built-in types may vary depending
+on the chosen ABI.
+
+@item
+The size and alignment of a new structure type will not be set by
+@code{libffi} until it has been passed to @code{ffi_prep_cif}.
+
+@item
+A structure type cannot be shared across ABIs.  Instead each ABI needs
+its own copy of the structure type.
+@end itemize
+
+So, before examining these fields, it is safest to pass the
+@code{ffi_type} object to @code{ffi_prep_cif} first.  This function
+will do all the needed setup.
+
+@example
+ffi_type *desired_type;
+ffi_abi desired_abi;
+...
+ffi_cif cif;
+if (ffi_prep_cif (&cif, desired_abi, 0, desired_type, NULL) == FFI_OK)
+  @{
+    size_t size = desired_type->size;
+    unsigned short alignment = desired_type->alignment;
+  @}
+@end example
+
+@node Arrays and Unions
+@subsection Arrays and Unions
+
+@subsubsection Arrays
+
+@samp{libffi} does not have direct support for arrays or unions.
+However, they can be emulated using structures.
+
+To emulate an array, simply create an @code{ffi_type} using
+@code{FFI_TYPE_STRUCT} with as many members as there are elements in
+the array.
+
+@example
+ffi_type array_type;
+ffi_type **elements
+int i;
+
+elements = malloc ((n + 1) * sizeof (ffi_type *));
+for (i = 0; i < n; ++i)
+  elements[i] = array_element_type;
+elements[n] = NULL;
+
+array_type.size = array_type.alignment = 0;
+array_type.type = FFI_TYPE_STRUCT;
+array_type.elements = elements;
+@end example
+
+Note that arrays cannot be passed or returned by value in C --
+structure types created like this should only be used to refer to
+members of real @code{FFI_TYPE_STRUCT} objects.
+
+However, a phony array type like this will not cause any errors from
+@samp{libffi} if you use it as an argument or return type.  This may
+be confusing.
+
+@subsubsection Unions
+
+A union can also be emulated using @code{FFI_TYPE_STRUCT}.  In this
+case, however, you must make sure that the size and alignment match
+the real requirements of the union.
+
+One simple way to do this is to ensue that each element type is laid
+out.  Then, give the new structure type a single element; the size of
+the largest element; and the largest alignment seen as well.
+
+This example uses the @code{ffi_prep_cif} trick to ensure that each
+element type is laid out.
+
+@example
+ffi_abi desired_abi;
+ffi_type union_type;
+ffi_type **union_elements;
+
+int i;
+ffi_type element_types[2];
+
+element_types[1] = NULL;
+
+union_type.size = union_type.alignment = 0;
+union_type.type = FFI_TYPE_STRUCT;
+union_type.elements = element_types;
+
+for (i = 0; union_elements[i]; ++i)
+  @{
+    ffi_cif cif;
+    if (ffi_prep_cif (&cif, desired_abi, 0, union_elements[i], NULL) == FFI_OK)
+      @{
+        if (union_elements[i]->size > union_type.size)
+          @{
+            union_type.size = union_elements[i];
+            size = union_elements[i]->size;
+          @}
+        if (union_elements[i]->alignment > union_type.alignment)
+          union_type.alignment = union_elements[i]->alignment;
+      @}
+  @}
+@end example
 
 @node Type Example
 @subsection Type Example
@@ -636,6 +772,8 @@ Prepare a closure function.
 the writable address returned by @code{ffi_closure_alloc}.
 
 @var{cif} is the @code{ffi_cif} describing the function parameters.
+Note that this object, and the types to which it refers, must be kept
+alive until the closure itself is freed.
 
 @var{user_data} is an arbitrary datum that is passed, uninterpreted,
 to your closure function.
@@ -733,6 +871,28 @@ int main()
 
 @end example
 
+@node Thread Safety
+@section Thread Safety
+
+@code{libffi} is not completely thread-safe.  However, many parts are,
+and if you follow some simple rules, you can use it safely in a
+multi-threaded program.
+
+@itemize @bullet
+@item
+@code{ffi_prep_cif} may modify the @code{ffi_type} objects passed to
+it.  It is best to ensure that only a single thread prepares a given
+@code{ffi_cif} at a time.
+
+@item
+On some platforms, @code{ffi_prep_cif} may modify the size and
+alignment of some types, depending on the chosen ABI.  On these
+platforms, if you switch between ABIs, you must ensure that there is
+only one call to @code{ffi_prep_cif} at a time.
+
+Currently the only affected platform is PowerPC and the only affected
+type is @code{long double}.
+@end itemize
 
 @node Missing Features
 @chapter Missing Features
@@ -749,8 +909,6 @@ There is no support for bit fields in structures.
 
 @item
 The ``raw'' API is undocumented.
-@c argument promotion?
-@c unions?
 @c anything else?
 @end itemize
 

Re: documentation patch, oddities, and proposals

[Andrew Pinski]

On Wed, Nov 4, 2015 at 1:32 PM, Tom Tromey <tom@tromey.com> wrote:
> Hi!
>
> [ I tried to send this once before but I don't know that it went
> through, so trying again... ]
>
> I've been working on an FFI:
>
>     https://github.com/tromey/emacs-ffi
>
> While doing so I refreshed my memory of some of the oddities of libffi,
> and discovered some I had never run across before.
>
> I found a few things that weren't documented.  I wrote a documentation
> patch that I think covers them all.  It is appended.  If you like it I
> can write a ChangeLog entry and submit a pull request.
>
> Some questions, observations, and proposals:
>
> * The ffi_arg business for return values I (re-)learned from reading the
>   archvies.  I still haven't fixed my own code to deal with this yet.
>
>   I wasn't sure -- is this needed in some way for the closure API as
>   well?  If so, and someone can explain where (that is, arguments,
>   return values, or both), I will write a documentation patch.
>
> * I wanted to compute the layout of a struct.  libffi does this
>   internally but doesn't expose the results.
>
>   I think it would be great to have a new function to do this.  In fact
>   I wrote one, but...

Yes this would be useful as it would be nice to move libobjc away from
its current mess of using GCC's internal headers too.

Thanks,
Andrew


>
> * ... this lead me to the discovery that ffi_prep_types modifies some of
>   the libffi globals, meaning that libffi is not type-safe when using
>   multiple ABIs.
>
>   Currently, AFAICT, this only affects long doubles on PPC.  So, not
>   quite as bad as it seems -- but still it is a bug.
>
> * Also due to ffi_prep_types, you can't really use the size and
>   alignment fields of an ffi_type unless you know that the ABI hasn't
>   changed.  And, because of this, you also can't share structure types
>   across ABIs.  (Maybe that is obvious?  It was a bit of a surprise to
>   me.)
>
> * As you can see from the docs, I also had to resort to some shenanigans
>   to get arrays and unions working.  I think it would be preferable to
>   have FFI_TYPE_ARRAY and FFI_TYPE_UNION directly in the library.
>
> * This doesn't impact the docs, but ffi.h.in is a mix of public and
>   private stuff.  I think it would be nice to separate these.
>
> * I didn't write docs for the raw or java APIs.  It's probably time for
>   those to die.
>
> * Someone who understands what it does should perhaps write
>   documentation for the Go closure code.  Or, if this is only for use by
>   gccgo, then maybe it should be relegated to the aforementioned
>   internal-only header.
>
> Tom
>
> diff --git a/doc/libffi.texi b/doc/libffi.texi
> index ff72e58..16c1b53 100644
> --- a/doc/libffi.texi
> +++ b/doc/libffi.texi
> @@ -107,6 +107,7 @@ values passed between the two languages.
>  * Multiple ABIs::               Different passing styles on one platform.
>  * The Closure API::             Writing a generic function.
>  * Closure Example::             A closure example.
> +* Thread Safety::               Thread safety.
>  @end menu
>
>
> @@ -190,12 +191,23 @@ to ensure this.  If @var{cif} declares that the function returns
>  @code{void} (using @code{ffi_type_void}), then @var{rvalue} is
>  ignored.
>
> +For types like @code{char} that are narrower than the system register
> +size, @samp{libffi} provides a type, @code{ffi_arg}, that can be used
> +as the return type.
> +
>  @var{avalues} is a vector of @code{void *} pointers that point to the
>  memory locations holding the argument values for a call.  If @var{cif}
>  declares that the function has no arguments (i.e., @var{nargs} was 0),
>  then @var{avalues} is ignored.  Note that argument values may be
>  modified by the callee (for instance, structs passed by value); the
>  burden of copying pass-by-value arguments is placed on the caller.
> +
> +Note that while the return value must be register-sized, arguments
> +should exactly match their declared type.  For example, if an argument
> +is a @code{short}, then the entry is @var{avalues} should point to an
> +object declared as @code{short}; but if the return type is
> +@code{short}, then @var{rvalue} should point to an object declared as
> +a larger type -- usually @code{ffi_arg}.
>  @end defun
>
>
> @@ -246,6 +258,8 @@ int main()
>  @menu
>  * Primitive Types::             Built-in types.
>  * Structures::                  Structure types.
> +* Size and Alignment::          Size and alignment of types.
> +* Arrays and Unions::           Arrays and unions.
>  * Type Example::                Structure type example.
>  * Complex::                     Complex types.
>  * Complex Type Example::        Complex type example.
> @@ -370,8 +384,7 @@ when passing to @code{ffi_prep_cif}.
>  @node Structures
>  @subsection Structures
>
> -Although @samp{libffi} has no special support for unions or
> -bit-fields, it is perfectly happy passing structures back and forth.
> +@samp{libffi} is perfectly happy passing structures back and forth.
>  You must first describe the structure to @samp{libffi} by creating a
>  new @code{ffi_type} object for it.
>
> @@ -391,9 +404,132 @@ For a structure, this should be set to @code{FFI_TYPE_STRUCT}.
>  @item ffi_type **elements
>  This is a @samp{NULL}-terminated array of pointers to @code{ffi_type}
>  objects.  There is one element per field of the struct.
> +
> +Note that @samp{libffi} has no special support for bit-fields.  You
> +must manage these manually.
>  @end table
>  @end deftp
>
> +The @code{size} and @code{alignment} fields will be filled in by
> +@code{ffi_prep_cif} or @code{ffi_prep_cif_var}, as needed.
> +
> +@node Size and Alignment
> +@subsection Size and Alignment
> +
> +@code{libffi} will set the @code{size} and @code{alignment} fields of
> +an @code{ffi_type} object for you.  It does so using its knowledge of
> +the ABI.
> +
> +You might expect that you can simply read these fields for a type that
> +has been laid out by @code{libffi}.  However, there are some caveats.
> +
> +@itemize @bullet
> +@item
> +The size or alignment of some of the built-in types may vary depending
> +on the chosen ABI.
> +
> +@item
> +The size and alignment of a new structure type will not be set by
> +@code{libffi} until it has been passed to @code{ffi_prep_cif}.
> +
> +@item
> +A structure type cannot be shared across ABIs.  Instead each ABI needs
> +its own copy of the structure type.
> +@end itemize
> +
> +So, before examining these fields, it is safest to pass the
> +@code{ffi_type} object to @code{ffi_prep_cif} first.  This function
> +will do all the needed setup.
> +
> +@example
> +ffi_type *desired_type;
> +ffi_abi desired_abi;
> +...
> +ffi_cif cif;
> +if (ffi_prep_cif (&cif, desired_abi, 0, desired_type, NULL) == FFI_OK)
> +  @{
> +    size_t size = desired_type->size;
> +    unsigned short alignment = desired_type->alignment;
> +  @}
> +@end example
> +
> +@node Arrays and Unions
> +@subsection Arrays and Unions
> +
> +@subsubsection Arrays
> +
> +@samp{libffi} does not have direct support for arrays or unions.
> +However, they can be emulated using structures.
> +
> +To emulate an array, simply create an @code{ffi_type} using
> +@code{FFI_TYPE_STRUCT} with as many members as there are elements in
> +the array.
> +
> +@example
> +ffi_type array_type;
> +ffi_type **elements
> +int i;
> +
> +elements = malloc ((n + 1) * sizeof (ffi_type *));
> +for (i = 0; i < n; ++i)
> +  elements[i] = array_element_type;
> +elements[n] = NULL;
> +
> +array_type.size = array_type.alignment = 0;
> +array_type.type = FFI_TYPE_STRUCT;
> +array_type.elements = elements;
> +@end example
> +
> +Note that arrays cannot be passed or returned by value in C --
> +structure types created like this should only be used to refer to
> +members of real @code{FFI_TYPE_STRUCT} objects.
> +
> +However, a phony array type like this will not cause any errors from
> +@samp{libffi} if you use it as an argument or return type.  This may
> +be confusing.
> +
> +@subsubsection Unions
> +
> +A union can also be emulated using @code{FFI_TYPE_STRUCT}.  In this
> +case, however, you must make sure that the size and alignment match
> +the real requirements of the union.
> +
> +One simple way to do this is to ensue that each element type is laid
> +out.  Then, give the new structure type a single element; the size of
> +the largest element; and the largest alignment seen as well.
> +
> +This example uses the @code{ffi_prep_cif} trick to ensure that each
> +element type is laid out.
> +
> +@example
> +ffi_abi desired_abi;
> +ffi_type union_type;
> +ffi_type **union_elements;
> +
> +int i;
> +ffi_type element_types[2];
> +
> +element_types[1] = NULL;
> +
> +union_type.size = union_type.alignment = 0;
> +union_type.type = FFI_TYPE_STRUCT;
> +union_type.elements = element_types;
> +
> +for (i = 0; union_elements[i]; ++i)
> +  @{
> +    ffi_cif cif;
> +    if (ffi_prep_cif (&cif, desired_abi, 0, union_elements[i], NULL) == FFI_OK)
> +      @{
> +        if (union_elements[i]->size > union_type.size)
> +          @{
> +            union_type.size = union_elements[i];
> +            size = union_elements[i]->size;
> +          @}
> +        if (union_elements[i]->alignment > union_type.alignment)
> +          union_type.alignment = union_elements[i]->alignment;
> +      @}
> +  @}
> +@end example
>
>  @node Type Example
>  @subsection Type Example
> @@ -636,6 +772,8 @@ Prepare a closure function.
>  the writable address returned by @code{ffi_closure_alloc}.
>
>  @var{cif} is the @code{ffi_cif} describing the function parameters.
> +Note that this object, and the types to which it refers, must be kept
> +alive until the closure itself is freed.
>
>  @var{user_data} is an arbitrary datum that is passed, uninterpreted,
>  to your closure function.
> @@ -733,6 +871,28 @@ int main()
>
>  @end example
>
> +@node Thread Safety
> +@section Thread Safety
> +
> +@code{libffi} is not completely thread-safe.  However, many parts are,
> +and if you follow some simple rules, you can use it safely in a
> +multi-threaded program.
> +
> +@itemize @bullet
> +@item
> +@code{ffi_prep_cif} may modify the @code{ffi_type} objects passed to
> +it.  It is best to ensure that only a single thread prepares a given
> +@code{ffi_cif} at a time.
> +
> +@item
> +On some platforms, @code{ffi_prep_cif} may modify the size and
> +alignment of some types, depending on the chosen ABI.  On these
> +platforms, if you switch between ABIs, you must ensure that there is
> +only one call to @code{ffi_prep_cif} at a time.
> +
> +Currently the only affected platform is PowerPC and the only affected
> +type is @code{long double}.
> +@end itemize
>
>  @node Missing Features
>  @chapter Missing Features
> @@ -749,8 +909,6 @@ There is no support for bit fields in structures.
>
>  @item
>  The ``raw'' API is undocumented.
> -@c argument promotion?
> -@c unions?
>  @c anything else?
>  @end itemize
>

Re: documentation patch, oddities, and proposals

[Richard Henderson]

On 11/04/2015 06:32 AM, Tom Tromey wrote:
> * The ffi_arg business for return values I (re-)learned from reading the
>    archvies.  I still haven't fixed my own code to deal with this yet.
>
>    I wasn't sure -- is this needed in some way for the closure API as
>    well?  If so, and someone can explain where (that is, arguments,
>    return values, or both), I will write a documentation patch.

As far as I can tell it's just historical, and now for api compatability with 
earlier libffi.  But perhaps Anthony remembers more.

> * I wanted to compute the layout of a struct.  libffi does this
>    internally but doesn't expose the results.
>
>    I think it would be great to have a new function to do this.  In fact
>    I wrote one, but...

Indeed.  It would be very nice to require a separate layout call, rather than 
imply it from ffi_prep_cif.  This would also nicely fix the problem that libgo 
encountered wrt zero-sized structures.

> * ... this lead me to the discovery that ffi_prep_types modifies some of
>    the libffi globals, meaning that libffi is not type-safe when using
>    multiple ABIs.
>
>    Currently, AFAICT, this only affects long doubles on PPC.  So, not
>    quite as bad as it seems -- but still it is a bug.

Indeed.  I've thought about ways to fix this in the past, and the best I can 
think of imply an api+abi change.

Basically, stop exporting all of the ffi_type_foo variables (which themselves 
imply nasty abi compatibility problems, due to the COPY relocs they require).

Instead, have a function call taking an enumeration for the basic types, as 
well as the abi enum.  Which means that ppc can have two separate structures 
for its longdouble, which means that both can be read-only.

> * Also due to ffi_prep_types, you can't really use the size and
>    alignment fields of an ffi_type unless you know that the ABI hasn't
>    changed.  And, because of this, you also can't share structure types
>    across ABIs.  (Maybe that is obvious?  It was a bit of a surprise to
>    me.)

No, it's not obvious.  Thankfully (?) ppc32 is the only offender here.

> * As you can see from the docs, I also had to resort to some shenanigans
>    to get arrays and unions working.  I think it would be preferable to
>    have FFI_TYPE_ARRAY and FFI_TYPE_UNION directly in the library.

That would be nice.

> * This doesn't impact the docs, but ffi.h.in is a mix of public and
>    private stuff.  I think it would be nice to separate these.

Indeed.

> * I didn't write docs for the raw or java APIs.  It's probably time for
>    those to die.
>
> * Someone who understands what it does should perhaps write
>    documentation for the Go closure code.  Or, if this is only for use by
>    gccgo, then maybe it should be relegated to the aforementioned
>    internal-only header.

Hmm..  They're certainly not truly internal functions, but I can't imagine 
ffi_prep_go_closure being of use to anything but libgo.  Within Go itself, one 
would use the standard library to call out to C functions.

On the other hand, ffi_call_go is useful to anyone wanting to call into a 
function written in Go.  Though I expect writing general-purpose libraries in 
Go to be as rare as invisible pink unicorns...

I'll have a look at the actual doc changes later.


r~

Re: documentation patch, oddities, and proposals

[Tom Tromey]

>> I wasn't sure -- is this needed in some way for the closure API as
>> well?  If so, and someone can explain where (that is, arguments,
>> return values, or both), I will write a documentation patch.

rth> As far as I can tell it's just historical, and now for api
rth> compatability with earlier libffi.  But perhaps Anthony remembers
rth> more.

I also didn't check to see how the "struct { char x; }" case is
handled.  Does that need ffi_arg?  And if so how is it unpacked?

rth> Instead, have a function call taking an enumeration for the basic
rth> types, as well as the abi enum.  Which means that ppc can have two
rth> separate structures for its longdouble, which means that both can be
rth> read-only.

That's what I was thinking as well.

Tom

Re: documentation patch, oddities, and proposals

[Richard Henderson]

On 11/04/2015 03:20 PM, Tom Tromey wrote:
>>> I wasn't sure -- is this needed in some way for the closure API as
>>> well?  If so, and someone can explain where (that is, arguments,
>>> return values, or both), I will write a documentation patch.
>
> rth> As far as I can tell it's just historical, and now for api
> rth> compatability with earlier libffi.  But perhaps Anthony remembers
> rth> more.
>
> I also didn't check to see how the "struct { char x; }" case is
> handled.  Does that need ffi_arg?

No, it should not.  Just the integral types.


r~

Re: documentation patch, oddities, and proposals

[Tom Tromey]

rth> Indeed.  It would be very nice to require a separate layout call,
rth> rather than imply it from ffi_prep_cif.  This would also nicely fix
rth> the problem that libgo encountered wrt zero-sized structures.

What was that problem btw?  If it's something users might encounter I
could update the docs to account for it.

rth> I'll have a look at the actual doc changes later.

Here's a newer version of the patch.  I've expanded the section on
ffi_arg use a bit.  I also updated the closure docs to note that this
behavior is not needed there.

I did a test to see what return pointer is passed to a closure declared
as returning void, since the docs had:

 @c FIXME: is this NULL for void-returning functions?

Experimentally it is not null, so I imagine it is just garbage... ?
I stuck another comment in there, maybe this is worth a bug report.

I'm not sure the thread safety section is correct.  It depends on what
parts of a CIF are referred to after it has bee prepped.  For example if
ffi_call refers to the alignment, then thread safety is hard to achieve
when also using multiple ABIs -- I think you'd have to re-prep the CIF
before each call and also only allow one call at a time (or, really,
just one ABI to be in use at a time).

Tom

diff --git a/doc/libffi.texi b/doc/libffi.texi
index ff72e58..50fc103 100644
--- a/doc/libffi.texi
+++ b/doc/libffi.texi
@@ -107,6 +107,7 @@ values passed between the two languages.
 * Multiple ABIs::               Different passing styles on one platform.
 * The Closure API::             Writing a generic function.
 * Closure Example::             A closure example.
+* Thread Safety::               Thread safety.
 @end menu
 
 
@@ -190,12 +191,29 @@ to ensure this.  If @var{cif} declares that the function returns
 @code{void} (using @code{ffi_type_void}), then @var{rvalue} is
 ignored.
 
+In most situations, @samp{libffi} will handle promotion according to
+the ABI.  However, for historical reasons, there is a special case
+with return values that must be handled by your code.  In particular,
+for integral (not @code{struct}) types that are narrower than the
+system register size, the return value will be widened by
+@samp{libffi}.  @samp{libffi} provides a type, @code{ffi_arg}, that
+can be used as the return type.  For example, if the CIF was defined
+with a return type of @code{char}, @samp{libffi} will try to store a
+full @code{ffi_arg} into the return value.
+
 @var{avalues} is a vector of @code{void *} pointers that point to the
 memory locations holding the argument values for a call.  If @var{cif}
 declares that the function has no arguments (i.e., @var{nargs} was 0),
 then @var{avalues} is ignored.  Note that argument values may be
 modified by the callee (for instance, structs passed by value); the
 burden of copying pass-by-value arguments is placed on the caller.
+
+Note that while the return value must be register-sized, arguments
+should exactly match their declared type.  For example, if an argument
+is a @code{short}, then the entry is @var{avalues} should point to an
+object declared as @code{short}; but if the return type is
+@code{short}, then @var{rvalue} should point to an object declared as
+a larger type -- usually @code{ffi_arg}.
 @end defun
 
 
@@ -246,6 +264,8 @@ int main()
 @menu
 * Primitive Types::             Built-in types.
 * Structures::                  Structure types.
+* Size and Alignment::          Size and alignment of types.
+* Arrays and Unions::           Arrays and unions.
 * Type Example::                Structure type example.
 * Complex::                     Complex types.
 * Complex Type Example::        Complex type example.
@@ -370,8 +390,7 @@ when passing to @code{ffi_prep_cif}.
 @node Structures
 @subsection Structures
 
-Although @samp{libffi} has no special support for unions or
-bit-fields, it is perfectly happy passing structures back and forth.
+@samp{libffi} is perfectly happy passing structures back and forth.
 You must first describe the structure to @samp{libffi} by creating a
 new @code{ffi_type} object for it.
 
@@ -391,9 +410,132 @@ For a structure, this should be set to @code{FFI_TYPE_STRUCT}.
 @item ffi_type **elements
 This is a @samp{NULL}-terminated array of pointers to @code{ffi_type}
 objects.  There is one element per field of the struct.
+
+Note that @samp{libffi} has no special support for bit-fields.  You
+must manage these manually.
 @end table
 @end deftp
 
+The @code{size} and @code{alignment} fields will be filled in by
+@code{ffi_prep_cif} or @code{ffi_prep_cif_var}, as needed.
+
+@node Size and Alignment
+@subsection Size and Alignment
+
+@code{libffi} will set the @code{size} and @code{alignment} fields of
+an @code{ffi_type} object for you.  It does so using its knowledge of
+the ABI.
+
+You might expect that you can simply read these fields for a type that
+has been laid out by @code{libffi}.  However, there are some caveats.
+
+@itemize @bullet
+@item
+The size or alignment of some of the built-in types may vary depending
+on the chosen ABI.
+
+@item
+The size and alignment of a new structure type will not be set by
+@code{libffi} until it has been passed to @code{ffi_prep_cif}.
+
+@item
+A structure type cannot be shared across ABIs.  Instead each ABI needs
+its own copy of the structure type.
+@end itemize
+
+So, before examining these fields, it is safest to pass the
+@code{ffi_type} object to @code{ffi_prep_cif} first.  This function
+will do all the needed setup.
+
+@example
+ffi_type *desired_type;
+ffi_abi desired_abi;
+@dots{}
+ffi_cif cif;
+if (ffi_prep_cif (&cif, desired_abi, 0, desired_type, NULL) == FFI_OK)
+  @{
+    size_t size = desired_type->size;
+    unsigned short alignment = desired_type->alignment;
+  @}
+@end example
+
+@node Arrays and Unions
+@subsection Arrays and Unions
+
+@subsubsection Arrays
+
+@samp{libffi} does not have direct support for arrays or unions.
+However, they can be emulated using structures.
+
+To emulate an array, simply create an @code{ffi_type} using
+@code{FFI_TYPE_STRUCT} with as many members as there are elements in
+the array.
+
+@example
+ffi_type array_type;
+ffi_type **elements
+int i;
+
+elements = malloc ((n + 1) * sizeof (ffi_type *));
+for (i = 0; i < n; ++i)
+  elements[i] = array_element_type;
+elements[n] = NULL;
+
+array_type.size = array_type.alignment = 0;
+array_type.type = FFI_TYPE_STRUCT;
+array_type.elements = elements;
+@end example
+
+Note that arrays cannot be passed or returned by value in C --
+structure types created like this should only be used to refer to
+members of real @code{FFI_TYPE_STRUCT} objects.
+
+However, a phony array type like this will not cause any errors from
+@samp{libffi} if you use it as an argument or return type.  This may
+be confusing.
+
+@subsubsection Unions
+
+A union can also be emulated using @code{FFI_TYPE_STRUCT}.  In this
+case, however, you must make sure that the size and alignment match
+the real requirements of the union.
+
+One simple way to do this is to ensue that each element type is laid
+out.  Then, give the new structure type a single element; the size of
+the largest element; and the largest alignment seen as well.
+
+This example uses the @code{ffi_prep_cif} trick to ensure that each
+element type is laid out.
+
+@example
+ffi_abi desired_abi;
+ffi_type union_type;
+ffi_type **union_elements;
+
+int i;
+ffi_type element_types[2];
+
+element_types[1] = NULL;
+
+union_type.size = union_type.alignment = 0;
+union_type.type = FFI_TYPE_STRUCT;
+union_type.elements = element_types;
+
+for (i = 0; union_elements[i]; ++i)
+  @{
+    ffi_cif cif;
+    if (ffi_prep_cif (&cif, desired_abi, 0, union_elements[i], NULL) == FFI_OK)
+      @{
+        if (union_elements[i]->size > union_type.size)
+          @{
+            union_type.size = union_elements[i];
+            size = union_elements[i]->size;
+          @}
+        if (union_elements[i]->alignment > union_type.alignment)
+          union_type.alignment = union_elements[i]->alignment;
+      @}
+  @}
+@end example
 
 @node Type Example
 @subsection Type Example
@@ -636,6 +778,8 @@ Prepare a closure function.
 the writable address returned by @code{ffi_closure_alloc}.
 
 @var{cif} is the @code{ffi_cif} describing the function parameters.
+Note that this object, and the types to which it refers, must be kept
+alive until the closure itself is freed.
 
 @var{user_data} is an arbitrary datum that is passed, uninterpreted,
 to your closure function.
@@ -652,8 +796,12 @@ The @code{ffi_cif} passed to @code{ffi_prep_closure_loc}.
 @item ret
 A pointer to the memory used for the function's return value.
 @var{fun} must fill this, unless the function is declared as returning
-@code{void}.
+@code{void}.  Note that this points to memory that is exactly the size
+of the type given as the return type when initializing the CIF.  In
+particular, closures do not have the special promotion behavior of
+@code{ffi_call}.
 @c FIXME: is this NULL for void-returning functions?
+@c (experimentally it is not, but it seems like a good idea)
 
 @item args
 A vector of pointers to memory holding the arguments to the function.
@@ -664,8 +812,7 @@ The same @var{user_data} that was passed to
 @end table
 
 @code{ffi_prep_closure_loc} will return @code{FFI_OK} if everything
-went ok, and something else on error.
-@c FIXME: what?
+went ok, and one of the other @code{ffi_status} values on error.
 
 After calling @code{ffi_prep_closure_loc}, you can cast @var{codeloc}
 to the appropriate pointer-to-function type.
@@ -733,6 +880,28 @@ int main()
 
 @end example
 
+@node Thread Safety
+@section Thread Safety
+
+@code{libffi} is not completely thread-safe.  However, many parts are,
+and if you follow some simple rules, you can use it safely in a
+multi-threaded program.
+
+@itemize @bullet
+@item
+@code{ffi_prep_cif} may modify the @code{ffi_type} objects passed to
+it.  It is best to ensure that only a single thread prepares a given
+@code{ffi_cif} at a time.
+
+@item
+On some platforms, @code{ffi_prep_cif} may modify the size and
+alignment of some types, depending on the chosen ABI.  On these
+platforms, if you switch between ABIs, you must ensure that there is
+only one call to @code{ffi_prep_cif} at a time.
+
+Currently the only affected platform is PowerPC and the only affected
+type is @code{long double}.
+@end itemize
 
 @node Missing Features
 @chapter Missing Features
@@ -749,9 +918,10 @@ There is no support for bit fields in structures.
 
 @item
 The ``raw'' API is undocumented.
-@c argument promotion?
-@c unions?
 @c anything else?
+
+@item
+The Go API is undocumented.
 @end itemize
 
 Note that variadic support is very new and tested on a relatively

Re: documentation patch, oddities, and proposals

[Richard Henderson]

On 11/07/2015 08:00 PM, Tom Tromey wrote:
> rth> Indeed.  It would be very nice to require a separate layout call,
> rth> rather than imply it from ffi_prep_cif.  This would also nicely fix
> rth> the problem that libgo encountered wrt zero-sized structures.
>
> What was that problem btw?  If it's something users might encounter I
> could update the docs to account for it.

The fact that size 0 is the trigger that the structure hasn't been layed out. 
So every call to such a function re-enters the layout for a zero-sized struct.
In the end it still works, but there's more overhead than needed.


r~

Re: documentation patch, oddities, and proposals

[Tom Tromey]

I sent my documentation patch as a PR.

[...]
rth> Indeed.  I've thought about ways to fix this in the past, and the best
rth> I can think of imply an api+abi change.

rth> Basically, stop exporting all of the ffi_type_foo variables (which
rth> themselves imply nasty abi compatibility problems, due to the COPY
rth> relocs they require).

rth> Instead, have a function call taking an enumeration for the basic
rth> types, as well as the abi enum.  Which means that ppc can have two
rth> separate structures for its longdouble, which means that both can be
rth> read-only.

Ok, following up on this, here's my wish-list if an ABI break is
possible.

* Require each ABI to have a string name and provide a way to look up
  ABI by name.

  This would let generic FFI wrappers expose ABIs to their callers
  without undue difficulty.  This would be handy at least for Lisp
  bindings; and it seems reasonably lightweight.

  Also, document all ABIs -- not really an ABI problem, but worth
  pointing out.

  I suppose this could be added without breaking ABI.

* As you said, get rid of ffi_type_* globals, replace with per-ABI calls
  using the FFI_TYPE_* constants.

* Require a separate type-prep stage.  For struct types allow an out
  parameter that would fill in field offsets.  Requiring the separate
  call would make it possible for all the calls taking an ffi_type to be
  const-correct.

* Add FFI_TYPE_UNION and FFI_TYPE_ARRAY.

* Mark each ffi_type with an "ABI-specific" flag.  This would taint a
  type as ABI-specific.  Layout of a struct (or union or array) would
  propagate this flag from the members to the aggregate.  The idea here
  is that in most cases this just doesn't matter and this would give an
  easy way to notice when it does.  ffi_prep_cif could check this flag.


Adding bitfield support might be nice, not sure.

I don't care a whole lot about the ffi_arg thing, but if it isn't
completely crazy to fix it, it might be a good idea.

Tom

Re: documentation patch, oddities, and proposals

[Tom Tromey]

Tom> * Require each ABI to have a string name and provide a way to look up
Tom>   ABI by name.
[...]
Tom>   Also, document all ABIs -- not really an ABI problem, but worth
Tom>   pointing out.
Tom>   I suppose this could be added without breaking ABI.

I was taking a stab at this today but I realized I don't fully
understand the PPC ABI stuff.

Unlike the other ports, src/powerpc/ffitarget.h puts a bunch of flags
into the ffi_abi enum.  This is fine (a bit odd maybe), but what I don't
know is whether all possible combinations of flags are valid.

For one host:

  FFI_LINUX = 8,
  /* This and following bits can reuse FFI_COMPAT values.  */
  FFI_LINUX_STRUCT_ALIGN = 1,
  FFI_LINUX_LONG_DOUBLE_128 = 2,

FFI_LINUX is required but can zero or more of the others be added in any
combination?

Another host:

  FFI_SYSV = 8,
  FFI_SYSV_SOFT_FLOAT = 1,
  FFI_SYSV_STRUCT_RET = 2,
  FFI_SYSV_IBM_LONG_DOUBLE = 4,
  FFI_SYSV_LONG_DOUBLE_128 = 16,

Same question here.

If they are all valid then I guess I'll need to rethink my simple naming
scheme, since having 16 different multi-word ABI names seems unpleasant.

Tom

Re: documentation patch, oddities, and proposals

[Richard Henderson]

On 11/16/2015 05:26 AM, Tom Tromey wrote:
> Unlike the other ports, src/powerpc/ffitarget.h puts a bunch of flags
> into the ffi_abi enum.  This is fine (a bit odd maybe), but what I don't
> know is whether all possible combinations of flags are valid.

No, I don't believe they are.  I think it's set up that way simply for 
convenience within the backend, testing properties rather than enumerating all 
of the possibilites.


r~