[LLD] [COFF] Allow using custom .edata from input object files

[mstorsjo]

Summary

This is used by Wine for manually crafting export tables.

If the input object contains .edata sections, GNU ld references them in the export directory instead of synthesizing an export table using either export directives or the normal auto export mechanism. (AFAIK, historically, way way back, GNU ld didn't support synthesizing the export table - one was supposed to generate it using dlltool and link it in instead.)

If faced with --out-implib and --output-def, GNU ld still populates those output files with the same export info as it would have generated otherwise, disregarding the input .edata. As this isn't an intended usage combination, I'm not adding checks for that in tests.

[ruiu]

Is this due to the historical reason that wine is creating an .edata section instead of -export directive? I feel like this feature is obscure, and if there's a way to avoid adding this, I'd like to try that first. If GNU ld is fine with -export directive now, and wine's source code can be updated to use the directive (which is I believe a good refactoring as it would simplify the code), can wine's code be updated?

[mstorsjo]

In D65903#1620514, @ruiu wrote:

Is this due to the historical reason that wine is creating an .edata section instead of -export directive? I feel like this feature is obscure, and if there's a way to avoid adding this, I'd like to try that first. If GNU ld is fine with -export directive now, and wine's source code can be updated to use the directive (which is I believe a good refactoring as it would simplify the code), can wine's code be updated?

Wine supports building modules both as ELF and PE, and in ELF they generate the .edata manually, so they still need to keep that around, so using export directives isn't a simplification, it's yet another extra codepath.

And if I understood @jacek correctly, there was some case with relay stubs that don't map trivially to export directives.

While this is a very obscure feature, the implementation of it in lld turned out to be very straightforward (kind of like .idata chunks from long import libs, but much simpler).

[ruiu]

I agree with you that this implementation is straightforward and simple, and I'm fine to add this if changing wine is not easy.

Could you add a little bit of comment to the code to explain what this is for?

Do you think it makes sense to show a warning if both -export and .edata are used?

[mstorsjo]

In D65903#1620642, @ruiu wrote:

I agree with you that this implementation is straightforward and simple, and I'm fine to add this if changing wine is not easy.

We discussed it with the Wine maintainer already, and he preferred this approach over using export directives.

Could you add a little bit of comment to the code to explain what this is for?

Sure, I can do that.

Do you think it makes sense to show a warning if both -export and .edata are used?

That would probably make sense. Unfortunately, I think we only know whether we have custom .edata at the end, after we've autoexported all symbols (if no export directives were found), but I can store a bool to distinguish between the cases.

[mstorsjo]

Added a comment, and a warning if there's both .edata sections and explicit exports. Adjusted the testcase to test the warning. Adjusted how the edata size is calculated, to account for potential alignment padding.

[mstorsjo]

Ping @ruiu

[ruiu]

COFF/Writer.cpp

1026–1029

Do you have to use these two variables?

1372–1374

Looks like you can check if !edataSec->chunks.empty() here and use edataSec->chunks.front() and back() in the following lines.

[mstorsjo]

COFF/Writer.cpp

1372–1374

No, I can't. Edata ends up merged into rdata, so at this point, edataSec is empty.

[ruiu]

LGTM

COFF/Writer.cpp

private:
 
   std::unique_ptr<FileOutputBuffer> &buffer;
   std::map<PartialSectionKey, PartialSection *> partialSections;
   std::vector<char> strtab;
   std::vector<llvm::object::coff_symbol16> outputSymtab;
   IdataContents idata;
   Chunk *importTableStart = nullptr;
   uint64_t importTableSize = 0;
+  Chunk *edataStart = nullptr;
+  uint64_t edataSize = 0;
   Chunk *iatStart = nullptr;
   uint64_t iatSize = 0;
   DelayLoadContents delayIdata;
   EdataContents edata;
   bool setNoSEHCharacteristic = false;
 
   DebugDirectoryChunk *debugDirectory = nullptr;
   std::vector<Chunk *> debugRecords;
for (Chunk *c : symtab->getChunks()) {
     if (shouldStripSectionSuffix(sc, name))
       name = name.split('$').first;
     PartialSection *pSec = createPartialSection(name,
                                                 c->getOutputCharacteristics());
     pSec->chunks.push_back(c);
   }
 
   fixPartialSectionChars(".rsrc", data | r);
+  fixPartialSectionChars(".edata", data | r);
   // Even in non MinGW cases, we might need to link against GNU import
   // libraries.
   bool hasIdata = fixGnuImportChunks();
   if (!idata.empty())
     hasIdata = true;
 
   if (hasIdata)
     addSyntheticIdata();
if (!delayIdata.empty()) {
     for (Chunk *c : delayIdata.getDataChunks())
       dataSec->addChunk(c);
     for (Chunk *c : delayIdata.getCodeChunks())
       textSec->addChunk(c);
   }
 }
 
 void Writer::createExportTable() {
-  if (config->exports.empty())
-    return;
+  if (!config->exports.empty() && edataSec->chunks.empty()) {
     for (Chunk *c : edata.chunks)
       edataSec->addChunk(c);
   }
+  if (!edataSec->chunks.empty()) {
+    edataStart = edataSec->chunks.front();
+    for (Chunk *c : edataSec->chunks)
+      edataSize += c->getSize();
+  }
+}
 
 void Writer::removeUnusedSections() {
   // Remove sections that we can be sure won't get content, to avoid
   // allocating space for their section headers.
   auto isUnused = [this](OutputSection *s) {
     if (s == relocSec)
       return false; // This section is populated later.
     // MergeChunks have zero size at this point, as their size is finalized
     // later. Only remove sections that have no Chunks at all.
     return s->chunks.empty();
   };
if (textSec->getVirtualSize()) {
     pe->BaseOfCode = textSec->getRVA();
     pe->SizeOfCode = textSec->getRawSize();
   }
   pe->SizeOfInitializedData = getSizeOfInitializedData();
 
   // Write data directory
   auto *dir = reinterpret_cast<data_directory *>(buf);
   buf += sizeof(*dir) * numberOfDataDirectory;
-  if (!config->exports.empty()) {
-    dir[EXPORT_TABLE].RelativeVirtualAddress = edata.getRVA();
-    dir[EXPORT_TABLE].Size = edata.getSize();
+  if (edataSize) {
+    dir[EXPORT_TABLE].RelativeVirtualAddress = edataStart->getRVA();
+    dir[EXPORT_TABLE].Size = edataSize;
   }
   if (importTableStart) {
     dir[IMPORT_TABLE].RelativeVirtualAddress = importTableStart->getRVA();
     dir[IMPORT_TABLE].Size = importTableSize;
   }
   if (iatStart) {
     dir[IAT].RelativeVirtualAddress = iatStart->getRVA();
     dir[IAT].Size = iatSize;

test/COFF/edata.s

+# REQUIRES: x86
+
+# RUN: llvm-mc -filetype=obj -triple=x86_64-mingw32 -o %t.o %s
+# RUN: lld-link -lldmingw -dll -out:%t.dll %t.o -entry:__ImageBase
+# RUN: llvm-readobj --coff-exports %t.dll | FileCheck %s
+
+# Check that the export table contains the manually crafted content
+# instead of the linker generated exports.
+
+# CHECK:      Export {
+# CHECK-NEXT:   Ordinal: 1
+# CHECK-NEXT:   Name: myfunc
+# CHECK-NEXT:   RVA:
+# CHECK-NEXT: }
+# CHECK-EMPTY:
+
+    .text
+    .globl myfunc
+myfunc:
+    ret
+    .globl exportfunc
+exportfunc:
+    ret
+
+// The object contains an export directive for exportfunc
+    .section .drectve
+    .asciz "-export:exportfunc"
+
+// The object also contains a manually crafted .edata section,
+// which instead exports myfunc, not exportfunc.
+    .section .edata, "drw"
+    .align 4
+exports:
+    .long 0           // ExportFlags
+    .long 0           // TimeDateStamp
+    .long 0           // MajorVersion + MinorVersion
+    .rva name         // NameRVA
+    .long 1           // OrdinalBase
+    .long 1           // AddressTableEntries
+    .long 1           // NumberOfNamePointers
+    .rva functions    // ExportAddressTableRVA
+    .rva names        // NamePointerRVA
+    .rva nameordinals // OrdinalTableRVA
+
+names:
+    .rva funcname_myfunc
+
+nameordinals:
+    .short 0
+
+functions:
+    .rva myfunc
+    .long 0
+
+funcname_myfunc:
+    .asciz "myfunc"
+
+name:
+    .asciz "mydll.dll"