Re: GDB 10.2 release (respin) -- 2021-01-31 Update

[Joel Brobecker <brobecker@adacore.com>]

> > Simon also requested we consider:
> > 
> >   * [SimonM] <PR symtab/26813>
> >     DW_FORM_rnglistx and DW_FORM_loclistx not fully supported
> >     https://sourceware.org/bugzilla/show_bug.cgi?id=26813
> > 
> >     Simon posted a patch series on Jan 20th:
> >     [PATCH 00/13] DWARF 5 rnglists & loclists fixes (PR 26813)
> >     https://sourceware.org/pipermail/gdb-patches/2021-January/175221.html
> 
> This is now pushed on master.  Do I have your OK to backport it?  My
> argument for backporting it is that support for processing these DWARF5
> attributes was new in GDB 10, but GDB chokes on any non-trivial use of
> them (like when there are two compile units using rnglists).
> 
> Not all patches are tagged with PR 26813 (because they are
> cleanups/refactor), but the important ones, that contain the actual
> fixes, do have it.  Is that ok for the release branch?

I don't know if I'll be able to provide a well educated answer,
but here are some thoughts and questions.

Generally speaking, anything that touches purely on rnglist
seems OK to backport because I understand this support is entirely
new. I'm a little less sure about the changes to liclist support,
though. Is there something I don't know that puts the changes to
loclist handling in the same category as the rnglist changes?

Is it possible to skip some patches that are not strictly necessary,
and if yes, would that actually be a good idea?

With that in mind, my best thoughts on the matter so far:

  [PATCH 01/13] gdb/dwarf: change read_loclist_index complaints into errors

        Although unnecessary, I think this one is fine, and perhaps
        even desirable to avoid some weird behavior in GDB...

  [PATCH 02/13] gdb/dwarf: fix bound check in read_rnglist_index

        OK for gdb-10-branch

  [PATCH 03/13] gdb/dwarf: add missing bound check to read_loclist_index

        Seems straightforward and adds safety; OK for gdb-10-branch.

  [PATCH 04/13] gdb/dwarf: remove unnecessary check in read_{rng,loc}list_index

        Maybe drop this patch, on the basis that this is just
        a cleanup that should, in fine, be a no-op in practice.

  [PATCH 05/13] gdb/dwarf: few fixes for handling DW_FORM_{rng,loc}listx

        After verification in the DWARF 5 standard that those are
        unsigned ULEBs, this one looks good to me for gdb-10-branch.

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

        I will need to trust you on that one, as I think I would need
        to delve more deeply into DWARF 5, and I'm lacking the time
        (at least until this weekend).

  [PATCH 07/13] gdb/dwarf: read DW_AT_ranges value as unsigned in partial_die_info::read

        A little scarier for me, but the justification that we are
        already doing this in dwarf2_get_pc_bounds is convincing.
        OK for gdb-10-branch.

  [PATCH 08/13] gdb/testsuite: add .debug_rnglists tests
  [PATCH 09/13] gdb/testsuite: DWARF assembler: add context parameters to _location
  [PATCH 10/13] gdb/testsuite: add .debug_loclists tests

        No problem for me.

  [PATCH 11/13] gdb/dwarf: split dwarf2_cu::ranges_base in two

        Do we need this? Looks like you are saying that this is
        an enhancement for a case that is unlikely to happen
        in practice?

  [PATCH 12/13] gdb/dwarf: make read_{loc, rng}list_index return sect_offset

        OK for gdb-10-branch, with perhaps a question regarding
        the gains-vs-risks ratio?

  [PATCH 13/13] gdb/testsuite: add test for .debug_{rng, loc}lists section without offset array

        No problem for me.

-- 
Joel