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

Rafael Avila de Espindola via llvm-commits llvm-commits at lists.llvm.org
Tue May 2 07:54:12 PDT 2017


We really should not change the behavior if there is no symbol table.

In addition to the semantic problem, there is also the issue that
* This impacts the non lto case too.
* It can cause way more code to be LTO in.

If we want to support this, the proper way to do it is to handle the
members as if they were part of a --start-lib/--end-lib. Even then I
think we should keep a warning.

Cheers,
Rafael

Rui Ueyama via Phabricator via llvm-commits
<llvm-commits at lists.llvm.org> writes:

> ruiu created this revision.
>
> It seems virtually everyone who tries to do LTO build with Clang and
> LLD was hit by a mistake to forget using llvm-ar command to create
> archive files. I wasn't an exception. Since this is an annoying common
> issue, it is probably better to handle that gracefully rather than
> reporting an error and tell the user to redo build with different
> configuration.
>
>
> https://reviews.llvm.org/D32721
>
> Files:
>   lld/ELF/Config.h
>   lld/ELF/Driver.cpp
>   lld/ELF/InputFiles.cpp
>   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.cpp
> ===================================================================
> --- lld/ELF/InputFiles.cpp
> +++ lld/ELF/InputFiles.cpp
> @@ -602,9 +602,6 @@
>    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
> @@ -152,6 +152,13 @@
>    return V;
>  }
>  
> +static bool hasSymbolTable(MemoryBufferRef MB) {
> +  std::unique_ptr<Archive> File =
> +      check(Archive::create(MB),
> +            MB.getBufferIdentifier() + ": failed to parse archive");
> +  return File->symbols().begin() != File->symbols().end();
> +}
> +
>  // Opens and parses a file. Path has to be resolved already.
>  // Newly created memory buffers are owned by this driver.
>  void LinkerDriver::addFile(StringRef Path, bool WithLOption) {
> @@ -172,11 +179,24 @@
>      readLinkerScript(MBRef);
>      return;
>    case file_magic::archive:
> +    // Handle -whole-archive.
>      if (InWholeArchive) {
>        for (MemoryBufferRef MB : getArchiveMembers(MBRef))
>          Files.push_back(createObjectFile(MB, Path));
>        return;
>      }
> +
> +    // 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 (!hasSymbolTable(MBRef)) {
> +      for (MemoryBufferRef MB : getArchiveMembers(MBRef))
> +        Files.push_back(make<LazyObjectFile>(MB));
> +      return;
> +    }
> +
> +    // Handle the regular case.
>      Files.push_back(make<ArchiveFile>(MBRef));
>      return;
>    case file_magic::elf_shared_object:
> 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;
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits


More information about the llvm-commits mailing list