rust-lang / rust-bindgen

Automatically generates Rust FFI bindings to C (and some C++) libraries.
https://rust-lang.github.io/rust-bindgen/
BSD 3-Clause "New" or "Revised" License
4.43k stars 695 forks source link

Bindgen doesn't generate "stdcall" calling convention on x86_64-pc-windows-msvc #1433

Closed AndrewGaspar closed 1 year ago

AndrewGaspar commented 5 years ago

Versions

Windows 10, 64-bit

clang

PS ~\Projects\scratch> clang --version
clang version 7.0.0 (tags/RELEASE_700/final)
Target: x86_64-pc-windows-msvc
Thread model: posix
InstalledDir: C:\Program Files\LLVM\bin

bindgen

PS ~\Projects\scratch> bindgen --version
bindgen 0.43.0

Input C/C++ Header

(snipped from MS-MPI's mpi.h header)

#define MPIAPI __stdcall
#define MPI_METHOD int MPIAPI

typedef int MPI_Datatype;
typedef int MPI_Op;

typedef
void
(MPIAPI MPI_User_function)(
     void* invec,
     void* inoutvec,
     int* len,
     MPI_Datatype* datatype
    );

MPI_METHOD
MPI_Op_create(
    MPI_User_function* user_fn,
    int commute,
    MPI_Op* op
    );

Bindgen Invocation

$ bindgen test.h

Actual Results

/* automatically generated by rust-bindgen */

pub type MPI_Datatype = ::std::os::raw::c_int;
pub type MPI_Op = ::std::os::raw::c_int;
extern "C" {
    pub fn MPI_Op_create(
        user_fn: ::std::option::Option<unsafe extern "C" fn()>,
        commute: ::std::os::raw::c_int,
        op: *mut MPI_Op,
    ) -> ::std::os::raw::c_int;
}

Expected Results

/* automatically generated by rust-bindgen */

pub type MPI_Datatype = ::std::os::raw::c_int;
pub type MPI_Op = ::std::os::raw::c_int;
pub type MPI_User_function = ::std::option::Option<
    unsafe extern "C" fn(
        invec: *mut ::std::os::raw::c_void,
        inoutvec: *mut ::std::os::raw::c_void,
        len: *mut ::std::os::raw::c_int,
        datatype: *mut MPI_Datatype,
    ),
>;
extern "C" {
    pub fn MPI_Op_create(
        user_fn: MPI_User_function,
        commute: ::std::os::raw::c_int,
        op: *mut MPI_Op,
    ) -> ::std::os::raw::c_int;
}

Comments

emit-clang-ast.txt

I've attached the results of --emit-clang-ast. I haven't looked at this output before, so apologies if I'm misinterpreting. It does appear that both the function MPI_Op_create and the function prototype MPI_User_function are receiving cconv = 1, which looks like it's the C calling convention. I think this is actually correct for 64-bit Windows - stdcall and C are now the same.

Even if it's correct to mark them as extern "C", which I'm fine with, it's still a bug that MPI_User_function isn't being defined, and the type signature of MPI_Op_create is incorrect.

Thank you!

AndrewGaspar commented 5 years ago

On further thought, I don't like that bindgen produces extern "C" for functions that are marked __stdcall on 64-bit Windows since then code becomes non-portable between 32-bit and 64-bit Windows.

e.g. if I run bindgen using bindgen test.h -- --target=i686-pc-win32

/* automatically generated by rust-bindgen */

pub type MPI_Datatype = ::std::os::raw::c_int;
pub type MPI_Op = ::std::os::raw::c_int;
extern "stdcall" {
    #[link_name = "\u{1}_MPI_Op_create@12"]
    pub fn MPI_Op_create(
        user_fn: ::std::option::Option<unsafe extern "stdcall" fn()>,
        commute: ::std::os::raw::c_int,
        op: *mut MPI_Op,
    ) -> ::std::os::raw::c_int;
}

Even once the function signature issue is fixed, this would then result in the user having to do cfg switches between 32-bit and 64-bit Windows when declaring the calling convention of any function that is an MPI_User_function.

Of course, I don't know if there's anything bindgen can do if Clang is reporting that the calling convention is cdecl on 64-bit Windows, even when __stdcall was used.

emilio commented 5 years ago

Yeah, I agree it'd be nice to use "stdcall", but as of right now I don't think libclang exposes an API for that unfortunately.

For reference this is the diff between bindgen test.h --emit-clang-ast -- -target i686-pc-windows-msvc and bindgen test.h --emit-clang-ast -- -target x86_64-pc-windows-msvc:

```diff diff --git a/emit.txt b/emit2.txt index 5f0a8f22..8c42dd99 100644 --- a/emit.txt +++ b/emit2.txt @@ -333,23 +333,23 @@ semantic-parent.kind = TranslationUnit semantic-parent.spelling = "t.h" semantic-parent.location = builtin definitions semantic-parent.is-definition? false semantic-parent.is-declaration? false semantic-parent.is-inlined-function? false type.kind = FunctionProto - type.cconv = 2 - type.spelling = "int (MPI_User_function *, int, MPI_Op *) __attribute__((stdcall))" + type.cconv = 1 + type.spelling = "int (MPI_User_function *, int, MPI_Op *)" type.is-variadic? false type.canonical.kind = FunctionProto - type.canonical.cconv = 2 - type.canonical.spelling = "int (void (*)(void *, void *, int *, int *) __attribute__((stdcall)), int, int *) __attribute__((stdcall))" + type.canonical.cconv = 1 + type.canonical.spelling = "int (void (*)(void *, void *, int *, int *), int, int *)" type.canonical.is-variadic? false type.canonical.return.kind = Int type.canonical.return.cconv = 100 type.canonical.return.spelling = "int" type.canonical.return.is-variadic? false type.return.kind = Int @@ -385,22 +385,22 @@ type.kind = Pointer type.cconv = 100 type.spelling = "MPI_User_function *" type.is-variadic? false type.canonical.kind = Pointer type.canonical.cconv = 100 - type.canonical.spelling = "void (*)(void *, void *, int *, int *) __attribute__((stdcall))" + type.canonical.spelling = "void (*)(void *, void *, int *, int *)" type.canonical.is-variadic? false type.canonical.pointee.kind = FunctionProto - type.canonical.pointee.cconv = 2 - type.canonical.pointee.spelling = "void (void *, void *, int *, int *) __attribute__((stdcall))" + type.canonical.pointee.cconv = 1 + type.canonical.pointee.spelling = "void (void *, void *, int *, int *)" type.canonical.pointee.is-variadic? false type.canonical.pointee.return.kind = Void type.canonical.pointee.return.cconv = 100 type.canonical.pointee.return.spelling = "void" type.canonical.pointee.return.is-variadic? false type.pointee.kind = FunctionProto @@ -602,16 +602,15 @@ type.declaration.semantic-parent.is-inlined-function? false ) ) ) /* automatically generated by rust-bindgen */ pub type MPI_Datatype = ::std::os::raw::c_int; pub type MPI_Op = ::std::os::raw::c_int; -extern "stdcall" { - #[link_name = "\u{1}_MPI_Op_create@12"] +extern "C" { pub fn MPI_Op_create( - user_fn: ::std::option::Option, + user_fn: ::std::option::Option, commute: ::std::os::raw::c_int, op: *mut MPI_Op, ) -> ::std::os::raw::c_int; } ```

So we cannot even try to detect it hackily via checking spelling or something like that. You can try to file an upstream LLVM issue, or generate your bindings with the i386 target.

emilio commented 5 years ago

Closing as it's an upstream libclang issue and I don't think we can work around it easily.

Thanks for reporting it though! :)

