[PATCH] D32721: Accept archive files with no symbol table instad of warning on them.
Peter Collingbourne via llvm-commits
llvm-commits at lists.llvm.org
Tue May 2 10:16:09 PDT 2017
On Tue, May 2, 2017 at 7:54 AM, Rafael Avila de Espindola <
rafael.espindola at gmail.com> wrote:
> 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.
>
I suppose that's true; this change causes us to create an extra Archive
object, which causes us to read all archives twice. Although it certainly
seems possible to change the code to avoid that.
* 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.
That is in fact what Rui's patch is doing.
> Even then I
> think we should keep a warning.
>
A warning sounds reasonable to me, especially if we think there may be
semantic issues.
Peter
>
> 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
>
--
--
Peter
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20170502/c6f7ed29/attachment.html>
More information about the llvm-commits
mailing list