[LLD] [COFF] Add support for GNU binutils import libraries

[mstorsjo]

Summary

GNU binutils import libraries aren't the same kind of short import libraries as link.exe and LLD produce, but are a plain static library containing .idata section chunks. MSVC link.exe can successfully link to them.

In order for imports from GNU import libraries to mix properly with the normal import chunks, the chunks from the existing mechanism needs to be added into named sections like .idata$2.

These GNU import libraries consist of one header object, a number of object files, one for each imported function/variable, and one trailer. Within the import libraries, the object files are ordered alphabetically in this order. The chunks stemming from these libraries have to be grouped by what library they originate from and sorted, to make sure the section chunks for headers and trailers for the lists are ordered as intended. This is done on all sections named .idata$*, before adding the synthesized chunks to them.

[ruiu]

Import library files are tricky, and I don't fully understand how it works in the MSVC linker. It seems that because import libraries contain sections such as .idata$2, .idata$3, etc., they would be "naturally" processed and sorted in the MSVC linker, and as a result a correct output would be created. But I couldn't figure that out. So, lld implements a special logic for the import library.

I have two questions:

1. Do you think you can figure out what MSVC does for the import libary?
2. Why does GNU binutils create an import library that is different from MSVC's?

[compnerd]

@ruiu I'm not sure that I understand the first question. Might be easier to discuss this on IRC or at the social.

As to why binutils generates different import libraries, is because a long time ago, import libraries used to be complete archives with object files. This was larger but simpler. They based the behaviour on that, but added some extensions to actually be able to sort the sections correctly (which in my experience is still insufficient as there have been cases where it would inject the null terminator too early, or swap things earlier). To make things worse, they actually don't get the ordering correct with the short import library format (which is what MSVC currently generates). In order to work with binutils, you need to generate the binutils specific format of the import library.

[mstorsjo]

In D38513#887477, @ruiu wrote:

Import library files are tricky, and I don't fully understand how it works in the MSVC linker. It seems that because import libraries contain sections such as .idata$2, .idata$3, etc., they would be "naturally" processed and sorted in the MSVC linker, and as a result a correct output would be created. But I couldn't figure that out. So, lld implements a special logic for the import library.

Ok, thanks for the honest comment on this part!

I have two questions:

1. Do you think you can figure out what MSVC does for the import libary?

I guess I'd have to try to reverse-engineer the logic somehow, alternatively try to look at what GNU binutils does in case that gives any clues.

This whole patch turned out to be a much larger task than I had anticipated, so I posted it early in case anyone of you would have had good insights on how to implement it. I'll probably continue with this with a bit lower priority and focus on other easier features inbetween though.

[smeenai]

Could you elaborate on "the first object file in the static import library to be included is the function itself"? Is the function in question the linker-provided import thunk, or something else?

[mstorsjo]

In D38513#895246, @smeenai wrote:

Could you elaborate on "the first object file in the static import library to be included is the function itself"? Is the function in question the linker-provided import thunk, or something else?

It's the import library provided import thunk.

To elaborate:
Given an import library for a dll testlib.dll, it contains a number of object files, d000000.o, d000001.o etc.

The first one, d000000.o, contains (among others) the symbols __IMPORT_DESCRIPTOR_testlib and __head_testlib_dll, which among others contains references to local zero bytes sections .idata$4 and .idata$5, intended to get a pointer to the start of the IAT for this DLL, and a reference to _testlib_dll_iname.

A later one, d000010.o in my test case, contains the symbols func and __imp_func, and contains 4 bytes of data for .idata$4 and .idata$5 (filling in one entry in the IAT), and a reference to __head_testlib_dll.

Finally, the last one in the library, d000011.o in my test, contains _testlib_dll_iname and 4 null byte terminators for .idata$4 and .idata$5.

Now while linking the module that might reference this import library, initially no object files from the import library are included. When encountering an undefined reference to func or __imp_func, it will pull in d000010.o which defines those. This object has an undefined reference to __head_testlib_dll which pulls in d000000.o, which in turn has got an undefined reference to _testlib_dll_iname which pulls in d000011.o.

This means that the object files, ordered by the order they are referenced (as I think lld does it right now?) is d000010.o, d000000.o and d000011.o. This means that the .idata$5 section group will be: [func entry 4 bytes] [0 bytes section chunk, referenced from the import descriptor] [4 null bytes terminator]. This means that the import descriptor actually points at the terminator. And if any other functions from the same import library were to be linked later, they would end up after the terminator. Or worse, if imports are done from multiple import libraries, they end up intermixed.

