[LLD] [COFF] Add support for automatically exporting all symbols

[mstorsjo]

Summary

GNU ld automatically exports all symbols if no symbols have been chosen to export via either def files or dllexport attributes. The same behaviour can also be enabled via the GNU ld option --export-all-symbols, in case some symbols are marked for export via a def file or dllexport attribute.

The list of excluded symbols is from GNU ld, minus the cygwin specific symbols.

Also add support for outputting the actual list of exported symbols in a def file, as in the GNU ld option --output-def.

This currently exports all symbols from object files pulled in from libmingwex.

These options in GNU ld are documented in https://sourceware.org/binutils/docs/ld/WIN32.html.

[ruiu]

COFF/Driver.cpp

1212

Is it doable for the mingw driver to always add -export-all-symbols if -export is empty? If you can do this, you can remove Config->MinGW && Config->Exports.empty() from this expression.

1212

This needs comment to make it clear that this is for MinGW, and in MinGW, all symbols are automatically exported if no symbols are chosen to be exported.

1214–1244

Can you make this a separate function which returns a std::set?

1248

We generally prefer early returns, so please flip the condition and return here.

1249

count is more appropriate than find.

[mstorsjo]

Thanks for the review - one inline response to a comment, will do the other suggested changes tomorrow.

COFF/Driver.cpp

1212

No, the mingw driver doesn't know whether the object files will contain any dllexport directives, so it can't take care of setting that flag.

[zturner]

Isn't there a hard limit on the number of files that can be exported in COFF?

COFF/Driver.cpp

1214

Can you make this a vector<StringRef>, but add an additional _ to the beginning of each item?

Then, below you can write:

if (Config->Machine != I386) {
  for (auto &Str : ExcludeSymbols) {
    Str = Str.drop_front().substr(0, Str.find('@'));
  }
}

saving unnecessary allocations on all code paths.

1243–1244

Since you've explicitly defined this set above, and there is no way to get additional items into it, why do we need a set? Seems like you can just std::sort the original vector and then std::binary_search for symbols.

[mstorsjo]

COFF/Driver.cpp

1212

Ok, will add such a comment.

1214

Ok, that should be doable.

1243–1244

Ok, changing this into a separate function that returns a sorted vector.

1248

Ok, will do

[mstorsjo]

Changed the exclude list as suggested by Zachary (write all entries in fully decorated form and just substr the others), moved it to a separate function and using std::sort and std::binary_search instead of making a set of it. Added a comment and changed a condition into an early return, as suggested by Rui.

[mstorsjo]

Print DATA in the output def for data symbols, adjusted the stdcall decoration for DllEntryPoint.

I did notice however that this also outputs everything from libmingwex.a that gets linked in; in practice this should probably be restricted to the object files passed directly on the command line, not libraries. Any suggestions on how to implement that? (GNU ld also has got other options like --exclude-libs that probably relates to that.) Or is this sensible as a first implementation already without that?

[mstorsjo]

In D38760#893870, @zturner wrote:

Isn't there a hard limit on the number of files that can be exported in COFF?

Did you mean files, or perhaps symbols? In either case, I don't really know. But as GNU ld does this, we probably should try to replicate it, at least for the simpler default cases. (E.g. any autotools project that doesn't override what to export.)

[ruiu]

COFF/Driver.cpp

580

Please mention that this is MinGW specific.

1214

Actually, I prefer a simple std::set because the number of items in the set is really small. I wouldn't avoid string copy at all because it's cost is negligible. I'd put emphasis simplicity over efficiency unless it is a performance-critical part of the linker.

1267

Please mention that this is MinGW specific.

[zturner]

COFF/Driver.cpp

1214

Fair enough, but even in that case I prefer llvm::StringSet over std::set. I rarely find a valid use case for std::set in normal code anymore when various LLVM data structures are almost always better.

[mstorsjo]

Added more comments about this being mingw specific, changed to use a StringSet instead of a sorted vector.

Updated the commit message to indicate that this currently exports more functions than what is ideal, in case you think it's sensible to commit it in this form. GNU ld by default ignores object files in libs named libgcc, libmingw32, libmingwex, libstdc++ and a few other similar ones, and more libs to ignore can be specified with --exclude-libs.

[ruiu]

By the way, why does GNU ld export all symbols by default? Is it to emulate the ELF-ism on Windows?

[mstorsjo]

In D38760#895075, @ruiu wrote:

By the way, why does GNU ld export all symbols by default? Is it to emulate the ELF-ism on Windows?

Yes, I guess so. With MinGW tools, you can build e.g. quite a number of generic unix autotools based libraries without any modifications (as long as the library is linked with -no-undefined); it obviously isn't ideal windows packaging, but in many cases good enough.

[ruiu]

COFF/Driver.cpp

552