AndrewGaspar commented 5 years ago

Thanks for looking at this emilio. However, there's a second issue here I think you missed: The function signature for any "stdcall" function pointer type is decaying to just void(*)(). e.g. In the example you posted, the signature for MPI_Op_create should be:

pub type MPI_User_function = ::std::option::Option<
    unsafe extern "C" fn(
        invec: *mut ::std::os::raw::c_void,
        inoutvec: *mut ::std::os::raw::c_void,
        len: *mut ::std::os::raw::c_int,
        datatype: *mut MPI_Datatype,
    ),
>;
extern "C" {
    pub fn MPI_Op_create(
        user_fn: MPI_User_function,
        commute: ::std::os::raw::c_int,
        op: *mut MPI_Op,
    ) -> ::std::os::raw::c_int;
}

but instead it's just:

extern "C" {
    pub fn MPI_Op_create(
        user_fn: ::std::option::Option<unsafe extern "C" fn()>,
        commute: ::std::os::raw::c_int,
        op: *mut MPI_Op,
    ) -> ::std::os::raw::c_int;
}
emilio commented 5 years ago

Hmm, sorry, yeah, I had missed that.

It preserves the calling convention right, but you're right that the alias is not properly generated, that deserves a bit more investigation.

FWIW the stdcall stuff is a bit messy, see #1403 and previous bits. The issue is that clang only puts the real calling convention at some places, so maybe we're just confused, or maybe it's the clang ast messing with us.

