[Linux/x86] GenericPInvokeCalliHelper problem

[t-mustafin]

GenericPInvokeCalliHelper produces a problem on linux.

GenericPInvokeCalliHelper takes away VASigCookie argument from the stack and moves on its place return address value. It breaks 16 byte stack alignment and leads to further problems on esp usage: execution could break inside CHECK_STACK_ALIGNMENT macro or on movap [esp+offset] instruction. The problem spreads until caller of GenericPInvokeCalliHelper will restore esp from ebp on self epilog.

runtime/src/coreclr/vm/i386/asmhelpers.S

Lines 619 to 677 in aa52611

	// ==========================================================================
	// Invoked for marshaling-required unmanaged CALLI calls as a stub.
	// EAX - the unmanaged target
	// ECX, EDX - arguments
	// [ESP + 4] - the VASigCookie
	//
	LEAF_ENTRY GenericPInvokeCalliHelper, _TEXT
	// save the target
	push eax
	// EAX <- VASigCookie
	mov eax, [esp + 8] // skip target and retaddr
	mov eax, [eax + VASigCookie__StubOffset]
	test eax, eax
	jz LOCAL_LABEL(GoCallCalliWorker)
	// ---------------------------------------
	push eax
	// stack layout at this point:
	//
	// | ... |
	// | stack arguments | ESP + 16
	// +----------------------+
	// | VASigCookie* | ESP + 12
	// +----------------------+
	// | return address | ESP + 8
	// +----------------------+
	// | CALLI target address | ESP + 4
	// +----------------------+
	// | stub entry point | ESP + 0
	// ------------------------
	// remove VASigCookie from the stack
	mov eax, [esp + 8]
	mov [esp + 12], eax
	// move stub entry point below the RA
	mov eax, [esp]
	mov [esp + 8], eax
	// load EAX with the target address
	pop eax
	pop eax
	// stack layout at this point:
	//
	// | ... |
	// | stack arguments | ESP + 8
	// +----------------------+
	// | return address | ESP + 4
	// +----------------------+
	// | stub entry point | ESP + 0
	// ------------------------
	// CALLI target address is in EAX
	ret

I see couple of options to resolve the problem:

1. Besides VASigCookie GenericPInvokeCalliHelper takes the unmanaged target argument via eax register. Caller could create a structure {the unmanaged target; VASigCookie} and pass a pointer to the structure via eax. GenericPInvokeCalliHelper able to read the structure and place the unmanaged target in eax to the moment of IL_STUB_PIvoke call. So this option changes only caller->GenericPInvokeCalliHelper interface, GenericPInvokeCalliHelper->IL_STUB_PIvoke interface remains unchanged.
2. Change all caller->IL_STUB_PIvoke calling convention to use only stack instead of ecx + edx + stack to pass arguments. It will free ecx register which could be used during caller->GenericPInvokeCalliHelper to keep VASigCookie value.

@jkotas Will such changes break some code outside of GenericPInvokeCalliHelper case? Which variant is preferable?

I couldn't figure out the best area label to add to this issue. If you have write-permissions please help me learn by adding exactly one area label.

[t-mustafin]

cc @alpencolt @gbalykov

[jkotas]

The strategy used to deal with this problem on ARM was to pass the cookie in callee saved register:

runtime/src/coreclr/vm/arm/pinvokestubs.S

Line 181 in 39803d4

// r4 = VASigCookie*

I think it would make sense to use the same strategy here as well and pass the cookie e.g. in ebx. If it helps, I think it would be fine to change Windows x86 to be on the same plan as well - it may get rid of a few ifdefs in the JIT.

Caller could create a structure

I agree that this approach would work too, but creating the structure in the JIT would be probably more involved than just using callee saved register like on ARM.

Change all caller->IL_STUB_PIvoke calling convention to use only stack

You would also need to change the calling convention for the callee. I think it would be involved change, touching many places.

For completeness, another option would be to get rid of the GenericPInvokeCalliHelper helper completely and instead change the JIT to call the IL stub directly. It is how it is done for NativeAOT (look for convertPInvokeCalliToCall method on JIT/EE interface). But that is probably pretty involved as well.

[jkotas]

One more option: Pass in the unmanaged target as regular argument to the stub and use the special register (eax, r12, etc.) for the cookie. It would reduce the dependencies on passing things in special registers that is a good thing, but it should be less involved than going all the way to convertPInvokeCalliToCall.