Re: bugs in external debug info support in libbacktrace

[Ian Lance Taylor <iant at golang dot org>]

On Mon, Nov 27, 2017 at 2:23 AM, Milian Wolff <milian.wolff@kdab.com> wrote:
>
> I was made aware that libbacktrace got support for external debug info with
> [1], great work! I have just synced the latest libbacktrace into heaptrack [2]
> in a local branch and played around with it and noticed two limitations:
>
> [1]: https://github.com/gcc-mirror/gcc/commit/
> b919941efc58035debbcf69b645c072b7dd6ba4e
> [2]: https://github.com/KDE/heaptrack
>
> a) elf_open_debugfile_by_debuglink checks the crc, even if it is not provided
> by the debug file. I.e. I have a file where `debuglink_crc == 0`, but the
> got_crc calculated from elf_crc32_file is non-zero. I have patched this
> locally with the following to make it work for me:
>
> diff --git a/libbacktrace/elf.c b/libbacktrace/elf.c
> index 06823fcf59b..24bf58728fd 100644
> --- a/libbacktrace/elf.c
> +++ b/libbacktrace/elf.c
> @@ -1005,7 +1005,7 @@ elf_open_debugfile_by_debuglink (struct backtrace_state
> *state,
>    if (ddescriptor < 0)
>      return -1;
>
> -  got_crc = elf_crc32_file (state, ddescriptor, error_callback, data);
> +  got_crc = debuglink_crc ? elf_crc32_file (state, ddescriptor,
> error_callback, data) : 0;
>    if (got_crc != debuglink_crc)
>      {
>        backtrace_close (ddescriptor, error_callback, data);

Thanks.  I fixed this problem with a slightly different patch,
appended.  Committed to mainline.


> b) elf_add guards the code to inspect the symtab-shndx with a `&& !debuginfo`
> check in loc 2797. This results in all files with separate debug info yielding
> `found_sym = 0` when calling elf_add, and symbol resolution is broken.
> Personally I have patched this check out to make symbol resolution work for
> me:
>
> diff --git a/libbacktrace/elf.c b/libbacktrace/elf.c
> index 06823fcf59b..6876bd3ed8e 100644
> --- a/libbacktrace/elf.c
> +++ b/libbacktrace/elf.c
> @@ -2794,7 +2794,7 @@ elf_add (struct backtrace_state *state, const char
> *filename, int descriptor,
>
>    if (symtab_shndx == 0)
>      symtab_shndx = dynsym_shndx;
> -  if (symtab_shndx != 0 && !debuginfo)
> +  if (symtab_shndx != 0)
>      {
>        const b_elf_shdr *symtab_shdr;
>        unsigned int strtab_shndx;
>
> Could you please check whether the two patches above could be upstreamed?

I don't think that's the right approach.  In the appended patch I skip
clearing *found_sym if debuginfo is set.  I hope that will fix the
problem.

Ian

2018-01-25  Ian Lance Taylor  <iant@golang.org>

* elf.c (elf_open_debugfile_by_debuglink): Don't check CRC if the
desired CRC is zero.
(elf_add): Don't clear *found_sym and *found_dwarf if debuginfo.

Attachment: patch.txt
Description: Text document