By enforcing the object files to be ordered alphabetically (or e.g. by their respective order in the import library archive), I make sure that the .idata$5 section group starts with [0 bytes section chunk referenced from the import descriptor], then [func entry 4 bytes] for every function referenced in the library, finishing with [4 null bytes terminator].

But this hack then turned out to break other things when linking msvcrt.lib (which contains a number of actual object files as well, in addition to the normal import library entries), exactly what it broke is yet undiagnosed.

[smeenai]

Thanks for the detailed explanation! I'm pretty familiar with how short import libraries work, but I hadn't looked into MinGW-style import libraries too much before.

I played around with them a bit, and judging from the output of running link with /verbose, link also pulls libraries in order of reference, so I'm not sure how the ordering works out there. My hypothesis is that link might special-case zero-sized sections, but I have no evidence to back that up yet. I'm planning to play around with custom object files with zero-sized sections constructed via yaml2obj to see if that hypothesis holds up.

[smeenai]

Also adding more Windows people in case they have any ideas.

[mstorsjo]

In D38513#896073, @smeenai wrote:

My hypothesis is that link might special-case zero-sized sections, but I have no evidence to back that up yet.

I don't think that's it. Consider linking two separate GNU import libraries in the same module; if just piling up section chunks into .idata$5 as they are referenced, you'd have an IAT with entries mixed from both. So some sort of grouping per import library is needed, but the exact conditions for it aren't really known. Perhaps only ordering .idata$* chunks like this, but not other ones?

One could try to read the source of GNU ld, but i'm not sure how easy it is to find the answer to the question by sifting through that source...

[smeenai]

In D38513#896150, @mstorsjo wrote:

In D38513#896073, @smeenai wrote:

My hypothesis is that link might special-case zero-sized sections, but I have no evidence to back that up yet.

I don't think that's it. Consider linking two separate GNU import libraries in the same module; if just piling up section chunks into .idata$5 as they are referenced, you'd have an IAT with entries mixed from both. So some sort of grouping per import library is needed, but the exact conditions for it aren't really known. Perhaps only ordering .idata$* chunks like this, but not other ones?

You're right; there appears to be no special-casing for zero-sized sections in general.

I don't think it's a library ordering thing either though. I extracted the individual object files out of the import library and linked against those rather the import library (in the order in which they would have been referenced, i.e. the object file containing the func entry was before the object file containing the zero byte section), and the imports of the linked image were still correct.

In general, link.exe is definitely doing some special-casing for .idata$* chunks though:

• They end up in the .rdata section in the final image (whereas LLD produces a separate .idata output section).
• The import address table chunks (.idata$5) end up at the start of the .rdata section, so regular grouping order isn't respected there.

[smeenai]

I looked into this some more. MinGW import libraries don't appear to contain the null directory table entry that's supposed to terminate the import directory table. This doesn't cause usually cause problems for link.exe, because you'll probably be linking against a non-MinGW import library, which provides the null entry (as .idata$3). Even if you're only linking against a MinGW import library, it looks like the loader doesn't require the entire terminating entry to be null; if either the Name RVA or the IAT RVA is null, the entry is considered to be a terminator. You can craft examples which cause problems though. All of the below discussion assumes 32-bit VS tools and MinGW, for reasons that will become apparent later.

Create a file fs.c:

__declspec(noreturn) void __stdcall ExitProcess(unsigned exitCode);

__declspec(dllexport) int f() { return 1; }
__declspec(dllexport) int g() { return 2; }
__declspec(dllexport) int h() { return 3; }
__declspec(dllexport) int i() { return 4; }
__declspec(dllexport noreturn) void ex(unsigned it) { ExitProcess(it); }

Create a library from it with MSVC:

cl /LD /MD /O1 fs.c

Create a file fsmain.c:

__declspec(dllimport) int f();
__declspec(dllimport) int g();
__declspec(dllimport) int h();
__declspec(dllimport) int i();
__declspec(dllimport noreturn) void ex();

void main() { ex(f() + g() + h() + i()); }

We're gonna wanna only link this with a single import library, so let's do that with the cl-generated import library first and convince ourselves that it works:

cl /O1 /Zl fsmain.c fs.lib /link /entry:main
fsmain
echo %ERRORLEVEL%
(outputs 10)

Now create a def file fs.def:

LIBRARY fs
EXPORTS
  f
  g
  h
  i
  ex

Create an import library with MinGW and see our executable fail to start properly when linked against that import library:

dlltool -d fs.def -l fs.lib
cl /O1 /Zl fsmain.c fs.lib /link /entry:main
fsmain
(you should get an "application was unable to start correctly" error)

