Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

callconv(.Fastcall) broken on i386-windows #10363

Open
Tracked by #537
drew-gpf opened this issue Dec 19, 2021 · 1 comment
Open
Tracked by #537

callconv(.Fastcall) broken on i386-windows #10363

drew-gpf opened this issue Dec 19, 2021 · 1 comment
Labels
arch-x86 32-bit x86 backend-llvm The LLVM backend outputs an LLVM IR Module. bug Observed behavior contradicts documented or intended behavior os-windows
Milestone

Comments

@drew-gpf
Copy link
Contributor

drew-gpf commented Dec 19, 2021

Zig Version

0.9.0-dev.1945+efdb94486

Steps to Reproduce

test.zig:

const std = @import("std");
extern fn call_zig(function: usize) callconv(.Fastcall) void;

fn fastcall(ecx: u32, edx: u32) callconv(.Fastcall) void {
    std.debug.print("{}, {}\n", .{ecx, edx});
}

pub fn main() void {
    @call(.{ .modifier = .never_inline }, call_zig, .{@ptrToInt(fastcall)});
}

test.c:

#include <stdint.h>

typedef void(__fastcall*zig_exported_fn)(uint32_t ecx, uint32_t edx);

void __fastcall call_zig(zig_exported_fn fn)
{
    fn(56, 1337);
}

Build with: zig build-exe --strip -OReleaseSmall -target i386-windows test.zig test.c (stripped and made small because IDA Pro likes to crash when loading a PDB generated by zig)

Expected Behavior

This should produce test.exe which prints "56, 1337" when ran.

Actual Behavior

This will not link:

lld-link: error: undefined symbol: @call_zig@4
>>> referenced by test.exe.lto.obj:(_wWinMainCRTStartup@0)
error: LLDReportedFailure

zig will correctly mangle call_zig's name with a leading @ to mean fastcall, but it miscalculates the number of (stack) bytes needed for arguments, as the first argument should be present in the ecx register and not in a stack location. It seems to be handling arguments as if it were stdcall but still doing name mangling for fastcall.

Removing __fastcall and callconv(.Fastcall) from call_zig will link but yield an incorrect (undefined) output (ex. 0, 0) because fastcall() will read the arguments ecx and edx from stack locations--call_zig() in test.c will instead correctly pass them in their corresponding registers, ecx and edx.

When not used in a situation where link errors are involved - I ran into this while using MS Detours - it either causes something to crash because it pops too many arg bytes off the stack or it just behaves incorrectly. Conveniently for me it turned out the C code being replaced only used __fastcall to emulate __thiscall, which works correctly with callconv(.Thiscall).

@drew-gpf drew-gpf added the bug Observed behavior contradicts documented or intended behavior label Dec 19, 2021
@LemonBoy
Copy link
Contributor

Good catch. Zig must specify inreg for the function parameters when applicable.

@andrewrk andrewrk added arch-x86 32-bit x86 os-windows backend-llvm The LLVM backend outputs an LLVM IR Module. labels Dec 19, 2021
@andrewrk andrewrk added this to the 0.11.0 milestone Dec 19, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
arch-x86 32-bit x86 backend-llvm The LLVM backend outputs an LLVM IR Module. bug Observed behavior contradicts documented or intended behavior os-windows
Projects
None yet
Development

No branches or pull requests

3 participants