Re: [PATCH] Fix issues with reading rnglists, especially from dwo files, for DWARF v5

[Caroline Tice <cmtice@google.com>]

Ping?  Could somebody please review this?

-- Caroline
cmtice@google.com


On Tue, Jun 9, 2020 at 4:32 PM Caroline Tice <cmtice@google.com> wrote:

> While playing with this a bit more (getting ready to work on my .dwp
> file patch), I noticed that there was an important bit I left out of
> the original patch: dwarf2_rnglists_process, in the original code,
> always reads out of the rnglist section in the main objfile, even when
> there is a .rnglist section in a .dwo file that it should be reading
> instead.  I had some local code that fixes this, but it didn't make it
> into my previous patch (I apologize for that).  The attached patch has
> been updated to contain this fix as well.  It also contains my fixes
> and testcase from the previous patch.
>
> I ran the testsuite again (adding the -gdwarf-5 and -gsplit-dwarf
> flags in testsuite/lib/gdb.exp), and compiling everything with clang:
>
> $ make check RUNTESTFLAGS="CC_FOR_TARGET=${CLANG_CC}
> CXX_FOR_TARGET=${CLANG_CXX}"
>
> This time my patched GDB passed quite a few tests that the unpatched
> version failed (when compiled with clang and passed the flags):
>
> Testsuite summary from unpatched GDB:
>
> === gdb Summary ===
> # of expected passes 55725
> # of unexpected failures 2956
> # of unexpected successes 83
> # of expected failures 169
> # of known failures 57
> # of unresolved testcases 115
> # of untested testcases 175
> # of unsupported tests 272
> # of paths in test names 1
> # of duplicate test names 269
>
>
> Testsuite summary from patched GDB:
>
> === gdb Summary ===
>
> # of expected passes 56173
> # of unexpected failures 2523
> # of unexpected successes 2
> # of expected failures 250
> # of known failures 57
> # of unresolved testcases 111
> # of untested testcases 174
> # of unsupported tests 272
> # of paths in test names 1
> # of duplicate test names 269
>
> Some of the tests that have more passes with the patch include:
> gdb.base/break-entry.exp, gdb.base/info-fun.exp,
> gdb.base/info-shared.exp,gdb.base/return-nodebug.exp,
> gdb.dwarf2/fission-base.exp
> (I can send you a complete list if you really want it)
>
> Anyway below is the updated patch for review.
>
> -- Caroline
> cmtice@google.com
>
> gdb/ChangeLog:
>
> 2020-06-09  Caroline Tice  <cmtice@google.com>
>
>         * dwarf2/read.c (struct dwop_section_names): Add rnglists_dwo.
>         (dwop_section_names): Add .debug_rnglists.dwo,
> .zdebug_rnglists.dwo.
>         (struct dwarf2_cu): Add cu_ranges_from_skeleton field.
>         (struct dwo_sections): Add rnglists field.
>         (dwarf2_ranges_read): Add tag parameter.
>         (cu_debug_rnglist_section): New function (decl & definition).
>         (cutu_reader::cutu_reader): Before replacing the skeleton unit
>         comp_unit_die with the dwo comp_unit_die, check to see if the
> skeleton
>         unit die has a DW_AT_ranges, and if so set the
> cu_ranges_from_skeleton
>         field in the cu.
>         (dwarf2_locate_dwo_sections): Add code to read rnglists_dwo
> section.
>         (dwarf2_rnglists_process): Add a dwarf_tag parameter, for the kind
> of
>         die whose range is being checked; add code to read the rnglist
> section
>         from the dwo file rather than from the main objfile, if
> appropriate.
>         Add cases for DW_RLE_base_addressx,
>         DW_RLE_startx_length, DW_RLE_startx_endx.  Also, update to only add
>         the base address to DW_RLE_offset_pairs (not to all ranges).
>         (dwarf2_ranges_process): Add dwarf tag parameter and pass it to
>         dwarf2_rnglists_process.
>         (dwarf2_ranges_read): Add dwarf tag parameter and pass it to
>         dwarf2_ranges_process.
>         (dwarf2_get_pc_bounds): Check for DW_FORM_rnglistx when setting
>         need_ranges_base.  Also pass die tag to dwarf2_ranges_read.
>         (dwarf2_record_block_ranges): Check for DW_FORM_rnglistx when
> setting
>         need_ranges_base.  Also pass die tag to dwarf2_ranges_read.
>         (read_full_die_1): Add code to read DW_AT_rnglists_base and assign
> to
>         cu->ranges_base.
>         (partial_die_info::read): Check for DW_FORM_rnglistx when setting
>         need_ranges_base.  Also pass die tag to dwarf2_ranges_read.
>         (read_rnglist_index): New function.
>         (read_attribute_reprocess):  Add code for DW_FORM_rnglistx.
>         (read_attribute_value): Mark DW_FORM_rnglistx with need_reprocess.
>
> gdb/testsuite/ChangeLog:
>
> 2020-06-09  Caroline Tice  <cmtice@google.com>
>
>         * gdb.dwarf2/dw5-rnglist-test.cc: New file.
>         * gdb.dwarf2/dw5-rnglist-test.exp: New file.
>
>
> On Thu, Jun 4, 2020 at 2:39 PM Caroline Tice <cmtice@google.com> wrote:
> >
> > I've been playing with running the testsuite with -gdwarf-5
> > -gsplit-dwarf and using the clang compiler. I did not find any tests
> > that showed the issues I was running into (but I did discover a small
> > bug in my patch, which my newer patch fixes).  With my fixed patch,
> > there are no testsuite differences.  I have created a new testsuite
> > test case, which does fail with current ToT and passes with my patch,
> > but that only works if you compile it with clang -- if you compile it
> > with GCC it passes in both cases (because GCC is not generating the
> > DW_FORM_rmglistx that my patch is handling/fixing).
> >
> > I've updated the patch to include the requested format changes, remove
> > the pieces that are only used in an upcoming patch,  fix the small
> > issue I found while testing, and I've added the test to the testsuite.
> > To run the test and see the issue:
> >
> > $  make check RUNTESTFLAGS="CC_FOR_TARGET=${CLANG_CC}
> > CXX_FOR_TARGET=${CLANG_CXX} dw5-rnglist-test.exp"
> >
> > gdb/ChangeLog (updated)
> >
> > 2020-06-04  Caroline Tice  <cmtice@google.com>
> >
> >         * dwarf2/read.c (struct dwop_section_names): Add rnglists_dwo.
> >         (dwop_section_names): Add .debug_rnglists.dwo,
> .zdebug_rnglists.dwo.
> >         (struct dwo_sections): Add rnglists field.
> >         (cu_debug_rnglist_section): New function (decl & definition).
> >         (dwarf2_locate_dwo_sections): Add code to read rnglists_dwo
> section.
> >         (dwarf2_rnglists_process): Add cases for DW_RLE_base_addressx,
> >         DW_RLE_startx_length, DW_RLE_startx_endx.  Also, update to only
> add
> >         the base address to DW_RLE_offset_pairs (not to all ranges).
> >         (dwarf2_get_pc_bounds): Check for DW_FORM_rnglistx when setting
> >         need_ranges_base.
> >         (dwarf2_record_block_ranges): Check for DW_FORM_rnglistx when
> setting
> >         need_ranges_base.
> >         (read_full_die_1): Add code to read DW_AT_rnglists_base and
> assign to
> >         cu->ranges_base.
> >         (partial_die_info::read): Check for DW_FORM_rnglistx when setting
> >         need_ranges_base.
> >         (read_rnglist_index): New function.
> >         (read_attribute_reprocess):  Add code for DW_FORM_rnglistx.
> >         (read_attribute_value): Mark DW_FORM_rnglistx with
> need_reprocess.
> >
> > gdb/testsuite/ChangeLog:
> >
> > 2020-06-04  Caroline Tice  <cmtice@google.com>
> >
> >         * gdb.dwarf2/dw5-rnglist-test.cc: New file.
> >         * gdb.dwarf2/dw5-rnglist-test.exp: New file.
> >
> >
> >
> >
> >
> > On Wed, Jun 3, 2020 at 7:49 AM Tom Tromey <tom@tromey.com> wrote:
> > >
> > > Caroline> $ clang++ -gdwarf-5 -O0 -gsplit-dwarf pre-order.cpp
> > > Caroline> pre-order-common.cpp -o pre-order
> > >
> > > I wonder if the test suite can be run this way.
> > >
> > > Caroline> Oh.  Those get used in my next upcoming patch (where I
> update GDB to
> > > Caroline> handle DWARFv5 .dwp files).  I can either leave them in this
> patch, or
> > > Caroline> remove them from here and put them in the next one,
> whichever you
> > > Caroline> prefer.
> > >
> > > I think it's better for patches to be reasonably self-contained when
> > > possible.
> > >
> > > Also, there were other patches for DWARFv5 on the list in "recent"
> (this
> > > year) times.  I wonder if those are relevant / related.
> > >
> > > Tom
>