What's happening here is that the ILT is placed immediately after the first (and only) import directory table entry, so the loader tries to read the ILT as if it were a directory entry, and we have enough ILT entries that both the Name RVA and IAT RVA fields are non-null, so the directory entry is considered non-null, and you get a crash from the malformed entry.

You can alter the number of exports in the def file (and adjust the expression in main accordingly) to see how the loader is behaving. Only exporting ex should work because the IAT RVA (but not the Name RVA) will end up being null. Exporting ex and f will crash because both fields will end up non-null. Exporting ex, f, and g will work because the Name RVA (but not the IAT RVA) will end up being null. Exporting ex, f, g, and h will work because the IAT RVA (but not the Name RVA) will end up being null.

This is also why it's important to use 32-bit tools. With 64-bit tools, unless you have a huge import library, the upper 4 bytes of each ILT entry will be 0, so you'll always end up with a Name RVA of 0, and the problem will be masked.

[mstorsjo]

FWIW, I noticed this detail when linking to a GNU import library using link.exe; if linking with -verbose, link.exe prints a list of object files at the end:

testlib.lib(d000004.o)
testlib.lib(d000005.o)
testlib.lib(d000006.o)
testlib.lib(d000000.o)
testlib.lib(d000011.o)

These clearly are grouped by import library, which confirms one detail of it. But this also is more or less the same order as lld would end up linking them right now, and that doesn't work for lld, since d000000.o would have a reference to the end of the IAT instead of the start.

[smeenai]

The grouping by import library is definitely important for both Microsoft-style and binutils-style import libraries. I'm pretty sure the second issue (how to get the correct ILT and IAT start addresses inside d000000.o) is solved by them special casing imports handling, as I mentioned before.

In D38513#896182, @smeenai wrote:

In general, link.exe is definitely doing some special-casing for .idata$* chunks though:

• They end up in the .rdata section in the final image (whereas LLD produces a separate .idata output section).
• The import address table chunks (.idata$5) end up at the start of the .rdata section, so regular grouping order isn't respected there.

[mstorsjo]

Finally updated this patch to a pretty solid, not all too messy implementation (or much less messy than before at least).

