ar: add llvm-dlltool support

[martell]

Summary

A PE COFF spec compliant import library generator.
Intended to be used with mingw-w64.

Supports:
PE COFF spec (section 8, Import Library Format)
PE COFF spec (Aux Format 3: Weak Externals)

[martell]

Updated as per some suggestions from rui:

• Now uses a single function called createImportLibrary from within COFFImportFile.

Originally suggested as std::vector<uint8_t> createImportLibrary(std::vector<ExportSymbol>)

The writeArchive function is needed to create this so it is a to file allocation.
Also 2 pieces of information need to be passed in.

The MachineType i.e x86, x64, arm, etc
The DllName i.e we need the internal name of the dll the functions expect to be found in.

So I had to adjust to

std::error_code llvm::object::createImportLibrary(StringRef DllName, std::string &Path, std::vector<COFFShortExport> Exports, MachineTypes Machine)

• added pcc as a reviewer.
• added ruiu as a subscriber in case any more feedback is needed on the above.
• added compnerd as he asked me to keep him up to date with this.
• added a simple testcase.
• put on llvm-commits and made public for review

[ruiu]

Before taking a look at details, I have a question. This change copies code from LLD to LLVM, but can you move the code instead? If it is a part of the plan and will be done in a follow-up patch, that's OK, but having duplicate code isn't good.

include/llvm/DllDriver/DllDriver.h

1 ↗

