From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from linux.microsoft.com (linux.microsoft.com [13.77.154.182]) by sourceware.org (Postfix) with ESMTP id 5A90D3850400 for ; Wed, 27 Jan 2021 22:43:47 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org 5A90D3850400 Received: from [192.168.254.32] (unknown [47.187.219.45]) by linux.microsoft.com (Postfix) with ESMTPSA id BD77420B7192; Wed, 27 Jan 2021 14:43:46 -0800 (PST) DKIM-Filter: OpenDKIM Filter v2.11.0 linux.microsoft.com BD77420B7192 Subject: Re: [RFC PATCH v3 1/5] Libffi Static Trampolines To: DJ Delorie Cc: libffi-discuss@sourceware.org References: From: "Madhavan T. Venkataraman" Message-ID: Date: Wed, 27 Jan 2021 16:43:45 -0600 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.10.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit X-Spam-Status: No, score=-20.9 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, ENV_AND_HDR_SPF_MATCH, NICE_REPLY_A, SPF_HELO_PASS, SPF_PASS, TXREP, USER_IN_DEF_DKIM_WL, USER_IN_DEF_SPF_WL autolearn=ham autolearn_force=no version=3.4.2 X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on server2.sourceware.org X-BeenThere: libffi-discuss@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Libffi-discuss mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 27 Jan 2021 22:43:49 -0000 On 1/27/21 4:15 PM, DJ Delorie wrote: > "Madhavan T. Venkataraman" 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 (>ramp.size, >ramp.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