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

Wed Mar 29 12:13:08 PDT 2017

Something to maybe keep in mind is that we might "want" to emit the
errors/notes in Visual Studio format (sorry :-P)

If a line starts with:
Directory/File.cpp(42): error, blabla!

It will be picked up by the error list window and also be double-clickable
in the output window.

I'm not asking for a feature request, but it would be nice to have this in
mind when designing the API for error reporting.

Thank you,

 Filipe

On Wed, 29 Mar 2017 at 19:53, Rui Ueyama via llvm-dev <
llvm-dev at lists.llvm.org> wrote:

> Put it all together, the following error messages should work for
> everybody. I'll create a patch to make this change and send it for review.
> Thank you guys for the inputs!
>
>
> Undefined symbol error:
>
> bin/ld.lld: error: undefined symbol:
> lld::elf::EhFrameSection<llvm::object::ELFType<(llvm::support::endianness)0,
> true>
> *>>> defined at* Writer.cpp:207 (/ssd/llvm-project/lld/ELF/Writer.cpp:207)
> *>>>*            Writer.cpp.o in archive lib/liblldELF.a
>
>
> Duplicate symbol error:
>
> 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/clang-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/clang-cmake-aarch64-39vma/llvm/tools/lld/ELF/SyntheticSections.cpp:673)
> *>>>*            SyntheticSections.cpp.o in archive lib/liblldELF.a
>
>
> On Tue, Mar 28, 2017 at 5:58 PM, Michael Spencer via llvm-dev <
> llvm-dev at lists.llvm.org> wrote:
>
> On Sat, Mar 25, 2017 at 6:56 AM, Hal Finkel via llvm-dev <
> llvm-dev at lists.llvm.org> 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/
> clang-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/
> clang-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
>
>
> +1
>
> - Michael Spencer
>
>
>
>
> 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/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
>
>
>
>
>
> _______________________________________________
> 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
>
>
> _______________________________________________
> 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 list
> llvm-dev at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
>
-- 
  F
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20170329/423a3d6a/attachment.html>