(On Diff #93157)

Probably it should be DlltoolDriver as not "dll" but "dlltool" is the name of the tool.

21 ↗

(On Diff #93157)

dlltool is a one word, so dlltoolDriverMain.

ARgs?

lib/DllDriver/Config.h

1 ↗

(On Diff #93157)

This is a direct translation of LLD, but it is too LLD-ish and not good for code that lives in llvm/lib. You want to handle errors as return values using Expected or Error, for example. You don't want to have a global variable "Configulation".

[martell]

In D29892#712675, @ruiu wrote:

Before taking a look at details, I have a question. This change copies code from LLD to LLVM, but can you move the code instead? If it is a part of the plan and will be done in a follow-up patch, that's OK, but having duplicate code isn't good.

I had planned to do it in a follow-up patch which just removes it from lld and just calls createImportLibrary or if you would prefer we can call llvm-lib directly,
I'm sure you have a better understanding of what is best there.
I use git-svn and doing it all as one commit for the sub repos seem awkward.
On that I left a comment about the parser to reduce duplication also.

lib/DllDriver/Config.h

1 ↗

(On Diff #93157)

Makes sense.
I will have a look at using Expected or Error

For the Configuration it might be best to add to the parser and have that as part of llvm also because it can mostly be reused for llvm-lib

Have you any suggestions on where this would live?

[ruiu]

I want to see a patch to remove code from LLD as they should be checked in as a pair with this patch.

[• rafael]

lib/Object/ArchiveWriter.cpp

318 ↗

(On Diff #93355)

I don't think this is correct in general. For a regular .a file I don't think the .a should include undefined weak symbols in the list.

[martell]

In D29892#713457, @ruiu wrote:

I want to see a patch to remove code from LLD as they should be checked in as a pair with this patch.

I can do that, my question on the parser remains to do this though because that is also mostly shared with lld bar a few tweaks?

lib/Object/ArchiveWriter.cpp

318 ↗

(On Diff #93355)

For archives containing COFF objects we need to include the undefined weak symbols in the list to support PE COFF spec (Aux Format 3: Weak Externals)
LLD and MSVC Link won't resolve any symbol created with the createWeakExternal function without this.
I will add a testcase to highlight this..

Are you saying this specific implementation will affect ELF archives in some way?

[ruiu]

In D29892#713608, @martell wrote:

In D29892#713457, @ruiu wrote:

I want to see a patch to remove code from LLD as they should be checked in as a pair with this patch.

I can do that, my question on the parser remains to do this though because that is also mostly shared with lld bar a few tweaks?

That is an unanswered question. You are copying this large code, and if you would fail to find a good way to remove the duplicated code from LLD in a follow-up patch, we'd have two duplicate parsers, which I want to avoid. You want to find a good way to avoid duplication.

[martell]

In D29892#713612, @ruiu wrote:

In D29892#713608, @martell wrote:

In D29892#713457, @ruiu wrote:

I want to see a patch to remove code from LLD as they should be checked in as a pair with this patch.

I can do that, my question on the parser remains to do this though because that is also mostly shared with lld bar a few tweaks?

That is an unanswered question. You are copying this large code, and if you would fail to find a good way to remove the duplicated code from LLD in a follow-up patch, we'd have two duplicate parsers, which I want to avoid. You want to find a good way to avoid duplication.

I think you missed my inline comment and this got out of context

Makes sense.
I will have a look at using Expected or Error

For the Configuration it might be best to add to the parser and have that as part of llvm also because it can mostly be reused for llvm-lib

Have you any suggestions on where this would live?

I'm looking for an acceptable location to have the .def parser within the llvm code base.

[ruiu]

Sorry, I don't have a good idea where we should move that code. Probably a few trial-and-error is needed to find the best spot.

[jrmuizel]

What about just putting .def parser in lib/Support?

[ruiu]

Are you suggesting we keep all the other duplicate code? If so, I don't think that's a good thing to do.

[martell]

In D29892#737439, @ruiu wrote:

Are you suggesting we keep all the other duplicate code? If so, I don't think that's a good thing to do.

I think he means move it to lib/Support and remove the code from lld.
I will do an update for this now.

Also I will setup a separate differential to address the duplication concerns you have.
I need to update the parser to take into account a new feature added recently also so will try to get all that done today

[martell]

Putting this on hold until D32689 gets merged.
Should be a lot easier after we resolve the migration.

[martell]

light update to take D32689 into account.

note:
also seems llvm-readobj can't read weak externals
will have to add support on top of the short import libs support I done previously.

[pcc]

lib/CMakeLists.txt

24 ↗

(On Diff #98637)

Just a comment on the location: I'm not sure if we want too many of these tool drivers to live in their own top-level directories under lib. If we're planning to add this one and maybe others (cvtres?) we should think about creating a new top-level directory and moving the drivers underneath that directory. My preferred bikeshed colour is: lib/ToolDrivers/llvm-lib, lib/ToolDrivers/llvm-dlltool, etc.

If this sounds good I'll go ahead and move LibDriver.

[ruiu]

include/llvm/DlltoolDriver/DlltoolDriver.h

25 ↗

(On Diff #98637)

Your file doesn't end with '\n'. Please fix.

include/llvm/Object/COFFImportFile.h

86 ↗

(On Diff #98637)

Please run clang-format-diff before uploading a patch.

87–88 ↗

(On Diff #98637)

if (foo)
  return true;
return false;

is the same as

return foo;

include/llvm/Object/COFFModuleDefinition.h

45 ↗

(On Diff #98637)

I don't like to add a magical boolean flag to slightly change the existing behavior. Why do you need this? This should be removed unless absolutely necessary.

lib/DlltoolDriver/CMakeLists.txt

10 ↗

(On Diff #98637)

Your file doesn't end with '\n'. Please fix.

lib/DlltoolDriver/DlltoolDriver.cpp

103–131 ↗

(On Diff #98637)

You can factor this out as a function that returns opt::InputArgList.

135 ↗

(On Diff #98637)

clang-format-diff please.

135 ↗

(On Diff #98637)

In LLVM coding style, all local variables should start with uppercase letter.

150 ↗

(On Diff #98637)

ditto -- formatting errors are distracting which take reviewer's focus away from actual code. You really want to fix all mechanical errors before uploading new patches.

152 ↗

(On Diff #98637)

Why -2?

160 ↗

(On Diff #98637)

Indentation

177 ↗

(On Diff #98637)

Format error. Please fix everything using clang-format-diff.

[martell]

I only updated this for my sanity of having it inline with the other patch.
I should have noted it is not ready for review. Sorry @ruiu.
I think I should wait for @pcc to move the lib driver before updating this again

lib/CMakeLists.txt

24 ↗

(On Diff #98637)

Sure that sounds good to me.

[pcc]

lib/CMakeLists.txt

24 ↗

(On Diff #98637)

r302995

[martell]

lib/CMakeLists.txt

24 ↗

(On Diff #98637)

thanks, I will follow with an update

[ruiu]

Fix all formatting errors before uploading any new patch, please.

[martell]

updated to master.
updated to address clang-format.
updated to have dlltool arguments and not genlib

[martell]

Looking for feedback on how to deal with the change to ArchiveWriter.cpp to only impact COFF and not ELF.

include/llvm/Object/COFFModuleDefinition.h

45 ↗

(On Diff #98637)

dlltool's defs are a little different then lib.exe's
specifically mangling, see isDecorated.
and the support for '==' for weak aliasing,
although support for the latter won't hurt lib.exe

[pcc]

lib/Object/ArchiveWriter.cpp

318 ↗

(On Diff #93355)

Yes, ELF undefined weak symbols would have the flags SF_Undefinedand SF_Weak set, so this would change the behaviour for ELF.

What I would suggest is to change the code in COFFObjectFile::getSymbolFlags() to also set the flag SF_Indirect if it sees a weak external. Then you can test for presence of the flag SF_Indirect here instead of SF_Weak.

[martell]

added work around as suggested by @pcc
passes the test @rafael added in rL303075

[martell]

added context
fixed cmake missing newline.

[ruiu]

include/llvm/Object/COFFModuleDefinition.h

45 ↗

(On Diff #98637)

Why are they different?

lib/Object/COFFImportFile.cpp

584–585 ↗

(On Diff #101330)

Please don't use raw news. You really want to avoid doing that, not only in this patch, but in other patches as well. You are leaking memory. I believe you are doing this because otherwise you would be accessing free'd memory, but leaking memory is also a bug which is not acceptable. Using new is not a solution for a use-after-free bug.

[martell]

lib/Object/COFFImportFile.cpp

584–585 ↗

(On Diff #101330)

Sorry about that Rui, I didn't even realise I used new here.
This doesn't even need new because it is pushed back onto the vector.
Updating the patch now. Also fixed and tested with mingw-w64.

lib/Object/COFFObjectFile.cpp

232 ↗

(On Diff #101330)

This was meant to be ||.

[martell]

updated to master.
addressed comments.
fixed tests.
added clearer comment about indirect

[martell]

include/llvm/Object/COFFModuleDefinition.h

45 ↗

(On Diff #98637)

mingw and wine def files does not mangle _ which is important for x86.
It also does some weird things in the mingw crt like prepending _ to all x86 crt functions so that they end up the same as x64 at link time.
I would like to not do this but this is a legacy problem with binutils dlltool.
I would have to update binutils to stop doing this before I could get mingw-w64 to change creating an abi breakage.
Some time in the future we could have this behind some flag like --legacy-mode and encourage mingw-w64 and wine to recreate all their .def files.

[martell]

ping @ruiu :)

[ruiu]

include/llvm/Object/COFFModuleDefinition.h

45 ↗

(On Diff #98637)

Please add that as a comment, including the fact that what mingw does is weird and should be removed.

[mstorsjo]

Just an additional comment (hope it doesn't delay adding this tool); this functionality is available in lib.exe in the MSVC universe, and via "link.exe -lib" (which iirc is what lib.exe calls). Is it possible to hook this up into lld later as well, in case most of the actual functionality is in libs and not in the dlltool frontend? That at least seems to be the case, so that's good. (I guess I can try to do that myself once the rest of this has landed as well.)

[martell]

added a comment per request of rui

[martell]

In D29892#804264, @mstorsjo wrote:

Just an additional comment (hope it doesn't delay adding this tool); this functionality is available in lib.exe in the MSVC universe, and via "link.exe -lib" (which iirc is what lib.exe calls). Is it possible to hook this up into lld later as well, in case most of the actual functionality is in libs and not in the dlltool frontend? That at least seems to be the case, so that's good. (I guess I can try to do that myself once the rest of this has landed as well.)

Code was already migrated from LLD to LLVM so that LLD, llvm-lib and llvm-dlltool can share the same code base.
I purposefully done this with llvm-lib in mind, you just need to hook up the frontend api of llvm-lib to COFFModuleDefinition and the Archiver to achieve this.

[martell]

updated comment to mention we should remove it in future.

[ruiu]

lib/Object/ArchiveWriter.cpp

322 ↗

(On Diff #105991)

Indentation. Please use clang-format-diff to format your patch before requesting code review.

lib/Object/COFFImportFile.cpp

584–585 ↗

(On Diff #101330)

Why do you have to pass buffers in the first place? Their lifetime is till the end of this block, so it seems unnecessary.

lib/Object/COFFModuleDefinition.cpp

59 ↗

(On Diff #105991)

... because mingw doesn't prepend "_".

87 ↗

(On Diff #105991)

Please format comment. This comment has two space characters. Please add a full stop at end of a sentence.

222–227 ↗

(On Diff #105991)

Do you think you can just do this?

if (Machine == IMAGE_FILE_MACHINE_I386 && !MingwDef) {
  if (!isDecorated(E.Name))
    E.Name = (std::string("_").append(E.Name));
  if (!E.ExtName.empty() && !isDecorated(E.ExtName))
    E.ExtName = (std::string("_").append(E.ExtName));
}

lib/ToolDrivers/llvm-dlltool/DlltoolDriver.cpp

53 ↗

(On Diff #105991)

There's a off-by-one error. X6 should be X7 and so on.

112–115 ↗

(On Diff #105991)

Let's simplify this.

llvm::errs() << Args.getArgString(MissingIndex) + ": missing argument\n";

119–123 ↗

(On Diff #105991)

Merge this with the following if so that it just prints out the help message.

128 ↗

(On Diff #105991)

It is easier to just add i386, i386:x86-64, arm at end of this line

134 ↗

(On Diff #105991)

No -> no

135 ↗

(On Diff #105991)

and this line.

141 ↗

(On Diff #105991)

Remove llvm:: as you did using namespace llvm.

166 ↗

(On Diff #105991)

Why?

173–175 ↗

(On Diff #105991)

Reverse the condition and return 0 at end of main.

[martell]

address comments

[martell]

lib/Object/ArchiveWriter.cpp

322 ↗

(On Diff #105991)

Sorry I thought the indentation of 2 spaces is correct already.
It actually wraps it on the next line to the external bracket.

lib/Object/COFFModuleDefinition.cpp

222–227 ↗

(On Diff #105991)

No because @ and ? are are still valid decorators for mingw

lib/ToolDrivers/llvm-dlltool/DlltoolDriver.cpp

53 ↗

(On Diff #105991)

Look at lib/ToolDrivers/llvm-lib/LibDriver.cpp#L44
I can change it but every infoTable in the llvm source tree has this off by 1 for the #define

112–115 ↗

(On Diff #105991)

assuming << and not +

128 ↗

(On Diff #105991)

Thought it would be better to have that list in once place but that works also.

141 ↗

(On Diff #105991)

This is all based on the rest of the source code of llvm.
llvm::errs seems to be used throughout for clarity over errs()
Just looking at lib/ToolDrivers/llvm-lib/LibDriver.cpp or any other source file in llvm I can see it follows this pattern.
We shouldn't be too pedantic about this.

[ruiu]

lib/Object/COFFImportFile.cpp

584–585 ↗

(On Diff #101330)

Can you address this comment?

lib/ToolDrivers/llvm-dlltool/DlltoolDriver.cpp

53 ↗

(On Diff #105991)

This is an obvious mistake, and it is not a good idea to blindly copy mistakes.

[martell]

lib/Object/COFFImportFile.cpp

584–585 ↗

(On Diff #101330)

In general I am just following the style of the rest of the function.

std::vector<uint8_t> ImportDescriptor; -> createImportDescriptor
std::vector<uint8_t> NullImportDescriptor; -> createNullImportDescriptor
std::vector<uint8_t> NullThunk; -> createNullThunk

std::vector<uint8_t> WeakBuffer1;
std::vector<uint8_t> WeakBuffer2;

Maybe a suggestion of a better name?
Unless you want a change to the signature of createWeakExternal?

[martell]

lib/Object/COFFImportFile.cpp

584–585 ↗

(On Diff #101330)

I would assume the others use std::vector<uint8_t> in the case that someone wants to use this function on a premade buffer of their own. Possible Public API???
I am happy to put the buffer inside the createWeakExternal function itself if you want?

[ruiu]

lib/Object/COFFImportFile.cpp

584–585 ↗

(On Diff #101330)

I don't think it's about style. Why do you have to pass WeakBuffer1 and WeakBuffer2? If there's no need, please don't do that.

[martell]

lib/Object/COFFImportFile.cpp

584–585 ↗

(On Diff #101330)

Gotcha, It has been awhile since I wrote this so I had to take some time to re read everything.
You have to pass WeakBuffer1 and WeakBuffer2 because you need to have the buffers in memory up until the writeArchive function is called.
By having the buffer in the function itself combined with the fact we call the function twice means we are using the same memory space for both buffers which obviously creates problems like having the same import created over and over.

md5sum e15c2aae621caebae2f1cc91a040cf62 /martell/llvm/usr/x86_64-w64-mingw32/lib/libmsvcrt.a
vs
md5sum 6cfae6b5ba015594e4b3ba8f3ba9bde0 /martell/llvm/usr/x86_64-w64-mingw32/lib/libmsvcrt.a

I will clarify what I mean by style.
ObjectFactory has a BumpPtrAllocator so we could create the buffer from within the function like createShortImport but all the other functions that create full sections like createNullImportDescriptor createNullImportDescriptor and createNullThunk all use std::vector<uint8_t>
For the record all of these functions could also use the BumpPtrAllocator I just assume they don't because it is cleaner and more readable which is why I done the same for createWeakExternal

[ruiu]

lib/Object/COFFImportFile.cpp

584–585 ↗

(On Diff #101330)

When writeArchive is called, both WeakBuffer1 and WeakBuffer2 are dead because their lifetime ends at the end of their enclosing innermost block. (i.e. they are invalid after line 591.) That means this patch has a use-after-free bug.

[martell]

use the allocator

[ruiu]

lib/Object/COFFImportFile.cpp

559–560 ↗

(On Diff #106640)

This must not be correct. You are memcpy'ing to some memory region and returning something different.

[martell]

lib/Object/COFFImportFile.cpp

559–560 ↗

(On Diff #106640)

In a cleanup I removed char *Buf so simplify not even realising I needed that reference.
Should be good now. Sorry for the confusion.

[ruiu]

lib/ToolDrivers/llvm-dlltool/DlltoolDriver.cpp

72 ↗

(On Diff #106717)

Remove llvm::.

99–100 ↗

(On Diff #106717)

These variables are dead. Please remove. (Please take a look at your code more carefully so that you won't add dead code.)

101 ↗

(On Diff #106717)

Move this to Line 134 so that the variable definition is closer to the use of the variable.

103 ↗

(On Diff #106717)

Move this to line 134.

104 ↗

(On Diff #106717)

Move this to line 131.

158 ↗

(On Diff #106717)

Use empty() instead.

[ruiu]

Please read your patch throughly again and again to fix all errors of the same kind until you think your patch is perfect. I think I'm pointing out the same errors probably too many times.

lib/Object/COFFModuleDefinition.cpp

87–89 ↗

(On Diff #106738)

This is not safe because Buf may contain just one character (and if that's the case, accessing Buf[1] can cause a runtime error.) You want to do this.

Buf = Buf.drop_front();
// GNU dlltool accepts both = and ==.
if (Buf.startswith('='))
  Buf = Buf.drop_front();
return Token(Equal, "=");

lib/ToolDrivers/llvm-dlltool/DlltoolDriver.cpp

101 ↗

(On Diff #106738)

Remove llvm::.

129–131 ↗

(On Diff #106738)

std::string Path = Args.getLastArgValue(OPT_l);

143 ↗

(On Diff #106738)

You want to do this only when MB has contents, no?

155–156 ↗

(On Diff #106738)

OK, so delete the above definition of Path and write everything here.

std::string Path = Args.getLastArgValue(OPT_l);
if (Path.empty())
  Path = getImplibPath(Def->OutputFile);

[martell]

lib/Object/COFFModuleDefinition.cpp

87–89 ↗

(On Diff #106738)

I suppose we could have a bad user that does something like hellofunc = without finishing the line.
Will do startswith but with a NULL terminated StringRef instead of a char.

lib/ToolDrivers/llvm-dlltool/DlltoolDriver.cpp

101 ↗

(On Diff #106738)

This is not consistent with the rest of the llvm project.
Anywhere opt is used regardless of using namespace llvm; it is displayed as llvm::opt
Similar to llvm::errs and llvm::outs
For example llvm::opt::OptTable is mentioned in this file twice and every other driver.
Just looking at the lib driver I can see llvm::InputArgList in main.

143 ↗

(On Diff #106738)

Added a check after the creation of MB to say definition file is empty in that case.

[martell]

removed all the unneeded headers in a cleanup.

check-llvm looks good

ninja check-llvm
  Expected Passes    : 14861
  Expected Failures  : 61
  Unsupported Tests  : 6273

rebuilt mingw-w64 crt with this and tested with LLD

/Users/martell/llvm/usr/bin/lld -flavor link -OUT:a.exe /Users/martell/llvm/usr/x86_64-w64-mingw32/lib/crt2.o /Users/martell/llvm/usr/x86_64-w64-mingw32/lib/crtbegin.o hello.o /Users/martell/llvm/usr/x86_64-w64-mingw32/lib/libmingw32.a /Users/martell/llvm/usr/lib/clang/5.0.0/lib/windows/libclang_rt.builtins-x86_64.a /Users/martell/llvm/usr/x86_64-w64-mingw32/lib/libmoldname.a /Users/martell/llvm/usr/x86_64-w64-mingw32/lib/libmingwex.a /Users/martell/llvm/usr/x86_64-w64-mingw32/lib/libmsvcrt.a /Users/martell/llvm/usr/x86_64-w64-mingw32/lib/libadvapi32.a /Users/martell/llvm/usr/x86_64-w64-mingw32/lib/libshell32.a /Users/martell/llvm/usr/x86_64-w64-mingw32/lib/libuser32.a /Users/martell/llvm/usr/x86_64-w64-mingw32/lib/libkernel32.a /Users/martell/llvm/usr/x86_64-w64-mingw32/lib/crtend.o /alternatename:__image_base__=__ImageBase

wine a.exe
hello world

Can we get a LGTM now? :)

[ruiu]

lib/Object/COFFImportFile.cpp

167 ↗

(On Diff #106768)

LLVM coding style: imp -> Imp

(As I said, I really appreciate if you carefully review your patch yourself before sending it so that I don't need to point out all these style errors. It seems we are spending too much time on these kind of stuff and am honestly a bit tired of pointing them out.)

483 ↗

(On Diff #106768)

imp -> Imp

484–486 ↗

(On Diff #106768)

Why static?

[martell]

lib/Object/COFFImportFile.cpp

167 ↗

(On Diff #106768)

Damn it, I really thought we had everything that time.
I don't know how we missed that in the previous iterations.
I assume it just became more apparent when std::vector<uint8_t> &Buffer was removed.
I just read through everything extra carefully and I ran clang-format again.
Updating now.

484–486 ↗

(On Diff #106768)

The reason for doing this was to not have to initialise the variables on every call to the function.

createNullImportDescriptor has
static const uint32_t NumberOfSections = 1;
static const uint32_t NumberOfSymbols = 1;

createImportDescriptor has
static const uint32_t NumberOfSections = 2;
static const uint32_t NumberOfSymbols = 7;
static const uint32_t NumberOfRelocations = 3;

createNullThunk has
static const uint32_t NumberOfSections = 2;
static const uint32_t NumberOfSymbols = 1;

We do not need the variables for the lifetime of the program but if I am to remove static here it should also be done for the other functions.

Which option will I go with?

[martell]

rebased on master.
ran clang-format on the files even fixing style errors in files that were not my own.

[martell]

I would REALLY like to make the 5.0 window with this. :)

lib/Object/ArchiveWriter.cpp

380 ↗

(On Diff #107054)

clang-format picked this up previously.
I just never added it to the diff.
Adding it here to show that I am actually using clang-format.

[ruiu]

lib/Object/ArchiveWriter.cpp

380 ↗

(On Diff #107054)

Well, what you want to use is actually clang-format-diff, not clang-format, as I said before. clang-format-diff formats only the part you modified while clang-format formats entire file. Pleaser revert this part of change so that you don't change unrelated code.

lib/Object/COFFImportFile.cpp

484–486 ↗

(On Diff #106768)

Compilers can optimize them out, so remove static.

549–550 ↗

(On Diff #107054)

Are these the same as

"__imp_" + Sym

and

"__imp_" + Weak

? If so, write that way.

lib/ToolDrivers/llvm-dlltool/DlltoolDriver.cpp

144–146 ↗

(On Diff #107054)

Is OutputFile guaranteed to have some non-empty value at this point?

[martell]

lib/Object/COFFImportFile.cpp

549–550 ↗

(On Diff #107054)

writeStringTable(Buffer, {"__imp_" + Sym, "__imp_" + Weak});
error: no matching function for call to 'writeStringTable'
note: candidate function not viable: cannot convert initializer list argument to 'ArrayRef<const std::string>' (aka 'ArrayRef<const basic_string<char, char_traits<char>, allocator<char> > >')

Using Twine fails here also and converting is pointless as we might as well just use std::string at that point.

E.Name and ENameExt use this same format E.Name = (std::string("_").append(E.Name));
Seems appropriate considering the type.

lib/ToolDrivers/llvm-dlltool/DlltoolDriver.cpp

144–146 ↗

(On Diff #107054)

parseOne in parseCOFFModuleDefinition sets it if Name is in the definition.
Is possible if -D is not passed and no name is in the def file.
I was letting writeArchive error out on this but we can exit sooner here.

[ruiu]

LGTM

[alekseyshl]

http://lab.llvm.org:8011/builders/sanitizer-x86_64-linux-fast/builds/6544 is not happy about this patch.

[pcc]

llvm/trunk/lib/Object/COFFImportFile.cpp

540

I would use IMAGE_WEAK_EXTERN_SEARCH_ALIAS here instead of the magic number 3.

[martell]

llvm/trunk/lib/Object/COFFImportFile.cpp

540

I will get a patch ready for that and stop setting SF_Undefined in the correct scenario.
Standby for an update. :)

llvm/trunk/include/llvm/Object/COFFModuleDefinition.h

 // A parser for the module-definition file (.def file).
 // Parsed results are directly written to Config global variable.
 //
 // The format of module-definition files are described in this document:
 // https://msdn.microsoft.com/en-us/library/28d6s79h.aspx
 //
 //===----------------------------------------------------------------------===//
 
-
 #ifndef LLVM_OBJECT_COFF_MODULE_DEFINITION_H
 #define LLVM_OBJECT_COFF_MODULE_DEFINITION_H
 
 #include "llvm/Object/COFF.h"
 #include "llvm/Object/COFFImportFile.h"
 
 namespace llvm {
 namespace object {
 
 struct COFFModuleDefinition {
   std::vector<COFFShortExport> Exports;
   std::string OutputFile;
   uint64_t ImageBase = 0;
   uint64_t StackReserve = 0;
   uint64_t StackCommit = 0;
   uint64_t HeapReserve = 0;
   uint64_t HeapCommit = 0;
   uint32_t MajorImageVersion = 0;
   uint32_t MinorImageVersion = 0;
   uint32_t MajorOSVersion = 0;
   uint32_t MinorOSVersion = 0;
 };
 
+// mingw and wine def files do not mangle _ for x86 which
+// is a consequence of legacy binutils' dlltool functionality.
+// This MingwDef flag should be removed once mingw stops this pratice.
 Expected<COFFModuleDefinition>
-parseCOFFModuleDefinition(MemoryBufferRef MB, COFF::MachineTypes Machine);
+parseCOFFModuleDefinition(MemoryBufferRef MB, COFF::MachineTypes Machine,
+                          bool MingwDef = false);
 
 } // End namespace object.
 } // End namespace llvm.
 
 #endif

llvm/trunk/include/llvm/ToolDrivers/llvm-dlltool/DlltoolDriver.h

+//===- DlltoolDriver.h - dlltool.exe-compatible driver ----------*- C++ -*-===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+//
+// Defines an interface to a dlltool.exe-compatible driver.
+// Used by llvm-dlltool.
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_TOOLDRIVERS_LLVM_DLLTOOL_DLLTOOLDRIVER_H
+#define LLVM_TOOLDRIVERS_LLVM_DLLTOOL_DLLTOOLDRIVER_H
+
+namespace llvm {
+template <typename T> class ArrayRef;
+
+int dlltoolDriverMain(ArrayRef<const char *> ArgsArr);
+} // namespace llvm
+
+#endif

llvm/trunk/lib/Object/ArchiveWriter.cpp

for (unsigned MemberNum = 0, N = Members.size(); MemberNum < N; ++MemberNum) {
     }
 
     for (const object::BasicSymbolRef &S : Obj.symbols()) {
       uint32_t Symflags = S.getFlags();
       if (Symflags & object::SymbolRef::SF_FormatSpecific)
         continue;
       if (!(Symflags & object::SymbolRef::SF_Global))
         continue;
-      if (Symflags & object::SymbolRef::SF_Undefined)
+      if (Symflags & object::SymbolRef::SF_Undefined &&
+          !(Symflags & object::SymbolRef::SF_Indirect))
         continue;
 
       unsigned NameOffset = NameOS.tell();
       if (auto EC = S.printName(NameOS))
         return EC;
       NameOS << '\0';
       MemberOffsetRefs.push_back(MemberNum);
       if (isBSDLike(Kind))

llvm/trunk/lib/Object/COFFImportFile.cpp

public:
   // NULL Import Address Table entry and a NULL Import Lookup Table Entry.  It
   // is used to terminate the IAT and ILT.
   NewArchiveMember createNullThunk(std::vector<uint8_t> &Buffer);
 
   // Create a short import file which is described in PE/COFF spec 7. Import
   // Library Format.
   NewArchiveMember createShortImport(StringRef Sym, uint16_t Ordinal,
                                      ImportType Type, ImportNameType NameType);
+
+  // Create a weak external file which is described in PE/COFF Aux Format 3.
+  NewArchiveMember createWeakExternal(StringRef Sym, StringRef Weak, bool Imp);
 };
 } // namespace
 
 NewArchiveMember
 ObjectFactory::createImportDescriptor(std::vector<uint8_t> &Buffer) {
-  static const uint32_t NumberOfSections = 2;
-  static const uint32_t NumberOfSymbols = 7;
-  static const uint32_t NumberOfRelocations = 3;
+  const uint32_t NumberOfSections = 2;
+  const uint32_t NumberOfSymbols = 7;
+  const uint32_t NumberOfRelocations = 3;
 
   // COFF Header
   coff_file_header Header{
       u16(Machine),
       u16(NumberOfSections),
       u32(0),
       u32(sizeof(Header) + (NumberOfSections * sizeof(coff_section)) +
           // .idata$2
           sizeof(coff_import_directory_table_entry) +
           NumberOfRelocations * sizeof(coff_relocation) +
           // .idata$4
           (DLLName.size() + 1)),
       u32(NumberOfSymbols),
       u16(0),
       u16(is32bit(Machine) ? IMAGE_FILE_32BIT_MACHINE : 0),
   };
   append(Buffer, Header);
 
   // Section Header Table
-  static const coff_section SectionTable[NumberOfSections] = {
+  const coff_section SectionTable[NumberOfSections] = {
       {{'.', 'i', 'd', 'a', 't', 'a', '$', '2'},
        u32(0),
        u32(0),
        u32(sizeof(coff_import_directory_table_entry)),
        u32(sizeof(coff_file_header) + NumberOfSections * sizeof(coff_section)),
        u32(sizeof(coff_file_header) + NumberOfSections * sizeof(coff_section) +
            sizeof(coff_import_directory_table_entry)),
        u32(0),
const coff_section SectionTable[NumberOfSections] = {
        u16(0),
        u16(0),
        u32(IMAGE_SCN_ALIGN_2BYTES | IMAGE_SCN_CNT_INITIALIZED_DATA |
            IMAGE_SCN_MEM_READ | IMAGE_SCN_MEM_WRITE)},
   };
   append(Buffer, SectionTable);
 
   // .idata$2
-  static const coff_import_directory_table_entry ImportDescriptor{
+  const coff_import_directory_table_entry ImportDescriptor{
       u32(0), u32(0), u32(0), u32(0), u32(0),
   };
   append(Buffer, ImportDescriptor);
 
-  static const coff_relocation RelocationTable[NumberOfRelocations] = {
+  const coff_relocation RelocationTable[NumberOfRelocations] = {
       {u32(offsetof(coff_import_directory_table_entry, NameRVA)), u32(2),
        u16(getImgRelRelocation(Machine))},
       {u32(offsetof(coff_import_directory_table_entry, ImportLookupTableRVA)),
        u32(3), u16(getImgRelRelocation(Machine))},
       {u32(offsetof(coff_import_directory_table_entry, ImportAddressTableRVA)),
        u32(4), u16(getImgRelRelocation(Machine))},
   };
   append(Buffer, RelocationTable);
writeStringTable(Buffer,
                     NullThunkSymbolName});
 
   StringRef F{reinterpret_cast<const char *>(Buffer.data()), Buffer.size()};
   return {MemoryBufferRef(F, DLLName)};
 }
 
 NewArchiveMember
 ObjectFactory::createNullImportDescriptor(std::vector<uint8_t> &Buffer) {
-  static const uint32_t NumberOfSections = 1;
-  static const uint32_t NumberOfSymbols = 1;
+  const uint32_t NumberOfSections = 1;
+  const uint32_t NumberOfSymbols = 1;
 
   // COFF Header
   coff_file_header Header{
       u16(Machine),
       u16(NumberOfSections),
       u32(0),
       u32(sizeof(Header) + (NumberOfSections * sizeof(coff_section)) +
           // .idata$3
           sizeof(coff_import_directory_table_entry)),
       u32(NumberOfSymbols),
       u16(0),
       u16(is32bit(Machine) ? IMAGE_FILE_32BIT_MACHINE : 0),
   };
   append(Buffer, Header);
 
   // Section Header Table
-  static const coff_section SectionTable[NumberOfSections] = {
+  const coff_section SectionTable[NumberOfSections] = {
       {{'.', 'i', 'd', 'a', 't', 'a', '$', '3'},
        u32(0),
        u32(0),
        u32(sizeof(coff_import_directory_table_entry)),
        u32(sizeof(coff_file_header) +
            (NumberOfSections * sizeof(coff_section))),
        u32(0),
        u32(0),
        u16(0),
        u16(0),
        u32(IMAGE_SCN_ALIGN_4BYTES | IMAGE_SCN_CNT_INITIALIZED_DATA |
            IMAGE_SCN_MEM_READ | IMAGE_SCN_MEM_WRITE)},
   };
   append(Buffer, SectionTable);
 
   // .idata$3
-  static const coff_import_directory_table_entry ImportDescriptor{
+  const coff_import_directory_table_entry ImportDescriptor{
       u32(0), u32(0), u32(0), u32(0), u32(0),
   };
   append(Buffer, ImportDescriptor);
 
   // Symbol Table
   coff_symbol16 SymbolTable[NumberOfSymbols] = {
       {{{0, 0, 0, 0, 0, 0, 0, 0}},
        u32(0),
        u16(1),
        u16(0),
        IMAGE_SYM_CLASS_EXTERNAL,
        0},
   };
   SymbolTable[0].Name.Offset.Offset = sizeof(uint32_t);
   append(Buffer, SymbolTable);
 
   // String Table
   writeStringTable(Buffer, {NullImportDescriptorSymbolName});
 
   StringRef F{reinterpret_cast<const char *>(Buffer.data()), Buffer.size()};
   return {MemoryBufferRef(F, DLLName)};
 }
 
 NewArchiveMember ObjectFactory::createNullThunk(std::vector<uint8_t> &Buffer) {
-  static const uint32_t NumberOfSections = 2;
-  static const uint32_t NumberOfSymbols = 1;
+  const uint32_t NumberOfSections = 2;
+  const uint32_t NumberOfSymbols = 1;
   uint32_t VASize = is32bit(Machine) ? 4 : 8;
 
   // COFF Header
   coff_file_header Header{
       u16(Machine),
       u16(NumberOfSections),
       u32(0),
       u32(sizeof(Header) + (NumberOfSections * sizeof(coff_section)) +
           // .idata$5
           VASize +
           // .idata$4
           VASize),
       u32(NumberOfSymbols),
       u16(0),
       u16(is32bit(Machine) ? IMAGE_FILE_32BIT_MACHINE : 0),
   };
   append(Buffer, Header);
 
   // Section Header Table
-  static const coff_section SectionTable[NumberOfSections] = {
+  const coff_section SectionTable[NumberOfSections] = {
       {{'.', 'i', 'd', 'a', 't', 'a', '$', '5'},
        u32(0),
        u32(0),
        u32(VASize),
        u32(sizeof(coff_file_header) + NumberOfSections * sizeof(coff_section)),
        u32(0),
        u32(0),
        u16(0),
NewArchiveMember ObjectFactory::createShortImport(StringRef Sym,
   // Write symbol name and DLL name.
   memcpy(P, Sym.data(), Sym.size());
   P += Sym.size() + 1;
   memcpy(P, DLLName.data(), DLLName.size());
 
   return {MemoryBufferRef(StringRef(Buf, Size), DLLName)};
 }
 
+NewArchiveMember ObjectFactory::createWeakExternal(StringRef Sym,
+                                                   StringRef Weak, bool Imp) {
+  std::vector<uint8_t> Buffer;
+  const uint32_t NumberOfSections = 1;
+  const uint32_t NumberOfSymbols = 5;
+
+  // COFF Header
+  coff_file_header Header{
+      u16(0),
+      u16(NumberOfSections),
+      u32(0),
+      u32(sizeof(Header) + (NumberOfSections * sizeof(coff_section))),
+      u32(NumberOfSymbols),
+      u16(0),
+      u16(0),
+  };
+  append(Buffer, Header);
+
+  // Section Header Table
+  const coff_section SectionTable[NumberOfSections] = {
+      {{'.', 'd', 'r', 'e', 'c', 't', 'v', 'e'},
+       u32(0),
+       u32(0),
+       u32(0),
+       u32(0),
+       u32(0),
+       u32(0),
+       u16(0),
+       u16(0),
+       u32(IMAGE_SCN_LNK_INFO | IMAGE_SCN_LNK_REMOVE)}};
+  append(Buffer, SectionTable);
+
+  // Symbol Table
+  coff_symbol16 SymbolTable[NumberOfSymbols] = {
+      {{{'@', 'c', 'o', 'm', 'p', '.', 'i', 'd'}},
+       u32(0),
+       u16(0xFFFF),
+       u16(0),
+       IMAGE_SYM_CLASS_STATIC,
+       0},
+      {{{'@', 'f', 'e', 'a', 't', '.', '0', '0'}},
+       u32(0),
+       u16(0xFFFF),
+       u16(0),
+       IMAGE_SYM_CLASS_STATIC,
+       0},
+      {{{0, 0, 0, 0, 0, 0, 0, 0}},
+       u32(0),
+       u16(0),
+       u16(0),
+       IMAGE_SYM_CLASS_EXTERNAL,
+       0},
+      {{{0, 0, 0, 0, 0, 0, 0, 0}},
+       u32(0),
+       u16(0),
+       u16(0),
+       IMAGE_SYM_CLASS_WEAK_EXTERNAL,
+       1},
+      {{{2, 0, 0, 0, 3, 0, 0, 0}}, u32(0), u16(0), u16(0), uint8_t(0), 0},
+  };
+  reinterpret_cast<StringTableOffset &>(SymbolTable[2].Name).Offset =
+      sizeof(uint32_t);
+
+  //__imp_ String Table
+  if (Imp) {
+    reinterpret_cast<StringTableOffset &>(SymbolTable[3].Name).Offset =
+        sizeof(uint32_t) + Sym.size() + 1 + 6;
+    writeStringTable(Buffer, {std::string("__imp_").append(Sym),
+                              std::string("__imp_").append(Weak)});
+  } else {
+    reinterpret_cast<StringTableOffset &>(SymbolTable[3].Name).Offset =
+        sizeof(uint32_t) + Sym.size() + 1;
+    writeStringTable(Buffer, {Sym, Weak});
+  }
+  append(Buffer, SymbolTable);
+
+  // Copied here so we can still use writeStringTable
+  char *Buf = Alloc.Allocate<char>(Buffer.size());
+  memcpy(Buf, Buffer.data(), Buffer.size());
+  return {MemoryBufferRef(StringRef(Buf, Buffer.size()), DLLName)};
+}
+
 std::error_code writeImportLibrary(StringRef DLLName, StringRef Path,
                                    ArrayRef<COFFShortExport> Exports,
                                    MachineTypes Machine) {
 
   std::vector<NewArchiveMember> Members;
   ObjectFactory OF(llvm::sys::path::filename(DLLName), Machine);
 
   std::vector<uint8_t> ImportDescriptor;
   Members.push_back(OF.createImportDescriptor(ImportDescriptor));
 
   std::vector<uint8_t> NullImportDescriptor;
   Members.push_back(OF.createNullImportDescriptor(NullImportDescriptor));
 
   std::vector<uint8_t> NullThunk;
   Members.push_back(OF.createNullThunk(NullThunk));
 
   for (COFFShortExport E : Exports) {
     if (E.Private)
       continue;
 
+    if (E.isWeak()) {
+      Members.push_back(OF.createWeakExternal(E.Name, E.ExtName, false));
+      Members.push_back(OF.createWeakExternal(E.Name, E.ExtName, true));
+      continue;
+    }
+
     ImportType ImportType = IMPORT_CODE;
     if (E.Data)
       ImportType = IMPORT_DATA;
     if (E.Constant)
       ImportType = IMPORT_CONST;
 
     StringRef SymbolName = E.isWeak() ? E.ExtName : E.Name;
     ImportNameType NameType = getNameType(SymbolName, E.Name, Machine);

llvm/trunk/lib/Object/COFFModuleDefinition.cpp

 };
 
 struct Token {
   explicit Token(Kind T = Unknown, StringRef S = "") : K(T), Value(S) {}
   Kind K;
   StringRef Value;
 };
 
-static bool isDecorated(StringRef Sym) {
-  return Sym.startswith("_") || Sym.startswith("@") || Sym.startswith("?");
+static bool isDecorated(StringRef Sym, bool MingwDef) {
+  // mingw does not prepend "_".
+  return (!MingwDef && Sym.startswith("_")) || Sym.startswith("@") ||
+         Sym.startswith("?");
 }
 
 static Error createError(const Twine &Err) {
   return make_error<StringError>(StringRef(Err.str()),
                                  object_error::parse_failed);
 }
 
 class Lexer {
case '\0':
       return Token(Eof);
     case ';': {
       size_t End = Buf.find('\n');
       Buf = (End == Buf.npos) ? "" : Buf.drop_front(End);
       return lex();
     }
     case '=':
       Buf = Buf.drop_front();
+      // GNU dlltool accepts both = and ==.
+      if (Buf.startswith("="))
+        Buf = Buf.drop_front();
       return Token(Equal, "=");
     case ',':
       Buf = Buf.drop_front();
       return Token(Comma, ",");
     case '"': {
       StringRef S;
       std::tie(S, Buf) = Buf.substr(1).split('"');
       return Token(Identifier, S);
public:
   }
 
 private:
   StringRef Buf;
 };
 
 class Parser {
 public:
-  explicit Parser(StringRef S, MachineTypes M) : Lex(S), Machine(M) {}
+  explicit Parser(StringRef S, MachineTypes M, bool B)
+      : Lex(S), Machine(M), MingwDef(B) {}
 
   Expected<COFFModuleDefinition> parse() {
     do {
       if (Error Err = parseOne())
         return std::move(Err);
     } while (Tok.K != Eof);
     return Info;
   }
if (Tok.K == Equal) {
         return createError("identifier expected, but got " + Tok.Value);
       E.ExtName = E.Name;
       E.Name = Tok.Value;
     } else {
       unget();
     }
 
     if (Machine == IMAGE_FILE_MACHINE_I386) {
-      if (!isDecorated(E.Name))
+      if (!isDecorated(E.Name, MingwDef))
         E.Name = (std::string("_").append(E.Name));
-      if (!E.ExtName.empty() && !isDecorated(E.ExtName))
+      if (!E.ExtName.empty() && !isDecorated(E.ExtName, MingwDef))
         E.ExtName = (std::string("_").append(E.ExtName));
     }
 
     for (;;) {
       read();
       if (Tok.K == Identifier && Tok.Value[0] == '@') {
         Tok.Value.drop_front().getAsInteger(10, E.Ordinal);
         read();
Error parseVersion(uint32_t *Major, uint32_t *Minor) {
     return Error::success();
   }
 
   Lexer Lex;
   Token Tok;
   std::vector<Token> Stack;
   MachineTypes Machine;
   COFFModuleDefinition Info;
+  bool MingwDef;
 };
 
 Expected<COFFModuleDefinition> parseCOFFModuleDefinition(MemoryBufferRef MB,
-                                                         MachineTypes Machine) {
-  return Parser(MB.getBuffer(), Machine).parse();
+                                                         MachineTypes Machine,
+                                                         bool MingwDef) {
+  return Parser(MB.getBuffer(), Machine, MingwDef).parse();
 }
 
 } // namespace object
 } // namespace llvm

llvm/trunk/lib/Object/COFFObjectFile.cpp

 
 uint32_t COFFObjectFile::getSymbolFlags(DataRefImpl Ref) const {
   COFFSymbolRef Symb = getCOFFSymbol(Ref);
   uint32_t Result = SymbolRef::SF_None;
 
   if (Symb.isExternal() || Symb.isWeakExternal())
     Result |= SymbolRef::SF_Global;
 
-  if (Symb.isWeakExternal())
+  if (Symb.isWeakExternal()) {
     Result |= SymbolRef::SF_Weak;
+    // We use indirect to allow the archiver to write weak externs
+    Result |= SymbolRef::SF_Indirect;
+  }
 
   if (Symb.getSectionNumber() == COFF::IMAGE_SYM_ABSOLUTE)
     Result |= SymbolRef::SF_Absolute;
 
   if (Symb.isFileRecord())
     Result |= SymbolRef::SF_FormatSpecific;
 
   if (Symb.isSectionDefinition())

llvm/trunk/lib/ToolDrivers/CMakeLists.txt

+add_subdirectory(llvm-dlltool)
 add_subdirectory(llvm-lib)

llvm/trunk/lib/ToolDrivers/LLVMBuild.txt

 ;
 ; For more information on the LLVMBuild system, please see:
 ;
 ;   http://llvm.org/docs/LLVMBuild.html
 ;
 ;===------------------------------------------------------------------------===;
 
 [common]
-subdirectories = llvm-lib
+subdirectories = llvm-dlltool llvm-lib
 
 [component_0]
 type = Group
 name = ToolDrivers
 parent = Libraries

llvm/trunk/lib/ToolDrivers/llvm-dlltool/CMakeLists.txt

+set(LLVM_TARGET_DEFINITIONS Options.td)
+tablegen(LLVM Options.inc -gen-opt-parser-defs)
+add_public_tablegen_target(DllOptionsTableGen)
+
+add_llvm_library(LLVMDlltoolDriver
+  DlltoolDriver.cpp
+  )
+
+add_dependencies(LLVMDlltoolDriver DllOptionsTableGen)

llvm/trunk/lib/ToolDrivers/llvm-dlltool/DlltoolDriver.cpp

+//===- DlltoolDriver.cpp - dlltool.exe-compatible driver ------------------===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+//
+// Defines an interface to a dlltool.exe-compatible driver.
+//
+//===----------------------------------------------------------------------===//
+
+#include "llvm/ToolDrivers/llvm-dlltool/DlltoolDriver.h"
+#include "llvm/Object/ArchiveWriter.h"
+#include "llvm/Object/COFF.h"
+#include "llvm/Object/COFFImportFile.h"
+#include "llvm/Object/COFFModuleDefinition.h"
+#include "llvm/Option/Arg.h"
+#include "llvm/Option/ArgList.h"
+#include "llvm/Option/Option.h"
+#include "llvm/Support/Path.h"
+
+#include <string>
+#include <vector>
+
+using namespace llvm;
+using namespace llvm::object;
+using namespace llvm::COFF;
+
+namespace {
+
+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
+};
+
+#define PREFIX(NAME, VALUE) const char *const NAME[] = VALUE;
+#include "Options.inc"
+#undef PREFIX
+
+static const llvm::opt::OptTable::Info infoTable[] = {
+#define OPTION(X1, X2, ID, KIND, GROUP, ALIAS, X7, X8, X9, X10, X11, X12)      \
+  {X1, X2, X10,         X11,         OPT_##ID, llvm::opt::Option::KIND##Class, \
+   X9, X8, OPT_##GROUP, OPT_##ALIAS, X7,       X12},
+#include "Options.inc"
+#undef OPTION
+};
+
+class DllOptTable : public llvm::opt::OptTable {
+public:
+  DllOptTable() : OptTable(infoTable, false) {}
+};
+
+} // namespace
+
+std::vector<std::unique_ptr<MemoryBuffer>> OwningMBs;
+
+// Opens a file. Path has to be resolved already.
+// Newly created memory buffers are owned by this driver.
+MemoryBufferRef openFile(StringRef Path) {
+  ErrorOr<std::unique_ptr<llvm::MemoryBuffer>> MB = MemoryBuffer::getFile(Path);
+
+  if (std::error_code EC = MB.getError())
+    llvm::errs() << "fail openFile: " << EC.message() << "\n";
+
+  MemoryBufferRef MBRef = MB.get()->getMemBufferRef();
+  OwningMBs.push_back(std::move(MB.get())); // take ownership
+  return MBRef;
+}
+
+static MachineTypes getEmulation(StringRef S) {
+  return StringSwitch<MachineTypes>(S)
+      .Case("i386", IMAGE_FILE_MACHINE_I386)
+      .Case("i386:x86-64", IMAGE_FILE_MACHINE_AMD64)
+      .Case("arm", IMAGE_FILE_MACHINE_ARMNT)
+      .Default(IMAGE_FILE_MACHINE_UNKNOWN);
+}
+
+static std::string getImplibPath(std::string Path) {
+  SmallString<128> Out = StringRef("lib");
+  Out.append(Path);
+  sys::path::replace_extension(Out, ".a");
+  return Out.str();
+}
+
+int llvm::dlltoolDriverMain(llvm::ArrayRef<const char *> ArgsArr) {
+  DllOptTable Table;
+  unsigned MissingIndex;
+  unsigned MissingCount;
+  llvm::opt::InputArgList Args =
+      Table.ParseArgs(ArgsArr.slice(1), MissingIndex, MissingCount);
+  if (MissingCount) {
+    llvm::errs() << Args.getArgString(MissingIndex) << ": missing argument\n";
+    return 1;
+  }
+
+  // Handle when no input or output is specified
+  if (Args.hasArgNoClaim(OPT_INPUT) ||
+      (!Args.hasArgNoClaim(OPT_d) && !Args.hasArgNoClaim(OPT_l))) {
+    Table.PrintHelp(outs(), ArgsArr[0], "dlltool", false);
+    llvm::outs() << "\nTARGETS: i386, i386:x86-64, arm\n";
+    return 1;
+  }
+
+  if (!Args.hasArgNoClaim(OPT_m) && Args.hasArgNoClaim(OPT_d)) {
+    llvm::errs() << "error: no target machine specified\n"
+                 << "supported targets: i386, i386:x86-64, arm\n";
+    return 1;
+  }
+
+  for (auto *Arg : Args.filtered(OPT_UNKNOWN))
+    llvm::errs() << "ignoring unknown argument: " << Arg->getSpelling() << "\n";
+
+  MemoryBufferRef MB;
+  if (auto *Arg = Args.getLastArg(OPT_d))
+    MB = openFile(Arg->getValue());
+
+  if (!MB.getBufferSize()) {
+    llvm::errs() << "definition file empty\n";
+    return 1;
+  }
+
+  COFF::MachineTypes Machine = IMAGE_FILE_MACHINE_UNKNOWN;
+  if (auto *Arg = Args.getLastArg(OPT_m))
+    Machine = getEmulation(Arg->getValue());
+
+  if (Machine == IMAGE_FILE_MACHINE_UNKNOWN) {
+    llvm::errs() << "unknown target\n";
+    return 1;
+  }
+
+  Expected<COFFModuleDefinition> Def =
+      parseCOFFModuleDefinition(MB, Machine, true);
+
+  if (!Def) {
+    llvm::errs() << "error parsing definition\n"
+                 << errorToErrorCode(Def.takeError()).message();
+    return 1;
+  }
+
+  // Do this after the parser because parseCOFFModuleDefinition sets OutputFile.
+  if (auto *Arg = Args.getLastArg(OPT_D))
+    Def->OutputFile = Arg->getValue();
+
+  if (Def->OutputFile.empty()) {
+    llvm::errs() << "no output file specified\n";
+    return 1;
+  }
+
+  std::string Path = Args.getLastArgValue(OPT_l);
+  if (Path.empty())
+    Path = getImplibPath(Def->OutputFile);
+
+  if (writeImportLibrary(Def->OutputFile, Path, Def->Exports, Machine))
+    return 1;
+  return 0;
+}

llvm/trunk/lib/ToolDrivers/llvm-dlltool/LLVMBuild.txt

+;===- ./lib/ToolDrivers/llvm-dlltool/LLVMBuild.txt -------------*- Conf -*--===;
+;
+;                     The LLVM Compiler Infrastructure
+;
+; This file is distributed under the University of Illinois Open Source
+; License. See LICENSE.TXT for details.
+;
+;===------------------------------------------------------------------------===;
+;
+; This is an LLVMBuild description file for the components in this subdirectory.
+;
+; For more information on the LLVMBuild system, please see:
+;
+;   http://llvm.org/docs/LLVMBuild.html
+;
+;===------------------------------------------------------------------------===;
+
+[component_0]
+type = Library
+name = DlltoolDriver
+parent = Libraries
+required_libraries = Object Option Support

llvm/trunk/lib/ToolDrivers/llvm-dlltool/Options.td

+include "llvm/Option/OptParser.td"
+
+def m: JoinedOrSeparate<["-"], "m">, HelpText<"Set target machine">;
+def m_long : JoinedOrSeparate<["--"], "machine">, Alias<m>;
+
+def l: JoinedOrSeparate<["-"], "l">, HelpText<"Generate an import lib">;
+def l_long : JoinedOrSeparate<["--"], "output-lib">, Alias<l>;
+
+def D: JoinedOrSeparate<["-"], "D">, HelpText<"Specify the input DLL Name">;
+def D_long : JoinedOrSeparate<["--"], "dllname">, Alias<D>;
+
+def d: JoinedOrSeparate<["-"], "d">, HelpText<"Input .def File">;
+def d_long : JoinedOrSeparate<["--"], "input-def">, Alias<d>;
+
+//==============================================================================
+// The flags below do nothing. They are defined only for dlltool compatibility.
+//==============================================================================
+
+def k: Flag<["-"], "k">, HelpText<"Kill @n Symbol from export">;
+def k_alias: Flag<["--"], "kill-at">, Alias<k>;
+
+def S: JoinedOrSeparate<["-"], "S">, HelpText<"Assembler">;
+def S_alias: JoinedOrSeparate<["--"], "as">, Alias<S>;
+
+def f: JoinedOrSeparate<["-"], "f">, HelpText<"Assembler Flags">;
+def f_alias: JoinedOrSeparate<["--"], "as-flags">, Alias<f>;

llvm/trunk/test/DllTool/coff-exports.def

+; RUN: llvm-dlltool -m i386:x86-64 --input-def %s --output-lib %t.a
+; RUN: llvm-readobj -coff-exports %t.a | FileCheck %s
+
+LIBRARY test.dll
+EXPORTS
+TestFunction
+
+; CHECK: File: test.dll
+; CHECK: Format: COFF-import-file
+; CHECK: Type: code
+; CHECK: Name type: name
+; CHECK: Symbol: __imp_TestFunction
+; CHECK: Symbol: TestFunction

llvm/trunk/test/DllTool/coff-weak-exports.def

+; RUN: llvm-dlltool -m i386:x86-64 --input-def %s --output-lib %t.a
+; RUN: llvm-readobj -coff-exports %t.a | FileCheck %s
+
+LIBRARY test.dll
+EXPORTS
+TestFunction==AltTestFunction
+
+; CHECK: File: test.dll
+; CHECK: Format: COFF-x86-64
+; CHECK: Arch: x86_64
+; CHECK: AddressSize: 64bit
+; CHECK: File: test.dll
+; CHECK: Format: COFF-x86-64
+; CHECK: Arch: x86_64
+; CHECK: AddressSize: 64bit
+; CHECK: File: test.dll
+; CHECK: Format: COFF-x86-64
+; CHECK: Arch: x86_64
+; CHECK: AddressSize: 64bit

llvm/trunk/test/DllTool/lit.local.cfg

+config.suffixes = ['.def']

llvm/trunk/tools/llvm-ar/CMakeLists.txt

 set(LLVM_LINK_COMPONENTS
   ${LLVM_TARGETS_TO_BUILD}
   Core
+  DlltoolDriver
   LibDriver
   Object
   Support
   )
 
 add_llvm_tool(llvm-ar
   llvm-ar.cpp
 
   DEPENDS
   intrinsics_gen
   )
 
 add_llvm_tool_symlink(llvm-ranlib llvm-ar)
 add_llvm_tool_symlink(llvm-lib llvm-ar)
+add_llvm_tool_symlink(llvm-dlltool llvm-ar)

llvm/trunk/tools/llvm-ar/llvm-ar.cpp

 // bitcode or other files.
 //
 //===----------------------------------------------------------------------===//
 
 #include "llvm/ADT/StringSwitch.h"
 #include "llvm/ADT/Triple.h"
 #include "llvm/IR/LLVMContext.h"
 #include "llvm/IR/Module.h"
+#include "llvm/ToolDrivers/llvm-dlltool/DlltoolDriver.h"
 #include "llvm/ToolDrivers/llvm-lib/LibDriver.h"
 #include "llvm/Object/Archive.h"
 #include "llvm/Object/ArchiveWriter.h"
 #include "llvm/Object/MachO.h"
 #include "llvm/Object/ObjectFile.h"
 #include "llvm/Support/Chrono.h"
 #include "llvm/Support/CommandLine.h"
 #include "llvm/Support/Errc.h"
int main(int argc, char **argv) {
   PrettyStackTraceProgram X(argc, argv);
   llvm_shutdown_obj Y;  // Call llvm_shutdown() on exit.
 
   llvm::InitializeAllTargetInfos();
   llvm::InitializeAllTargetMCs();
   llvm::InitializeAllAsmParsers();
 
   StringRef Stem = sys::path::stem(ToolName);
+  if (Stem.find("dlltool") != StringRef::npos)
+    return dlltoolDriverMain(makeArrayRef(argv, argc));
+
   if (Stem.find("ranlib") == StringRef::npos &&
       Stem.find("lib") != StringRef::npos)
     return libDriverMain(makeArrayRef(argv, argc));
 
   // Have the command line options parsed and handle things
   // like --help and --version.
   cl::ParseCommandLineOptions(argc, argv,
     "LLVM Archiver (llvm-ar)\n\n"
     "  This program archives bitcode files into single libraries\n"
   );
 
   if (Stem.find("ranlib") != StringRef::npos)
     return ranlib_main();
   if (Stem.find("ar") != StringRef::npos)
     return ar_main();
-  fail("Not ranlib, ar or lib!");
+  fail("Not ranlib, ar, lib or dlltool!");
 }