With this in place, lld can finally use a normal MinGW sysroot built with GCC and GNU binutils, making lld a proper drop-in replacement for ld.bfd in these environments. (There still are cases with more advanced C++ compiled by GCC which doesn't work properly with lld though.)

[ruiu]

Wow, this patch is almost one year old.

COFF/Writer.cpp

563 ↗

(On Diff #165934)

Perhaps this is a good place to explain what is GNU import libraries? We shouldn't assume that code readers know what that is when reading this code.

820 ↗

(On Diff #165934)

Flip the condition and continue early.

[mstorsjo]

In D38513#1239919, @ruiu wrote:

Wow, this patch is almost one year old.

Yup, I put it on hold last year as it seemed complex, and I had more important things to focus on first. Now after reading a bit of binutils source, I think I understand how it's supposed to be handled (and my previous attempt didn't seem to be too far off). And after looking at the code a second time (with more experience with LLD), I managed to clean it up more so it's very lean and pretty nonintrusive now, and no longer with an RFC tag, but this should be good to go now.

COFF/Writer.cpp

563 ↗

(On Diff #165934)

Ok, I'll expand the comment.

820 ↗

(On Diff #165934)

Ok, will do.

[mstorsjo]

Expanded a code comment to explain what it does and why, changed a condition to use early continue as suggested.

[ruiu]

Yeah, I agree with you that this patch is lean. It actually looks pretty good.

COFF/Writer.cpp

378 ↗

(On Diff #166233)

nit: we omit const if it is obviously a const.

383 ↗

(On Diff #166233)

Can you assign Pair.first.first and Pair.first.second to local variables for the sake of documentation?

397 ↗

(On Diff #166233)

Ditto

413 ↗

(On Diff #166233)

toString is written for constructing a debug or log message, so it is better to avoid using it as part of the actual logic.

478 ↗

(On Diff #166233)

nit: AddChunk -> Add since the scope of this variable is very narrow.

636 ↗

(On Diff #166233)

I wonder if you should use assert. If this line is unreachable unless there's a bug in lld, you should use assert.

[zturner]

COFF/Writer.cpp

378 ↗

(On Diff #166233)

Do we? It doesn't hurt anything and prevents the user from modifying the value later. I don't see a good reason to omit it.

[mstorsjo]

In D38513#1240693, @ruiu wrote:

Yeah, I agree with you that this patch is lean. It actually looks pretty good.

Thanks! I'll update it according to your other comments, except for the one I'd prefer avoid changing here.

COFF/Writer.cpp

636 ↗

(On Diff #166233)

This check is just moved, so I'd avoid changing it within this patch.

I think it might theoretically be possible to trigger it with very pathological input, but I don't remember exactly right now...

[ruiu]

COFF/Writer.cpp

378 ↗

(On Diff #166233)

At least in many places in lld, we don't to keep it terse.

[mstorsjo]

Applied @ruiu's suggestions.

[ruiu]

COFF/Writer.cpp

383 ↗

(On Diff #166356)

StringRef is a small object and we usually copy them instead of creating references or pointers to them.

399 ↗

(On Diff #166356)

Ditto

428 ↗

(On Diff #166356)

This function got a bit too long. Do you think there's a way to refactor?

[mstorsjo]

Split two functions out of createSections, changed const StringRef& into plain StringRef.

[ruiu]

LGTM. Beautiful!

COFF/Writer.cpp

428 ↗

(On Diff #166511)

Add a blank line.

442 ↗

(On Diff #166511)

Add a blank line before a multiline comment.

450–451 ↗

(On Diff #166511)

Please write a function comment.

lld/trunk/COFF/DLL.h

 #include "Symbols.h"
 
 namespace lld {
 namespace coff {
 
 // Windows-specific.
 // IdataContents creates all chunks for the DLL import table.
 // You are supposed to call add() to add symbols and then
-// call getChunks() to get a list of chunks.
+// call create() to populate the chunk vectors.
 class IdataContents {
 public:
   void add(DefinedImportData *Sym) { Imports.push_back(Sym); }
   bool empty() { return Imports.empty(); }
-  std::vector<Chunk *> getChunks();
 
-  uint64_t getDirRVA() { return Dirs[0]->getRVA(); }
-  uint64_t getDirSize();
-  uint64_t getIATRVA() { return Addresses[0]->getRVA(); }
-  uint64_t getIATSize();
-
-private:
   void create();
 
   std::vector<DefinedImportData *> Imports;
   std::vector<Chunk *> Dirs;
   std::vector<Chunk *> Lookups;
   std::vector<Chunk *> Addresses;
   std::vector<Chunk *> Hints;
   std::vector<Chunk *> DLLNames;

lld/trunk/COFF/DLL.cpp

public:
   }
 
 private:
   size_t Size;
 };
 
 } // anonymous namespace
 
-uint64_t IdataContents::getDirSize() {
-  return Dirs.size() * sizeof(ImportDirectoryTableEntry);
-}
-
-uint64_t IdataContents::getIATSize() {
-  return Addresses.size() * ptrSize();
-}
-
-// Returns a list of .idata contents.
-// See Microsoft PE/COFF spec 5.4 for details.
-std::vector<Chunk *> IdataContents::getChunks() {
-  create();
-
-  // The loader assumes a specific order of data.
-  // Add each type in the correct order.
-  std::vector<Chunk *> V;
-  V.insert(V.end(), Dirs.begin(), Dirs.end());
-  V.insert(V.end(), Lookups.begin(), Lookups.end());
-  V.insert(V.end(), Addresses.begin(), Addresses.end());
-  V.insert(V.end(), Hints.begin(), Hints.end());
-  V.insert(V.end(), DLLNames.begin(), DLLNames.end());
-  return V;
-}
-
 void IdataContents::create() {
   std::vector<std::vector<DefinedImportData *>> V = binImports(Imports);
 
   // Create .idata contents for each DLL.
   for (std::vector<DefinedImportData *> &Syms : V) {
     // Create lookup and address tables. If they have external names,
     // we need to create HintName chunks to store the names.
     // If they don't (if they are import-by-ordinals), we store only

lld/trunk/COFF/Writer.cpp

 public:
   Writer() : Buffer(errorHandler().OutputBuffer) {}
   void run();
 
 private:
   void createSections();
   void createMiscChunks();
   void createImportTables();
+  void appendImportThunks();
+  void locateImportTables(
+      std::map<std::pair<StringRef, uint32_t>, std::vector<Chunk *>> &Map);
   void createExportTable();
   void mergeSections();
   void assignAddresses();
   void removeEmptySections();
   void createSymbolAndStringTable();
   void openFile(StringRef OutputPath);
   template <typename PEHeaderTy> void writeHeader();
   void createSEHTable();
private:
   uint32_t getSizeOfInitializedData();
   std::map<StringRef, std::vector<DefinedImportData *>> binImports();
 
   std::unique_ptr<FileOutputBuffer> &Buffer;
   std::vector<OutputSection *> OutputSections;
   std::vector<char> Strtab;
   std::vector<llvm::object::coff_symbol16> OutputSymtab;
   IdataContents Idata;
+  Chunk *ImportTableStart = nullptr;
+  uint64_t ImportTableSize = 0;
+  Chunk *IATStart = nullptr;
+  uint64_t IATSize = 0;
   DelayLoadContents DelayIdata;
   EdataContents Edata;
   bool SetNoSEHCharacteristic = false;
 
   DebugDirectoryChunk *DebugDirectory = nullptr;
   std::vector<Chunk *> DebugRecords;
   CVDebugRecordChunk *BuildId = nullptr;
   ArrayRef<uint8_t> SectionTable;

 
 } // namespace coff
 } // namespace lld
 
 // The main function of the writer.
 void Writer::run() {
   ScopedTimer T1(CodeLayoutTimer);
 
+  createImportTables();
   createSections();
   createMiscChunks();
-  createImportTables();
+  appendImportThunks();
   createExportTable();
   mergeSections();
   assignAddresses();
   removeEmptySections();
   setSectionPermissions();
   createSymbolAndStringTable();
 
   if (FileSize > UINT32_MAX)
static void sortBySectionOrder(std::vector<Chunk *> &Chunks) {
   };
 
   std::stable_sort(Chunks.begin(), Chunks.end(),
                    [=](const Chunk *A, const Chunk *B) {
                      return GetPriority(A) < GetPriority(B);
                    });
 }
 
+// Sort concrete section chunks from GNU import libraries.
+//
+// GNU binutils doesn't use short import files, but instead produces import
+// libraries that consist of object files, with section chunks for the .idata$*
+// sections. These are linked just as regular static libraries. Each import
+// library consists of one header object, one object file for every imported
+// symbol, and one trailer object. In order for the .idata tables/lists to
+// be formed correctly, the section chunks within each .idata$* section need
+// to be grouped by library, and sorted alphabetically within each library
+// (which makes sure the header comes first and the trailer last).
+static bool fixGnuImportChunks(
+    std::map<std::pair<StringRef, uint32_t>, std::vector<Chunk *>> &Map) {
+  uint32_t RDATA = IMAGE_SCN_CNT_INITIALIZED_DATA | IMAGE_SCN_MEM_READ;
+
+  // Make sure all .idata$* section chunks are mapped as RDATA in order to
+  // be sorted into the same sections as our own synthesized .idata chunks.
+  for (auto &Pair : Map) {
+    StringRef SectionName = Pair.first.first;
+    uint32_t OutChars = Pair.first.second;
+    if (!SectionName.startswith(".idata"))
+      continue;
+    if (OutChars == RDATA)
+      continue;
+    std::vector<Chunk *> &SrcVect = Pair.second;
+    std::vector<Chunk *> &DestVect = Map[{SectionName, RDATA}];
+    DestVect.insert(DestVect.end(), SrcVect.begin(), SrcVect.end());
+    SrcVect.clear();
+  }
+
+  bool HasIdata = false;
+  // Sort all .idata$* chunks, grouping chunks from the same library,
+  // with alphabetical ordering of the object fils within a library.
+  for (auto &Pair : Map) {
+    StringRef SectionName = Pair.first.first;
+    if (!SectionName.startswith(".idata"))
+      continue;
+
+    std::vector<Chunk *> &Chunks = Pair.second;
+    if (!Chunks.empty())
+      HasIdata = true;
+    std::stable_sort(Chunks.begin(), Chunks.end(), [&](Chunk *S, Chunk *T) {
+      SectionChunk *SC1 = dyn_cast_or_null<SectionChunk>(S);
+      SectionChunk *SC2 = dyn_cast_or_null<SectionChunk>(T);
+      if (!SC1 || !SC2) {
+        // if SC1, order them ascending. If SC2 or both null,
+        // S is not less than T.
+        return SC1 != nullptr;
+      }
+      // Make a string with "libraryname/objectfile" for sorting, achieving
+      // both grouping by library and sorting of objects within a library,
+      // at once.
+      std::string Key1 =
+          (SC1->File->ParentName + "/" + SC1->File->getName()).str();
+      std::string Key2 =
+          (SC2->File->ParentName + "/" + SC2->File->getName()).str();
+      return Key1 < Key2;
+    });
+  }
+  return HasIdata;
+}
+
+// Add generated idata chunks, for imported symbols and DLLs, and a
+// terminator in .idata$2.
+static void addSyntheticIdata(
+    IdataContents &Idata,
+    std::map<std::pair<StringRef, uint32_t>, std::vector<Chunk *>> &Map) {
+  uint32_t RDATA = IMAGE_SCN_CNT_INITIALIZED_DATA | IMAGE_SCN_MEM_READ;
+  Idata.create();
+
+  // Add the .idata content in the right section groups, to allow
+  // chunks from other linked in object files to be grouped together.
+  // See Microsoft PE/COFF spec 5.4 for details.
+  auto Add = [&](StringRef N, std::vector<Chunk *> &V) {
+    std::vector<Chunk *> &DestVect = Map[{N, RDATA}];
+    DestVect.insert(DestVect.end(), V.begin(), V.end());
+  };
+
+  // The loader assumes a specific order of data.
+  // Add each type in the correct order.
+  Add(".idata$2", Idata.Dirs);
+  Add(".idata$4", Idata.Lookups);
+  Add(".idata$5", Idata.Addresses);
+  Add(".idata$6", Idata.Hints);
+  Add(".idata$7", Idata.DLLNames);
+}
+
+// Locate the first Chunk and size of the import directory list and the
+// IAT.
+void Writer::locateImportTables(
+    std::map<std::pair<StringRef, uint32_t>, std::vector<Chunk *>> &Map) {
+  uint32_t RDATA = IMAGE_SCN_CNT_INITIALIZED_DATA | IMAGE_SCN_MEM_READ;
+  std::vector<Chunk *> &ImportTables = Map[{".idata$2", RDATA}];
+  if (!ImportTables.empty())
+    ImportTableStart = ImportTables.front();
+  for (Chunk *C : ImportTables)
+    ImportTableSize += C->getSize();
+
+  std::vector<Chunk *> &IAT = Map[{".idata$5", RDATA}];
+  if (!IAT.empty())
+    IATStart = IAT.front();
+  for (Chunk *C : IAT)
+    IATSize += C->getSize();
+}
+
 // Create output section objects and add them to OutputSections.
 void Writer::createSections() {
   // First, create the builtin sections.
   const uint32_t DATA = IMAGE_SCN_CNT_INITIALIZED_DATA;
   const uint32_t BSS = IMAGE_SCN_CNT_UNINITIALIZED_DATA;
   const uint32_t CODE = IMAGE_SCN_CNT_CODE;
   const uint32_t DISCARDABLE = IMAGE_SCN_MEM_DISCARDABLE;
   const uint32_t R = IMAGE_SCN_MEM_READ;
for (Chunk *C : Symtab->getChunks()) {
     if (SC && !SC->Live) {
       if (Config->Verbose)
         SC->printDiscardedMessage();
       continue;
     }
     Map[{C->getSectionName(), C->getOutputCharacteristics()}].push_back(C);
   }
 
+  // Even in non MinGW cases, we might need to link against GNU import
+  // libraries.
+  bool HasIdata = fixGnuImportChunks(Map);
+  if (!Idata.empty())
+    HasIdata = true;
+
+  if (HasIdata)
+    addSyntheticIdata(Idata, Map);
+
   // Process an /order option.
   if (!Config->Order.empty())
     for (auto &Pair : Map)
       sortBySectionOrder(Pair.second);
 
+  if (HasIdata)
+    locateImportTables(Map);
+
   // Then create an OutputSection for each section.
   // '$' and all following characters in input section names are
   // discarded when determining output section. So, .text$foo
   // contributes to .text, for example. See PE/COFF spec 3.2.
   for (auto &Pair : Map) {
     StringRef Name = getOutputSectionName(Pair.first.first);
     uint32_t OutChars = Pair.first.second;
 
void Writer::createMiscChunks() {
   }
 }
 
 // Create .idata section for the DLL-imported symbol table.
 // The format of this section is inherently Windows-specific.
 // IdataContents class abstracted away the details for us,
 // so we just let it create chunks and add them to the section.
 void Writer::createImportTables() {
-  if (ImportFile::Instances.empty())
-    return;
-
   // Initialize DLLOrder so that import entries are ordered in
   // the same order as in the command line. (That affects DLL
   // initialization order, and this ordering is MSVC-compatible.)
   for (ImportFile *File : ImportFile::Instances) {
     if (!File->Live)
       continue;
 
     std::string DLL = StringRef(File->DLLName).lower();
     if (Config->DLLOrder.count(DLL) == 0)
       Config->DLLOrder[DLL] = Config->DLLOrder.size();
 
-    if (File->ThunkSym) {
-      if (!isa<DefinedImportThunk>(File->ThunkSym))
-        fatal(toString(*File->ThunkSym) + " was replaced");
-      DefinedImportThunk *Thunk = cast<DefinedImportThunk>(File->ThunkSym);
-      if (File->ThunkLive)
-        TextSec->addChunk(Thunk->getChunk());
-    }
-
     if (File->ImpSym && !isa<DefinedImportData>(File->ImpSym))
       fatal(toString(*File->ImpSym) + " was replaced");
     DefinedImportData *ImpSym = cast_or_null<DefinedImportData>(File->ImpSym);
     if (Config->DelayLoads.count(StringRef(File->DLLName).lower())) {
       if (!File->ThunkSym)
         fatal("cannot delay-load " + toString(File) +
               " due to import of data: " + toString(*ImpSym));
       DelayIdata.add(ImpSym);
     } else {
       Idata.add(ImpSym);
     }
   }
+}
 
-  if (!Idata.empty())
-    for (Chunk *C : Idata.getChunks())
-      IdataSec->addChunk(C);
+void Writer::appendImportThunks() {
+  if (ImportFile::Instances.empty())
+    return;
+
+  for (ImportFile *File : ImportFile::Instances) {
+    if (!File->Live)
+      continue;
+
+    if (!File->ThunkSym)
+      continue;
+
+    if (!isa<DefinedImportThunk>(File->ThunkSym))
+      fatal(toString(*File->ThunkSym) + " was replaced");
+    DefinedImportThunk *Thunk = cast<DefinedImportThunk>(File->ThunkSym);
+    if (File->ThunkLive)
+      TextSec->addChunk(Thunk->getChunk());
+  }
 
   if (!DelayIdata.empty()) {
     Defined *Helper = cast<Defined>(Config->DelayLoadHelper);
     DelayIdata.create(Helper);
     for (Chunk *C : DelayIdata.getChunks())
       DidatSec->addChunk(C);
     for (Chunk *C : DelayIdata.getDataChunks())
       DataSec->addChunk(C);
template <typename PEHeaderTy> void Writer::writeHeader() {
 
   // Write data directory
   auto *Dir = reinterpret_cast<data_directory *>(Buf);
   Buf += sizeof(*Dir) * NumberfOfDataDirectory;
   if (!Config->Exports.empty()) {
     Dir[EXPORT_TABLE].RelativeVirtualAddress = Edata.getRVA();
     Dir[EXPORT_TABLE].Size = Edata.getSize();
   }
-  if (!Idata.empty()) {
-    Dir[IMPORT_TABLE].RelativeVirtualAddress = Idata.getDirRVA();
-    Dir[IMPORT_TABLE].Size = Idata.getDirSize();
-    Dir[IAT].RelativeVirtualAddress = Idata.getIATRVA();
-    Dir[IAT].Size = Idata.getIATSize();
+  if (ImportTableStart) {
+    Dir[IMPORT_TABLE].RelativeVirtualAddress = ImportTableStart->getRVA();
+    Dir[IMPORT_TABLE].Size = ImportTableSize;
+  }
+  if (IATStart) {
+    Dir[IAT].RelativeVirtualAddress = IATStart->getRVA();
+    Dir[IAT].Size = IATSize;
   }
   if (RsrcSec->getVirtualSize()) {
     Dir[RESOURCE_TABLE].RelativeVirtualAddress = RsrcSec->getRVA();
     Dir[RESOURCE_TABLE].Size = RsrcSec->getVirtualSize();
   }
   if (FirstPdata) {
     Dir[EXCEPTION_TABLE].RelativeVirtualAddress = FirstPdata->getRVA();
     Dir[EXCEPTION_TABLE].Size =

lld/trunk/test/COFF/Inputs/gnu-implib-func.s

+        .text
+        .global         func
+        .global         __imp_func
+func:
+        jmp             *__imp_func
+
+        # The data that is emitted into .idata$7 here is isn't needed for
+        # the import data structures, but we need to emit something which
+        # produces a relocation against _head_test_lib, to pull in the
+        # header and trailer objects.
+
+        .section        .idata$7
+        .rva            _head_test_lib
+
+        .section        .idata$5
+__imp_func:
+        .rva            .Lhint_name
+        .long           0
+
+        .section        .idata$4
+        .rva            .Lhint_name
+        .long           0
+
+        .section        .idata$6
+.Lhint_name:
+        .short          0
+        .asciz          "func"

lld/trunk/test/COFF/Inputs/gnu-implib-head.s

+        .section        .idata$2
+        .global         _head_test_lib
+_head_test_lib:
+        .rva            hname
+        .long           0
+        .long           0
+        .rva            __test_lib_iname
+        .rva            fthunk
+
+        .section        .idata$5
+fthunk:
+        .section        .idata$4
+hname:

lld/trunk/test/COFF/Inputs/gnu-implib-tail.s

+        .section        .idata$4
+        .long           0
+        .long           0
+        .section        .idata$5
+        .long           0
+        .long           0
+
+        .section        .idata$7
+        .global         __test_lib_iname
+__test_lib_iname:
+       .asciz           "foo.dll"

lld/trunk/test/COFF/imports-gnu-only.s

+# REQUIRES: x86
+#
+# RUN: llvm-mc -triple=x86_64-windows-gnu %p/Inputs/gnu-implib-head.s -filetype=obj -o %t-dabcdh.o
+# RUN: llvm-mc -triple=x86_64-windows-gnu %p/Inputs/gnu-implib-func.s -filetype=obj -o %t-dabcds00000.o
+# RUN: llvm-mc -triple=x86_64-windows-gnu %p/Inputs/gnu-implib-tail.s -filetype=obj -o %t-dabcdt.o
+# RUN: rm -f %t-implib.a
+# RUN: llvm-ar rcs %t-implib.a %t-dabcdh.o %t-dabcds00000.o %t-dabcdt.o
+# RUN: llvm-mc -triple=x86_64-windows-gnu %s -filetype=obj -o %t.obj
+# RUN: lld-link -out:%t.exe -entry:main -subsystem:console \
+# RUN:   %t.obj %t-implib.a
+# RUN: llvm-objdump -s %t.exe | FileCheck -check-prefix=DATA %s
+
+        .text
+        .global main
+main:
+        call func
+        ret
+
+# Check that the linker inserted the null terminating import descriptor,
+# even if there were no normal import libraries, only gnu ones.
+
+# DATA: Contents of section .rdata:
+# First import descriptor
+# DATA:  140002000 28200000 00000000 00000000 53200000
+# Last word from first import descriptor, null terminator descriptor
+# DATA:  140002010 38200000 00000000 00000000 00000000
+# Null terminator descriptor and import lookup table.
+# DATA:  140002020 00000000 00000000 48200000 00000000

lld/trunk/test/COFF/imports-gnu.test

+# REQUIRES: x86
+# Verify that the lld can link to GNU import libs.
+#
+# RUN: llvm-mc -triple=x86_64-windows-gnu %p/Inputs/gnu-implib-head.s -filetype=obj -o %t-dabcdh.o
+# RUN: llvm-mc -triple=x86_64-windows-gnu %p/Inputs/gnu-implib-func.s -filetype=obj -o %t-dabcds00000.o
+# RUN: llvm-mc -triple=x86_64-windows-gnu %p/Inputs/gnu-implib-tail.s -filetype=obj -o %t-dabcdt.o
+# RUN: rm -f %t-implib.a
+# RUN: llvm-ar rcs %t-implib.a %t-dabcdh.o %t-dabcds00000.o %t-dabcdt.o
+# Not linking with -lldmingw; one can link to GNU import libs even if not targeting MinGW.
+# RUN: lld-link -out:%t.exe -entry:main -subsystem:console \
+# RUN:   %p/Inputs/hello64.obj %p/Inputs/std64.lib %t-implib.a -include:func
+# RUN: llvm-readobj -coff-imports %t.exe | FileCheck -check-prefix=IMPORT %s
+
+# Check that import entries from both libraries show up.
+
+IMPORT:      Import {
+IMPORT-NEXT:   Name: foo.dll
+IMPORT-NEXT:   ImportLookupTableRVA:
+IMPORT-NEXT:   ImportAddressTableRVA:
+IMPORT-NEXT:   Symbol: func (0)
+IMPORT-NEXT: }
+IMPORT-NEXT: Import {
+IMPORT-NEXT:   Name: std64.dll
+IMPORT-NEXT:   ImportLookupTableRVA:
+IMPORT-NEXT:   ImportAddressTableRVA:
+IMPORT-NEXT:   Symbol: ExitProcess (0)
+IMPORT-NEXT:   Symbol:  (50)
+IMPORT-NEXT:   Symbol: MessageBoxA (1)
+IMPORT-NEXT: }

lld/trunk/test/COFF/no-idata.s

+# REQUIRES: x86
+#
+# RUN: llvm-mc -triple=x86_64-windows-gnu %s -filetype=obj -o %t.obj
+# RUN: lld-link -out:%t.exe -entry:main -subsystem:console %t.obj
+# RUN: llvm-objdump -s %t.exe | FileCheck -check-prefix=DUMP %s
+# RUN: llvm-readobj -file-headers %t.exe | FileCheck -check-prefix=DIRECTORY %s
+
+        .text
+        .global main
+main:
+        ret
+
+# Check that no .idata (.rdata) entries were added, no null terminator
+# for the import descriptor table.
+# DUMP: Contents of section .text:
+# DUMP-NEXT: 140001000 c3
+# DUMP-NEXT-EMPTY:
+
+# DIRECTORY: ImportTableRVA: 0x0
+# DIRECTORY: ImportTableSize: 0x0