[LLD] [COFF] Exclude certain static libraries and object files when exporting all symbols

[mstorsjo]

Summary

This more or less matches what GNU ld does.

The clang_rt.builtins-$arch cases are a bit problematic; an alternative would be to trim the library name at the first period, and just have it match against libclang_rt.

[ruiu]

At this point I feel like we should define a function bool shouldExport(Defined *Sym) or something, so that we can move the code to that function.

[mstorsjo]

In D38937#898214, @ruiu wrote:

At this point I feel like we should define a function bool shouldExport(Defined *Sym) or something, so that we can move the code to that function.

Sounds like a good idea. How do you suggest to handle the exclude StringSets in that case - recreate them in each function invocation? Or have a static StringSet<> ExcludeSymbols in the global scope?

For future direction; GNU ld has got options for adding symbols and libraries to the exclude libs, so in case someone actually needs and wants that feature, we might need to add support for such options. I haven't felt a great need for it so far though, since I don't think I've ever seen it used.

[ruiu]

In D38937#898215, @mstorsjo wrote:

In D38937#898214, @ruiu wrote:

At this point I feel like we should define a function bool shouldExport(Defined *Sym) or something, so that we can move the code to that function.

Sounds like a good idea. How do you suggest to handle the exclude StringSets in that case - recreate them in each function invocation? Or have a static StringSet<> ExcludeSymbols in the global scope?

I don't know if this is a good idea, but since the number of strings you need to check is small, you may be able to just iterate over the list like this?

for (StringRef S : {"libgcc", "libgcc_s", ...})
  if (LibName == S)
    return false;

[mstorsjo]

In D38937#898216, @ruiu wrote:

In D38937#898215, @mstorsjo wrote:

In D38937#898214, @ruiu wrote:

At this point I feel like we should define a function bool shouldExport(Defined *Sym) or something, so that we can move the code to that function.

Sounds like a good idea. How do you suggest to handle the exclude StringSets in that case - recreate them in each function invocation? Or have a static StringSet<> ExcludeSymbols in the global scope?

I don't know if this is a good idea, but since the number of strings you need to check is small, you may be able to just iterate over the list like this?

for (StringRef S : {"libgcc", "libgcc_s", ...})
  if (LibName == S)
    return false;

I feel that would end up pretty unwieldy. Additionally, the set of symbols to exclude is different depending on the architecture.

The symbol/lib/object sets should be easy to encapsulate in a class though - I'll update the diff with such a suggestion.

[mstorsjo]

Wrap up all the sets and logic for deciding what symbols to export into a class.

[mstorsjo]

Any further comments on this version?

[ruiu]

I don't see a reason to use OO over plain functions in this patch, but this is probably okay. We can revisit it later. But I feel we are having too many functions for MinGW in Driver.cpp, so I was thinking that it might be a good idea to great MinGW.{h,cpp} in the COFF directory to put MinGW-related stuff there. What do you think?

[mstorsjo]

In D38937#901090, @ruiu wrote:

I don't see a reason to use OO over plain functions in this patch, but this is probably okay.

The reason for OO was (as you probably guess) keeping the sets around for each invocation. It might be a case of premature optimization. The alternative while keeping them around, with plain functions, would be to store the sets in the calling context and passing them to the function, and at that point, wrapping them up in a class seemed to simplify the code.

We can revisit it later. But I feel we are having too many functions for MinGW in Driver.cpp, so I was thinking that it might be a good idea to great MinGW.{h,cpp} in the COFF directory to put MinGW-related stuff there. What do you think?

That's probably sensible. Do you want a preparatory patch doing that, and then rebasing this one on top of that?

[ruiu]

LGTM given that you are going to create a follow-up patch to move MinGW-related stuff to new files.

COFF/Driver.cpp

return {
       // These are the MinGW names that differ from the standard
       // ones (lacking an extra underscore).
       "DllMain",
       "DllEntryPoint",
       "DllMainCRTStartup",
   };
 }
 
