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

[DJ Delorie]

"Madhavan T. Venkataraman" <madvenka@linux.microsoft.com> writes:
>> Extern, but local to this port, yes?
>
> Yes. So, is this declaration acceptable?

Yup!

>>> +  /* 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.

>> 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 ;-)

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 could add a small comment saying "see comment above trampoline_code_table".
> Is that acceptable?

That would be good too :-)