search

microsoft / windows-drivers-rs

Platform that enables Windows driver development in Rust. Developed by Surface.
Apache License 2.0
1.51k stars 67 forks source link

generated bindings should use `extern "system"` instead of `extern "C"` #156

Open HoShiMin opened 6 months ago

HoShiMin commented 6 months ago

As we know, almost all Windows functions have stdcall convention, so, this is strange that all generated bindings are marked as extern "C". It seems that the convention should be changed to extern "stdcall", isn't it? Of course, for functions that are really "stdcall".

For example, ZwCreateFile.
It has the following prototype:

NTSYSAPI
NTSTATUS
NTAPI // <-- Here it is!
ZwCreateFile(...);
wmmc88 commented 6 months ago

Are you encountering any buggy behavior that is affected by this? Technically, I think the most correct solution here is that bindgen generates extern system:

extern "system" -- Usually the same as extern "C", except on Win32, in which case it's "stdcall", or what you should use to link to the Windows API itself --From https://doc.rust-lang.org/reference/items/external-blocks.html

Functionally, there should be no difference in behavior here because on ARM and x64 processors, __stdcall is accepted and ignored by the compiler. 64-bit targets use the x64 calling convention. On Windows targets, extern "C" is equivalent to extern "win64", which is Rust's moniker for Window's x64 calling convention

HoShiMin commented 6 months ago

@wmmc88, sure, there are no problems in x64, but for the correctness it would be great to use "stdcall" or "system" as you stated above - for example, for cases of building a 32-bit driver for legacy Windows. And just because it's just correct 🙂

wmmc88 commented 6 months ago

Per our current support policy:

This project was built with support of WDM, KMDF, and UMDF drivers in mind, as well as Win32 Services. This includes support for all versions of WDF included in WDK 22H2 and newer.

Since NI WDK (22H2) does not support 32-bit drivers, we do not currently have plans to support this scenario. However, if the change to emit extern "system" instead of extern "C" is minimally invasive, then we would consider a PR.