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.

Sign up for GitHub

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

Jump to bottom

Variadic functions don't allow "system" on non-x86 Windows #110505

Closed
kennykerr opened this issue Apr 18, 2023 · 8 comments · Fixed by #119587
Closed

Variadic functions don't allow "system" on non-x86 Windows #110505

kennykerr opened this issue Apr 18, 2023 · 8 comments · Fixed by #119587
Labels
A-abi Area: Concerning the application binary interface (ABI). C-bug Category: This is a bug. O-windows Operating system: Windows T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@kennykerr
Copy link
Contributor

kennykerr commented Apr 18, 2023 •
edited
Loading

While testing variadic function support with raw-dylib I discovered that Rust requires such functions to use either "C" or "cdecl" ABIs but raw-dylib requires the "system" ABI on non-x86 Windows.

My undertstanding from discussing with @dpaoliello is that this is not an issue with raw-dylib itself but with Rust's insistence that such functions aren't allowed to use the "system" ABI. It's just that raw-dylib is very strict about calling convention so that's where the issue surfaced.

#[link(name = "library", modifiers = " verbatim")]
extern "system" {
    pub fn DbgPrint(format: *const u8, ...) -> u32;
}

On x64 I should be able to compile this but Rust says no:

error[E0045]: C-variadic function must have a compatible calling convention, like `C` or `cdecl`
 --> src\lib.rs:3:5
  |
3 |     pub fn DbgPrint(format: *const u8, ...) -> u32;
  |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ C-variadic function must have a compatible calling convention

This example works if I change the ABI to "cdecl" but that's not meant to be a valid ABI on x64 and so when I try to compile this with raw-dylib it fails:

#[link(name = "library", kind = "raw-dylib", modifiers = " verbatim")]
extern "cdecl" {
    pub fn DbgPrint(format: *const u8, ...) -> u32;
}
error: ABI not supported by `#[link(kind = "raw-dylib")]` on this architecture
 --> src\lib.rs:3:5
  |
3 |     pub fn DbgPrint(format: *const u8, ...) -> u32;
  |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

The practical result is that it is impossible to link this function with raw-dylib on non-x86.
@kennykerr kennykerr added the C-bug Category: This is a bug. label Apr 18, 2023
This was referenced Apr 18, 2023
Merged
Closed
@compiler-errors
Copy link
Member

cc #97971 which originally added variadics support to some other calling conventions (though not extern "system" it seems) behind the feature gate #![feature(extended_varargs_abi_support)].

@kennykerr kennykerr mentioned this issue Apr 19, 2023
Merged
@kennykerr
Copy link
Contributor Author

A workaround is to use "C" on non-x86 as I implemented here: microsoft/windows-rs#2458

@dpaoliello
Copy link
Contributor

@wesleywiser @nagisa Thoughts on permitting variadic parameters with the system calling convention for non-x86 Windows? If #87678 gets merged then we'll be pushing folks to use system so that they get the correct calling convention for Windows APIs on all architectures, but that breaks compatibility with variadic parameters.

@jyn514 jyn514 added O-windows Operating system: Windows T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. A-abi Area: Concerning the application binary interface (ABI). labels Apr 20, 2023
@kennykerr kennykerr mentioned this issue Jul 18, 2023
Merged
@workingjubilee workingjubilee mentioned this issue Oct 19, 2023
Open
@RalfJung
Copy link
Member

RalfJung commented Jan 4, 2024

A workaround is to use "C" on non-x86 as I implemented here: microsoft/windows-rs#2458

This work-around is causing pain. Would be great if there was a way that it could be avoided. :)

@RalfJung
Copy link
Member

RalfJung commented Jan 4, 2024 •
edited
Loading

Thoughts on permitting variadic parameters with the system calling convention for non-x86 Windows?

There's a comment saying this can't work:

rust/compiler/rustc_target/src/spec/abi/mod.rs

Lines 70 to 78 in ef7ebaa

// * C and Cdecl obviously support varargs.
// * C can be based on Aapcs, SysV64 or Win64, so they must support varargs.
// * EfiApi is based on Win64 or C, so it also supports it.
//
// * Stdcall does not, because it would be impossible for the callee to clean
// up the arguments. (callee doesn't know how many arguments are there)
// * Same for Fastcall, Vectorcall and Thiscall.
// * System can become Stdcall, so is also a no-no.
// * Other calling conventions are related to hardware or the compiler itself. 

        // * Stdcall does not, because it would be impossible for the callee to clean
        //   up the arguments. (callee doesn't know how many arguments are there)
        // * System can become Stdcall, so is also a no-no.

I don't understand anything about these ABIs so I can't say how accurate this is.

Also I see nothing here special-casing x86, so why does the issue specifically mention non-x86?

@RalfJung RalfJung mentioned this issue Jan 4, 2024
Merged
@kennykerr
Copy link
Contributor Author

Would be great if there was a way that it could be avoided.

Happy to update windows-rs as soon as there's a way to avoid it. 😊

@beepster4096
Copy link
Contributor

Can we make system equal to cdecl when applied to a variadic function? It's supposed to be whatever ABI is correct for the system api and cdecl is the correct ABI for a variadic win32 function. According to the docs __stdcall acts as __cdecl when applied to a variadic function.

@beepster4096 beepster4096 mentioned this issue Jan 4, 2024
Merged
@beepster4096
Copy link
Contributor

Went ahead and opened #119587 for this.

@bors bors closed this as completed in 7b507db Jan 13, 2024
rust-timer added a commit to rust-lang-ci/rust that referenced this issue Jan 13, 2024
@rust-timer
Unrolled build for rust-lang#119587
7aabe50 
Rollup merge of rust-lang#119587 - beepster4096:system_varargs, r=petrochenkov

Varargs support for system ABI

This PR allows functions with the `system` ABI to be variadic (under the `extended_varargs_abi_support` feature tracked in rust-lang#100189). On x86 windows, the `system` ABI is equivalent to `C` for variadic functions. On other platforms, `system` is already equivalent to `C`.

Fixes rust-lang#110505
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
A-abi Area: Concerning the application binary interface (ABI). C-bug Category: This is a bug. O-windows Operating system: Windows T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Varargs support for system ABI beepster4096/rust
6 participants
@RalfJung @compiler-errors @kennykerr @dpaoliello @beepster4096 @jyn514