Since what this function constructs is a set containing predetermined elements, it is probably better to construct it directly, as it is more obvious what we are doing.

static StringSet<> getExportExcludeSymbols() {
  if (Config->Machine == I386)
    return {"__NULL_IMPORT_DESCRIPTOR",
            "__pei386_runtime_relocator",
            "_do_pseudo_reloc",
            "_impure_ptr",
            "__impure_ptr",
            "__fmode",
            "_environ",
            "___dso_handle",
            // These are the MinGW names that differ from the standard
            // ones (lacking an extra underscore).
            "_DllMain@12",
            "_DllEntryPoint@12",
            "_DllMainCRTStartup@12"};

  return {"_NULL_IMPORT_DESCRIPTOR",
          "_pei386_runtime_relocator",
          "do_pseudo_reloc",
          "impure_ptr",
          "_impure_ptr",
          "_fmode",
          "environ",
          "__dso_handle",
          "DllMain",
          "DllEntryPoint",
          "DllMainCRTStartup"};
}

[mstorsjo]

Constructing the exclude string sets directly instead of doing string transformations.

[ruiu]

LGTM

COFF/Driver.cpp

598–599

You can combine the two lines to

if (auto *Def = dyn_cast_or_null<Defined>(E.Sym))

COFF/Driver.cpp

for (COFFShortExport E1 : M.Exports) {
     E2.Noname = E1.Noname;
     E2.Data = E1.Data;
     E2.Private = E1.Private;
     E2.Constant = E1.Constant;
     Config->Exports.push_back(E2);
   }
 }
 