bugproof commented 4 years ago

did anything change with clang?

AndrewGaspar commented 4 years ago

It seems that stdcall is now included in the AST as of clang 10:

❯ clang++ -cc1 -ast-dump .\test.h
TranslationUnitDecl 0x19dcef06178 <<invalid sloc>> <invalid sloc>
|-TypedefDecl 0x19dcef06a10 <<invalid sloc>> <invalid sloc> implicit __int128_t '__int128'
| `-BuiltinType 0x19dcef06710 '__int128'
|-TypedefDecl 0x19dcef06a80 <<invalid sloc>> <invalid sloc> implicit __uint128_t 'unsigned __int128'
| `-BuiltinType 0x19dcef06730 'unsigned __int128'
|-TypedefDecl 0x19dcef06da8 <<invalid sloc>> <invalid sloc> implicit __NSConstantString 'struct __NSConstantString_tag'
| `-RecordType 0x19dcef06b60 'struct __NSConstantString_tag'
|   `-Record 0x19dcef06ad8 '__NSConstantString_tag'
|-TypedefDecl 0x19dcef06e40 <<invalid sloc>> <invalid sloc> implicit __builtin_ms_va_list 'char *'
| `-PointerType 0x19dcef06e00 'char *'
|   `-BuiltinType 0x19dcef06210 'char'
|-TypedefDecl 0x19dcef06eb0 <<invalid sloc>> <invalid sloc> implicit __builtin_va_list 'char *'
| `-PointerType 0x19dcef06e00 'char *'
|   `-BuiltinType 0x19dcef06210 'char'
|-TypedefDecl 0x19dcef06f20 <.\test.h:4:1, col:13> col:13 referenced MPI_Datatype 'int'
| `-BuiltinType 0x19dcef06270 'int'
|-TypedefDecl 0x19dcef06f90 <line:5:1, col:13> col:13 referenced MPI_Op 'int'
| `-BuiltinType 0x19dcef06270 'int'
|-TypedefDecl 0x19dcf12c300 <line:7:1, line:9:9> col:9 referenced MPI_User_function 'void (void *, void *, int *, MPI_Datatype *) __attribute__((stdcall))':'void (void *, void *, int *, MPI_Datatype *)'
| `-AttributedType 0x19dcf12c280 'void (void *, void *, int *, MPI_Datatype *) __attribute__((stdcall))' sugar
|   |-ParenType 0x19dcf12c230 'void (void *, void *, int *, MPI_Datatype *)' sugar
|   | `-FunctionProtoType 0x19dcf12c1e0 'void (void *, void *, int *, MPI_Datatype *)' cdecl
|   |   |-BuiltinType 0x19dcef061d0 'void'
|   |   |-PointerType 0x19dcef06990 'void *'
|   |   | `-BuiltinType 0x19dcef061d0 'void'
|   |   |-PointerType 0x19dcef06990 'void *'
|   |   | `-BuiltinType 0x19dcef061d0 'void'
|   |   |-PointerType 0x19dcef070f0 'int *'
|   |   | `-BuiltinType 0x19dcef06270 'int'
|   |   `-PointerType 0x19dcf12c0f0 'MPI_Datatype *'
|   |     `-TypedefType 0x19dcf12c0d0 'MPI_Datatype' sugar
|   |       |-Typedef 0x19dcef06f20 'MPI_Datatype'
|   |       `-BuiltinType 0x19dcef06270 'int'
|   `-ParenType 0x19dcf12c230 'void (void *, void *, int *, MPI_Datatype *)' sugar
|     `-FunctionProtoType 0x19dcf12c1e0 'void (void *, void *, int *, MPI_Datatype *)' cdecl
|       |-BuiltinType 0x19dcef061d0 'void'
|       |-PointerType 0x19dcef06990 'void *'
|       | `-BuiltinType 0x19dcef061d0 'void'
|       |-PointerType 0x19dcef06990 'void *'
|       | `-BuiltinType 0x19dcef061d0 'void'
|       |-PointerType 0x19dcef070f0 'int *'
|       | `-BuiltinType 0x19dcef06270 'int'
|       `-PointerType 0x19dcf12c0f0 'MPI_Datatype *'
|         `-TypedefType 0x19dcf12c0d0 'MPI_Datatype' sugar
|           |-Typedef 0x19dcef06f20 'MPI_Datatype'
|           `-BuiltinType 0x19dcef06270 'int'
`-FunctionDecl 0x19dcf12c6b8 <line:2:20, line:21:5> line:17:1 MPI_Op_create 'int (MPI_User_function *, int, MPI_Op *) __attribute__((stdcall))':'int (MPI_User_function *, int, MPI_Op *)'
  |-ParmVarDecl 0x19dcf12c3e8 <line:18:5, col:24> col:24 user_fn 'MPI_User_function *'
  |-ParmVarDecl 0x19dcf12c468 <line:19:5, col:9> col:9 commute 'int'
  `-ParmVarDecl 0x19dcf12c528 <line:20:5, col:13> col:13 op 'MPI_Op *'
