windows-rs icon indicating copy to clipboard operation
windows-rs copied to clipboard

Discrepancy in symbol names between i686*/lib/*windows.* and .lib files in windows SDK

Open glandium opened this issue 2 years ago • 1 comments

I think I missed some libraries from the SDK, but I dumped symbol lists from windows SDK/MSVC for x86 and compared to what's in i686_msvc, and there are tons of discrepancies (this is after #1979).

Discrepancies where the numbers are different (windows.lib on the left, SDK on the right): __imp__CfUpdateSyncProviderStatus@8 __imp__CfUpdateSyncProviderStatus@12 __imp__CfReportProviderProgress@28 __imp__CfReportProviderProgress@32 __imp__CfReportProviderProgress2@40 __imp__CfReportProviderProgress2@44 __imp__CfQuerySyncProviderStatus@8 __imp__CfQuerySyncProviderStatus@12 __imp__CfDisconnectSyncRoot@4 __imp__CfDisconnectSyncRoot@8 __imp__RegisterClusterNotifyV2@24 __imp__RegisterClusterNotifyV2@28 __imp__EapHostPeerQueryUserBlobFromCredentialInputFields@44 __imp__EapHostPeerQueryUserBlobFromCredentialInputFields@48 __imp__EapHostPeerQueryCredentialInputFields@36 __imp__EapHostPeerQueryCredentialInputFields@40 __imp__EapHostPeerInvokeIdentityUI@60 __imp__EapHostPeerInvokeIdentityUI@64 __imp__EapHostPeerInvokeConfigUI@40 __imp__EapHostPeerInvokeConfigUI@44 __imp__EapHostPeerGetMethodProperties@48 __imp__EapHostPeerGetMethodProperties@52 __imp__EapHostPeerConfigBlob2Xml@32 __imp__EapHostPeerConfigBlob2Xml@36 __imp__EapHostPeerGetIdentity@64 __imp__EapHostPeerGetIdentity@68 __imp__EapHostPeerBeginSession@64 __imp__EapHostPeerBeginSession@68 __imp__alljoyn_interfacedescription_property_getannotationscount@12 __imp__alljoyn_interfacedescription_property_getannotationscount@16 __imp__alljoyn_interfacedescription_property_getannotationatindex@32 __imp__alljoyn_interfacedescription_property_getannotationatindex@36 __imp__alljoyn_interfacedescription_property_getannotation@24 __imp__alljoyn_interfacedescription_property_getannotation@28 __imp__alljoyn_interfacedescription_property_eql@24 __imp__alljoyn_interfacedescription_property_eql@32 __imp__RtmConvertIpv6AddressAndLengthToNetAddress@16 __imp__RtmConvertIpv6AddressAndLengthToNetAddress@28 __imp__WinBioRemoveCredential@24 __imp__WinBioRemoveCredential@80 __imp__WinBioGetCredentialState@28 __imp__WinBioGetCredentialState@84 __imp__WlanSetProfileEapUserData@40 __imp__WlanSetProfileEapUserData@44 (this is the full list I got, i686_gnu has the same list, but most numbers are different (+1 when different))

Examples of discrepancies where there are no numbers in the .lib in the SDK: __imp__ber_free@8 __imp__ber_free __imp__ber_flatten@8 __imp__ber_flatten __imp__ber_first_element@12 __imp__ber_first_element __imp__ber_bvfree@4 __imp__ber_bvfree __imp__ber_bvecfree@4 __imp__ber_bvecfree __imp__ber_bvdup@4 __imp__ber_bvdup __imp__ber_alloc_t@4 __imp__ber_alloc_t __imp__LdapUnicodeToUTF8@16 __imp__LdapUnicodeToUTF8 __imp__LdapUTF8ToUnicode@16 __imp__LdapUTF8ToUnicode __imp__LdapMapErrorToWin32@4 __imp__LdapMapErrorToWin32 __imp__LdapGetLastError@0 __imp__LdapGetLastError __imp__CreateFX@16 __imp__CreateFX __imp__DtcGetTransactionManagerExW@24 __imp__DtcGetTransactionManagerExW __imp__DtcGetTransactionManagerExA@24 __imp__DtcGetTransactionManagerExA __imp__DtcGetTransactionManagerC@28 __imp__DtcGetTransactionManagerC __imp__DtcGetTransactionManager@28 __imp__DtcGetTransactionManager

the full list for those is 1283 lines, and the symbols come from a variety of libraries (advapi32, authz, avifil32, cabinat, comsvcs, fontsub, icu, mi, msajapi, msvfw32, mtxdm, netsh, oledlg, rpcrt4, rtutils, setupapi, shlwapi, snmpapi, user32, wdsclientapi, wdsmc, wdspxe, windows, wldap32, xaudio2_8, xolehlp). In some of them, all the symbols are without numbers (e.g. icu), but in others, some are and some aren't. I haven't looked exactly which ones are which.

glandium avatar Aug 25 '22 03:08 glandium

In some of them, all the symbols are without numbers (e.g. icu)

Right, I suspect most of these issues will actually root in upstream win32 metadata. In the case of ICU functions, for example, https://github.com/microsoft/win32metadata/issues/1013.

If you'd like to share the list, we can help start filing/working those issues.

riverar avatar Aug 25 '22 04:08 riverar

Well, currently, the @number is added for all symbols, so there's at least a part of the problem on this side. I can share the full list I have, but I don't guarantee it's actually complete.

glandium avatar Aug 25 '22 05:08 glandium

Full list here: https://gist.github.com/glandium/395e52e494b56094184ab8d898340d89

glandium avatar Aug 25 '22 05:08 glandium

Full list here: https://gist.github.com/glandium/395e52e494b56094184ab8d898340d89

Great, thanks!

Well, currently, the @ number is added for all symbols, so there's at least a part of the problem on this side.

Agreed.

Though arguably developers should really skip the ICU baked into the OS and just use a separate crate/SDK instead. It seems the OS ICU work may have already been abandoned, if we go by the outdated documentation. https://docs.microsoft.com/windows/win32/intl/international-components-for-unicode--icu-#history-of-changes-to-the-icu-library-in-windows

riverar avatar Aug 25 '22 05:08 riverar

ICU is only a small part of the list, though.

glandium avatar Aug 25 '22 06:08 glandium

Examples of discrepancies where there are no numbers in the .lib in the SDK

Those are likely because those functions aren't stdcall at all - many are likely cdecl. It would be most helpful if those are reported to the win32metadata repo since the metadata would need to indicate the calling convention before the Rust project can do anything about it.

As of now, we just assume all functions are stdcall since the metadata doesn't appear to set the CallingConvention field.

kennykerr avatar Aug 25 '22 13:08 kennykerr

Discrepancies where the numbers are different

I can dig into these.

kennykerr avatar Aug 25 '22 13:08 kennykerr

Discrepancies where the numbers are different

Looks like these are also win32metadata bugs. For example CfDisconnectSyncRoot is defined as follows in metadata:

public struct CF_CONNECTION_KEY
{
    public IntPtr Value;
}

public static extern HRESULT CfDisconnectSyncRoot([In] CF_CONNECTION_KEY ConnectionKey);

That explains why windows-rs says the stack size is 4 since IntPtr is 4 bytes on x86.

But that's not accurate since the definition of CF_CONNECTION_KEY in the Windows SDK is always 8 bytes, regardless of architecture:

#define DECLARE_OPAQUE_KEY( name )          \
    typedef struct name##__ {               \
        LONGLONG Internal;                  \
    } name, *P##name

DECLARE_OPAQUE_KEY( CF_CONNECTION_KEY );

LONGLONG is just a typedef for __int64.

kennykerr avatar Aug 25 '22 14:08 kennykerr

Thanks for reporting! I think the issues here are all covered by the following metadata issues I just created:

https://github.com/microsoft/win32metadata/issues/1133 https://github.com/microsoft/win32metadata/issues/1134

kennykerr avatar Aug 25 '22 14:08 kennykerr

Shouldn't this stay open for the part where windows-rs will have to be adjusted for cdecl functions? (or are cdecl functions filtered-out?)

glandium avatar Aug 25 '22 20:08 glandium

I'd keep it open but its blocked on those issues being resolved and I don't know when they'll be fixed. I prefer not to keep issues open indefinitely. I'll keep an eye on those and if they get fixed soon I can plumb through to Rust pretty quickly.

kennykerr avatar Aug 25 '22 23:08 kennykerr

These issues have now been fixed in the win32 metadata. Reopening to deal with ingestion.

kennykerr avatar Aug 31 '22 14:08 kennykerr

Now that I have the updated metadata (#1996) I can finish this up. There are a few outstanding issues:

  1. Sometimes the stdcall function stack size calculation isn't quite right. For example RegisterClusterNotifyV2 still reports 24 when it should be 28 because it fails to account for struct padding (a 64-bit field follows a 32-bit field). This can get quite tricky with nested structs.

  2. The metadata crate needs to be updated to retrieve the calling convention.

  3. The bindgen crate needs to use the calling convention to change the way functions are imported from extern "system" to whatever is appropriate. This may require some more substantial code gen refactoring as all the imports are currently lumped into a single extern block for the windows-sys crate.

kennykerr avatar Sep 01 '22 13:09 kennykerr

Hey @glandium please take a look at #1999 - I have validated all of the discrepancies you found and confirmed they work (with tests).

kennykerr avatar Sep 01 '22 19:09 kennykerr

I'm going to leave this open until these two issues are resolved:

https://github.com/microsoft/win32metadata/issues/1217 https://github.com/microsoft/win32metadata/issues/1221

kennykerr avatar Sep 02 '22 17:09 kennykerr

Both blocking issues have now been resolved. Will update windows-rs as soon as they publish a build.

kennykerr avatar Sep 09 '22 14:09 kennykerr

And here's the PR that pulls in the win32 metadata update that resolves these issues: https://github.com/microsoft/windows-rs/pull/2015

kennykerr avatar Sep 09 '22 19:09 kennykerr