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

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

Closed
glandium opened this issue Aug 25, 2022 · 17 comments · Fixed by #1999 or #2015
Closed

Comments

@glandium
Copy link
Contributor

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.

@riverar
Copy link
Collaborator

riverar commented Aug 25, 2022

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, microsoft/win32metadata#1013.

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

@glandium
Copy link
Contributor Author

glandium commented Aug 25, 2022

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
Copy link
Contributor Author

@riverar
Copy link
Collaborator

riverar commented Aug 25, 2022

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

@glandium
Copy link
Contributor Author

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

@kennykerr
Copy link
Collaborator

kennykerr commented Aug 25, 2022

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
Copy link
Collaborator

Discrepancies where the numbers are different

I can dig into these.

@kennykerr
Copy link
Collaborator

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
Copy link
Collaborator

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

microsoft/win32metadata#1133
microsoft/win32metadata#1134

@kennykerr kennykerr added question Further information is requested and removed bug Something isn't working labels Aug 25, 2022
@glandium
Copy link
Contributor Author

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?)

@kennykerr
Copy link
Collaborator

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
Copy link
Collaborator

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

@kennykerr kennykerr reopened this Aug 31, 2022
@kennykerr kennykerr added bug Something isn't working and removed question Further information is requested labels Aug 31, 2022
@kennykerr
Copy link
Collaborator

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
Copy link
Collaborator

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

@kennykerr
Copy link
Collaborator

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

microsoft/win32metadata#1217
microsoft/win32metadata#1221

@kennykerr kennykerr reopened this Sep 2, 2022
@kennykerr kennykerr removed the bug Something isn't working label Sep 7, 2022
@kennykerr
Copy link
Collaborator

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

@kennykerr
Copy link
Collaborator

And here's the PR that pulls in the win32 metadata update that resolves these issues: #2015

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants