[lld] r259475 - ELF: Include archive names in error messages.
Rui Ueyama via llvm-commits
llvm-commits at lists.llvm.org
Tue Feb 2 12:17:16 PST 2016
On Tue, Feb 2, 2016 at 12:13 PM, Sean Silva <chisophugis at gmail.com> wrote:
> 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().
>
Yeah, I think this can be StringRef.
This however cannot be Archive* because for archives in --whole-archive
--no-whole-archive, we do not create Archive objects. We just extract all
files from archives and pass all of them to the symbol table, so there's no
need to instantiate Archive for such archives.
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/afac1956/attachment.html>
More information about the llvm-commits
mailing list