<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Tue, May 2, 2017 at 7:54 AM, Rafael Avila de Espindola <span dir="ltr"><<a href="mailto:rafael.espindola@gmail.com" target="_blank">rafael.espindola@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">We really should not change the behavior if there is no symbol table.<br>
<br>
In addition to the semantic problem, there is also the issue that<br>
* This impacts the non lto case too.<br></blockquote><div><br></div><div>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.</div><div><br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
* It can cause way more code to be LTO in.<br>
<br>
If we want to support this, the proper way to do it is to handle the<br>
members as if they were part of a --start-lib/--end-lib.</blockquote><div><br></div><div>That is in fact what Rui's patch is doing.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Even then I<br>
think we should keep a warning.<br></blockquote><div><br></div><div>A warning sounds reasonable to me, especially if we think there may be semantic issues.</div><div><br></div><div>Peter</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
Cheers,<br>
Rafael<br>
<br>
Rui Ueyama via Phabricator via llvm-commits<br>
<div><div class="h5"><<a href="mailto:llvm-commits@lists.llvm.org">llvm-commits@lists.llvm.org</a>> writes:<br>
<br>
> ruiu created this revision.<br>
><br>
> It seems virtually everyone who tries to do LTO build with Clang and<br>
> LLD was hit by a mistake to forget using llvm-ar command to create<br>
> archive files. I wasn't an exception. Since this is an annoying common<br>
> issue, it is probably better to handle that gracefully rather than<br>
> reporting an error and tell the user to redo build with different<br>
> configuration.<br>
><br>
><br>
> <a href="https://reviews.llvm.org/D32721" rel="noreferrer" target="_blank">https://reviews.llvm.org/<wbr>D32721</a><br>
><br>
> Files:<br>
> lld/ELF/Config.h<br>
> lld/ELF/Driver.cpp<br>
> lld/ELF/InputFiles.cpp<br>
> lld/ELF/Relocations.cpp<br>
> lld/test/ELF/lto/archive-no-<wbr>index.ll<br>
><br>
</div></div>> Index: lld/test/ELF/lto/archive-no-<wbr>index.ll<br>
> ==============================<wbr>==============================<wbr>=======<br>
> --- lld/test/ELF/lto/archive-no-<wbr>index.ll<br>
> +++ lld/test/ELF/lto/archive-no-<wbr>index.ll<br>
> @@ -4,29 +4,15 @@<br>
> ; encountered an empty archive index and undefined references (to prevent<br>
> ; noisy false alarms).<br>
><br>
> -; RUN: rm -fr %T/archive-no-index<br>
> -; RUN: mkdir %T/archive-no-index<br>
> -; RUN: llvm-as %S/Inputs/archive.ll -o %T/archive-no-index/f.o<br>
> -; RUN: llvm-ar cr %T/archive-no-index/libf.a<br>
> -; RUN: llvm-ar qS %T/archive-no-index/libf.a %T/archive-no-index/f.o<br>
> -; RUN: llvm-as %s -o %t.o<br>
> -; RUN: not ld.lld -emain -m elf_x86_64 %t.o -o %t %T/archive-no-index/libf.a \<br>
> -; RUN: 2>&1 | FileCheck --check-prefix=NOTE %s<br>
> +; RUN: llvm-as -o %t1.o %s<br>
> +; RUN: llvm-as -o %t2.o %S/Inputs/archive.ll<br>
><br>
> -; RUN: llvm-ar crs %T/archive-no-index/libfs.a %T/archive-no-index/f.o<br>
> -; RUN: ld.lld -emain -m elf_x86_64 %t.o -o %t %T/archive-no-index/libf.a \<br>
> -; RUN: %T/archive-no-index/libfs.a<br>
> +; RUN: rm -f %t1.a %t2.a<br>
> +; RUN: llvm-ar crS %t1.a %t2.o<br>
> +; RUN: llvm-ar crs %t2.a %t2.o<br>
><br>
> -; RUN: llvm-as %S/Inputs/archive-3.ll -o %T/archive-no-index/foo.o<br>
> -; RUN: llvm-ar crs %T/archive-no-index/libfoo.a %T/archive-no-index/foo.o<br>
> -; RUN: not ld.lld -emain -m elf_x86_64 %t.o -o %t %T/archive-no-index/libfoo.a \<br>
> -; RUN: 2>&1 | FileCheck --check-prefix=NO-NOTE %s<br>
> -<br>
> -; NOTE: undefined symbol: f<br>
> -; NOTE: archive listed no symbols<br>
> -<br>
> -; NO-NOTE: undefined symbol: f<br>
> -; NO-NOTE-NOT: archive listed no symbols<br>
> +; RUN: ld.lld -o %t -emain -m elf_x86_64 %t1.o %t1.a<br>
> +; RUN: ld.lld -o %t -emain -m elf_x86_64 %t1.o %t2.a<br>
><br>
> target datalayout = "e-m:e-i64:64-f80:128-n8:16:<wbr>32:64-S128"<br>
> target triple = "x86_64-unknown-linux-gnu"<br>
> Index: lld/ELF/Relocations.cpp<br>
> ==============================<wbr>==============================<wbr>=======<br>
> --- lld/ELF/Relocations.cpp<br>
> +++ lld/ELF/Relocations.cpp<br>
> @@ -694,17 +694,6 @@<br>
> warn(Msg);<br>
> } else {<br>
> error(Msg);<br>
> -<br>
> - if (Config-><wbr>ArchiveWithoutSymbolsSeen) {<br>
> - message("At least one archive listed no symbols in its index."<br>
> - " This can happen when creating archives with a version"<br>
> - " of ar that does not understand the object files in"<br>
> - " the archive. For example, if you are using LLVM"<br>
> - " bitcode objects (such as created by -flto), you may"<br>
> - " need to use llvm-ar or GNU ar with a plugin.");<br>
> - // Reset to false so that we print the message only once.<br>
> - Config-><wbr>ArchiveWithoutSymbolsSeen = false;<br>
> - }<br>
> }<br>
> }<br>
><br>
> Index: lld/ELF/InputFiles.cpp<br>
> ==============================<wbr>==============================<wbr>=======<br>
> --- lld/ELF/InputFiles.cpp<br>
> +++ lld/ELF/InputFiles.cpp<br>
> @@ -602,9 +602,6 @@<br>
> for (const Archive::Symbol &Sym : File->symbols()) {<br>
> Symtab<ELFT>::X-><wbr>addLazyArchive(this, Sym);<br>
> }<br>
> -<br>
> - if (File->symbols().begin() == File->symbols().end())<br>
> - Config-><wbr>ArchiveWithoutSymbolsSeen = true;<br>
> }<br>
><br>
> // Returns a buffer pointing to a member file containing a given symbol.<br>
> Index: lld/ELF/Driver.cpp<br>
> ==============================<wbr>==============================<wbr>=======<br>
> --- lld/ELF/Driver.cpp<br>
> +++ lld/ELF/Driver.cpp<br>
> @@ -152,6 +152,13 @@<br>
> return V;<br>
> }<br>
><br>
> +static bool hasSymbolTable(MemoryBufferRef MB) {<br>
> + std::unique_ptr<Archive> File =<br>
> + check(Archive::create(MB),<br>
> + MB.getBufferIdentifier() + ": failed to parse archive");<br>
> + return File->symbols().begin() != File->symbols().end();<br>
> +}<br>
> +<br>
> // Opens and parses a file. Path has to be resolved already.<br>
> // Newly created memory buffers are owned by this driver.<br>
> void LinkerDriver::addFile(<wbr>StringRef Path, bool WithLOption) {<br>
> @@ -172,11 +179,24 @@<br>
> readLinkerScript(MBRef);<br>
> return;<br>
> case file_magic::archive:<br>
> + // Handle -whole-archive.<br>
> if (InWholeArchive) {<br>
> for (MemoryBufferRef MB : getArchiveMembers(MBRef))<br>
> Files.push_back(<wbr>createObjectFile(MB, Path));<br>
> return;<br>
> }<br>
> +<br>
> + // If an archive file has no symbol table, it is likely that a user<br>
> + // is attempting LTO and using a default ar command that doesn't<br>
> + // understand the LLVM bitcode file. It is a pretty common error, so<br>
> + // we'll handle it as if it had a symbol table.<br>
> + if (!hasSymbolTable(MBRef)) {<br>
> + for (MemoryBufferRef MB : getArchiveMembers(MBRef))<br>
> + Files.push_back(make<<wbr>LazyObjectFile>(MB));<br>
> + return;<br>
> + }<br>
> +<br>
> + // Handle the regular case.<br>
> Files.push_back(make<<wbr>ArchiveFile>(MBRef));<br>
> return;<br>
> case file_magic::elf_shared_object:<br>
> Index: lld/ELF/Config.h<br>
> ==============================<wbr>==============================<wbr>=======<br>
> --- lld/ELF/Config.h<br>
> +++ lld/ELF/Config.h<br>
> @@ -99,7 +99,6 @@<br>
> std::vector<SymbolVersion> VersionScriptLocals;<br>
> std::vector<uint8_t> BuildIdVector;<br>
> bool AllowMultipleDefinition;<br>
> - bool ArchiveWithoutSymbolsSeen = false;<br>
> bool AsNeeded = false;<br>
> bool Bsymbolic;<br>
> bool BsymbolicFunctions;<br>
> ______________________________<wbr>_________________<br>
> llvm-commits mailing list<br>
> <a href="mailto:llvm-commits@lists.llvm.org">llvm-commits@lists.llvm.org</a><br>
> <a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits" rel="noreferrer" target="_blank">http://lists.llvm.org/cgi-bin/<wbr>mailman/listinfo/llvm-commits</a><br>
</blockquote></div><br><br clear="all"><div><br></div>-- <br><div class="gmail_signature" data-smartmail="gmail_signature"><div dir="ltr">-- <div>Peter</div></div></div>
</div></div>