Skip to content

Conversation

BrzVlad
Copy link
Member

@BrzVlad BrzVlad commented Jul 6, 2023

First, we no longer assert that the call convetion is cdecl on the slowpath. The pinvoke trampoline on x86 restores the saved stack pointer so it doesn't matter if the pinvoke callee pops or not the stack. Therefore both stdcall and cdecl calls should be supported with the same path. Untested.

Second, we use the fastpath (which uses the C call convention) only if the pinvoke signature is also cdecl, otherwise we have cconv mismatch.

@ghost
Copy link

ghost commented Jul 6, 2023

Tagging subscribers to this area: @BrzVlad, @kotlarmilos
See info in area-owners.md if you want to be subscribed.

Issue Details

First, we no longer assert that the call convetion is cdecl on the slowpath. The pinvoke trampoline on x86 restores the saved stack pointer so it doesn't matter if the pinvoke callee pops or not the stack. Therefore both stdcall and cdecl calls should be supported with the same path. Untested.

Second, we use the fastpath (which uses the C call convention) only if the pinvoke signature is also cdecl, otherwise we have cconv mismatch.

Author: BrzVlad
Assignees: BrzVlad
Labels:

area-Codegen-Interpreter-mono

Milestone: -

First, we no longer assert that the call convetion is cdecl on the slowpath. The pinvoke trampoline on x86 restores the saved stack pointer so it doesn't matter if the pinvoke callee pops or not the stack. Therefore both stdcall and cdecl calls should be supported with the same path. Untested.

Second, we use the fastpath (which uses the C call convention) only if the pinvoke signature is also cdecl, otherwise we have cconv mismatch.
@BrzVlad BrzVlad force-pushed the fix-interp-x86-cconv branch from cd379a1 to dbfd395 Compare July 7, 2023 06:47
@BrzVlad BrzVlad merged commit 2e26fbd into dotnet:main Jul 7, 2023
@ghost ghost locked as resolved and limited conversation to collaborators Aug 6, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants