[LLD] [COFF] Add support for the -wholearchive option

[mstorsjo]

Summary

This fixes PR31824.

[ruiu]

COFF/Options.td

99–100 ↗

(On Diff #114682)

Since they are lld-specific flags, I'd name -lld-whole-archive and -lld-no-whole-archive.

[pcc]

COFF/Options.td

99–100 ↗

(On Diff #114682)

I don't think you need to add lld-specific flags to support --whole-archive. Instead, you could implement support for the equivalent link.exe flag, /wholearchive. See also PR31824.

[mstorsjo]

COFF/Options.td

99–100 ↗

(On Diff #114682)

@pcc - Oh, that probably makes sense - I hadn't seen that option before. Then the MinGW driver just needs to inject -wholearchive: before each library between --whole-archive and --no-whole-archive. Since the driver doesn't do file magic detection, it probably needs to only match this for *.a, *.lib and -lfoo entries, but that's probably fine for most sensible use cases.

[pcc]

COFF/Options.td

99–100 ↗

(On Diff #114682)

Right. I think you wouldn't even need to do the matching; MSVC link.exe allows /wholearchive to be used with object files, and we can do the same.

[mstorsjo]

Adjusted the COFF driver to implement the link.exe option -wholearchive instead.

[ruiu]

Generally looking good.

Could you split this patch into two, one for COFF and one for MinGW?

COFF/Driver.cpp

135 ↗

(On Diff #114880)

It's not new code, but can you write it in two lines?

Symtab->addFile(make<ArchiveFile>(MBRef));
return;

Because addFile returns void and this function returns void too, it is legal to write return Symtab->addFile(...), but this is still an odd feature of C++, and I believe we generally want to avoid this unless it's generated as instantiated templates.

137 ↗

(On Diff #114880)

nit: add a blank line befor ethis line.

138 ↗

(On Diff #114880)

Likewise, separate into two lines.

975 ↗

(On Diff #114880)

Replacing WholeArchive with Args.hasArg(OPT_wholearchive_flag) might improve readability.

MinGW/Driver.cpp

175 ↗

(On Diff #114880)

I think I'd name this just Prefix as we generally prefer shorter variable names in lld.

183 ↗

(On Diff #114880)

80 cols (please run clang-format if you have installed it.)

[mstorsjo]

In D37709#868490, @ruiu wrote:

Could you split this patch into two, one for COFF and one for MinGW?

Sure, will repost with it split, and with your suggestions applied.

COFF/Driver.cpp

135 ↗

(On Diff #114880)

Sure, I can fix that at the same time.

[mstorsjo]

Split the patch, moving the MinGW specific parts to a separate patch. Applied Rui's style suggestions.

[ruiu]

LGTM

COFF/Driver.cpp

131 ↗

(On Diff #114900)

nit: I'd rename Member M.

132 ↗

(On Diff #114900)

Instead of "*", let's pass something like "<whole-archive>" instead.

[pcc]

COFF/Driver.cpp

975 ↗

(On Diff #114880)

Wouldn't that create O(N^2) complexity in the size of the argument list though?

233 ↗

(On Diff #114900)

Are you sure that /wholearchive has no effect on archives specified by linker directives?

[mstorsjo]

COFF/Driver.cpp

233 ↗

(On Diff #114900)

No, I can try to construct a test to figure out how it's supposed to behave.

[mstorsjo]

COFF/Driver.cpp

975 ↗

(On Diff #114880)

So it seems, based on reading ArgList.h. Will push with this changed back to a bool WholeArchiveFlag variable, to avoid the O(N^2) complexity.

233 ↗

(On Diff #114900)

I checked - link.exe doesn't apply the -wholearchive option to -defaultlib options, neither on the command line nor in directives. So I'll update the handling of the command line version of -defaultlib accordingly as well.

lld/trunk/COFF/Driver.h

private:
   // This function looks up the symbol table and resolve corresponding
   // entry point name.
   StringRef findDefaultEntry();
   WindowsSubsystem inferSubsystem();
 
   void invokeMSVC(llvm::opt::InputArgList &Args);
 
   MemoryBufferRef takeBuffer(std::unique_ptr<MemoryBuffer> MB);
-  void addBuffer(std::unique_ptr<MemoryBuffer> MB);
+  void addBuffer(std::unique_ptr<MemoryBuffer> MB, bool WholeArchive);
   void addArchiveBuffer(MemoryBufferRef MBRef, StringRef SymName,
                         StringRef ParentName);
 
-  void enqueuePath(StringRef Path);
+  void enqueuePath(StringRef Path, bool WholeArchive);
 
   void enqueueTask(std::function<void()> Task);
   bool run();
 
   std::list<std::function<void()>> TaskQueue;
   std::vector<StringRef> FilePaths;
   std::vector<MemoryBufferRef> Resources;
 };

lld/trunk/COFF/Driver.cpp

MemoryBufferRef LinkerDriver::takeBuffer(std::unique_ptr<MemoryBuffer> MB) {
   make<std::unique_ptr<MemoryBuffer>>(std::move(MB)); // take ownership
 
   if (Driver->Tar)
     Driver->Tar->append(relativeToRoot(MBRef.getBufferIdentifier()),
                         MBRef.getBuffer());
   return MBRef;
 }
 
-void LinkerDriver::addBuffer(std::unique_ptr<MemoryBuffer> MB) {
+void LinkerDriver::addBuffer(std::unique_ptr<MemoryBuffer> MB,
+                             bool WholeArchive) {
   MemoryBufferRef MBRef = takeBuffer(std::move(MB));
 
   // File type is detected by contents, not by file extension.
   file_magic Magic = identify_magic(MBRef.getBuffer());
   if (Magic == file_magic::windows_resource) {
     Resources.push_back(MBRef);
     return;
   }
 
   FilePaths.push_back(MBRef.getBufferIdentifier());
-  if (Magic == file_magic::archive)
-    return Symtab->addFile(make<ArchiveFile>(MBRef));
-  if (Magic == file_magic::bitcode)
-    return Symtab->addFile(make<BitcodeFile>(MBRef));
+  if (Magic == file_magic::archive) {
+    if (WholeArchive) {
+      std::unique_ptr<Archive> File =
+          check(Archive::create(MBRef),
+                MBRef.getBufferIdentifier() + ": failed to parse archive");
+
+      for (MemoryBufferRef M : getArchiveMembers(File.get()))
+        addArchiveBuffer(M, "<whole-archive>", MBRef.getBufferIdentifier());
+      return;
+    }
+    Symtab->addFile(make<ArchiveFile>(MBRef));
+    return;
+  }
+
+  if (Magic == file_magic::bitcode) {
+    Symtab->addFile(make<BitcodeFile>(MBRef));
+    return;
+  }
 
   if (Magic == file_magic::coff_cl_gl_object)
     error(MBRef.getBufferIdentifier() + ": is not a native COFF file. "
           "Recompile without /GL");
   else
     Symtab->addFile(make<ObjFile>(MBRef));
 }
 
-void LinkerDriver::enqueuePath(StringRef Path) {
+void LinkerDriver::enqueuePath(StringRef Path, bool WholeArchive) {
   auto Future =
       std::make_shared<std::future<MBErrPair>>(createFutureForFile(Path));
   std::string PathStr = Path;
   enqueueTask([=]() {
     auto MBOrErr = Future->get();
     if (MBOrErr.second)
       error("could not open " + PathStr + ": " + MBOrErr.second.message());
     else
-      Driver->addBuffer(std::move(MBOrErr.first));
+      Driver->addBuffer(std::move(MBOrErr.first), WholeArchive);
   });
 }
 
 void LinkerDriver::addArchiveBuffer(MemoryBufferRef MB, StringRef SymName,
                                     StringRef ParentName) {
   file_magic Magic = identify_magic(MB.getBuffer());
   if (Magic == file_magic::coff_import_library) {
     Symtab->addFile(make<ImportFile>(MB));
for (auto *Arg : Args) {
     case OPT_aligncomm:
       parseAligncomm(Arg->getValue());
       break;
     case OPT_alternatename:
       parseAlternateName(Arg->getValue());
       break;
     case OPT_defaultlib:
       if (Optional<StringRef> Path = findLib(Arg->getValue()))
-        enqueuePath(*Path);
+        enqueuePath(*Path, false);
       break;
     case OPT_export: {
       Export E = parseExport(Arg->getValue());
       E.Directives = true;
       Config->Exports.push_back(E);
       break;
     }
     case OPT_failifmismatch:
void LinkerDriver::link(ArrayRef<const char *> ArgsArr) {
   if (Args.hasArg(OPT_nosymtab))
     Config->WriteSymtab = false;
 
   Config->MapFile = getMapFile(Args);
 
   if (ErrorCount)
     return;
 
+  bool WholeArchiveFlag = Args.hasArg(OPT_wholearchive_flag);
   // Create a list of input files. Files can be given as arguments
   // for /defaultlib option.
   std::vector<MemoryBufferRef> MBs;
-  for (auto *Arg : Args.filtered(OPT_INPUT))
+  for (auto *Arg : Args.filtered(OPT_INPUT, OPT_wholearchive_file)) {
+    switch (Arg->getOption().getID()) {
+    case OPT_INPUT:
       if (Optional<StringRef> Path = findFile(Arg->getValue()))
-      enqueuePath(*Path);
+        enqueuePath(*Path, WholeArchiveFlag);
+      break;
+    case OPT_wholearchive_file:
+      if (Optional<StringRef> Path = findFile(Arg->getValue()))
+        enqueuePath(*Path, true);
+      break;
+    }
+  }
   for (auto *Arg : Args.filtered(OPT_defaultlib))
     if (Optional<StringRef> Path = findLib(Arg->getValue()))
-      enqueuePath(*Path);
+      enqueuePath(*Path, false);
 
   // Windows specific -- Create a resource file containing a manifest file.
   if (Config->Manifest == Configuration::Embed)
-    addBuffer(createManifestRes());
+    addBuffer(createManifestRes(), false);
 
   // Read all input files given via the command line.
   run();
 
   // We should have inferred a machine type by now from the input files, but if
   // not we assume x64.
   if (Config->Machine == IMAGE_FILE_MACHINE_UNKNOWN) {
     warn("/machine is not specified. x64 is assumed");
     Config->Machine = AMD64;
   }
 
   // Input files can be Windows resource files (.res files). We use
   // WindowsResource to convert resource files to a regular COFF file,
   // then link the resulting file normally.
   if (!Resources.empty())
-    addBuffer(convertResToCOFF(Resources));
+    addBuffer(convertResToCOFF(Resources), false);
 
   if (Tar)
     Tar->append("response.txt",
                 createResponseFile(Args, FilePaths,
                                    ArrayRef<StringRef>(SearchPaths).slice(1)));
 
   // Handle /largeaddressaware
   if (Config->is64() || Args.hasArg(OPT_largeaddressaware))

lld/trunk/COFF/Options.td

 def opt     : P<"opt", "Control optimizations">;
 def out     : P<"out", "Path to file to write output">;
 def pdb : P<"pdb", "PDB file path">;
 def section : P<"section", "Specify section attributes">;
 def stack   : P<"stack", "Size of the stack">;
 def stub    : P<"stub", "Specify DOS stub file">;
 def subsystem : P<"subsystem", "Specify subsystem">;
 def version : P<"version", "Specify a version number in the PE header">;
+def wholearchive_file : P<"wholearchive", "Include all object files from this archive">;
 
 def disallowlib : Joined<["/", "-", "-?"], "disallowlib:">, Alias<nodefaultlib>;
 
 def manifest : F<"manifest">;
 def manifest_colon : P<"manifest", "Create manifest file">;
 def manifestuac : P<"manifestuac", "User access control">;
 def manifestfile : P<"manifestfile", "Manifest file path">;
 def manifestdependency : P<"manifestdependency",

 def dll : F<"dll">, HelpText<"Create a DLL">;
 def driver : P<"driver", "Generate a Windows NT Kernel Mode Driver">;
 def nodefaultlib_all : F<"nodefaultlib">;
 def noentry : F<"noentry">;
 def profile : F<"profile">;
 def swaprun_cd : F<"swaprun:cd">;
 def swaprun_net : F<"swaprun:net">;
 def verbose : F<"verbose">;
+def wholearchive_flag : F<"wholearchive">;
 
 def force : F<"force">,
     HelpText<"Allow undefined symbols when creating executables">;
 def force_unresolved : F<"force:unresolved">;
 
 defm allowbind: B<"allowbind", "Disable DLL binding">;
 defm allowisolation : B<"allowisolation", "Set NO_ISOLATION bit">;
 defm appcontainer : B<"appcontainer",

lld/trunk/test/COFF/wholearchive.s

+# REQEUIRES: x86
+
+# RUN: yaml2obj < %p/Inputs/export.yaml > %t.archive.obj
+# RUN: llvm-ar rcs %t.archive.lib %t.archive.obj
+# RUN: llvm-mc -triple=x86_64-windows-msvc %s -filetype=obj -o %t.main.obj
+
+# RUN: lld-link -dll -out:%t.dll -entry:main %t.main.obj -wholearchive:%t.archive.lib -implib:%t.lib
+# RUN: llvm-readobj %t.lib | FileCheck %s -check-prefix CHECK-IMPLIB
+
+# RUN: lld-link -dll -out:%t.dll -entry:main %t.main.obj -wholearchive %t.archive.lib -implib:%t.lib
+# RUN: llvm-readobj %t.lib | FileCheck %s -check-prefix CHECK-IMPLIB
+
+# CHECK-IMPLIB: Symbol: __imp_exportfn3
+# CHECK-IMPLIB: Symbol: exportfn3
+
+.global main
+.text
+main:
+  ret