Re: [RFC PATCH v3 1/5] Libffi Static Trampolines

["Madhavan T. Venkataraman" <madvenka@linux.microsoft.com>]

On 1/27/21 4:15 PM, DJ Delorie wrote:
> "Madhavan T. Venkataraman" <madvenka@linux.microsoft.com> writes:
>>>> +case "$target" in
>>>> +     *-linux*)
>>>> +       AC_DEFINE(FFI_EXEC_STATIC_TRAMP, 1,
>>>> +                 [Define this if you want statically defined trampolines])
>>>> +     ;;
>>>> +esac
>>>
>>> Ok.  Might want to conditional for the arches that are supported too.
>>>
>>
>> ok. I will add the arches although the code currently handles that by testing
>> ffi_tramp_arch == NULL.
> 
> /me ponders...
> 
> Is the intent here that a Linux client has to check for static
> trampolines in configure (i.e. it's on linux) but *also* check if it's
> supported at runtime?  If we could minimize the number of checks, that
> would help the developer.  If the API is always present (or always
> hidden, so that the client need do nothing), that's best.
> 
> Ideally, the client would need do nothing regardless, but that only
> works if the static trampoline functionality is 100% hidden.
> 

libffi clients do not need to do anything. They do not even need to recompile.
They call the closure API which internally uses static trampolines (if supported)
or legacy trampolines (if not supported). As far as they are concerned, static
trampolines are 100% hidden.

For apps/libraries that wish to use the static trampoline API directly in the future,
there will be changes to use the API (and to refactor their code, if necessary). See
the examples I sent to AG.

The above ffi_tramp_arch check is done internally by the API to see if the
underlying architecture supports it or not. So, let us say libffi is running
on linux on SPARC. There is no ffi_tramp_arch() defined for SPARC. So, the API
functions will all return 0 and the caller (the closure code) will fallback to
using the legacy trampolines.

>> My intention is to allow applications and libraries that currently use dynamic code
>> to use the libffi API to eliminate their dynamic code. The maintainers of these
>> apps/libs would rewrite their dynamic code into static code. All they need is a
>> way to pass data to their static code (that uses PC-relative access). The libffi
>> API will let them do that.
> 
> Ok, as long as there's a use case.
> 

I know that there are a lot of use cases out there. But I have not been able to find
one in open source by googling. If someone can point me to an open source app/lib
that uses dynamic code, I could attempt to show how to use the static trampoline
API to replace the dynamic code with static code.

> Note that adding public APIs is MUCH easier than removing them later...
> 

Agreed. So, I have suggested to Anthony that we could expose the public API
after these changes have soaked in libffi for some time and have been well
exercised. I could submit a patch after, say, 6 months just to expose the
API.

>> My intention is to always have the API functions defined in the library. If
>> FFI_EXEC_STATIC_TRAMP is defined, then, the API will supply static trampolines.
>> Else, the stub API functions will return an error and the caller can fall back
>> on his existing method if any.
> 
> Ok.  I guess that answers one of my earlier questions (above) ;-)
> 
>>>> @@ -943,12 +966,17 @@ void *
>>>>  ffi_data_to_code_pointer (void *data)
>>>>  {
>>>>    msegmentptr seg = segment_holding (gm, data);
>>>> +
>>>
>>> Extraneous but ok.
>>
>> This is needed.
> 
> I meant the extra blank line ;-)

I am sorry I misunderstood. I will delete the extra blank line.

> 
>>>> +  if (table->nfree == gtramp.ntramp && gtramp.ntables > 1)
>>>
>>> You can't compare these.  The first is a count of *trampolines* and the
>>> second is a count of *tables*.
>>>
>>
>> There are two conditions here:
>>
>> If (table->nfree == gtramp.ntramp) is true, then all of the table's slots are free.
> 
> This reads as "If the number of trampolines equals the number of tables"
> which are not comparable.  If you're doing some math magic hack here to
> get the right answer, it *really* needs a big comment explaining how it
> works.
> 

gtramp.ntramp is not the number of tables. It is the number of trampolines
in a table. Here is the comment that describes the field:

/*
 * Trampoline globals.
...
 * ntramp
 *      Total number of trampolines in the trampoline code table.
...
 */

Here is how it gets initialized:

  /*
   * Get trampoline code table information from the architecture.
   */
  gtramp.text = ffi_tramp_arch (&gtramp.size, &gtramp.map_size);
  gtramp.ntramp = gtramp.map_size / gtramp.size;

gtramp.map_size is the size of a single code table mapping. gtramp.size
is the size of a single trampoline. So, gtramp.ntramp is the integral
number of trampolines that fit into a single code table.

>>> tramp_table_del unlinks the table, but doesn't deallocate its
>>> resources.  Are these resources lost/leaked?
>>>
>>
>> The table list contains tables that have free slots. If all slots of a table
>> are allocated, the table is removed from the list. Its trampoline resources
>> are held inside closures. When those closures are freed, the table will be back
>> on the list. When the table gets back all of its trampolines, it will be freed
>> along with all of its trampolines.
> 
> Ah, ok.
> 

Thanks!

Madhavan