fix(jit/win64): platform call conv for closure indirect calls - #2
Merged
Conversation
The CallClosure handler built its indirect-call signature with a hard- coded CallConv::SystemV. On x86_64-pc-windows-msvc the callee is compiled WindowsFastcall, so arguments landed in SysV registers (rdi, rsi) while the callee read MS x64 registers (rcx, rdx). The async poll fn is reached via CreateClosure -> CallClosure, so every JIT-executed async function on Windows read a garbage state-machine pointer and returned garbage (async_execution_tests / promise_combinator_tests: double(21) -> garbage instead of 42). Build the signature from module.make_signature() so it carries the platform default calling convention, matching the callee — the same fix already applied to the sibling indirect-call site. No-op on Unix (default is SystemV there); async + effect suites still green on macOS/Linux.
…vention The real cause of the Windows async garbage: the krio_adapter-generated async entry (`generate_promise_entry`), sync entry (`generate_sync_entry`), and poll fn (`reshape_to_poll_abi`) inherited HirFunction::new's default `CallingConvention::Fast`. On x86_64 Fast lowers to Cranelift's Fast, which happens to match `extern "C"` (SystemV) on Unix — so Linux/macOS worked — but on x86_64-pc-windows-msvc it does NOT match Win64. The runtime calls these functions through `extern "C"` transmutes, so on Windows the entry read its first argument (and the poll fn its state-machine pointer) from the wrong register, returning garbage for every JIT-executed async function. Pin the three runtime-facing generated functions to `CallingConvention::C` (= platform default = WindowsFastcall on Windows). No-op on Unix. Complements the CallClosure indirect-call fix in the same branch (inner- promise polls).
…ntion Regression guard for the Windows async-garbage fix: the async promise entry and sync entry must carry CallingConvention::C (platform default), not HirFunction::new's Fast default, or JIT-executed async functions read arguments from the wrong registers on x86_64-pc-windows-msvc.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes the Windows-only async garbage: JIT-executed async functions returned garbage on
x86_64-pc-windows-msvc(e.g.double(21)→ 174116032 instead of 42) acrossasync_execution_testsandpromise_combinator_tests.Root cause: the
CallClosureindirect-call site hard-codedCallConv::SystemV. The async poll fn is reached viaCreateClosure→CallClosure, and on Windows the callee is compiledWindowsFastcall— so args landed in SysV registers while the callee read MS x64 registers, makingpoll_fnread a garbage state-machine pointer. Same root cause and fix as the sibling indirect-call site (which already switched to the module default).Fix: build the signature from
module.make_signature()so it carries the platform default calling convention, matching the callee. No-op on Unix (default is SystemV there).Verifying via the Windows CI job on this PR.