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

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/clang-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/clang-cmake-aarch64-39vma/llvm/tools/lld/ELF/Writer.cpp:38
*>>>*            lib/liblldELF.a(Writer.cpp.o)
*>>> defined at*
/home/buildslave/buildslave/clang-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
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20170324/008af5d3/attachment.html>

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

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.

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/clang-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/clang-cmake-aarch64-39vma/
> llvm/tools/lld/ELF/Writer.cpp:38
> *>>>*            lib/liblldELF.a(Writer.cpp.o)
> *>>> defined at*
/home/buildslave/buildslave/clang-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
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20170324/11678f44/attachment.html>

On Fri, Mar 24, 2017 at 4: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
>
> 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.
>
That's an interesting idea, and I think I like it.

> 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
>>
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20170324/4839f6d9/attachment.html>

> On Mar 24, 2017, at 4:22 PM, Reid Kleckner via llvm-dev <llvm-dev at
lists.llvm.org> 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
I find this really nicely readable!

— 
Mehdi


> 
> 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.
> 
> On Fri, Mar 24, 2017 at 4:07 PM, Rui Ueyama via llvm-dev <llvm-dev at
lists.llvm.org <mailto:llvm-dev at lists.llvm.org>> wrote:
> On Fri, Mar 24, 2017 at 2:04 PM, Sean Silva <chisophugis at gmail.com
<mailto: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/clang-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/clang-cmake-aarch64-39vma/llvm/tools/lld/ELF/Writer.cpp:38
> >>>            lib/liblldELF.a(Writer.cpp.o)
> >>> defined at
/home/buildslave/buildslave/clang-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 <mailto: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 <mailto:llvm-dev at lists.llvm.org>
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
<http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev>
> 
> 
> 
> 
> _______________________________________________
> LLVM Developers mailing list
> llvm-dev at lists.llvm.org <mailto:llvm-dev at lists.llvm.org>
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
<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
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20170324/26315922/attachment.html>

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

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
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20170324/b2267d5e/attachment-0001.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: Screenshot_2017-03-24-22-23-53.png
Type: image/png
Size: 118228 bytes
Desc: not available
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20170324/b2267d5e/attachment-0001.png>