Support windows linker #4491

RX14 · 2017-05-31T18:31:30Z

This PR allows the crystal compiler to emit correct linker commands for --cross-compile for windows machines. This allows complication of simple --prelude=empty windows executables. Hopefully this is the first part of a gradual merge of windows support for crystal.

Using the below test case and crystal command, crystal emits a cl (equivalent of cc on windows) command to link the object file. Please test this out yourself if you have a machine with visual studio installed.

@[Link("kernel32")]
@[Link("libcmt")]
lib LibWindows
  STD_INPUT_HANDLE  = 0xFFFFFFF6_u32
  STD_OUTPUT_HANDLE = 0xFFFFFFF5_u32
  STD_ERROR_HANDLE  = 0xFFFFFFF4_u32

  alias DWord = UInt32
  alias Handle = Void*
  alias SizeT = UInt64 # FIXME

  struct Overlapped
    internal : SizeT*
    internal_high : SizeT*
    pointer : Void*
    event : Handle
  end

  fun get_std_handle = GetStdHandle(std_handle : DWord) : Handle
  fun get_file_type = GetFileType(file : Handle) : DWord
  fun write_file = WriteFile(file : Handle, buffer : UInt8*, size : DWord, written : DWord*, overlapped : Overlapped*) : Bool
  fun close_handle = CloseHandle(file : Handle) : Bool
end

handle = LibWindows.get_std_handle(LibWindows::STD_OUTPUT_HANDLE)

str = "Hello World!\n"
written = 0_u32
LibWindows.write_file(handle, pointerof(str.@c), str.@bytesize, pointerof(written), nil)

