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

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

[Joel Brobecker]

Hi everyone,

It's been 3 months since our 10.1 release, and we have since then
pushed 8 fixes to the gdb-10-branch, so now is a good time to think
about a corrective release.

The plan would be to create the release as soon as able, so we can
then start thinking about Release 11.1 soon after. For now, here is
a quick status update of the things I know about. Please let me know
if there are other issues that we should consider for this corrective
release.

Thank you!

Fixed Since the Previous Update:
--------------------------------

  N/A for this round (this is the first update for this release ;-) ).

Added Since the Last Update:
----------------------------

  I'll start from the list of bugs filed for 10.2, available via:
  https://sourceware.org/bugzilla/buglist.cgi?bug_status=UNCONFIRMED&bug_status=NEW&bug_status=ASSIGNED&bug_status=SUSPENDED&bug_status=WAITING&bug_status=REOPENED&list_id=24362&product=gdb&query_format=advanced&target_milestone=10.2

  * [TomDV/SimonM/PedroA?] <PR remote/26614>
    AddressSanitizer: heap-use-after-free of extended_remote_target in remote_async_inferior_event_handler
    https://sourceware.org/bugzilla/show_bug.cgi?id=26614

    According to the bug, a patch was pushed on Jan 7th.
    Are there other fixes needed for that one, or can we close?
    (I'll ask on the PR)

  * [SimonM] <PR gdb/26828>
    SIGSEGV in follow_die_offset dwarf2/read.c:22950
    https://sourceware.org/bugzilla/show_bug.cgi?id=26828

    I see that Simon posted a patch series that the bug reporter
    confirmed fixes the issue; 2 patches were pushed, and 2 remain.

    [PATCH 3/4] gdb/dwarf: don't enqueue CU in maybe_queue_comp_unit if already expanded
    https://sourceware.org/pipermail/gdb-patches/2021-January/175265.html
    (asking TomT if he has an opinion)

    [PATCH 4/4] gdb/dwarf: create and destroy dwarf2_per_bfd's CUs-to-expand queue
    https://sourceware.org/pipermail/gdb-patches/2020-December/173964.html
    Already approved by TomT

  * [<UNASSIGNED>] <PR backtrace/27147>
    [GNU/Linux, sparc64] GDB is unable to print full stack trace (got "previous frame inner to this frame" errors)
    https://sourceware.org/bugzilla/show_bug.cgi?id=27147

    As the subject says, it's an unwinder issue.
    Simon was able to reproduce, and tied this issue to a specific
    commit adding multi-target support.

    It's a 9->10 regression, so it would be nice to fix, but the risk
    at the moment, is that the PR is currently not assigned, so there is
    no "champion" to look after it.

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

Other Ongoing Items:
--------------------

  < none :) >

Not Critical, but Requested:
----------------------------

  < none :) >

-- 
Joel

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

[Simon Marchi via Gdb-patches]

On 2021-01-31 1:45 a.m., Joel Brobecker wrote:
>   * [SimonM] <PR gdb/26828>
>     SIGSEGV in follow_die_offset dwarf2/read.c:22950
>     https://sourceware.org/bugzilla/show_bug.cgi?id=26828
> 
>     I see that Simon posted a patch series that the bug reporter
>     confirmed fixes the issue; 2 patches were pushed, and 2 remain.
> 
>     [PATCH 3/4] gdb/dwarf: don't enqueue CU in maybe_queue_comp_unit if already expanded
>     https://sourceware.org/pipermail/gdb-patches/2021-January/175265.html
>     (asking TomT if he has an opinion)

Indeed, I am ideally waiting for Tom to OK it.  If he doesn't have time,
I am reasonably confident with the patch to merge it.  If we think he
won't have time, I think it's better to merge it earlier than later,
that gives more time for any potential problem to surface.

>   * [<UNASSIGNED>] <PR backtrace/27147>
>     [GNU/Linux, sparc64] GDB is unable to print full stack trace (got "previous frame inner to this frame" errors)
>     https://sourceware.org/bugzilla/show_bug.cgi?id=27147
> 
>     As the subject says, it's an unwinder issue.
>     Simon was able to reproduce, and tied this issue to a specific
>     commit adding multi-target support.
> 
>     It's a 9->10 regression, so it would be nice to fix, but the risk
>     at the moment, is that the PR is currently not assigned, so there is
>     no "champion" to look after it.
> 
> 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?

Simon

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

[Joel Brobecker]

> > 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

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

[Simon Marchi via Gdb-patches]

On 2021-02-03 12:50 a.m., Joel Brobecker wrote:
> 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?