+// Get a set of libs to not automatically export
+// when exporting all global symbols for MinGW.
+static StringSet<> getExportExcludeLibs() {
+  return {
+      "libgcc",
+      "libgcc_s",
+      "libstdc++",
+      "libmingw32",
+      "libmingwex",
+      "libg2c",
+      "libsupc++",
+      "libobjc",
+      "libgcj",
+      "libclang_rt.builtins-aarch64",
+      "libclang_rt.builtins-arm",
+      "libclang_rt.builtins-i386",
+      "libclang_rt.builtins-x86_64",
+  };
+}
+
+// Get a set of object files to not automatically export
+// when exporting all global symbols for MinGW.
+static StringSet<> getExportExcludeObjects() {
+  return {
+      "crt0.o",
+      "crt1.o",
+      "crt1u.o",
+      "crt2.o",
+      "crt2u.o",
+      "dllcrt1.o",
+      "dllcrt2.o",
+      "gcrt0.o",
+      "gcrt1.o",
+      "gcrt2.o",
+      "crtbegin.o",
+      "crtend.o",
+  };
+}
+
 // This is MinGW specific.
 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";
if (ErrorCount)
       return;
   }
 
   // In MinGW, all symbols are automatically exported if no symbols
   // are chosen to be exported.
   if (Config->DLL && ((Config->MinGW && Config->Exports.empty()) ||
                       Args.hasArg(OPT_export_all_symbols))) {
     StringSet<> ExcludeSymbols = getExportExcludeSymbols();
+    StringSet<> ExcludeLibs = getExportExcludeLibs();
+    StringSet<> ExcludeObjects = getExportExcludeObjects();
 
     Symtab->forEachSymbol([=](Symbol *S) {
       auto *Def = dyn_cast<Defined>(S->body());
       if (!Def || !Def->isLive() || !Def->getChunk())
         return;
       if (ExcludeSymbols.count(Def->getName()))
         return;
+      StringRef LibName = sys::path::filename(Def->getFile()->ParentName);
+      // Drop the file extension.
+      LibName = LibName.substr(0, LibName.rfind('.'));
+      if (ExcludeLibs.count(LibName))
+        return;
+      StringRef FileName = sys::path::filename(Def->getFile()->getName());
+      if (LibName.empty() && ExcludeObjects.count(FileName))
+        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

test/COFF/export-all.s

 # CHECK2: Name: exportfn1
 # CHECK2: Name: exportfn2
 # CHECK2: Name: exportfn3
 
 # CHECK2-DEF: EXPORTS
 # CHECK2-DEF: exportfn1 @3
 # CHECK2-DEF: exportfn2 @4
 # CHECK2-DEF: exportfn3 @5
+
+# Test ignoring certain object files and libs.
+
+# RUN: echo -e ".global foobar\n.global DllMainCRTStartup\n.text\nDllMainCRTStartup:\nret\nfoobar:\ncall mingwfunc\ncall crtfunc\nret\n" > %t.main.s
+# RUN: llvm-mc -triple=x86_64-windows-gnu %t.main.s -filetype=obj -o %t.main.obj
+# RUN: mkdir -p %T/libs
+# RUN: echo -e ".global mingwfunc\n.text\nmingwfunc:\nret\n" > %T/libs/mingwfunc.s
+# RUN: llvm-mc -triple=x86_64-windows-gnu %T/libs/mingwfunc.s -filetype=obj -o %T/libs/mingwfunc.o
+# RUN: llvm-ar rcs %T/libs/libmingwex.a %T/libs/mingwfunc.o
+# RUN: echo -e ".global crtfunc\n.text\ncrtfunc:\nret\n" > %T/libs/crtfunc.s
+# RUN: llvm-mc -triple=x86_64-windows-gnu %T/libs/crtfunc.s -filetype=obj -o %T/libs/crt2.o
+# RUN: lld-link -out:%t.dll -dll -entry:DllMainCRTStartup %t.main.obj -lldmingw %T/libs/crt2.o %T/libs/libmingwex.a -output-def:%t.def
+# RUN: echo "EOF" >> %t.def
+# RUN: cat %t.def | FileCheck -check-prefix=CHECK-EXCLUDE %s
+
+# CHECK-EXCLUDE: EXPORTS
+# CHECK-EXCLUDE-NEXT: foobar @1
+# CHECK-EXCLUDE-NEXT: EOF