[llvm-dev] [RFC] better link error messages

Sat Mar 25 14:42:10 PDT 2017

I hadn't noticed that detail, but I totally agree that the full path should
be present in the diagnostic.

On Mar 25, 2017 7:56 AM, "Hal Finkel" <hfinkel at anl.gov> wrote:

>
> On 03/24/2017 11:42 PM, Sean Silva via llvm-dev wrote:
>
>
>
> On Mar 24, 2017 5:22 PM, "Reid Kleckner" <rnk at google.com> wrote:
>
> I figured you might consider moving the basenames of the filename earlier
> in the diagnostic, something like:
>
> bin/ld.lld: *error:* duplicate symbol: lld::elf::MipsGotSection::addEntry(lld::elf::SymbolBody&,
> long, lld::elf::RelExpr)
> *>>> defined at* Writer.cpp:38 in /home/buildslave/buildslave/
> clang-cmake-aarch64-39vma/llvm/tools/lld/ELF/
> *>>>*            Writer.cpp.o in archive lib/liblldELF.a
> *>>> defined at* SyntheticSections.cpp:673 in /home/buildslave/buildslave/
> clang-cmake-aarch64-39vma/llvm/tools/lld/ELF/
> *>>>*            SyntheticSections.cpp.o in archive lib/liblldELF.a
>
>
> Please don't do this (split the base name from the path). Tools that match
> file names, grep, etc. won't be able to locate the files easily. I've
> worked with projects that, especially when all library dependencies are
> included, have lots of files with generic names (e.g. Writer.cpp). This is
> convenient for relatively self-contained projects where you can recognize
> the file names. We work on one such project. This is not the general case.
>
> If you'd like to make the base name easier to visually differentiate, I
> recommend just repeating it:
>
> bin/ld.lld: *error:* duplicate symbol: lld::elf::MipsGotSection::addEntry(lld::elf::SymbolBody&,
> long, lld::elf::RelExpr)
> *>>> defined at* Writer.cpp:38 (/home/buildslave/buildslave/c
> lang-cmake-aarch64-39vma/llvm/tools/lld/ELF/Writer.cpp:38)
> *>>>*            Writer.cpp.o in archive lib/liblldELF.a
> *>>> defined at* SyntheticSections.cpp:673 (/home/buildslave/buildslave/c
> lang-cmake-aarch64-39vma/llvm/tools/lld/ELF/SyntheticSections.cpp:673)
> *>>>*            SyntheticSections.cpp.o in archive lib/liblldELF.a
>
> I'd be happy with that.
>
> Thanks again,
> Hal
>
>
> Oftentimes filenames are unique enough that it tells you where to go
> immediately. Maybe it's a bad idea for the source location info, though.
> You want that to be copy-pastable or clickable in an IDE.
>
>
> I really like this.
>
> I'm on mobile right now which is a worst case, but this is pretty readable
> even there. I've attached a screenshot. One nit is that we want it to be
> clear that Writer.cpp.o is in an archive before reading "Writer.cpp.o".
> Maybe something like "archive member Writer.cpp.o in ...".
>
> I also think we might want some bold text for the input file lines to
> clarify better what they are. Maybe ">>> *from* *input* *file*: archive
> member ..." instead of just a blank ">>> archive member ..."
>
> -- Sean Silva
>
>
> On Fri, Mar 24, 2017 at 4:07 PM, Rui Ueyama via llvm-dev <
> llvm-dev at lists.llvm.org> wrote:
>
>> On Fri, Mar 24, 2017 at 2:04 PM, Sean Silva <chisophugis at gmail.com>
>> wrote:
>>
>>> I lile the idea of having it more structured and I think your suggested
>>> format is the right direction.
>>>
>>> I think one principle should be that we assume that file names and
>>> symbol names are "really long" (possibly wrapped by the terminal etc.).
>>>
>>
>> Right. That's what we should expect.
>>
>> I believe the current error message format for the existing linkers were
>> set more than 30 years ago when path names and symbol names were much
>> shorter than they are today. If they are short, error messages become
>> something like "src/libfoo/bar.o: undefined symbol: strbar", which is quite
>> easy to read. That is unfortunately no longer the case because we are
>> creating significantly larger programs than a few decades ago and C++ name
>> mangling makes symbol names significantly longer than before.
>>
>> So I would modify your suggested format to use the "note" color for the
>>> strings like "Object 1" so that even if those lines are wrapped by the
>>> terminal then they can still be easily visually parsed. We may also want to
>>> do something like ">>> Object 1:" so that places without color (like
>>> google's internal web ui for build logs, or if users are piping the build
>>> system's output into `less` or `tee`) are still a bit easier to parse in
>>> the presence of wrapped lines.
>>>
>>
>> Makes sense. I tried a few different formats based on suggestions from
>> Mehdi, Hans and you, and come up with this one. What do you think?
>>
>> bin/ld.lld: *error:* undefined symbol: lld::elf::EhFrameSection<llvm:
>> :object::ELFType<(llvm::support::endianness)0, true>
>> >::addSection(lld::elf::InputSectionBase*)
>> *>>> referenced by* /home/buildslave/buildslave/cl
>> ang-cmake-aarch64-39vma/llvm/tools/lld/ELF/Writer.cpp:207
>> *>>>*               lib/liblldELF.a(Writer.cpp.o)
>>
>>
>> bin/ld.lld: *error:* duplicate symbol: lld::elf::MipsGotSection::addEntry(lld::elf::SymbolBody&,
>> long, lld::elf::RelExpr)
>> *>>> defined at* /home/buildslave/buildslave/cl
>> ang-cmake-aarch64-39vma/llvm/tools/lld/ELF/Writer.cpp:38
>> *>>>*            lib/liblldELF.a(Writer.cpp.o)
>> *>>> defined at* /home/buildslave/buildslave/cl
>> ang-cmake-aarch64-39vma/llvm/tools/lld/ELF/SyntheticSections.cpp:673
>> *>>>*            lib/liblldELF.a(SyntheticSections.cpp.o)
>>
>>
>> This format prints out source file names and object file names in
>> different lines. I found that that is easier to read than print them in the
>> same line, as error messages with some amount of whitespace are easier to
>> find in dense build system outputs.
>>
>> This principle also suggests (and your suggested format does this) that
>>> to avoid having to parse past a symbol/file name to get to other
>>> information, there should be at most one symbol/file name on a given line
>>> and there should never be anything after it on the line. The ld64 format
>>> violates this, for example.
>>>
>>> -- Sean Silva
>>>
>>> On Mar 23, 2017 4:18 PM, "Rui Ueyama via llvm-dev" <
>>> llvm-dev at lists.llvm.org> wrote:
>>>
>>> Folks,
>>>
>>> I'd like propose a new error message format for LLD so that error
>>> message for undefined or duplicated symbols are more informative and easy
>>> to read.
>>>
>>> Below are examples of the current error messages (note that characters
>>> in red are actually red on terminal):
>>>
>>> *Undefined symbols*
>>> /ssd/clang/bin/ld.lld: error: /ssd/llvm-project/lld/ELF/Writer.cpp:207:
>>> undefined symbol 'lld::elf::EhFrameSection<llvm
>>> ::object::ELFType<(llvm::support::endianness)0, true>
>>> >::addSection(lld::elf::InputSectionBase*)'
>>>
>>> *Conflicting symbols*
>>> /ssd/clang/bin/ld.lld: error: /ssd/llvm-project/lld/ELF/Writer.cpp:38:
>>> duplicate symbol 'lld::elf::MipsGotSection::addEntry(lld::elf::SymbolBody&,
>>> long, lld::elf::RelExpr)'
>>> /ssd/clang/bin/ld.lld: error: /ssd/llvm-project/lld/ELF/SyntheticSections.cpp:673:
>>> previous definition was here
>>>
>>>
>>> For each error, we want to print out information about 1) symbol name,
>>> 2) source file name(s) and source location(s) if available and 3) source
>>> object file name(s) and archive file name(s) if available. Currently, these
>>> messages lack object file names, and I think the above error messages are a
>>> bit hard to grasp because too much information is packed into each line.
>>>
>>> I'm thinking of changing the format to something like the following:
>>>
>>> *Undefined symbols*
>>> /ssd/clang/bin/ld.lld: error: undefined symbol:
>>> lld::elf::EhFrameSection<llvm::object::ELFType<(llvm::support::endianness)0,
>>> true> >::addSection(lld::elf::InputSectionBase*)
>>>   Source: /ssd/llvm-project/lld/ELF/Writer.cpp:207
>>>   Object: lib/liblldELF.a(Writer.cpp.o)
>>>
>>> *Conflicting symbols*
>>> /ssd/clang/bin/ld.lld: error: duplicate symbol:
>>> lld::elf::MipsGotSection::addEntry(lld::elf::SymbolBody&, long,
>>> lld::elf::RelExpr)
>>>   Source 1: /ssd/llvm-project/lld/ELF/Writer.cpp:38
>>>   Source 2: /ssd/llvm-project/lld/ELF/SyntheticSections.cpp:673
>>>   Object 1: lib/liblldELF.a(Writer.cpp.o)
>>>   Object 2 : lib/liblldELF.a(SyntheticSections.cpp.o)
>>>
>>>
>>> The new error messages contain complete information that the linker is
>>> able to gather, and it uses more vertical space to improve readability.
>>>
>>> Thoughts?
>>>
>>> _______________________________________________
>>> LLVM Developers mailing list
>>> llvm-dev at lists.llvm.org
>>> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
>>>
>>>
>>>
>>
>> _______________________________________________
>> LLVM Developers mailing list
>> llvm-dev at lists.llvm.org
>> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
>>
>>
>
>
>
> _______________________________________________
> LLVM Developers mailing listllvm-dev at lists.llvm.orghttp://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
>
>
> --
> Hal Finkel
> Lead, Compiler Technology and Programming Languages
> Leadership Computing Facility
> Argonne National Laboratory
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20170325/6d40bdb3/attachment.html>