Re: [PATCH 06/13] gdb/dwarf: read correct rnglist/loclist header in read_{rng,loc}list_index

[Zoran Zaric via Gdb-patches <gdb-patches@sourceware.org>]

> On 2021-01-28 10:39 a.m., Zoran Zaric wrote:>> From: Simon Marchi <simon.marchi@efficios.com>
>>>
>>> When loading the binary from PR 26813 in GDB, we get:
>>>
>>>       DW_FORM_rnglistx index pointing outside of .debug_rnglists offset array [in module /home/simark/build/binutils-gdb/gdb/MagicPurse]
>>>
>>> ... and the symbols fail to load.
>>>
>>> In read_rnglist_index and read_loclist_index, we read the header
>>> (documented in sections 7.28 and 7.29 of DWARF 5) of the CU's
>>> contribution to the .debug_rnglists / .debug_loclists sections to
>>> validate that the index we want to read makes sense.  However, we always
>>> read the header at the beginning of the section, rather than the header
>>> for the contribution from which we want to read the index.
>>>
>>> To illustrate, here's what the binary from PR 26813 contains.  There are
>>> two compile units:
>>>
>>> 0x0000000c: DW_TAG_compile_unit 1
>>>                 DW_AT_ranges [DW_FORM_rnglistx]: 0x0
>>>                 DW_AT_rnglists_base [DW_FORM_sec_offset]: 0xC
>>>
>>> 0x00003ec9: DW_TAG_compile_unit 2
>>>                 DW_AT_ranges [DW_FORM_rnglistx]: 0xB
>>>                 DW_AT_rnglists_base [DW_FORM_sec_offset]: 0x85
>>>
>>> The layout of the .debug_rnglists is the following:
>>>
>>>       [0x00, 0x0B]: header for CU 1's contribution
>>>       [0x0C, 0x0F]: list of offsets for CU 1 (1 element)
>>>       [0x10, 0x78]: range lists data for CU 1
>>>
>>>       [0x79, 0x84]: header for CU 2's contribution
>>>       [0x85, 0xB4]: list of offsets for CU 2 (12 elements)
>>>       [0xB5, 0xBD7]: range lists data for CU 2
>>>
>>> The DW_AT_rnglists_base attrbute points to the beginning of the list of
>>> offsets for that CU, relative to the start of the .debug_rnglists
>>> section.  That's right after the header for that contribution.
>>>
>>> When we try to read the DW_AT_ranges attribute for CU 2,
>>> read_rnglist_index reads the header for CU 1 instead of the one for CU
>>> 2.  Since there's only one element in CU 1's offset list, it believes
>>> (wrongfully) that the index 0xB is out of range.
>>>
>>> Fix it by reading the header just before where DW_AT_rnglists_base
>>> points to.  With this patch, I am able to load GDB built with clang-11
>>> and -gdwarf-5 in itself, with and without -readnow.
>>>
>>
>> I came to the same conclusion, the issue was probably never noticed because the tested compilers only ever generated one rangelist entry.
>>
>> Even LLVM didn't seem to generate it before version 11, or at least I haven't noticed the issue when I was previously testing the DWARF 5 support.
> 
> Indeed, and I think it was only used on the compilation unit's DIE?

I think you are right.

>>> Change-Id: Ie53ff8251af8c1556f0a83a31aa8572044b79e3d
>>>
>>
>> Are these Gerrit change ID's? They shouldn't be part of the patches right?
>>
>> Zoran
> 
> I do use Gerrit personally to track the patches I work on, their
> versions, which ones have been merged and which ones haven't.  When I
> push the patches upstream and then sync my Gerrit instance's master
> branch, it automatically sees which patches have been merged in the
> master branch and closes the corresponding reviews.  So keeping the
> Gerrit Change-Id really helps me, and I don't think it bothers anyone,
> so I leave them there.
> 
> Simon
> 

Makes sense. I didn't notice change ID's on some other reviews so I 
wasn't sure what is the policy about those.

Zoran