+static void writeDefFile(StringRef Name) {
+  std::error_code EC;
+  raw_fd_ostream OS(Name, EC, sys::fs::F_None);
+  if (EC)
+    fatal("cannot open " + Name + ": " + EC.message());
+
+  OS << "EXPORTS\n";
+  for (Export &E : Config->Exports)
+    OS << "    " << E.ExportName << " "
+       << "@" << E.Ordinal << "\n";
+}
+
 // A helper function for filterBitcodeFiles.
 static bool needsRebuilding(MemoryBufferRef MB) {
   // The MSVC linker doesn't support thin archives, so if it's a thin
   // archive, we always need to rebuild it.
   std::unique_ptr<Archive> File =
       check(Archive::create(MB), "Failed to read " + MB.getBufferIdentifier());
   if (File->isThin())
     return true;
 
   // Returns true if the archive contains at least one bitcode file.
   for (MemoryBufferRef Member : getArchiveMembers(File.get()))
     if (identify_magic(Member.getBuffer()) == file_magic::bitcode)
       return true;
   return false;
 }
 
 // Opens a given path as an archive file and removes bitcode files
 // from them if exists. This function is to appease the MSVC linker as
 // their linker doesn't like archive files containing non-native
 // object files.
 //
 // If a given archive doesn't contain bitcode files, the archive path
 // is returned as-is. Otherwise, a new temporary file is created and
 // its path is returned.
 static Optional<std::string>
 filterBitcodeFiles(StringRef Path, std::vector<std::string> &TemporaryFiles) {
   std::unique_ptr<MemoryBuffer> MB = check(
       MemoryBuffer::getFile(Path, -1, false, true), "could not open " + Path);
   MemoryBufferRef MBRef = MB->getMemBufferRef();
   file_magic Magic = identify_magic(MBRef.getBuffer());
 
   if (Magic == file_magic::bitcode)
     return None;
   if (Magic != file_magic::archive)
     return Path.str();
   if (!needsRebuilding(MBRef))
     return Path.str();
 
   std::unique_ptr<Archive> File =
       check(Archive::create(MBRef),
             MBRef.getBufferIdentifier() + ": failed to parse archive");
 
   std::vector<NewArchiveMember> New;
   for (MemoryBufferRef Member : getArchiveMembers(File.get()))
     if (identify_magic(Member.getBuffer()) != file_magic::bitcode)
       New.emplace_back(Member);
void LinkerDriver::link(ArrayRef<const char *> ArgsArr) {
   if (Args.hasArg(OPT_safeseh)) {
     for (ObjFile *File : ObjFile::Instances)
       if (!File->SEHCompat)
         error("/safeseh: " + File->getName() + " is not compatible with SEH");
     if (ErrorCount)
       return;
   }
 
+  if (Config->DLL && ((Config->MinGW && Config->Exports.empty()) ||
+                      Args.hasArg(OPT_export_all_symbols))) {
+    std::vector<std::string> ExcludeSymbols = {
+        "_NULL_IMPORT_DESCRIPTOR",
+        // Runtime pseudo-reloc.
+        "_pei386_runtime_relocator",
+        "do_pseudo_reloc",
+        // Global vars that should not be exported.
+        "impure_ptr",
+        "_impure_ptr",
+        "_fmode",
+        "environ",
+        "__dso_handle",
+        // Entry point symbols, and entry hooks; these are the MinGW
+        // names that differ from the standard ones (lacking an extra
+        // underscore).
+        "DllMain@12",
+        "DllEntryPoint@0",
+        "DllMainCRTStartup@12",
+    };
+
+    if (Config->Machine == I386) {
+      // For I386, prepend the underscore - all the symbols in the exclude list
+      // are cdecl/stdcall that should have an underscore.
+      for (auto &Str : ExcludeSymbols)
+        Str = "_" + Str;
+    } else {
+      // For non-I386, drop the stdcall decoration.
+      for (auto &Str : ExcludeSymbols)
+        Str = Str.substr(0, Str.find('@'));
+    }
+    std::set<StringRef> ExcludeSymbolSet(ExcludeSymbols.begin(),
+                                         ExcludeSymbols.end());
+
+    Symtab->forEachSymbol([=](Symbol *S) {
+      auto *Def = dyn_cast<Defined>(S->body());
+      if (Def && Def->isLive() && Def->getChunk()) {
+        if (ExcludeSymbolSet.find(Def->getName()) != ExcludeSymbolSet.end())
+          return;
+        Export E;
+        E.Name = Def->getName();
+        E.Sym = Def;
+        Config->Exports.push_back(E);
+      }
+    });
+  }
+
   // Windows specific -- when we are creating a .dll file, we also
   // need to create a .lib file.
   if (!Config->Exports.empty() || Config->DLL) {
     fixupExports();
     createImportLibrary(/*AsLib=*/false);
     assignExportOrdinals();
   }
 
+  // Handle /output-def
+  if (auto *Arg = Args.getLastArg(OPT_output_def))
+    writeDefFile(Arg->getValue());
+
   // Set extra alignment for .comm symbols
   for (auto Pair : Config->AlignComm) {
     StringRef Name = Pair.first;
     uint32_t Alignment = Pair.second;
 
     Symbol *Sym = Symtab->find(Name);
     if (!Sym) {
       warn("/aligncomm symbol " + Name + " not found");

COFF/Options.td

 defm nxcompat : B<"nxcompat", "Disable data execution provention">;
 defm safeseh : B<"safeseh", "Produce an image with Safe Exception Handler">;
 defm tsaware  : B<"tsaware", "Create non-Terminal Server aware executable">;
 
 def help : F<"help">;
 def help_q : Flag<["/?", "-?"], "">, Alias<help>;
 
 // LLD extensions
+def export_all_symbols : F<"export-all-symbols">;
 def nopdb : F<"nopdb">, HelpText<"Disable PDB generation for DWARF users">;
 def nosymtab : F<"nosymtab">;
 def lldmingw : F<"lldmingw">;
 def msvclto : F<"msvclto">;
+def output_def : Joined<["/", "-"], "output-def:">;
 def rsp_quoting : Joined<["--"], "rsp-quoting=">,
   HelpText<"Quoting style for response files, 'windows' (default) or 'posix'">;
 
 // Flags for debugging
 def lldmap : F<"lldmap">;
 def lldmap_file : Joined<["/", "-"], "lldmap:">;
 
 //==============================================================================

test/COFF/export-all.s

+# REQEUIRES: x86
+
+# RUN: llvm-mc -triple=i686-windows-gnu %s -filetype=obj -o %t.obj
+
+# RUN: lld-link -lldmingw -dll -out:%t.dll -entry:DllMainCRTStartup@12 %t.obj -implib:%t.lib
+# RUN: llvm-readobj -coff-exports %t.dll | FileCheck %s
+
+# CHECK-NOT: Name: DllMainCRTStartup
+# CHECK: Name: foobar
+
+.global _foobar
+.global _DllMainCRTStartup@12
+.text
+_DllMainCRTStartup@12:
+  ret
+_foobar:
+  ret
+
+# Test specifying -export-all-symbols, on an object file that contains axi
+# dllexport directive for some of the symbols.
+
+# RUN: yaml2obj < %p/Inputs/export.yaml > %t.obj
+#
+# RUN: lld-link -out:%t.dll -dll %t.obj -lldmingw -export-all-symbols -output-def:%t.def
+# RUN: llvm-readobj -coff-exports %t.dll | FileCheck -check-prefix=CHECK2 %s
+# RUN: cat %t.def | FileCheck -check-prefix=CHECK2-DEF %s
+
+# Note, this will actually export _DllMainCRTStartup as well, since
+# it uses the standard spelling in this object file, not the MinGW one.
+
+# CHECK2: Name: exportfn1
+# CHECK2: Name: exportfn2
+# CHECK2: Name: exportfn3
+
+# CHECK2-DEF: EXPORTS
+# CHECK2-DEF: exportfn1 @3
+# CHECK2-DEF: exportfn2 @4
+# CHECK2-DEF: exportfn3 @5