I think the loc and rng changes fall in the same category, and kind of
mirror of each other.

Although there are loclist attributes in DWARF 4, the .debug_loclists
section is new to DWARF 5 (DWARF 4 had .debug_loc).  In the end, the
format of one location list hasn't changed much, but the format of the
section (the container of the location lists) changed quite a bit.  My
fixes have to do with parsing of the container, which is new stuff.

And it's pretty much the same with ranges, in DWARF 4, you had
.debug_ranges, in DWARF 5 you have .debug_rnglists.  My fixes have to do
with parsing the headers and finding the right list, not parsing the
lists themselves.

> 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...

Yes, and if I am not mistaken, this is new DWARF 5-only code to read the
DW_FORM_loclistx form, so it shouldn't affect DWARF 4.

> 
>   [PATCH 02/13] gdb/dwarf: fix bound check in read_rnglist_index
> 
>         OK for gdb-10-branch

Same, only affects new DWARF 5 stuff.

> 
>   [PATCH 03/13] gdb/dwarf: add missing bound check to read_loclist_index
> 
>         Seems straightforward and adds safety; OK for gdb-10-branch.

Same.

> 
>   [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.

Again, it only affects new DWARF 5 stuff.  But indeed it's really not
necessary, I'll skip it.

> 
>   [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.

Actually, this patch fixes a problem that was introduced by a refactor
that happened after GDB 10, the one that added methods on struct
attribute, 529908cbd0af ("Remove DW_UNSND").  So it's not relevant on
the 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).

If that makes it feel safer, this code is only on the code path when
reading a DW_FORM_rnglistx or DW_FORM_loclistx attribute, which is new
DWARF 5 stuff.  It should not be invoked when reading DWARF 4.

And it is an important one that can't be skipped, it's the main fix of
the series.  Without it, let's say we have two compilation units, each
with a contribution to .debug_rnglists, when reading a DW_FORM_rnglistx
attribute from the second compilation unit, we read the header of the
.debug_rnglists contribution of the first compilation unit.

>   [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.

This one is also not relavant on the gdb-10-branch, because it fixes a
regression also introduced by the "Remove DW_UNSND" commit.


>   [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.

Yeah, it's all tests, so it helps ensure that the backport works fine.

>   [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?

Indeed, probably not relevant for a backport since it fixes a corner
case not likely to happen.

>   [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?

I'll skip it, it doesn't change anything in practice.

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

Ok.

So this is what I'll push after regtesting a bit:

  - gdb/dwarf: change read_loclist_index complaints into errors
  - gdb/dwarf: fix bound check in read_rnglist_index
  - gdb/dwarf: add missing bound check to read_loclist_index
  - gdb/dwarf: read correct rnglist/loclist header in read_{rng,loc}list_index
  - gdb/testsuite: add .debug_rnglists tests
  - gdb/testsuite: DWARF assembler: add context parameters to _location
  - gdb/testsuite: add .debug_loclists tests
  - gdb/testsuite: add test for .debug_{rng,loc}lists section without offset array

Thanks!

Simon

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

[Simon Marchi via Gdb-patches]

On 2021-02-03 10:48 a.m., Simon Marchi via Gdb-patches wrote:
> So this is what I'll push after regtesting a bit:
> 
>   - gdb/dwarf: change read_loclist_index complaints into errors
>   - gdb/dwarf: fix bound check in read_rnglist_index
>   - gdb/dwarf: add missing bound check to read_loclist_index
>   - gdb/dwarf: read correct rnglist/loclist header in read_{rng,loc}list_index
>   - gdb/testsuite: add .debug_rnglists tests
>   - gdb/testsuite: DWARF assembler: add context parameters to _location
>   - gdb/testsuite: add .debug_loclists tests
>   - gdb/testsuite: add test for .debug_{rng,loc}lists section without offset array

This is now pushed.

Simon

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

[Joel Brobecker]

> > So this is what I'll push after regtesting a bit:
> > 
> >   - gdb/dwarf: change read_loclist_index complaints into errors
> >   - gdb/dwarf: fix bound check in read_rnglist_index
> >   - gdb/dwarf: add missing bound check to read_loclist_index
> >   - gdb/dwarf: read correct rnglist/loclist header in read_{rng,loc}list_index
> >   - gdb/testsuite: add .debug_rnglists tests
> >   - gdb/testsuite: DWARF assembler: add context parameters to _location
> >   - gdb/testsuite: add .debug_loclists tests
> >   - gdb/testsuite: add test for .debug_{rng,loc}lists section without offset array
> 
> This is now pushed.

Perfect. Thanks Simon.

-- 
Joel