AndrewGaspar commented 4 years ago

Additionally, the correct function pointer signature is emitted in the AST, but bindgen still has the undesirable behavior, so it's getting lost somewhere in translation.

emilio commented 4 years ago

We're using clang_getFunctionTypeCallingConv to get the right calling convention, the relevant functions return the C calling convention.

It does the right thing if you specify i686-pc-windows-msvc, so presumably something is wrong inside libclang.

pvdrz commented 1 year ago

I ran bindgen with this input

#define MPIAPI __stdcall
#define MPI_METHOD int MPIAPI

typedef int MPI_Datatype;
typedef int MPI_Op;

typedef
void
(MPIAPI MPI_User_function)(
     void* invec,
     void* inoutvec,
     int* len,
     MPI_Datatype* datatype
    );

MPI_METHOD
MPI_Op_create(
    MPI_User_function* user_fn,
    int commute,
    MPI_Op* op
    );

using both -- --target=i686-pc-win32 and -- --target=i686-pc-windows-msvc and got the following:


/* automatically generated by rust-bindgen 0.63.0 */

pub type MPI_Datatype = ::std::os::raw::c_int; pub type MPI_Op = ::std::os::raw::c_int; extern "stdcall" { pub fn MPI_Op_create( user_fn: ::std::option::Option< unsafe extern "stdcall" fn( arg1: mut ::std::os::raw::c_void, arg2: mut ::std::os::raw::c_void, arg3: mut ::std::os::raw::c_int, arg4: mut MPI_Datatype, ),

, commute: ::std::os::raw::c_int, op: *mut MPI_Op, ) -> ::std::os::raw::c_int; }

I have clang 14 on my system just in case.