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

Support -static and -dynamic .lib suffixes on Windows #13473

Merged

Conversation

HertzDevil
Copy link
Contributor

@HertzDevil HertzDevil commented May 15, 2023

As described in #11575 (comment), this PR makes the Crystal compiler look for foo-static.lib before foo.lib in each library search directory on Windows, and foo-dynamic.lib before foo.lib if -Dpreview_dll is specified. Only foo is affected; any path separator (/ or \) will disable the extra suffix behavior.

This will allow us to finally distribute static and DLL import libraries side-by-side in CRYSTAL_LIBRARY_PATH. The next step would be having CI collect the import libraries into $ORIGIN/lib/*-dynamic.lib, and the DLLs themselves into $ORIGIN/*.dll. The DLLs do not have their own directory since Crystal itself might look them up in the same directory as the executable.

This PR also enables delay-loading for all DLLs, even when -Dpreview_dll is not used. This is not a problem for static libraries because they simply do not import DLLs (this is already the case when the Crystal interpreter loads those .lib files).

@HertzDevil HertzDevil marked this pull request as draft May 16, 2023 11:47
@HertzDevil
Copy link
Contributor Author

HertzDevil commented May 16, 2023

Some symbols from WS2_32.dll are imported by their ordinal numbers:

> bin\crystal build spec\std\socket\socket_spec.cr

> dumpbin /imports:WS2_32.dll socket_spec.exe
Microsoft (R) COFF/PE Dumper Version 14.35.32215.0
Copyright (C) Microsoft Corporation.  All rights reserved.


Dump of file socket_spec.exe

File Type: EXECUTABLE IMAGE

  Section contains the following imports:

  Section contains the following delay load imports:

    WS2_32.dll
              00000001 Characteristics
      000000014039CCE8 Address of HMODULE
      00000001403923E0 Import Address Table
      000000014038F038 Import Name Table
      000000014038F2A8 Bound Import Name Table
      0000000000000000 Unload Import Name Table
                     0 time date stamp

                                    0000000140217F4E    B6 inet_ntop
                                    0000000140217F3C    B7 inet_pton
                                    0000000140217DA3       Ordinal   111
                                    0000000140217F2A       Ordinal     6
                                    0000000140217E2E    4E WSASend
                                    0000000140217E40    31 WSAGetOverlappedResult
                                    0000000140217E52       Ordinal     3
                                    0000000140217E64       Ordinal    23
                                    0000000140217E76       Ordinal   115
                                    0000000140217E88    3B WSAIoctl
                                    0000000140217E9A    58 WSASocketW
                                    0000000140217EAC       Ordinal    21
                                    0000000140217EBE    A6 getaddrinfo
                                    0000000140217ED0       Ordinal     2
                                    0000000140217EE2    A5 freeaddrinfo
                                    0000000140217EF4       Ordinal    13
                                    0000000140217F06    49 WSARecv
                                    0000000140217F18       Ordinal     4

  Summary

      170000 .data
       15000 .pdata
      126000 .rdata
        1000 .reloc
      26A000 .text
        1000 _RDATA

This reveals a bug where a LibC::BOOL is used in an if condition in the delay-load helper. This calls for something like #12365

Also note that in this case the executable does not know about the names of the symbols imported using ordinals (e.g. 115 is WSAStartUp), and only WS2_32.dll itself knows it.

@HertzDevil HertzDevil marked this pull request as ready for review May 16, 2023 20:10
Copy link
Member

@straight-shoota straight-shoota left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just have a small suggestion for a little optimization.
But it's optional. I'm approving either way.

src/compiler/crystal/loader/msvc.cr Show resolved Hide resolved
@straight-shoota straight-shoota added this to the 1.9.0 milestone May 22, 2023
@straight-shoota straight-shoota merged commit f690598 into crystal-lang:master May 23, 2023
46 checks passed
@HertzDevil HertzDevil deleted the feature/windows-lib-suffix branch May 23, 2023 08:39
straight-shoota added a commit that referenced this pull request Jul 10, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

None yet

3 participants