[lld] r259475 - ELF: Include archive names in error messages.
Sean Silva via llvm-commits
llvm-commits at lists.llvm.org
Tue Feb 2 12:13:20 PST 2016
Thanks!
I have some suggestion inline.
On Tue, Feb 2, 2016 at 12:22 AM, Rui Ueyama via llvm-commits <
llvm-commits at lists.llvm.org> wrote:
> Author: ruiu
> Date: Tue Feb 2 02:22:41 2016
> New Revision: 259475
>
> URL: http://llvm.org/viewvc/llvm-project?rev=259475&view=rev
> Log:
> ELF: Include archive names in error messages.
>
> If object files are drawn from archive files, the error message should
> be something like "conflict symbols in foo.a(bar.o) and baz.o" instead
> of "conflict symbols in bar.o and baz.o". This patch implements that.
>
> Added:
> lld/trunk/test/ELF/Inputs/conflict.s
> Modified:
> lld/trunk/ELF/Driver.cpp
> lld/trunk/ELF/InputFiles.cpp
> lld/trunk/ELF/InputFiles.h
> lld/trunk/ELF/SymbolTable.cpp
> lld/trunk/ELF/Symbols.cpp
> lld/trunk/test/ELF/conflict.s
>
> Modified: lld/trunk/ELF/Driver.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/lld/trunk/ELF/Driver.cpp?rev=259475&r1=259474&r2=259475&view=diff
>
> ==============================================================================
> --- lld/trunk/ELF/Driver.cpp (original)
> +++ lld/trunk/ELF/Driver.cpp Tue Feb 2 02:22:41 2016
> @@ -95,8 +95,9 @@ void LinkerDriver::addFile(StringRef Pat
> return;
> case file_magic::archive:
> if (WholeArchive) {
> + StringRef S = MBRef.getBufferIdentifier();
> for (MemoryBufferRef MB : getArchiveMembers(MBRef))
> - Files.push_back(createObjectFile(MB));
> + Files.push_back(createObjectFile(MB, S));
> return;
> }
> Files.push_back(make_unique<ArchiveFile>(MBRef));
>
> Modified: lld/trunk/ELF/InputFiles.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/lld/trunk/ELF/InputFiles.cpp?rev=259475&r1=259474&r2=259475&view=diff
>
> ==============================================================================
> --- lld/trunk/ELF/InputFiles.cpp (original)
> +++ lld/trunk/ELF/InputFiles.cpp Tue Feb 2 02:22:41 2016
> @@ -452,8 +452,11 @@ static std::unique_ptr<InputFile> create
> fatal("Invalid file class: " + MB.getBufferIdentifier());
> }
>
> -std::unique_ptr<InputFile> elf2::createObjectFile(MemoryBufferRef MB) {
> - return createELFFile<ObjectFile>(MB);
> +std::unique_ptr<InputFile> elf2::createObjectFile(MemoryBufferRef MB,
> + StringRef ArchiveName) {
> + std::unique_ptr<InputFile> F = createELFFile<ObjectFile>(MB);
> + F->ArchiveName = ArchiveName;
> + return F;
> }
>
> std::unique_ptr<InputFile> elf2::createSharedFile(MemoryBufferRef MB) {
>
> Modified: lld/trunk/ELF/InputFiles.h
> URL:
> http://llvm.org/viewvc/llvm-project/lld/trunk/ELF/InputFiles.h?rev=259475&r1=259474&r2=259475&view=diff
>
> ==============================================================================
> --- lld/trunk/ELF/InputFiles.h (original)
> +++ lld/trunk/ELF/InputFiles.h Tue Feb 2 02:22:41 2016
> @@ -38,6 +38,11 @@ public:
>
> StringRef getName() const { return MB.getBufferIdentifier(); }
>
> + // Filename of .a which contained this file. If this file was
> + // not in an archive file, it is the empty string. We use this
> + // string for creating error messages.
> + std::string ArchiveName;
> +
>
Can this be StringRef? The parent archive's MemoryBufferRef should already
have a StringRef to that same string, so I don't think there are any
lifetime issues.
That will avoid O(#files) std::string heap allocations and (at least for
me) clarify the code by avoiding to need to think about why the lifetime
had to be preserved with std::string.
Or perhaps simpler would be to do `Archive *ParentArchive = nullptr;`
instead of storing ArchiveName. Then you can do
ParentArchive->getFileName().
> protected:
> InputFile(Kind K, MemoryBufferRef M) : MB(M), FileKind(K) {}
> MemoryBufferRef MB;
> @@ -208,7 +213,8 @@ public:
> bool isNeeded() const { return !AsNeeded || IsUsed; }
> };
>
> -std::unique_ptr<InputFile> createObjectFile(MemoryBufferRef MB);
> +std::unique_ptr<InputFile> createObjectFile(MemoryBufferRef MB,
> + StringRef ArchiveName = "");
> std::unique_ptr<InputFile> createSharedFile(MemoryBufferRef MB);
>
> } // namespace elf2
>
> Modified: lld/trunk/ELF/SymbolTable.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/lld/trunk/ELF/SymbolTable.cpp?rev=259475&r1=259474&r2=259475&view=diff
>
> ==============================================================================
> --- lld/trunk/ELF/SymbolTable.cpp (original)
> +++ lld/trunk/ELF/SymbolTable.cpp Tue Feb 2 02:22:41 2016
> @@ -149,17 +149,23 @@ ELFFileBase<ELFT> *SymbolTable<ELFT>::fi
> return nullptr;
> }
>
> +// Returns "(internal)", "foo.a(bar.o)" or "baz.o".
> +template <class ELFT> static std::string getFilename(ELFFileBase<ELFT>
> *F) {
> + if (!F)
> + return "(internal)";
> + if (!F->ArchiveName.empty())
> + return (F->ArchiveName + "(" + F->getName() + ")").str();
> + return F->getName();
> +}
> +
> // Construct a string in the form of "Sym in File1 and File2".
> // Used to construct an error message.
> template <class ELFT>
> std::string SymbolTable<ELFT>::conflictMsg(SymbolBody *Old, SymbolBody
> *New) {
> - ELFFileBase<ELFT> *OldFile = findFile(Old);
> - ELFFileBase<ELFT> *NewFile = findFile(New);
> -
> + ELFFileBase<ELFT> *F1 = findFile(Old);
> + ELFFileBase<ELFT> *F2 = findFile(New);
> StringRef Sym = Old->getName();
> - StringRef F1 = OldFile ? OldFile->getName() : "(internal)";
> - StringRef F2 = NewFile ? NewFile->getName() : "(internal)";
> - return (demangle(Sym) + " in " + F1 + " and " + F2).str();
> + return demangle(Sym) + " in " + getFilename(F1) + " and " +
> getFilename(F2);
> }
>
> // This function resolves conflicts if there's an existing symbol with
>
> Modified: lld/trunk/ELF/Symbols.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/lld/trunk/ELF/Symbols.cpp?rev=259475&r1=259474&r2=259475&view=diff
>
> ==============================================================================
> --- lld/trunk/ELF/Symbols.cpp (original)
> +++ lld/trunk/ELF/Symbols.cpp Tue Feb 2 02:22:41 2016
> @@ -187,7 +187,7 @@ std::unique_ptr<InputFile> Lazy::getMemb
> // read from the library.
> if (MBRef.getBuffer().empty())
> return std::unique_ptr<InputFile>(nullptr);
> - return createObjectFile(MBRef);
> + return createObjectFile(MBRef, File->getName());
> }
>
> template <class ELFT> static void doInitSymbols() {
>
> Added: lld/trunk/test/ELF/Inputs/conflict.s
> URL:
> http://llvm.org/viewvc/llvm-project/lld/trunk/test/ELF/Inputs/conflict.s?rev=259475&view=auto
>
> ==============================================================================
> --- lld/trunk/test/ELF/Inputs/conflict.s (added)
> +++ lld/trunk/test/ELF/Inputs/conflict.s Tue Feb 2 02:22:41 2016
> @@ -0,0 +1,7 @@
> +.globl _Z3muldd, foo, baz
> +_Z3muldd:
> +foo:
> +baz:
> + mov $60, %rax
> + mov $42, %rdi
> + syscall
>
> Modified: lld/trunk/test/ELF/conflict.s
> URL:
> http://llvm.org/viewvc/llvm-project/lld/trunk/test/ELF/conflict.s?rev=259475&r1=259474&r2=259475&view=diff
>
> ==============================================================================
> --- lld/trunk/test/ELF/conflict.s (original)
> +++ lld/trunk/test/ELF/conflict.s Tue Feb 2 02:22:41 2016
> @@ -3,15 +3,21 @@
> # RUN: llvm-mc -filetype=obj -triple=x86_64-unknown-linux %s -o %t
> # RUN: not ld.lld %t %t -o %t2 2>&1 | FileCheck -check-prefix=DEMANGLE %s
>
> -# RUN: not ld.lld %t %t -o %t2 --no-demangle 2>&1 | \
> -# RUN: FileCheck -check-prefix=NO_DEMANGLE %s
> -
> # DEMANGLE: duplicate symbol: {{mul\(double, double\)|_Z3muldd}} in
> # DEMANGLE: duplicate symbol: foo in
>
> +# RUN: not ld.lld %t %t -o %t2 --no-demangle 2>&1 | \
> +# RUN: FileCheck -check-prefix=NO_DEMANGLE %s
> +
> # NO_DEMANGLE: duplicate symbol: _Z3muldd in
> # NO_DEMANGLE: duplicate symbol: foo in
>
> +# RUN: llvm-mc -filetype=obj -triple=x86_64-unknown-linux
> %S/Inputs/conflict.s -o %t2
> +# RUN: llvm-ar rcs %t.a %t2
> +# RUN: not ld.lld %t %t.a -u baz -o %t2 2>&1 | FileCheck
> -check-prefix=ARCHIVE %s
> +
> +# ARCHIVE: duplicate symbol: foo in {{.*}}conflict.s.tmp and
> {{.*}}.a(conflict.s.tmp2)
>
I think this is hardcoding some dependence on the %t names that lit chooses
(e.g. conflict.s.tmp). Could you use %t.o instead of %t2 and then check
{{.*}}.a({{[^)]}}.o) ?
-- Sean Silva
> +
> .globl _Z3muldd, foo
> _Z3muldd:
> foo:
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20160202/0975ff40/attachment.html>
More information about the llvm-commits
mailing list