[PATCH] D32721: Accept archive files with no symbol table instad of warning on them.

[Rafael Avila de Espindola via llvm-commits]

LGTM

Rui Ueyama via Phabricator <reviews at reviews.llvm.org> writes:

> ruiu updated this revision to Diff 97500.
> ruiu added a comment.
>
> - Instantiate Archive only once.
>
>
> https://reviews.llvm.org/D32721
>
> Files:
>   lld/ELF/Config.h
>   lld/ELF/Driver.cpp
>   lld/ELF/InputFiles.cpp
>   lld/ELF/InputFiles.h
>   lld/ELF/Relocations.cpp
>   lld/test/ELF/lto/archive-no-index.ll
>
> Index: lld/test/ELF/lto/archive-no-index.ll
> ===================================================================
> --- lld/test/ELF/lto/archive-no-index.ll
> +++ lld/test/ELF/lto/archive-no-index.ll
> @@ -4,29 +4,15 @@
>  ; encountered an empty archive index and undefined references (to prevent
>  ; noisy false alarms).
>  
> -; RUN: rm -fr %T/archive-no-index
> -; RUN: mkdir %T/archive-no-index
> -; RUN: llvm-as %S/Inputs/archive.ll -o %T/archive-no-index/f.o
> -; RUN: llvm-ar cr %T/archive-no-index/libf.a
> -; RUN: llvm-ar qS %T/archive-no-index/libf.a %T/archive-no-index/f.o
> -; RUN: llvm-as %s -o %t.o
> -; RUN: not ld.lld -emain -m elf_x86_64 %t.o -o %t %T/archive-no-index/libf.a \
> -; RUN:     2>&1 | FileCheck --check-prefix=NOTE %s
> +; RUN: llvm-as -o %t1.o %s
> +; RUN: llvm-as -o %t2.o %S/Inputs/archive.ll
>  
> -; RUN: llvm-ar crs %T/archive-no-index/libfs.a %T/archive-no-index/f.o
> -; RUN: ld.lld -emain -m elf_x86_64 %t.o -o %t %T/archive-no-index/libf.a \
> -; RUN:     %T/archive-no-index/libfs.a
> +; RUN: rm -f %t1.a %t2.a
> +; RUN: llvm-ar crS %t1.a %t2.o
> +; RUN: llvm-ar crs %t2.a %t2.o
>  
> -; RUN: llvm-as %S/Inputs/archive-3.ll -o %T/archive-no-index/foo.o
> -; RUN: llvm-ar crs %T/archive-no-index/libfoo.a %T/archive-no-index/foo.o
> -; RUN: not ld.lld -emain -m elf_x86_64 %t.o -o %t %T/archive-no-index/libfoo.a \
> -; RUN:     2>&1 | FileCheck --check-prefix=NO-NOTE %s
> -
> -; NOTE: undefined symbol: f
> -; NOTE: archive listed no symbols
> -
> -; NO-NOTE: undefined symbol: f
> -; NO-NOTE-NOT: archive listed no symbols
> +; RUN: ld.lld -o %t -emain -m elf_x86_64 %t1.o %t1.a
> +; RUN: ld.lld -o %t -emain -m elf_x86_64 %t1.o %t2.a
>  
>  target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
>  target triple = "x86_64-unknown-linux-gnu"
> Index: lld/ELF/Relocations.cpp
> ===================================================================
> --- lld/ELF/Relocations.cpp
> +++ lld/ELF/Relocations.cpp
> @@ -694,17 +694,6 @@
>      warn(Msg);
>    } else {
>      error(Msg);
> -
> -    if (Config->ArchiveWithoutSymbolsSeen) {
> -      message("At least one archive listed no symbols in its index."
> -              " This can happen when creating archives with a version"
> -              " of ar that does not understand the object files in"
> -              " the archive. For example, if you are using LLVM"
> -              " bitcode objects (such as created by -flto), you may"
> -              " need to use llvm-ar or GNU ar with a plugin.");
> -      // Reset to false so that we print the message only once.
> -      Config->ArchiveWithoutSymbolsSeen = false;
> -    }
>    }
>  }
>  
> Index: lld/ELF/InputFiles.h
> ===================================================================
> --- lld/ELF/InputFiles.h
> +++ lld/ELF/InputFiles.h
> @@ -239,7 +239,7 @@
>  // An ArchiveFile object represents a .a file.
>  class ArchiveFile : public InputFile {
>  public:
> -  explicit ArchiveFile(MemoryBufferRef M) : InputFile(ArchiveKind, M) {}
> +  explicit ArchiveFile(std::unique_ptr<Archive> &&File);
>    static bool classof(const InputFile *F) { return F->kind() == ArchiveKind; }
>    template <class ELFT> void parse();
>  
> Index: lld/ELF/InputFiles.cpp
> ===================================================================
> --- lld/ELF/InputFiles.cpp
> +++ lld/ELF/InputFiles.cpp
> @@ -596,17 +596,13 @@
>    }
>  }
>  
> -template <class ELFT> void ArchiveFile::parse() {
> -  File = check(Archive::create(MB),
> -               MB.getBufferIdentifier() + ": failed to parse archive");
> +ArchiveFile::ArchiveFile(std::unique_ptr<Archive> &&File)
> +    : InputFile(ArchiveKind, File->getMemoryBufferRef()),
> +      File(std::move(File)) {}
>  
> -  // Read the symbol table to construct Lazy objects.
> -  for (const Archive::Symbol &Sym : File->symbols()) {
> +template <class ELFT> void ArchiveFile::parse() {
> +  for (const Archive::Symbol &Sym : File->symbols())
>      Symtab<ELFT>::X->addLazyArchive(this, Sym);
> -  }
> -
> -  if (File->symbols().begin() == File->symbols().end())
> -    Config->ArchiveWithoutSymbolsSeen = true;
>  }
>  
>  // Returns a buffer pointing to a member file containing a given symbol.
> Index: lld/ELF/Driver.cpp
> ===================================================================
> --- lld/ELF/Driver.cpp
> +++ lld/ELF/Driver.cpp
> @@ -171,14 +171,31 @@
>    case file_magic::unknown:
>      readLinkerScript(MBRef);
>      return;
> -  case file_magic::archive:
> +  case file_magic::archive: {
> +    // Handle -whole-archive.
>      if (InWholeArchive) {
>        for (MemoryBufferRef MB : getArchiveMembers(MBRef))
>          Files.push_back(createObjectFile(MB, Path));
>        return;
>      }
> -    Files.push_back(make<ArchiveFile>(MBRef));
> +
> +    std::unique_ptr<Archive> File =
> +        check(Archive::create(MBRef), Path + ": failed to parse archive");
> +
> +    // If an archive file has no symbol table, it is likely that a user
> +    // is attempting LTO and using a default ar command that doesn't
> +    // understand the LLVM bitcode file. It is a pretty common error, so
> +    // we'll handle it as if it had a symbol table.
> +    if (!File->hasSymbolTable()) {
> +      for (MemoryBufferRef MB : getArchiveMembers(MBRef))
> +        Files.push_back(make<LazyObjectFile>(MB));
> +      return;
> +    }
> +
> +    // Handle the regular case.
> +    Files.push_back(make<ArchiveFile>(std::move(File)));
>      return;
> +  }
>    case file_magic::elf_shared_object:
>      if (Config->Relocatable) {
>        error("attempted static link of dynamic object " + Path);
> Index: lld/ELF/Config.h
> ===================================================================
> --- lld/ELF/Config.h
> +++ lld/ELF/Config.h
> @@ -99,7 +99,6 @@
>    std::vector<SymbolVersion> VersionScriptLocals;
>    std::vector<uint8_t> BuildIdVector;
>    bool AllowMultipleDefinition;
> -  bool ArchiveWithoutSymbolsSeen = false;
>    bool AsNeeded = false;
>    bool Bsymbolic;
>    bool BsymbolicFunctions;