[lld] Fix --exclude-libs broken when --whole-archive is used

[iid_iunknown]

Summary

Problem
--exclude-libs does not work for static libraries affected by the --whole-archive option.

Description
--exclude-libs creates a list of static library paths and does library lookups in this list.
--whole-archive splits the static libraries that follow it into separate objects. As a result, lld no longer sees static libraries among linked files and does no --exclude-libs lookups.

Solution
The proposed solution is to make --exclude-libs consider object files too. When lld finds an object file it checks whether this file originates from an archive and, if so, looks the archive up in the --exclude-libs list.

[ruiu]

ELF/Driver.cpp

994–995

I'd define a function

static StringRef getArchiveName(InputFile *File) {
  if (isa<ArchiveFile>(File))
    return File_>getName();
  return File->ArchiveName;
}

so that I can write

for (InputFile *File :Files)
  if ((All && isa<ArchiveFile>(File)) || Libs.count(path::filename(getArchiveName(File)))
    for (SymbolBody *Sym : F->getSymbols()
      if (!Sym->isLocal())
        Sym->symbol()->VersionId = VER_NDX_LOCAL;

[iid_iunknown]

Introducing getArchiveName

[iid_iunknown]

ELF/Driver.cpp

994–995

Thanks Rui!

This one is now fixed.
Please note that the suggested condition

if ((All && isa<ArchiveFile>(File)) || Libs.count(path::filename(getArchiveName(File)))

is not quite correct as it does not handle the --exclude-libs ALL --whole-archive case, so it was modified to produce the expected result.

[ruiu]

ELF/Driver.cpp

994–995

Do you actually have to check if ArchiveName is not empty? Since Libs can never contains an empty string, I don't think it is necessary.

[iid_iunknown]

ELF/Driver.cpp

994–995

Object files that do not originate from archives have empty ArchiveName.
Removing that check would involve all the objects and probably even shared libraries into symbols localization if --exclude-libs ALL is specified.

The check ensures that symbols localization is applied only to files associated with archives: either the file is an archive itself or an object extracted from an archive by e.g. --whole-archive.

[ruiu]

LGTM with this change.

ELF/Driver.cpp

994–995

Ah, okay, then I'd make this change

static Optional<StringRef> getArchiveName(InputFile *File) {
  if (isa<ArchiveFile>(File))
    return File->getName();
  if (!File->ArchiveName.emty())
    return File->ArchiveName;
  return None;
}

so that I can change this function to

for (InputFile *File : Files) {
  if (Optional<StringRef> Archive = getArchiveName(File))
    if (All || Libs.count(path::filename(*Archive)))
      ...

[iid_iunknown]

ELF/Driver.cpp

994–995

Good point.
I will update the patch.
Thanks!

[iid_iunknown]

Use Optional in getArchiveName()

ELF/Driver.cpp

for (;;) {
       Ret.insert(S.substr(0, Pos));
       S = S.substr(Pos + 1);
     }
     Ret.insert(S);
   }
   return Ret;
 }
 
+static Optional<StringRef> getArchiveName(InputFile *File) {
+  if (isa<ArchiveFile>(File))
+    return File->getName();
+  if (!File->ArchiveName.empty())
+    return File->ArchiveName;
+  return None;
+}
+
+
 // Handles the -exclude-libs option. If a static library file is specified
 // by the -exclude-libs option, all public symbols from the archive become
 // private unless otherwise specified by version scripts or something.
 // A special library name "ALL" means all archive files.
 //
 // This is not a popular option, but some programs such as bionic libc use it.
+template <class ELFT>
 static void excludeLibs(opt::InputArgList &Args, ArrayRef<InputFile *> Files) {
   DenseSet<StringRef> Libs = getExcludeLibs(Args);
   bool All = Libs.count("ALL");
 
-  for (InputFile *File : Files)
-    if (auto *F = dyn_cast<ArchiveFile>(File))
-      if (All || Libs.count(path::filename(F->getName())))
-        for (SymbolBody *Sym : F->getSymbols())
-          Sym->symbol()->VersionId = VER_NDX_LOCAL;
+  for (InputFile *File : Files) {
+    if (Optional<StringRef> Archive = getArchiveName(File))
+      if (All || Libs.count(path::filename(*Archive)))
+        for (SymbolBody *SymBody : File->getSymbols())
+          if (!SymBody->isLocal())
+            SymBody->symbol()->VersionId = VER_NDX_LOCAL;
+  }
 }
 
 // Do actual linking. Note that when this function is called,
 // all linker scripts have already been parsed.
 template <class ELFT> void LinkerDriver::link(opt::InputArgList &Args) {
   Target = getTarget();
 
   Config->MaxPageSize = getMaxPageSize(Args);
template <class ELFT> void LinkerDriver::link(opt::InputArgList &Args) {
   if (errorCount())
     return;
 
   // Handle undefined symbols in DSOs.
   Symtab->scanShlibUndefined<ELFT>();
 
   // Handle the -exclude-libs option.
   if (Args.hasArg(OPT_exclude_libs))
-    excludeLibs(Args, Files);
+    excludeLibs<ELFT>(Args, Files);
 
   // Apply version scripts.
   Symtab->scanVersionScript();
 
   // Create wrapped symbols for -wrap option.
   for (auto *Arg : Args.filtered(OPT_wrap))
     Symtab->addSymbolWrap<ELFT>(Arg->getValue());
 

test/ELF/exclude-libs.s

 // RUN: llvm-readobj -dyn-symbols %t.exe | FileCheck --check-prefix=EXCLUDE %s
 
 // RUN: ld.lld -shared %t.o %t.dir/exc.a -o %t.exe --exclude-libs foo:bar:exc.a
 // RUN: llvm-readobj -dyn-symbols %t.exe | FileCheck --check-prefix=EXCLUDE %s
 
 // RUN: ld.lld -shared %t.o %t.dir/exc.a -o %t.exe --exclude-libs=ALL
 // RUN: llvm-readobj -dyn-symbols %t.exe | FileCheck --check-prefix=EXCLUDE %s
 
+// RUN: ld.lld -shared --whole-archive %t.o %t.dir/exc.a -o %t.exe --exclude-libs foo,bar,exc.a
+// RUN: llvm-readobj -dyn-symbols %t.exe | FileCheck --check-prefix=EXCLUDE %s
+
+// RUN: ld.lld -shared --whole-archive %t.o %t.dir/exc.a -o %t.exe --exclude-libs=ALL
+// RUN: llvm-readobj -dyn-symbols %t.exe | FileCheck --check-prefix=EXCLUDE %s
+
 // DEFAULT: Name: fn
 // EXCLUDE-NOT: Name: fn
 
 .globl fn
 foo:
   call fn@PLT