$ bin/crystal build foo.cr --cross-compile --target x86_64-pc-windows-msvc --prelude=empty
Using compiled compiler at `.build/crystal'
cl "foo.o" "/Fefoo"  libcmt.lib kernel32.lib

One question is whether we want to emit libcmt.lib by default as part of every linker command: it is windows' libc implementation which calls the executables' main function. On linux, ld includes the equivalent of this automatically.

This commit allows the crystal compiler to emit correct linker commands for --cross-compile for windows machines. This allows complication of simple --prelude=empty windows executables.

bew · 2017-05-31T19:19:20Z

src/compiler/crystal/compiler.cr

@@ -18,6 +18,7 @@ module Crystal
  # optionally generates an executable.
  class Compiler
    CC = ENV["CC"]? || "cc"
+    CL = "cl"


I suggest renaming CC and CL to something like POSIX_LINKER and WINDOWS_LINKER

No, keeping the same name as ENV variables makes it easier to deal with.

Yeah that's a good idea. I want to work out an env var or commandline flag to set the linker, like CC on posix.

Why don't just use LINK https://msdn.microsoft.com/en-us/library/hx5b050y.aspx

The LINK tool prepends the options and arguments defined in the LINK environment variable and appends the options and arguments defined in the _LINK_ environment variable to the command line arguments before processing.

From this wording, it appears to me that the LINK env var is used to prepend options to the link command, not contain the path to the executable.

I mean the link command instead of cl

Why not use ld instead of cc? I don't know, but I think it'd beyond the scope of this PR. If we change to use the linker directly instead of the compiler directly we should do that on all platforms simultaneously.

@bcardiff Because, just like on Unix, calling the linker directly would require you to manually pass the correct C/base libraries to link. On Windows, it's more complicated than Unix; in addition to kernel32.lib, you need to link in the proper C library using its full path, depending on the architecture and debug vs release mode.

@kirbyfan64 you certainly don't need to pass the full path.

ysbaddaden · 2017-05-31T19:35:59Z

Great work!

bcardiff

Great! I was hoping to do something like this and get it to master.

I think is fine for know to not include libcmt.lib in the windows as part of the lib_flags or the linker command.
A LibWindows as in your submitted example should do the work IMO.

bcardiff · 2017-05-31T19:39:17Z

src/compiler/crystal/compiler.cr

@@ -18,6 +18,7 @@ module Crystal
  # optionally generates an executable.
  class Compiler
    CC = ENV["CC"]? || "cc"
+    CL = "cl"


Why don't just use LINK https://msdn.microsoft.com/en-us/library/hx5b050y.aspx

bcardiff · 2017-05-31T19:39:53Z

src/compiler/crystal/compiler.cr

-    private def codegen(program : Program, node, sources, output_filename)
-      @link_flags = "#{@link_flags} -rdynamic"
-
+    private def codegen(program, node : ASTNode, sources, output_filename)


why program has lost it's type restriction : Program?

It was superfluous and made the line longer. ASTNode is required to distinguish the override, Program is just repeating the information contained in the variable name program.

txe · 2017-06-01T08:49:44Z

I also can suggest as a temporary solution not to call cl.exe directly but call a batch file that makes necessary environment setup, for example cl.bat

@echo off
%comspec% /c ""C:\Program Files (x86)\Microsoft Visual C++ Build Tools\vcbuildtools.bat" x64 && cl.exe %*"

or put this command line in CL

RX14 · 2017-06-01T09:10:12Z

@txe I think that the idea is that you'd run the msvc environment setup batch (e.g. open the VS command prompt from the start menu) and then run crystal inside that prompt which already has the correct PATH and LIB env vars set up. That's certainly what I did when I was testing. I think it's the job of the user to set up their environment not the compiler.

txe · 2017-06-01T09:19:03Z

I thought it was a question of usability:

during the porting process the compiler is going to be run a lot and start the build prompt for that is inconvenient
fully ported compiler will still use cl.exe, I think users will be happy to simply run crystal everywhere as any other compiler.

RX14 · 2017-06-01T09:40:48Z

Why not just keep the build prompt open? To me that's like complaining you have to open a terminal when you want to use the crystal compiler. Sure you do, but you can just keep one open.

In addition, I don't think there's going to be a way to find that batch file consistently across all systems.

txe · 2017-06-01T09:54:20Z

Actually, I have a script which run the compiler and link without the prompt, so I talk about this feature which will run cl.exe directly and demand the prompt. Just a small step further and the prompt is not needed :)
I think, ./bin (where crystal script) is good place for one more script and this script is only for windows (looks like a windows way)
But anyway it's just suggestion, not a big deal.
And thanks for this feature 👍

Edit: sorry, I wrote the prompt and not the prompt, but maybe it's not clear what the difference is.
User can run command prompt cmd.exe just in place in exlporer (I usually do it). In case of the build prompt user needs to start prompt from menu, then change directory by cd /D <path_where_*.cr>. And surely, user can keep one build prompt open.

Edit2: there is a way to put ./bin in PATH during crystal startup for the current executing process, so CL can be kept simple like CL="cl.bat"

ysbaddaden · 2017-06-01T13:32:21Z

I believe this the user responsibility to set the CL env variable correctly, either through the VS console as explained above or manually. We use an env variable for a reason :)

This should be documented, thought, along with an example for usual VS installs.

RX14 · 2017-06-01T14:10:52Z

It seem to be a windows tradition to set the location of cl using only PATH. As far as I can see msbuild only accepts overriding the location of cl on the command line. So if we want an env var for the location of cl we'll need to make one up ourselves.

bcardiff · 2017-06-03T04:52:45Z

Thanks @RX14 . Once the compiler is ready for windows the environment setup will be addressed by a wrapper script, documentation or some other artifact. For now is great to have the command to link in windows and the refactor make senses.

Support windows linker

99e3888

This commit allows the crystal compiler to emit correct linker commands for --cross-compile for windows machines. This allows complication of simple --prelude=empty windows executables.

bew reviewed May 31, 2017

View reviewed changes

bcardiff reviewed May 31, 2017

View reviewed changes

bcardiff approved these changes May 31, 2017

View reviewed changes

bcardiff mentioned this pull request May 31, 2017

[WIP] Windows #3582

Closed

24 tasks

bcardiff added this to Issues in Windows Jun 3, 2017

bcardiff merged commit 3e6bd33 into crystal-lang:master Jun 3, 2017

bcardiff added this to the Next milestone Jun 3, 2017

Sija referenced this pull request Jun 27, 2017

Crystal 0.23.0 Changelog

3c3d3e2

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support windows linker #4491

Support windows linker #4491

RX14 commented May 31, 2017 •

edited

bew May 31, 2017

ysbaddaden May 31, 2017

RX14 May 31, 2017

bcardiff May 31, 2017

RX14 May 31, 2017 •

edited

bcardiff May 31, 2017

RX14 May 31, 2017

refi64 May 31, 2017

RX14 May 31, 2017

ysbaddaden commented May 31, 2017

bcardiff left a comment

bcardiff May 31, 2017

bcardiff May 31, 2017

RX14 May 31, 2017 •

edited

txe commented Jun 1, 2017

RX14 commented Jun 1, 2017 •

edited

txe commented Jun 1, 2017 •

edited

RX14 commented Jun 1, 2017

txe commented Jun 1, 2017 •

edited

ysbaddaden commented Jun 1, 2017

RX14 commented Jun 1, 2017

bcardiff commented Jun 3, 2017

Support windows linker #4491

Support windows linker #4491

Conversation

RX14 commented May 31, 2017 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

RX14 May 31, 2017 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ysbaddaden commented May 31, 2017

bcardiff left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

RX14 May 31, 2017 • edited

Choose a reason for hiding this comment

txe commented Jun 1, 2017

RX14 commented Jun 1, 2017 • edited

txe commented Jun 1, 2017 • edited

RX14 commented Jun 1, 2017

txe commented Jun 1, 2017 • edited

ysbaddaden commented Jun 1, 2017

RX14 commented Jun 1, 2017

bcardiff commented Jun 3, 2017

RX14 commented May 31, 2017 •

edited

RX14 May 31, 2017 •

edited

RX14 May 31, 2017 •

edited

RX14 commented Jun 1, 2017 •

edited

txe commented Jun 1, 2017 •

edited

txe commented Jun 1, 2017 •

edited