[RFC PATCH v3 2/5] x86: Support for Static Trampolines

[Madhavan T. Venkataraman]

On 1/28/21 4:17 PM, DJ Delorie wrote:
> "Madhavan T. Venkataraman" <madvenka@linux.microsoft.com> writes:
>>> Extern, but local to this port, yes?
>>
>> Yes. So, is this declaration acceptable?
> 
> Yup!

Great.

> 
>>>> +  /* Initialize the dynamic trampoline. */
>>>
>>> Should these new APIs be inside #if FFI_EXEC_STATIC_TRAMP ?
>>>
>>
>> Strictly speaking, these should be inside that ifdef. I did it this
>> way to avoid too many ifdefs in the code. If you prefer I put them
>> inside the ifdefs, I will do it. I will try to minimize the number of
>> ifdefs somehow.
> 
> No, it's ok, I was just worried that if ffi_tramp_is_present was in an
> #ifdef any callers would be too - but as you noted in another email,
> there's always a ffi_tramp_is_present even if it always returns false.
> 

ok.

>>> This hack to detect CET should be replaced by the logic in ffitarget.h,
>>> or add a #define CET_ENABLED to ffitarget.h
>>>
>>
>> So, _CET_ENDBR for x64 is either defined as:
>>
>> If CET is present:
>> 	#define _CET_ENDBR	endbr64
>> Otherwise:
>> 	#define _CET_ENDBR
>>
>> So, it is always defined. So, I cannot do something like:
> 
> I was thinking of the conditionals in src/x86/ffitarget.h:
> 
> #if !defined(GENERATE_LIBFFI_MAP) && defined(__ASSEMBLER__) \
>     && defined(__CET__)
> 
> Obviously you'd omit the __ASSEMBLER__ one ;-)

So, the definition is like this:

#if !defined(GENERATE_LIBFFI_MAP) && defined(__ASSEMBLER__) \
    && defined(__CET__)
# include <cet.h>
# define _CET_NOTRACK notrack
#else
# define _CET_ENDBR
# define _CET_NOTRACK
#endif

Where we include cet.h, CET_ENDBR may still be defined either
as endbr64 or empty depending on __CET__ & 1 being non-zero or
zero, right?

So, would something like this work?

#if !defined(GENERATE_LIBFFI_MAP) && defined(__CET__)
# include <cet.h>
# if (__CET__ & 1) != 0
#  define ENDBR_PRESENT
# endif
# define _CET_NOTRACK notrack
#else
# define _CET_ENDBR
# define _CET_NOTRACK
#endif

Then, I can do:

#ifdef ENDBR_PRESENT
blah blah

> 
> If you duplicate the ffitarget.h logic you get the same results.
> 
>>> Copies first argument to %r10, discards return address and arg - closure
>>> will return to whoever called it's caller.  I'm not sure how this works,
>>> which means *at least* a comment needs to be here ;-)
>>>
>> The target code in this case is the alt entry point.
>>
>> This is what the alt code is doing:
>> 	- load the data (closure) address in r10
>> 	- discard the original value of r10 saved on the stack
>> 	  since we are using r10 we don't need its original value
>> 	- restore the stack back to what it was when the static trampoline was
>> 	  invoked.
> 
> These kinds of short notes should be useful inline comments in the assembler:
> 
> 	movq	8(%rsp), %r10	/* load closure */
> 

I will add this.

>> I could add a small comment saying "see comment above trampoline_code_table".
>> Is that acceptable?
> 
> That would be good too :-)
> 

I will add this as well.

Thanks!

Madhavan