Incorrect Calling Conventions on Windows about wasmer HOT 9 CLOSED

It seems like the problem there is that ctx is actually passed after the f64 or something. f64 is in the XMM0 register even though the ABI requires it to be in the XMM1 register if it's the second argument, which is also not the case for Linux, so this would explain this being a Windows only problem again. I think I saw the ctx argument being moved to the first parameter in some PR. Maybe it didn't do that all the way?

Here you can see XMM1 being used:
https://rust.godbolt.org/z/v4BTVw

Here's the jitted code filling the value into XMM0:

Alternatively, cranelift may not know about the Windows calling convention and thus just fill the value into XMM0 like it would on Linux (either it isn't being told about it, or it doesn't support it yet).

from wasmer.

Defining the wrap function to always be of "sysv64" calling convention fixes the bug, but that seems more like a workaround rather than a proper fix, as cranelift should probably just emit the correct calling convention in the first place. I don't know where this is done though. (Update: This fixes some things but breaks others, I'm thinking cranelift actually is going for the windows calling convention but the ctx parameter isn't implemented correctly)

I'd encourage some maintainer to verify if all the calling conventions between the jit'ed code and rust are actually correct (all the trampolines, libcalls, ...).

from wasmer.

@CryZe Thanks for the deep dive! I "own" most of the runtime code, so I'll take a look at this on Monday. It's probably the calling convention getting set up wrong somewhere.

from wasmer.

@CryZe Sorry this took so long, but I've just had time to investigate and indeed yes, cranelift implements the x64 calling convention incorrectly. I honestly don't blame them, it seems to waste registers.

Here's the spec: https://docs.microsoft.com/en-us/cpp/build/x64-calling-convention?view=vs-2017#example-of-argument-passing-3---mixed-ints-and-floats

from wasmer.

We created an issue in Cranelift: bytecodealliance/cranelift#691

from wasmer.

This issue was fixed in: bytecodealliance/cranelift#701 (related issue in cranelift: bytecodealliance/cranelift#691)

Once we update the Cranelift version the issue should be fixed in our runtime.

Keeping the issue open until then!

from wasmer.

The implementation was unfortunately incorrect. So they still need to fix that. (Oh wait, they fixed that too).

from wasmer.

I had similar problems with floating point arguments on Windows, but indeed appears to work correctly on master with the the updated cranelift.

Are you guys planning to tag a new release in the near future? Would be convenient :)

from wasmer.

This seems to be resolved :)

from wasmer.