COFF: Introduce LD shim around LINK

[martell]

lld/lib/Shim/COFFLdOptions.td

21 ↗

(On Diff #101361)

This is enough to get hello worlds working and cmake passing checks.

lld/lib/Shim/COFFLdShim.cpp

1 ↗

(On Diff #101361)

Location Okay?

144 ↗

(On Diff #101361)

Do I let LINK.exe handle such errors?

lld/tools/lld/lld.cpp

123 ↗

(On Diff #101361)

I'm not quite sure if I should do this here or in the ELF's Driver.cpp where it checks the emulation.
Suggestions welcome

[ruiu]

• lib is not the right place to put these files. Create a directory mingw as lld/mingw and move new files to that directory.

lld/lib/Shim/COFFLdOptions.td

1 ↗

(On Diff #101361)

Rename this Options.td.

lld/lib/Shim/COFFLdShim.cpp

1 ↗

(On Diff #101361)

See the comment above.

Rename this file MingwDriver.cpp.

12 ↗

(On Diff #101361)

This is a linker driver for MinGW.

16–17 ↗

(On Diff #101361)

Remove duplicate.

65 ↗

(On Diff #101361)

COFFLdOptTable -> MingwOptTable.

73 ↗

(On Diff #101361)

You should create a new namespace lld::mingw.

77 ↗

(On Diff #101361)

Remove all occurences of llvm::.

80 ↗

(On Diff #101361)

Remove llvm::.

92 ↗

(On Diff #101361)

Remove {}.

107 ↗

(On Diff #101361)

Remove IgnoreShared as you are not using it.

121 ↗

(On Diff #101361)

Don't new std::string -- you are leaking memory.

144 ↗

(On Diff #101361)

Either is probably fine, but don't leave this kind of comment.

163 ↗

(On Diff #101361)

Maybe forward is a better name.

Don't return true/false for no reason. You are not using the return value, so return void.

173 ↗

(On Diff #101361)

Don't return a value unless you are actually using it.

176 ↗

(On Diff #101361)

Can you run clang-format before sending a new patch to review, please?

177 ↗

(On Diff #101361)

Memory leak.

lld/tools/lld/lld.cpp

52 ↗

(On Diff #101361)

isMingw

53 ↗

(On Diff #101361)

for (auto It = V.begin(); It + 1 != V.end(); ++It) {
  if (StringRef(*It) != "-m")
    continue;
  StringRef S = *(It + 1);
  return S == "i386pe" || S == "i386pep" || S == "thumb2pe";
}
return false;

123 ↗

(On Diff #101361)

Doing this right here is the correct approach

125 ↗

(On Diff #101361)

Remove else because the last if ends with return.

[martell]

updated to HEAD.
added new options from rL305805
addressed as many comments as possible.

@ruiu Naming everything Mingw Driver doesn't make a lot of sense because this also supports cygwin. That and the possible future targets such as midipix.
Have you any suggestions on a more generic naming scheme?

[martell]

prepend lld/ to the patch path to make it more readable

[mstorsjo]

In D33880#797799, @martell wrote:

@ruiu Naming everything Mingw Driver doesn't make a lot of sense because this also supports cygwin. That and the possible future targets such as midipix.
Have you any suggestions on a more generic naming scheme?

Just my 2 cents; I also prefer naming this something like "ld style frontend to the COFF linker" instead of "mingw driver". but in the end, Rui's opinion is what matters of course.

[ruiu]

lld/include/lld/Driver/Driver.h

21 ↗

(On Diff #105012)

Indentation. Please use clang-format-diff before sending patches so that human reviewers can focus on meaning of the code instead of formatting.

lld/lib/Shim/COFFLdShim.cpp

1 ↗

(On Diff #101361)

Please move this file as I wrote above.

[martell]

lld/lib/Shim/COFFLdShim.cpp

1 ↗

(On Diff #101361)

reiterating my previous question. It probably got lost in the thread.

Naming everything Mingw Driver doesn't make a lot of sense because this also supports cygwin. That and the possible future targets such as midipix.

Have you any thoughts or suggestions on a more generic naming scheme rui?

Also a comment from @mstorsjo above.

Just my 2 cents; I also prefer naming this something like "ld style frontend to the COFF linker" instead of "mingw driver". but in the end, Rui's opinion is what matters of course.

[martell]

lld/include/lld/Driver/Driver.h

21 ↗

(On Diff #105012)

I will use git diff -U999 HEAD^ | clang-format-diff.py -i -p1 going forward. Thanks.

[ruiu]

lld/include/lld/Driver/Driver.h

21 ↗

(On Diff #105012)

Can you upload a clang-formatted version? This indentation is still wrong.

lld/lib/Shim/COFFLdShim.cpp

1 ↗

(On Diff #105012)

Move this file and other files in this directory to lld/Mingw (do not include lib or something, but just that directory).

Mingw is probably not the best name, but we already have COFF and ELF, and I can't think of any good name that briefly conveys the idea that this is "GNU-style ld command line frontend for Windows".

[mstorsjo]

lld/lib/Shim/COFFLdShim.cpp

70 ↗

(On Diff #105012)

FWIW, this fails to link for me (on linux), with following error:

lib/liblldCOFFShim.a(COFFLdShim.cpp.o):(.bss._ZN3lld4coff5SaverE+0x0): multiple definition of `lld::coff::Saver'
lib/liblldCOFF.a(Driver.cpp.o):(.bss._ZN3lld4coff5SaverE+0x0): first defined here
lib/liblldCOFFShim.a(COFFLdShim.cpp.o):(.bss._ZN3lld4coff6BAllocE+0x0): multiple definition of `lld::coff::BAlloc'
lib/liblldCOFF.a(Driver.cpp.o):(.bss._ZN3lld4coff6BAllocE+0x0): first defined here

I did this modification locally to fix it:

-BumpPtrAllocator BAlloc;
-StringSaver Saver{BAlloc};
+extern StringSaver Saver;

[mstorsjo]

lld/lib/Shim/COFFLdShim.cpp

217 ↗

(On Diff #105012)

Isn't this a typo here, shouldn't this be i386pep mapping to X64?

[mstorsjo]

lld/lib/Shim/COFFLdShim.cpp

231 ↗

(On Diff #105012)

To get this working on i386, I had to do this modification:

auto M = Args.getLastArg(OPT_m);
if (M && StringRef(M->getValue()) == "i386pe")
  LinkArgs.push_back("/alternatename:__image_base__=___ImageBase");
else
  LinkArgs.push_back("/alternatename:__image_base__=__ImageBase");

[martell]

Have been super busy with work, should have got around to this sooner sorry.

[mstorsjo]

lib/CMakeLists.txt

4

Afaik Rui requested this to be a separate directory on the toplevel (next to COFF and ELF), not within lib.

lib/Mingw/CMakeLists.txt

10

I think Rui requested this to be named MingwDriver.cpp or Driver.cpp as well.

lib/Mingw/COFFLdShim.cpp

102

As far as I can see, nothing sets the Sysroot variable used here.

One might want to add a stub option --sysroot to Options.td as well, to handle the case if the user passes --sysroot to the clang frontend. In those cases, the clang driver already takes care of most of it (adding the right -L parameters to lld), so we can get pretty far by just ignoring it instead of erroring out.

[mstorsjo]

In D33880#831161, @martell wrote:

Have been super busy with work, should have got around to this sooner sorry.

If you're still too busy and don't mind, I could also take over trying to get this in a mergeable form.

[martell]

All the old comments got buried in the thread.
I see the comment about lld/Mingw all the other folders are named in all caps or all lowercase.
I went with all caps for now unless we specifically want Mingw with just a capital M.

Removed handling of sysroot
Though I am not sure this is correct behaviour because ELF handles this in the same manner.

[ruiu]

Please rename the directory MinGW, as MINGW is not an acronym.T

MINGW/Driver.cpp

36 ↗

(On Diff #109695)

This should be mingw instead of coff.

Since you are writing code that is not in COFF directory, do not depend on anything in COFF directory, except elf::coff::link. Please remove dependency to COFF directory.

[martell]

In D33880#831570, @mstorsjo wrote:

If you're still too busy and don't mind, I could also take over trying to get this in a mergeable form.

I can be on this to ensure it gets merged, there were 2 reasons why I left this slide for awhile.
I was having an issue with llvm-dlltool where if I used new to create memory for the strings it would work but if I done it the correct way as rui instructed it was not working correctly.
We now know that was because we were writing the strings beforehand the entry thanks to you :) So the 2 bugs because of the location of the the allocation seemed to nullify each other to some degree.
Which left me a little confused. So I left the lld support slide until llvm-dlltool was merged and also at the time I had started a new job.

I can still be somewhat useful here because I understand what is going on.
Where I probably won't be of any use is on the unwind exceptions, so might be an idea to commandeer that differential?
I'm sure this is something you know a lot more about and I can probably learn a lot from the patch that gets merged for it.

Also I am not sure how extreme I should go with test cases here, how many existing tests will I amend to test the mingw driver.

[mstorsjo]

In D33880#831661, @martell wrote:

In D33880#831570, @mstorsjo wrote:

If you're still too busy and don't mind, I could also take over trying to get this in a mergeable form.

I can be on this to ensure it gets merged, there were 2 reasons why I left this slide for awhile.
I was having an issue with llvm-dlltool where if I used new to create memory for the strings it would work but if I done it the correct way as rui instructed it was not working correctly.
We now know that was because we were writing the strings beforehand the entry thanks to you :) So the 2 bugs because of the location of the the allocation seemed to nullify each other to some degree.
Which left me a little confused. So I left the lld support slide until llvm-dlltool was merged and also at the time I had started a new job.

Sure - it's great to have at least one of the components merged finally :-)

I can still be somewhat useful here because I understand what is going on.
Where I probably won't be of any use is on the unwind exceptions, so might be an idea to commandeer that differential?
I'm sure this is something you know a lot more about and I can probably learn a lot from the patch that gets merged for it.

Nope, I know much less about that; I'll probably stay away from your libc++/libunwind patches until I've got everything necessary for building C programs upstreamed. The things left are this and the mingw ctor section handling (even though it's not used for C projects, it causes linker errors if not handled in one way or another).

[martell]

MinGW/Driver.cpp

80 ↗

(On Diff #109699)

Is this desirable. I had to do this in the case of a link error because I would get a stack crash.
I believe this might have been caused by bad libs generated by llvm-dlltool as I described in the comments. In general I will follow what rui thinks here about exiting early.

include/lld/Driver/Driver.h

25

renamed the function to link for consistency also

lld/lib/Shim/COFFLdShim.cpp

70 ↗

(On Diff #105012)

This was because it was sharing the COFF namespace which already has those for the coff linker.
Now that we are using the mingw namespace as rui asked this change should not be needed.

tools/lld/CMakeLists.txt

13

I also renamed this to MinGW, not sure if that is desirable for a lib name even though it is consistent.

tools/lld/lld.cpp

116

The description of what CanExitEarly is makes sense by why do we need a bool to even track this.
I don't think it is relevant to mingw::link because we use the COFF linker under the hood.
I think this is for error messaging?
I Just want to be sure because I use _exit(1); in the driver

[martell]

MinGW/CMakeLists.txt

9 ↗

(On Diff #109699)

I also renamed this to MinGW, not sure if that is desirable for a lib name even though it is consistent.

[mstorsjo]

MinGW/Driver.cpp

212 ↗

(On Diff #109699)

As I got the mingw-arm64 stuff complete enough to publish patches now and this isn't merged yet, you could add a mapping from arm64pe (or whatever we choose in D36364) to ARM64 here.

tools/lld/lld.cpp

57

... and here, and in ELF/Driver.cpp

[martell]

tools/lld/lld.cpp

57

I think I should remove the code in ELF/Driver.cpp because that is no longer a code path that can be followed

[martell]

removed defunct ELF path.
added support for arm64pe

[ruiu]

MinGW/Driver.cpp

73 ↗

(On Diff #109922)

remove this files global variable.

78–80 ↗

(On Diff #109922)

Just call exit() instead of _exit(). _exit is a performance hack that don't want to replicate to a thin wrapper.

83 ↗

(On Diff #109922)

Do you really need this?

141 ↗

(On Diff #109922)

Add {}

150 ↗

(On Diff #109922)

Just check if there's at least one OPT_l or OPT_INPUT.

154 ↗

(On Diff #109922)

Use Args.getLastArgValue instead.

168 ↗

(On Diff #109922)

Please spend a bit more time on reviewing your patch yourself before sending it to review so that everything follows the LLVM style. You want to read your patch from top to bottom several times (that's what I do when I send a patch.)

outArg -> OutArg.

[mstorsjo]

Ping @martell - do you have time to update this patch according to Rui's review?

[martell]

@mstorsjo on it

[martell]

MinGW/Driver.cpp

63 ↗

(On Diff #111685)

infoTable doesn't seem to follow the variable naming guide but this is done consistently across LLVM

[martell]

remove AddFile function.
I thought it looked cleaner with it but it is not needed anymore.

[martell]

I'm currently testing shared library building(dll's) and adding testcases.
Will be back with an update today/tomorrow once that is ironed out.

[mstorsjo]

In D33880#845576, @martell wrote:

I'm currently testing shared library building(dll's) and adding testcases.
Will be back with an update today/tomorrow once that is ironed out.

I know of some issues around that, but had meant to send my patches once this goes in, in order not to stall this one forever.

[martell]

In D33880#845605, @mstorsjo wrote:

I know of some issues around that, but had meant to send my patches once this goes in, in order not to stall this one forever.

With that in mind I updated this with just a test for .exe and ensured the default for exe generation is a.exe vs a.out for elf targets.
You might want to override this for a.dll when -shared is passed etc

[ruiu]

MinGW/CMakeLists.txt

9 ↗

(On Diff #109699)

Yeah, I think MinGW is not exactly right, but I can't come up with a better name. It is probably not that bad, as it is a bit inaccurate but clear on what it refers to.

MinGW/Driver.cpp

53 ↗

(On Diff #111701)

infoTable -> InfoTable

(Yet another style issue, and honestly I'm a bit tired of pointing them out, so please please fix them before sending your patch to review. If you do not pay attention to the documented and obvious coding style rules, I would probably suspend doing "real" code review and instead pointing out only style issues until your code follows the coding style. I understand that we cannot and do not want to reduce the error rate to zero, and I really appreciate your work on lld/mingw, but pointing out that variables are in CamelCase in the LLVM coding style on virtually every patch update is a waste of both your and my time.)

77–78 ↗

(On Diff #111701)

I don't think you need to call flush() because exit() will indirectly call them.

[martell]

MinGW/Driver.cpp

53 ↗

(On Diff #111701)

I addressed ALL the CamelCase variables in the patch and left a note for this one specifically before you reviewed it again. You should see a comment a few lines below where infoTable is used with the following text
infoTable doesn't seem to follow the variable naming guide but this is done consistently across LLVM

Here are references to the locations I found using this exact spelling which is why I left it.

LLD

COFF/DriverUtils.cpp
lib/Driver/DarwinLdDriver.cpp

LLVM

tools/llvm-mt/llvm-mt.cpp
tools/llvm-rc/llvm-rc.cpp
lib/ToolDrivers/llvm-lib/LibDriver.cpp
tools/llvm-cvtres/llvm-cvtres.cpp
unittests/Option/OptionParsingTest.cpp
etc

Do you still want me to change it?

[martell]

I followed up with rL311517 and rL311518 to ensure there is no confusion in future.

[mstorsjo]

MinGW/Driver.cpp

53 ↗

(On Diff #111701)

I understand you frustration Rui, I truly do (especially given earlier revisions of this and other patches), but this one was surely clearly pointed out by Martell before - unfortunately I guess the comment was buried in the log and not at the right spot in the inline diff.

If we give Martell the benefit of doubt that it is ok wrt to variable naming (since he did point this one out explicitly before with a reasoning) and other style issues I assume he's fixed with clang-format by now, what's the status of functional review of this patch? Is it almost done with a few minor issues yet to be found, or has that not been done thoroughly yet at all due to major style issues in all the previous iterations?

[ruiu]

MinGW/Driver.cpp

53 ↗

(On Diff #111701)

I'm sorry that I missed your previous comment, Martell. But infoTables in many files are just copypasta and shouldn't be intentional.

As to the functionality, I think this patch is towards a right direction, though there are a few things that have to be addressed.

71 ↗

(On Diff #112287)

I'd change the type of this variable to std::vector<std::string> and repack to std::vector<const char *> at end of link(). This should eliminate a lot of uses of Saver.save().

97 ↗

(On Diff #112287)

You are using this function only once, and at that call site IgnoreShared is not passed, so it is always false. Please remove that parameter.

120 ↗

(On Diff #112287)

Use Arg->getOption().getUnaliasedOption().getID() instead of Arg->getOption().getID() so that we don't need to worry about aliases.

125 ↗

(On Diff #112287)

Do you really have to call Saver.save here? The use of Saver.save is not consistent in this patch, and it might be saving a string that's already been saved.

I believe we should remove Saver entirely from this patch. Can you do that if you replace some occurrences of StringRef with std::strings in this patch, no?

131 ↗

(On Diff #112287)

You are using this function only once so inline this.

141 ↗

(On Diff #112287)

By convention, use empty instead of size.

150–151 ↗

(On Diff #112287)

Merge OutArg and OutValueArg and do that at caller side. For example, instead of doing

forwardValue(Args, OPT_m, "i386pe", "MACHINE", "X86");

, you can just do

forwardValue(Args, OPT_m, "i386pe", "MACHINE:X86");

Also, lowercase options are probably preferred, so it's even better if you do

forwardValue(Args, OPT_m, "i386pe", "machine:x86");

189 ↗

(On Diff #112287)

for (auto *Arg : Args.filtered(OPT_L))
  SearchPaths.push_back(Arg->getValue());

[martell]

MinGW/Driver.cpp

97 ↗

(On Diff #112287)

It is based on the -Bstatic flag.

addLibrary(Arg->getValue(), Args.hasArg(OPT_Bstatic));
which in turn calls
searchLibrary(Name, StaticOnly))
passing in the bool

[martell]

MinGW/Driver.cpp

190 ↗

(On Diff #112395)

I could probably use auto here instead of std::vector<std::string>::size_type?
Other then that I think I have addressed all the comments

[martell]

auto gives a cast warning so leave it with the full type.

[ruiu]

I forgot to mention that this patch needs a test. Please create a subdirectory MinGW under lld/tests and put a test file there for this driver.

MinGW/Driver.cpp

97 ↗

(On Diff #112287)

You can at least remove = false. It is nice to keep the variable name, so I'd rename this StaticOnly.

190 ↗

(On Diff #112395)

I think just size_t suffices. However, probably the best way of doing this is something like this:

std::vector<const char *> Vec;
for (const std::string &S : LinkArgs)
  Vec.push_back(S.c_str());
return coff::link(Vec);

128–129 ↗

(On Diff #112397)

StringRef OutArg -> const std::string &OutArg

to remove Twine and str().

You generally want to avoid C strings in C++, so const char *Default = nullptr -> std::string Default = "".

131 ↗

(On Diff #112397)

For consistency with addLibrary, remove {}.

139 ↗

(On Diff #112397)

If you change StringRef OutArg to const std::string &OutArg, then you can remove Twine and str().

170–174 ↗

(On Diff #112397)

I'd make them lowercase as well.

182–183 ↗

(On Diff #112397)

if (Args.getLastArgValue(OPT_m) == "i386pe")

MinGW/Options.td

35–38 ↗

(On Diff #112397)

You are not handling this one, so please remove.

[mstorsjo]

In D33880#850959, @ruiu wrote:

I forgot to mention that this patch needs a test. Please create a subdirectory MinGW under lld/tests and put a test file there for this driver.

He actually already adds one test case within test/COFF/out.test - linking and checking an implicitly named output a.exe. I guess you at least want that test split out to a separate file in a separate directory then?

[ruiu]

He actually already adds one test case within test/COFF/out.test - linking and checking an implicitly named output a.exe. I guess you at least want that test split out to a separate file in a separate directory then?

Yes, I think we want to have a separate directory under test as we have one at the top level directory.

Also, the test seems too simple. I want to see a test file that tests -o, -entry, -subs, -shared, -outlib, -stack and image_base.

[mstorsjo]

In D33880#851739, @ruiu wrote:

He actually already adds one test case within test/COFF/out.test - linking and checking an implicitly named output a.exe. I guess you at least want that test split out to a separate file in a separate directory then?

Yes, I think we want to have a separate directory under test as we have one at the top level directory.

Also, the test seems too simple. I want to see a test file that tests -o, -entry, -subs, -shared, -outlib, -stack and image_base.

Ok, makes sense.

Although I think we should just drop handling of -shared from this patch and focus on getting the rest of it OK'd. Actually producing working DLLs is incomplete in this patch anyway.

[mstorsjo]

Ping @martell - do you have time to follow up on Rui's latest review comments? Do you want assistance in e.g. writing the necessary test cases for it?

[martell]

address feedback
add MinGW tests
add -shared

[mstorsjo]

MinGW/Driver.cpp

179 ↗

(On Diff #114455)

Shouldn't you swap a.exe and a.dll here?

MinGW/Options.td

18 ↗

(On Diff #114455)

If testing -stack, you'll notice that this will recurse infinitely - remove the alias.

test/MinGW/output.test

10 ↗

(On Diff #114455)

This doesn't test that a.exe is produced by the right command.

Missing tests for -subs, -stack, -o, -outlib and actually referring to the image_base symbol.

If setting e.g. -subs, I think it would make sense to check for it in the readobj output, and check that the dll actually is a dll etc.

Also, since the input file is used only once, Rui has earlier preferred to move that data into this file, and rename it to .yaml.

[mstorsjo]

@martell I took the liberty to improve the tests - if you squash http://martin.st/temp/lld-mingw-tests.patch into it, it should test most things that are added in this patch, as suggested by Rui.

[martell]

update tests

[martell]

@mstorsjo I had actually done a bunch of these tests but as separate files yesterday.
Given that it is just a wrapper having just output.test probably makes more sense here.

I was actually looking through tests/COFF wondering what would be the best test for the __image_base__ symbol.
Thanks for the help there.

the x64 test did not actually check for IMAGE_FILE_MACHINE_AMD64 so I added that.
also I wanted to add a test to ensure that __ImageBase is still supported because long term I want binutils to use that also so we can use LINK.exe with mingw-w64 if needed.
So for that I added an arm and aarch64 test that uses __ImageBase instead, so we can check that and thumb2pe and arm64pe all in the one test.

[mstorsjo]

In D33880#865996, @martell wrote:

@mstorsjo I had actually done a bunch of these tests but as separate files yesterday.
Given that it is just a wrapper having just output.test probably makes more sense here.

Yeah, so far it's probably easiest to squeeze them all in one. When adding more tests later we can see what looks like it'd make most sense - and we'll see what Rui thinks of course.

I was actually looking through tests/COFF wondering what would be the best test for the __image_base__ symbol.
Thanks for the help there.

the x64 test did not actually check for IMAGE_FILE_MACHINE_AMD64 so I added that.

Oh, apparently I lost that line in some test/edit cycle when I ripped out parts of them. Thanks for noticing!

also I wanted to add a test to ensure that __ImageBase is still supported because long term I want binutils to use that also so we can use LINK.exe with mingw-w64 if needed.
So for that I added an arm and aarch64 test that uses __ImageBase instead, so we can check that and thumb2pe and arm64pe all in the one test.

Ok, that makes sense to test as well - thanks!

MinGW/Driver.cpp

86 ↗

(On Diff #114525)

A comment on the level of nitpickery: Some of the functions (e.g. findFile above) are static, while this one (findFromSearchPaths) isn't - it'd be nice to have all functions static unless they really need to be non-static.

[martell]

make findFromSearchPaths, searchLibrary, addLibrary and createFiles static

[mstorsjo]

Thanks @martell for the updates! @ruiu, any further comments on this now?

[ruiu]

LGTM. There are a few things that I want to fix in this patch, but we can do that later.

[mstorsjo]

Thanks Rui for the review effort! Hopefully later changes are easier to work with, as smaller incremental additions/changes.

include/lld/Driver/Driver.h

 
 #include "llvm/ADT/ArrayRef.h"
 #include "llvm/Support/raw_ostream.h"
 
 namespace lld {
 namespace coff {
 bool link(llvm::ArrayRef<const char *> Args,
           llvm::raw_ostream &Diag = llvm::errs());
+bool ldshim(llvm::ArrayRef<const char *> Args,
+            llvm::raw_ostream &Diag = llvm::errs());
 }
 
 namespace elf {
 bool link(llvm::ArrayRef<const char *> Args, bool CanExitEarly,
           llvm::raw_ostream &Diag = llvm::errs());
 }
 
 namespace mach_o {
 bool link(llvm::ArrayRef<const char *> Args,
           llvm::raw_ostream &Diag = llvm::errs());
 }
 }
 
 #endif

lib/CMakeLists.txt

 add_subdirectory(Config)
 add_subdirectory(Core)
 add_subdirectory(Driver)
+add_subdirectory(Mingw)
 add_subdirectory(ReaderWriter)

lib/Mingw/CMakeLists.txt

+set(LLVM_TARGET_DEFINITIONS Options.td)
+tablegen(LLVM Options.inc -gen-opt-parser-defs)
+add_public_tablegen_target(ShimOptionsTableGen)
+
+if(NOT LLD_BUILT_STANDALONE)
+  set(tablegen_deps intrinsics_gen)
+endif()
+
+add_lld_library(lldCOFFShim
+  COFFLdShim.cpp
+
+  LINK_LIBS
+  lldConfig
+  lldCore
+  ${LLVM_PTHREAD_LIB}
+
+  DEPENDS
+  ShimOptionsTableGen
+  ${tablegen_deps}
+)

lib/Mingw/COFFLdShim.cpp

+//===- lib/Mingw/COFFLdShim.cpp -------------------------------------------===//
+//
+//                             The LLVM Linker
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+///
+/// GNU ld style linker driver for COFF currently supporting mingw-w64.
+///
+//===----------------------------------------------------------------------===//
+
+#include "lld/Driver/Driver.h"
+#include "llvm/ADT/ArrayRef.h"
+#include "llvm/ADT/Optional.h"
+#include "llvm/ADT/StringRef.h"
+#include "llvm/ADT/StringSwitch.h"
+#include "llvm/ADT/Triple.h"
+#include "llvm/Option/Arg.h"
+#include "llvm/Option/ArgList.h"
+#include "llvm/Option/Option.h"
+#include "llvm/Support/CommandLine.h"
+#include "llvm/Support/FileSystem.h"
+#include "llvm/Support/Path.h"
+#include "llvm/Support/StringSaver.h"
+
+#if !defined(_MSC_VER) && !defined(__MINGW32__)
+#include <unistd.h>
+#endif
+
+using namespace lld;
+using namespace llvm;
+
+namespace lld {
+namespace coff {
+namespace {
+
+// Create OptTable
+enum {
+  OPT_INVALID = 0,
+#define OPTION(_1, _2, ID, _4, _5, _6, _7, _8, _9, _10, _11, _12) OPT_##ID,
+#include "Options.inc"
+#undef OPTION
+};
+
+// Create prefix string literals used in Options.td
+#define PREFIX(NAME, VALUE) const char *const NAME[] = VALUE;
+#include "Options.inc"
+#undef PREFIX
+
+// Create table mapping all options defined in Options.td
+static const opt::OptTable::Info infoTable[] = {
+#define OPTION(X1, X2, ID, KIND, GROUP, ALIAS, X7, X8, X9, X10, X11, X12)      \
+  {X1, X2, X10,         X11,         OPT_##ID, opt::Option::KIND##Class,       \
+   X9, X8, OPT_##GROUP, OPT_##ALIAS, X7,       X12},
+#include "Options.inc"
+#undef OPTION
+};
+
+class COFFLdOptTable : public opt::OptTable {
+public:
+  COFFLdOptTable() : OptTable(infoTable, false) {}
+  opt::InputArgList parse(ArrayRef<const char *> Argv);
+};
+
+} // namespace
+
+extern StringSaver Saver;
+static SmallVector<const char *, 256> LinkArgs;
+static std::vector<StringRef> SearchPaths;
+static StringRef Sysroot;
+static bool files = false;
+
+static void error(const Twine &Msg) {
+  errs() << Msg << "\n";
+  llvm_shutdown();
+  outs().flush();
+  errs().flush();
+  _exit(1);
+}
+
+static cl::TokenizerCallback getQuotingStyle(opt::InputArgList &Args) {
+  if (auto *Arg = Args.getLastArg(OPT_rsp_quoting)) {
+    StringRef S = Arg->getValue();
+    if (S != "windows" && S != "posix")
+      error("invalid response file quoting: " + S);
+    if (S == "windows")
+      return cl::TokenizeWindowsCommandLine;
+    return cl::TokenizeGNUCommandLine;
+  }
+  if (Triple(sys::getProcessTriple()).getOS() == Triple::Win32)
+    return cl::TokenizeWindowsCommandLine;
+  return cl::TokenizeGNUCommandLine;
+}
+
+// Find a file by concatenating given paths. If a resulting path
+// starts with "=", the character is replaced with a --sysroot value.
+static Optional<std::string> findFile(StringRef Path1, const Twine &Path2) {
+  SmallString<128> S;
+  if (Path1.startswith("="))
+    sys::path::append(S, Sysroot, Path1.substr(1), Path2);
+  else
+    sys::path::append(S, Path1, Path2);
+  if (sys::fs::exists(S))
+    return S.str().str();
+  return None;
+}
+
+Optional<std::string> findFromSearchPaths(StringRef Path) {
+  for (StringRef Dir : SearchPaths)
+    if (Optional<std::string> S = findFile(Dir, Path))
+      return S;
+  return None;
+}
+
+// This is for -lfoo. We'll look for libfoo.dll.a or libfoo.a from search paths.
+Optional<std::string> searchLibrary(StringRef Name, bool IgnoreShared = false) {
+  if (Name.startswith(":"))
+    return findFromSearchPaths(Name.substr(1));
+  for (StringRef Dir : SearchPaths) {
+    if (!IgnoreShared)
+      if (Optional<std::string> S = findFile(Dir, "lib" + Name + ".dll.a"))
+        return S;
+    if (Optional<std::string> S = findFile(Dir, "lib" + Name + ".a"))
+      return S;
+  }
+  return None;
+}
+
+void addFile(StringRef Path) {
+  LinkArgs.push_back(Saver.save(Path).data());
+  files = true;
+}
+
+// Add a given library by searching it from input search paths.
+void addLibrary(StringRef Name, bool StaticOnly) {
+  if (Optional<std::string> Path = searchLibrary(Name, StaticOnly))
+    addFile(*Path);
+  else
+    error("unable to find library -l" + Name);
+}
+
+void createFiles(opt::InputArgList &Args) {
+  for (auto *Arg : Args)
+    switch (Arg->getOption().getID()) {
+    case OPT_l:
+      addLibrary(Arg->getValue(), Args.hasArg(OPT_Bstatic));
+      break;
+    case OPT_INPUT:
+      addFile(Arg->getValue());
+      break;
+    }
+  if (!files)
+    error("no input files");
+}
+
+static StringRef getString(opt::InputArgList &Args, unsigned Key,
+                           StringRef Default = "") {
+  if (auto *Arg = Args.getLastArg(Key))
+    return Arg->getValue();
+  return Default;
+}
+
+static std::vector<StringRef> getArgs(opt::InputArgList &Args, int Id) {
+  std::vector<StringRef> V;
+  for (auto *Arg : Args.filtered(Id))
+    V.push_back(Arg->getValue());
+  return V;
+}
+
+static void forward(opt::InputArgList &Args, unsigned Key, StringRef outArg) {
+  StringRef S = getString(Args, Key);
+  if (S.size())
+    LinkArgs.push_back(Saver.save(Twine("-" + outArg + ":" + S)).data());
+}
+
+static void forwardValue(opt::InputArgList &Args, unsigned Key,
+                         StringRef cmpArg, StringRef outArg,
+                         StringRef outValueArg) {
+  StringRef S = getString(Args, Key);
+  if (S == cmpArg)
+    LinkArgs.push_back(
+        Saver.save(Twine("-" + outArg + ":" + outValueArg)).data());
+}
+
+// Parses a given list of options.
+opt::InputArgList COFFLdOptTable::parse(ArrayRef<const char *> Argv) {
+  // Make InputArgList from string vectors.
+  unsigned MissingIndex;
+  unsigned MissingCount;
+  SmallVector<const char *, 256> Vec(Argv.data(), Argv.data() + Argv.size());
+
+  opt::InputArgList Args = this->ParseArgs(Vec, MissingIndex, MissingCount);
+
+  // Expand response files (arguments in the form of @<filename>)
+  // and then parse the argument again.
+  cl::ExpandResponseFiles(Saver, getQuotingStyle(Args), Vec);
+  Args = this->ParseArgs(Vec, MissingIndex, MissingCount);
+
+  if (MissingCount)
+    error(Twine(Args.getArgString(MissingIndex)) + ": missing argument");
+
+  for (auto *Arg : Args.filtered(OPT_UNKNOWN))
+    error("unknown argument: " + Arg->getSpelling());
+  return Args;
+}
+
+bool ldshim(ArrayRef<const char *> ArgsArr, raw_ostream &Diag) {
+  COFFLdOptTable Parser;
+  opt::InputArgList Args = Parser.parse(ArgsArr.slice(1));
+  LinkArgs.push_back(ArgsArr[0]);
+
+  forwardValue(Args, OPT_m, "i386pe", "MACHINE", "X86");
+  forwardValue(Args, OPT_m, "i386pep", "MACHINE", "X64");
+  forwardValue(Args, OPT_m, "thumb2pe", "MACHINE", "ARM");
+
+  forward(Args, OPT_o, "OUT");
+  forward(Args, OPT_entry, "ENTRY");
+  forward(Args, OPT_subs, "SUBSYSTEM");
+  forward(Args, OPT_shared, "DLL");
+  forward(Args, OPT_outlib, "IMPLIB");
+  forward(Args, OPT_stack, "STACK");
+
+  SearchPaths = getArgs(Args, OPT_L);
+  createFiles(Args);
+
+  // handle __image_base__
+  auto M = Args.getLastArg(OPT_m);
+  if (M && StringRef(M->getValue()) == "i386pe")
+    LinkArgs.push_back("/alternatename:__image_base__=___ImageBase");
+  else
+    LinkArgs.push_back("/alternatename:__image_base__=__ImageBase");
+
+  return coff::link(LinkArgs);
+}
+
+} // namespace coff
+} // namespace lld

lib/Mingw/Options.td

+include "llvm/Option/OptParser.td"
+
+class F<string name>: Flag<["--", "-"], name>;
+class J<string name>: Joined<["--", "-"], name>;
+class S<string name>: Separate<["--", "-"], name>;
+
+def L: JoinedOrSeparate<["-"], "L">, MetaVarName<"<dir>">,
+  HelpText<"Add a directory to the library search path">;
+def entry: S<"entry">, MetaVarName<"<entry>">,
+  HelpText<"Name of entry point symbol">;
+def m: JoinedOrSeparate<["-"], "m">, HelpText<"Set target emulation">;
+def o: JoinedOrSeparate<["-"], "o">, MetaVarName<"<path>">,
+  HelpText<"Path to file to write output">;
+def l: JoinedOrSeparate<["-"], "l">, MetaVarName<"<libName>">,
+  HelpText<"Root name of library to use">;
+def shared: F<"shared">, HelpText<"Build a shared object">;
+def subs: Separate<["--"], "subsystem">, HelpText<"Specify subsystem">;
+def stack: Flag<["--"], "stack">, Alias<stack>;
+def outlib: Separate<["--"], "out-implib">, HelpText<"Import library name">;
+
+// Currently stubs to avoid errors
+def Bdynamic: F<"Bdynamic">, HelpText<"Link against shared libraries">;
+def Bstatic: F<"Bstatic">, HelpText<"Do not link against shared libraries">;
+def major_image_version: Separate<["--"], "major-image-version">;
+def minor_image_version: Separate<["--"], "minor-image-version">;
+def O: Joined<["-"], "O">, HelpText<"Optimize output file size">;
+def v: Flag<["-"], "v">, HelpText<"Display the version number">;
+def verbose: F<"verbose">, HelpText<"Verbose mode">;
+def version: F<"version">, HelpText<"Display the version number and exit">;
+
+// Alias
+def alias_entry_e: JoinedOrSeparate<["-"], "e">, Alias<entry>;
+
+// Style
+def rsp_quoting: J<"rsp-quoting=">,
+  HelpText<"Quoting style for response files. Values supported: windows|posix">;

tools/lld/CMakeLists.txt

 set(LLVM_LINK_COMPONENTS
   Support
   )
 
 add_lld_tool(lld
   lld.cpp
   )
 
 target_link_libraries(lld
   lldDriver
   lldCOFF
   lldELF
+  lldCOFFShim
   )
 
 install(TARGETS lld
   RUNTIME DESTINATION bin)
 
 if(NOT LLD_SYMLINKS_TO_CREATE)
   set(LLD_SYMLINKS_TO_CREATE lld-link ld.lld)
 endif()
 
 foreach(link ${LLD_SYMLINKS_TO_CREATE})
   add_lld_symlink(${link} lld)
 endforeach()

tools/lld/lld.cpp

 static Flavor getFlavor(StringRef S) {
   return StringSwitch<Flavor>(S)
       .CasesLower("ld", "ld.lld", "gnu", Gnu)
       .CaseLower("link", WinLink)
       .CaseLower("darwin", Darwin)
       .Default(Invalid);
 }
 
+static bool isPETarget(const std::vector<const char *> &V) {
+  for (auto It = V.begin(); It + 1 != V.end(); ++It) {
+    if (StringRef(*It) != "-m")
+      continue;
+    StringRef S = *(It + 1);
+    return S == "i386pe" || S == "i386pep" || S == "thumb2pe";
+  }
+  return false;
+}
+
 static Flavor parseProgname(StringRef Progname) {
 #if __APPLE__
   // Use Darwin driver for "ld" on Darwin.
   if (Progname == "ld")
     return Darwin;
 #endif
 
 #if LLVM_ON_UNIX
int main(int Argc, const char **Argv) {
   // Standard set up, so program fails gracefully.
   sys::PrintStackTraceOnErrorSignal(Argv[0]);
   PrettyStackTraceProgram StackPrinter(Argc, Argv);
   llvm_shutdown_obj Shutdown;
 
   std::vector<const char *> Args(Argv, Argv + Argc);
   switch (parseFlavor(Args)) {
   case Gnu:
+    if (isPETarget(Args))
+      return !coff::ldshim(Args);
     return !elf::link(Args, true);
   case WinLink:
     return !coff::link(Args);
   case Darwin:
     return !mach_o::link(Args);
   default:
     die("lld is a generic driver.\n"
         "Invoke ld.lld (Unix), ld (macOS) or lld-link (Windows) instead.");
   }
 }