[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 ↗

(On Diff #214106)

Do you have to use these two variables?

1376 ↗

(On Diff #214106)

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

1376 ↗

(On Diff #214106)

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

[ruiu]

LGTM

lld/trunk/COFF/Config.h

struct Configuration {
 
   std::set<std::string> noDefaultLibs;
   bool noDefaultLibAll = false;
 
   // True if we are creating a DLL.
   bool dll = false;
   StringRef implib;
   std::vector<Export> exports;
+  bool hadExplicitExports;
   std::set<std::string> delayLoads;
   std::map<std::string, int> dllOrder;
   Symbol *delayLoadHelper = nullptr;
 
   bool saveTemps = false;
 
   // /guard:cf
   GuardCFLevel guardCF = GuardCFLevel::Off;

lld/trunk/COFF/Driver.cpp

void LinkerDriver::link(ArrayRef<const char *> argsArr) {
   // references to the symbols we use from them.
   run();
 
   // Resolve remaining undefined symbols and warn about imported locals.
   symtab->resolveRemainingUndefines();
   if (errorCount())
     return;
 
+  config->hadExplicitExports = !config->exports.empty();
   if (config->mingw) {
     // In MinGW, all symbols are automatically exported if no symbols
     // are chosen to be exported.
     maybeExportMinGWSymbols(args);
 
     // Make sure the crtend.o object is the last object file. This object
     // file can contain terminating section chunks that need to be placed
     // last. GNU ld processes files and static libraries explicitly in the

lld/trunk/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;
+  Chunk *edataEnd = nullptr;
   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 (!edataSec->chunks.empty()) {
+    // Allow using a custom built export table from input object files, instead
+    // of having the linker synthesize the tables.
+    if (config->hadExplicitExports)
+      warn("literal .edata sections override exports");
+  } else if (!config->exports.empty()) {
     for (Chunk *c : edata.chunks)
       edataSec->addChunk(c);
   }
+  if (!edataSec->chunks.empty()) {
+    edataStart = edataSec->chunks.front();
+    edataEnd = edataSec->chunks.back();
+  }
+}
 
 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
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 (edataStart) {
+    dir[EXPORT_TABLE].RelativeVirtualAddress = edataStart->getRVA();
+    dir[EXPORT_TABLE].Size =
+        edataEnd->getRVA() + edataEnd->getSize() - edataStart->getRVA();
   }
   if (importTableStart) {
     dir[IMPORT_TABLE].RelativeVirtualAddress = importTableStart->getRVA();
     dir[IMPORT_TABLE].Size = importTableSize;
   }
   if (iatStart) {
     dir[IAT].RelativeVirtualAddress = iatStart->getRVA();
     dir[IAT].Size = iatSize;

lld/trunk/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 2>&1 | FileCheck %s --allow-empty --check-prefix=NOWARNING
+# RUN: llvm-readobj --coff-exports %t.dll | FileCheck %s
+# RUN: lld-link -lldmingw -dll -out:%t.dll %t.o -entry:__ImageBase -export:otherfunc 2>&1 | FileCheck %s --check-prefix=WARNING
+# 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:
+
+# NOWARNING-NOT: warning
+
+# WARNING: warning: literal .edata sections override exports
+
+    .text
+    .globl myfunc
+myfunc:
+    ret
+    .globl otherfunc
+otherfunc:
+    ret
+
+// The object contains a manually crafted .edata section, which exports
+// myfunc, not otherfunc.
+    .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"