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

[Caroline Tice <cmtice@google.com>]

[-- Attachment #1: Type: text/plain, Size: 14794 bytes --]

"This time for sure!" -- Bullwinkle Moose

I think I've got all of your requested changes in now, and I've
attached the updated patch.   About what you said at the very end of
your last message:

> I thought that somehow, when reading a CU that uses a DWO, we were creating
> a "logical" DIE tree by combining the DW_TAG_skeleton_unit DIE and the
> children of the DWO's DW_TAG_compile_unit DIE, and while doing this,
> overwriting the DW_TAG_skeleton_unit's DIE to use the DW_TAG_compile_unit
> tag instead.  Therefore making it appear to the rest of the DWARF reader
> as if it was a "standard" DW_TAG_compile_unit DIE.  But no, maybe I just dreamed
> all of this, or I can't find it anymore.
>

Actually your first thought was absolutely correct.  This is done in
cutu_reader::cutu_reader.  In my patched
read.c this is at line 7244:

comp_unit_die = dwo_comp_unit_die;

It's 4 lines above the comment:

         /* Yikes, we couldn't find the rest of the DIE, we only have
             the stub.  A complaint has already been logged.  There's
             not much more we can do except pass on the stub DIE to
             die_reader_func.  We don't want to throw an error on bad
             debug info.  */

> In fact, the reason the code was checking for DW_TAG_compile_unit must be that
> in the GCC/pre-standard version, the skeleton DIE in the executable is a
> DW_TAG_compile_unit.  With DWARF5, we'll see DW_TAG_skeleton_unit here.
>
> So I believe we should use
>
>  (tag != DW_TAG_compile_unit && tag != DW_TAG_skeleton_unit)
>
> to cover both versions, GCC pre-standard and DWARF 5.  Does that make sense?

I agree that we need to check both cases in cu_debug_rnglists_section,
because sometimes it gets called before the line above in cutu_reader,
and sometimes it gets called after (now that I'm also calling it in
dwarf2_rnglists_process).

> Wherever we use the logic:
>
>          int need_ranges_base = (die->tag != DW_TAG_compile_unit
>                                  && attr->form != DW_FORM_rnglistx);
>
> we should maybe check for DW_TAG_skeleton_unit as well?"

I don't think there's any point in checking for DW_TAG_skeleton_unit
in the need_ranges_base checks, because I believe that all of those
checks are called after the call to cutu_reader, so we never have a
DW_TAG_skeleton_unit by the time we get to those checks.

Please review the updated, attached patch and let me know if it is ok
now.  Thanks! :-)


-- Caroline
cmtice@google.com

gdb/ChangeLog

2020-07-14  Caroline Tice  <cmtice@google.com>

        * dwarf2/read.c (RNGLIST_HEADER_SIZE32) New constant definition.
        (RNGLIST_HEADER_SIZE64): New constant definition.
        (struct dwop_section_names): Add rnglists_dwo.
        (dwop_section_names): Add .debug_rnglists.dwo, .zdebug_rnglists.dwo.
        (struct loclist_header): Rename to 'loclists_rnglists_header'.
        (struct dwo_sections): Add rnglists field.
        (read_attribut_reprocess): Add tag parameter.
        (dwarf2_ranges_read): Add tag parameter & remove forward function decl.
        (cu_debug_rnglists_section): New function (decl & definition).
        (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; get rnglist section from
        cu_debug_rnglists_section, to get from either objfile or dwo file as
        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), moving
        test inside if-condition and updating complaint message.
        (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 and update comment appropriately.  Also pass die tag
        to dwarf2_ranges_read.
        (dwarf2_record_block_ranges): Check for DW_FORM_rnglistx when setting
        need_ranges_base and update comment appropriately.  Also pass die tag
        to dwarf2_ranges_process.
        (read_full_die_1): Add code to read DW_AT_rnglists_base and assign to
        cu->ranges_base.  Also pass die tag to read_attribute_reprocess.
        (partial_die_info::read): Check for DW_FORM_rnglistx when setting
        need_ranges_base and update comment appropriately.  Also pass die tag
        to read_attribute_reprocess and dwarf2_ranges_read.
        (read_loclist_header): Rename function to read_loclists_rnglists_header,
        and update function comment appropriately.
        (read_loclist_index): Call read_loclists_rnglists_header instead of
        read_loclist_header.
        (read_rnglist_index): New function.
        (read_attribute_reprocess):  Add tag parameter. Add code for
        DW_FORM_rnglistx, passing tag to read_rnglist_index.
        (read_attribute_value): Mark DW_FORM_rnglistx with need_reprocess.

gdb/testsuite/ChangeLog:

2020-07-14  Caroline Tice  <cmtice@google.com>

        * gdb.dwarf2/dw5-rnglist-test.cc: New file.
        * gdb.dwarf2/dw5-rnglist-test.exp: New file.




-- Caroline
cmtice@google.com


On Sat, Jul 11, 2020 at 10:54 AM Simon Marchi <simark@simark.ca> wrote:
>
> > @@ -1378,12 +1387,13 @@ static void read_call_site_scope (struct die_info *die, struct dwarf2_cu *cu);
> >
> >  static void read_variable (struct die_info *die, struct dwarf2_cu *cu);
> >
> > -static int dwarf2_ranges_read (unsigned, CORE_ADDR *, CORE_ADDR *,
> > -                            struct dwarf2_cu *, dwarf2_psymtab *);
> > -
> >  /* Return the .debug_loclists section to use for cu.  */
> >  static struct dwarf2_section_info *cu_debug_loc_section (struct dwarf2_cu *cu);
> >
> > +/* Return the .debug_rnglists section to use for cu.  */
> > +static struct dwarf2_section_info *cu_debug_rnglists_section
> > +(struct dwarf2_cu *cu, dwarf_tag tag);
>
> Indent this last line with two spaces.
>
> > @@ -13802,17 +13817,33 @@ dwarf2_rnglists_process (unsigned offset, struct dwarf2_cu *cu,
> >    const gdb_byte *buffer;
> >    CORE_ADDR baseaddr;
> >    bool overflow = false;
> > +  ULONGEST addr_index;
> > +  bool ignore_dwo_unit = false;
> > +  struct dwarf2_section_info *rnglists_section;
> >
> >    base = cu->base_address;
> >
> > -  per_objfile->per_bfd->rnglists.read (objfile);
> > -  if (offset >= per_objfile->per_bfd->rnglists.size)
> > +  /* Make sure we read the .debug_rnglists section from the file that
> > +     contains the DW_AT_ranges attribute we are reading.  Normally that
> > +     would be the .dwo file, if there is one.  However for DW_TAG_compile_unit
> > +     we always want to read from objfile/linked program (which would be the
> > +     DW_TAG_skeleton_unit DIE if we're using split dwarf).  */
> > +  if (tag == DW_TAG_compile_unit)
> > +    ignore_dwo_unit = true;
> > +
> > +  if (cu->dwo_unit != nullptr && !ignore_dwo_unit)
> > +      rnglists_section = &cu->dwo_unit->dwo_file->sections.rnglists;
> > +  else
> > +      rnglists_section = &per_objfile->per_bfd->rnglists;
>
> If think you can omit the `ignore_dwo_unit` variable and write this directly,
> without loss of readability:
>
>   if (cu->dwo_unit != nullptr && tag != DW_TAG_compile_unit)
>
> Also, should this use cu_debug_rnglists_section?
>
> > @@ -19094,13 +19184,65 @@ read_loclist_index (struct dwarf2_cu *cu, ULONGEST loclist_index)
> >      return bfd_get_64 (abfd, info_ptr) + loclist_base;
> >  }
> >
> > +/* Given a DW_FORM_rnglistx value rnglist_index, fetch the offset from the
>
> rnglist_index -> RNGLIST_INDEX
>
> > +   array of offsets in the .debug_rnglists section.  */
> > +static CORE_ADDR
> > +read_rnglist_index (struct dwarf2_cu *cu, ULONGEST rnglist_index,
> > +                 dwarf_tag tag)
> > +{
> > +  struct dwarf2_per_objfile *dwarf2_per_objfile = cu->per_objfile;
> > +  struct objfile *objfile = dwarf2_per_objfile->objfile;
> > +  bfd *abfd = objfile->obfd;
> > +  ULONGEST rnglist_header_size =
> > +    (cu->header.initial_length_size == 4 ? RNGLIST_HEADER_SIZE32
> > +     : RNGLIST_HEADER_SIZE64);
> > +  ULONGEST rnglist_base =
> > +      (cu->dwo_unit != nullptr) ? rnglist_header_size : cu->ranges_base;
> > +  ULONGEST start_offset =
> > +    rnglist_base + rnglist_index * cu->header.offset_size;
> > +
> > +  /* Get rnglists section.  */
> > +  struct dwarf2_section_info *section = cu_debug_rnglists_section (cu, tag);
> > +  if (section == nullptr)
> > +    error (_("Cannot find .debug_rnglists section [in module %s]"),
> > +        objfile_name (objfile));
>
> As of now, cu_debug_rnglists_section can't return nullptr.  And I think it should
> stay this way, it throws an error if it can't return a valid section.
>
> > +
> > +  /* Read the rnglists section content.  */
> > +  section->read (objfile);
> > +  if (section->buffer == nullptr)
> > +    error (_("DW_FORM_rnglistx used without .debug_rnglists section "
> > +          "[in module %s]"),
> > +       objfile_name (objfile));
>
> Align `objfile_name` with the opening parenthesis.
>
> > +
> > +  /* Verify the rnglist header is valid (same as loclist header).  */
>
> Is this comment accurate?  The code validates rnglist_index, not the header, right?
>
> > +  struct loclist_header header;
>
> Rename this type `loclists_rnglists_header`, to match the function.
>
> > +  read_loclists_rnglists_header (&header, section);
> > +  if (rnglist_index >= header.offset_entry_count)
> > +    error (_("DW_FORM_rnglistx index pointing outside of "
> > +          ".debug_rnglists offset array [in module %s]"),
> > +         objfile_name (objfile));
>
> Same here.
>
> > +
> > +  /* Validate that the offset is within the section's range.  */
> > +  if (start_offset >= section->size)
>
> Pendantically, I suppose we should verify not only that the start_offset
> is within the section's range, but also that there are enough bytes to read?
>
> For example, if we have a 100 bytes-long section, and start_offset is 99, we
> won't be able to read 4 or 8 bytes.
>
> > +    error (_("DW_FORM_rnglistx pointing outside of "
> > +             ".debug_rnglists section [in module %s]"),
> > +          objfile_name (objfile));
>
> Same here.
>
>
> > @@ -23383,6 +23530,30 @@ cu_debug_loc_section (struct dwarf2_cu *cu)
> >                                 : &per_objfile->per_bfd->loc);
> >  }
> >
> > +/* Return the .debug_rnglists section to use for CU.  */
> > +static struct dwarf2_section_info *
> > +cu_debug_rnglists_section (struct dwarf2_cu *cu, dwarf_tag tag)
> > +{
> > +  if (cu->header.version < 5)
> > +    error (_(".debug_rnglists section cannot be used in DWARF %d"),
> > +        cu->header.version);
> > +  struct dwarf2_per_objfile *dwarf2_per_objfile = cu->per_objfile;
> > +
> > +  /* Make sure we read the .debug_rnglists section from the file that
> > +     contains the DW_AT_ranges attribute we are reading.  Normally that
> > +     would be the .dwo file, if there is one.  However for DW_TAG_compile_unit
> > +     we always want to read from objfile/linked program (which would be the
> > +     DW_TAG_skeleton_unit DIE if we're using split dwarf).  */
> > +  if (cu->dwo_unit != nullptr && tag != DW_TAG_compile_unit)
> > +    {
> > +      struct dwo_sections *sections = &cu->dwo_unit->dwo_file->sections;
> > +
> > +      if (sections->rnglists.size > 0)
> > +     return &sections->rnglists;
> > +    }
> > +  return &dwarf2_per_objfile->per_bfd->rnglists;
>
> I expressed a concern about this earlier: if an attribute in the .dwo is of form
> DW_FORM_rnglistx, but for some reason the .dwo has no .debug_rnglist section, what
> happens?  It seems to me like we're going to return the section from the executable,
> which would be wrong.
>
> Also, I just debugged this function, and saw that it is called in this context:
>
> #0  cu_debug_rnglists_section (cu=0x615000026c80, tag=DW_TAG_skeleton_unit) at /home/simark/src/binutils-gdb/gdb/dwarf2/read.c:23557
> #1  0x0000557c4612cde5 in read_rnglist_index (cu=0x615000026c80, rnglist_index=0, tag=DW_TAG_skeleton_unit) at /home/simark/src/binutils-gdb/gdb/dwarf2/read.c:19205
> #2  0x0000557c4612d66e in read_attribute_reprocess (reader=0x7fffb47cf240, attr=0x621000186198, tag=DW_TAG_skeleton_unit) at /home/simark/src/binutils-gdb/gdb/dwarf2/read.c:19258
> #3  0x0000557c4612152d in read_full_die_1 (reader=0x7fffb47cf240, diep=0x7fffb47cf280, info_ptr=0x60400001e8c0 "", num_extra_attrs=0) at /home/simark/src/binutils-gdb/gdb/dwarf2/read.c:18253
> #4  0x0000557c461217d9 in read_full_die (reader=0x7fffb47cf240, diep=0x7fffb47cf280, info_ptr=0x60400001e8a4 "\001") at /home/simark/src/binutils-gdb/gdb/dwarf2/read.c:18268
> #5  0x0000557c46094af2 in cutu_reader::cutu_reader (this=0x7fffb47cf240, this_cu=0x621000183910, per_objfile=0x61300000a840, abbrev_table=0x60b00006ff30, existing_cu=0x0, skip_partial=false) at /
> home/simark/src/binutils-gdb/gdb/dwarf2/read.c:7202
> #6  0x0000557c4609b0ca in process_psymtab_comp_unit (this_cu=0x621000183910, per_objfile=0x61300000a840, want_partial_unit=false, pretend_language=language_minimal) at /home/simark/src/binutils-gdb/gdb/dwarf2/read.c:7667
> #7  0x0000557c460a0fd9 in dwarf2_build_psymtabs_hard (per_objfile=0x61300000a840) at /home/simark/src/binutils-gdb/gdb/dwarf2/read.c:8051
> #8  0x0000557c46086ae3 in dwarf2_build_psymtabs (objfile=0x614000006440) at /home/simark/src/binutils-gdb/gdb/dwarf2/read.c:6106
>
> As you can see, tag == DW_TAG_skeleton_unit.
>
> I thought that somehow, when reading a CU that uses a DWO, we were creating
> a "logical" DIE tree by combining the DW_TAG_skeleton_unit DIE and the
> children of the DWO's DW_TAG_compile_unit DIE, and while doing this,
> overwriting the DW_TAG_skeleton_unit's DIE to use the DW_TAG_compile_unit
> tag instead.  Therefore making it appear to the rest of the DWARF reader
> as if it was a "standard" DW_TAG_compile_unit DIE.  But no, maybe I just dreamed
> all of this, or I can't find it anymore.
>
> In fact, the reason the code was checking for DW_TAG_compile_unit must be that
> in the GCC/pre-standard version, the skeleton DIE in the executable is a
> DW_TAG_compile_unit.  With DWARF5, we'll see DW_TAG_skeleton_unit here.
>
> So I believe we should use
>
>   (tag != DW_TAG_compile_unit && tag != DW_TAG_skeleton_unit)
>
> to cover both versions, GCC pre-standard and DWARF 5.  Does that make sense?
>
> Wherever we use the logic:
>
>           int need_ranges_base = (die->tag != DW_TAG_compile_unit
>                                   && attr->form != DW_FORM_rnglistx);
>
> we should maybe check for DW_TAG_skeleton_unit as well?
>
> Simon

[-- Attachment #2: v5-0001-Update-GDB-to-fix-issues-with-handling-DWARF-v5-r.patch --]
[-- Type: application/octet-stream, Size: 27207 bytes --]

From 738901dfd29cb542468557ce802431cdbba0189d Mon Sep 17 00:00:00 2001
From: Caroline Tice <cmtice@google.com>
Date: Wed, 1 Jul 2020 12:39:08 -0700
Subject: [PATCH v5] Update GDB to fix issues with handling DWARF v5 rnglists &
 .dwo files.

While experimenting with GDB on DWARF 5 with split debug (dwo files),
I discovered that GDB was not reading the rnglist index
properly (it needed to be reprocessed in the same way the loclist
index does), and that there was no code for reading rnglists out of
dwo files at all.  Also, the rnglist address reading function
(dwarf2_rnglists_process) was adding the base address to all rnglist
entries, when it's only supposed to add it to the DW_RLE_offset_pair
entries (http://dwarfstd.org/doc/DWARF5.pdf, p. 53), and was not
handling several entry types.

- Added 'reprocessing' for reading rnglist index (as is done for loclist
  index).
- Added code for reading rnglists out of .dwo files.
- Added several missing rnglist forms to dwarf2_rnglists_process.
- Fixed bug that was alwayas adding base address for rnglists (only
  one form needs that).
- Updated dwarf2_rnglists_process to read rnglist out of dwo file when
  appropriate.
- Added new functions cu_debug_rnglist_section & read_rnglist_index.
- Added new testcase, dw5-rnglist-test.{cc,exp}
---
 gdb/dwarf2/read.c                             | 261 ++++++++++++++----
 gdb/testsuite/gdb.dwarf2/dw5-rnglist-test.cc  |  97 +++++++
 gdb/testsuite/gdb.dwarf2/dw5-rnglist-test.exp |  40 +++
 3 files changed, 350 insertions(+), 48 deletions(-)
 create mode 100644 gdb/testsuite/gdb.dwarf2/dw5-rnglist-test.cc
 create mode 100644 gdb/testsuite/gdb.dwarf2/dw5-rnglist-test.exp

diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c
index 558fad74f8..39ed455def 100644
--- a/gdb/dwarf2/read.c
+++ b/gdb/dwarf2/read.c
@@ -132,6 +132,12 @@ static int dwarf2_loclist_block_index;
 /* Size of .debug_loclists section header for 64-bit DWARF format.  */
 #define LOCLIST_HEADER_SIZE64 20
 
+/* Size of .debug_rnglists section header for 32-bit DWARF format.  */
+#define RNGLIST_HEADER_SIZE32 12
+
+/* Size of .debug_rnglists section header for 64-bit DWARF format.  */
+#define RNGLIST_HEADER_SIZE64 20
+
 /* An index into a (C++) symbol name component in a symbol name as
    recorded in the mapped_index's symbol table.  For each C++ symbol
    in the symbol table, we record one entry for the start of each
@@ -340,6 +346,7 @@ static const struct dwop_section_names
   struct dwarf2_section_names loclists_dwo;
   struct dwarf2_section_names macinfo_dwo;
   struct dwarf2_section_names macro_dwo;
+  struct dwarf2_section_names rnglists_dwo;
   struct dwarf2_section_names str_dwo;
   struct dwarf2_section_names str_offsets_dwo;
   struct dwarf2_section_names types_dwo;
@@ -355,6 +362,7 @@ dwop_section_names =
   { ".debug_loclists.dwo", ".zdebug_loclists.dwo" },
   { ".debug_macinfo.dwo", ".zdebug_macinfo.dwo" },
   { ".debug_macro.dwo", ".zdebug_macro.dwo" },
+  { ".debug_rnglists.dwo", ".zdebug_rnglists.dwo" },
   { ".debug_str.dwo", ".zdebug_str.dwo" },
   { ".debug_str_offsets.dwo", ".zdebug_str_offsets.dwo" },
   { ".debug_types.dwo", ".zdebug_types.dwo" },
@@ -364,9 +372,9 @@ dwop_section_names =
 
 /* local data types */
 
-/* The location list section (.debug_loclists) begins with a header,
-   which contains the following information.  */
-struct loclist_header
+/* The location list and range list sections (.debug_loclists & .debug_rnglists)
+   begin with a header,  which contains the following information.  */
+struct loclists_rnglists_header
 {
   /* A 4-byte or 12-byte length containing the length of the
      set of entries for this compilation unit, not including the
@@ -650,6 +658,7 @@ struct dwo_sections
   struct dwarf2_section_info loclists;
   struct dwarf2_section_info macinfo;
   struct dwarf2_section_info macro;
+  struct dwarf2_section_info rnglists;
   struct dwarf2_section_info str;
   struct dwarf2_section_info str_offsets;
   /* In the case of a virtual DWO file, these two are unused.  */
@@ -1274,7 +1283,7 @@ static const gdb_byte *read_attribute (const struct die_reader_specs *,
 				       const gdb_byte *, bool *need_reprocess);
 
 static void read_attribute_reprocess (const struct die_reader_specs *reader,
-				      struct attribute *attr);
+				      struct attribute *attr, dwarf_tag tag);
 
 static CORE_ADDR read_addr_index (struct dwarf2_cu *cu, unsigned int addr_index);
 
@@ -1378,12 +1387,13 @@ static void read_call_site_scope (struct die_info *die, struct dwarf2_cu *cu);
 
 static void read_variable (struct die_info *die, struct dwarf2_cu *cu);
 
-static int dwarf2_ranges_read (unsigned, CORE_ADDR *, CORE_ADDR *,
-			       struct dwarf2_cu *, dwarf2_psymtab *);
-
 /* Return the .debug_loclists section to use for cu.  */
 static struct dwarf2_section_info *cu_debug_loc_section (struct dwarf2_cu *cu);
 
+/* Return the .debug_rnglists section to use for cu.  */
+static struct dwarf2_section_info *cu_debug_rnglists_section
+  (struct dwarf2_cu *cu, dwarf_tag tag);
+
 /* How dwarf2_get_pc_bounds constructed its *LOWPC and *HIGHPC return
    values.  Keep the items ordered with increasing constraints compliance.  */
 enum pc_bounds_kind
@@ -12447,6 +12457,11 @@ dwarf2_locate_dwo_sections (bfd *abfd, asection *sectp, void *dwo_sections_ptr)
       dwo_sections->macro.s.section = sectp;
       dwo_sections->macro.size = bfd_section_size (sectp);
     }
+  else if (section_is_p (sectp->name, &names->rnglists_dwo))
+    {
+      dwo_sections->rnglists.s.section = sectp;
+      dwo_sections->rnglists.size = bfd_section_size (sectp);
+    }
   else if (section_is_p (sectp->name, &names->str_dwo))
     {
       dwo_sections->str.s.section = sectp;
@@ -13791,7 +13806,7 @@ read_variable (struct die_info *die, struct dwarf2_cu *cu)
 template <typename Callback>
 static bool
 dwarf2_rnglists_process (unsigned offset, struct dwarf2_cu *cu,
-			 Callback &&callback)
+			 dwarf_tag tag, Callback &&callback)
 {
   dwarf2_per_objfile *per_objfile = cu->per_objfile;
   struct objfile *objfile = per_objfile->objfile;
@@ -13801,17 +13816,20 @@ dwarf2_rnglists_process (unsigned offset, struct dwarf2_cu *cu,
   const gdb_byte *buffer;
   CORE_ADDR baseaddr;
   bool overflow = false;
+  ULONGEST addr_index;
+  struct dwarf2_section_info *rnglists_section;
 
   base = cu->base_address;
+  rnglists_section = cu_debug_rnglists_section (cu, tag);
+  rnglists_section->read (objfile);
 
-  per_objfile->per_bfd->rnglists.read (objfile);
-  if (offset >= per_objfile->per_bfd->rnglists.size)
+  if (offset >= rnglists_section->size)
     {
       complaint (_("Offset %d out of bounds for DW_AT_ranges attribute"),
 		 offset);
       return false;
     }
-  buffer = per_objfile->per_bfd->rnglists.buffer + offset;
+  buffer = rnglists_section->buffer + offset;
 
   baseaddr = objfile->text_section_offset ();
 
@@ -13819,8 +13837,8 @@ dwarf2_rnglists_process (unsigned offset, struct dwarf2_cu *cu,
     {
       /* Initialize it due to a false compiler warning.  */
       CORE_ADDR range_beginning = 0, range_end = 0;
-      const gdb_byte *buf_end = (per_objfile->per_bfd->rnglists.buffer
-				 + per_objfile->per_bfd->rnglists.size);
+      const gdb_byte *buf_end = (rnglists_section->buffer
+				 + rnglists_section->size);
       unsigned int bytes_read;
 
       if (buffer == buf_end)
@@ -13842,6 +13860,11 @@ dwarf2_rnglists_process (unsigned offset, struct dwarf2_cu *cu,
 	  base = cu->header.read_address (obfd, buffer, &bytes_read);
 	  buffer += bytes_read;
 	  break;
+        case DW_RLE_base_addressx:
+          addr_index = read_unsigned_leb128 (obfd, buffer, &bytes_read);
+          buffer += bytes_read;
+          base = read_addr_index (cu, addr_index);
+          break;
 	case DW_RLE_start_length:
 	  if (buffer + cu->header.addr_size > buf_end)
 	    {
@@ -13860,6 +13883,19 @@ dwarf2_rnglists_process (unsigned offset, struct dwarf2_cu *cu,
 	      break;
 	    }
 	  break;
+	case DW_RLE_startx_length:
+          addr_index = read_unsigned_leb128 (obfd, buffer, &bytes_read);
+          buffer += bytes_read;
+          range_beginning = read_addr_index (cu, addr_index);
+          if (buffer > buf_end)
+            {
+              overflow = true;
+              break;
+            }
+          range_end = (range_beginning
+                       + read_unsigned_leb128 (obfd, buffer, &bytes_read));
+          buffer += bytes_read;
+          break;
 	case DW_RLE_offset_pair:
 	  range_beginning = read_unsigned_leb128 (obfd, buffer, &bytes_read);
 	  buffer += bytes_read;
@@ -13888,6 +13924,19 @@ dwarf2_rnglists_process (unsigned offset, struct dwarf2_cu *cu,
 	  range_end = cu->header.read_address (obfd, buffer, &bytes_read);
 	  buffer += bytes_read;
 	  break;
+	case DW_RLE_startx_endx:
+          addr_index = read_unsigned_leb128 (obfd, buffer, &bytes_read);
+          buffer += bytes_read;
+          range_beginning = read_addr_index (cu, addr_index);
+          if (buffer > buf_end)
+            {
+              overflow = true;
+              break;
+            }
+          addr_index = read_unsigned_leb128 (obfd, buffer, &bytes_read);
+          buffer += bytes_read;
+          range_end = read_addr_index (cu, addr_index);
+          break;
 	default:
 	  complaint (_("Invalid .debug_rnglists data (no base address)"));
 	  return false;
@@ -13897,14 +13946,6 @@ dwarf2_rnglists_process (unsigned offset, struct dwarf2_cu *cu,
       if (rlet == DW_RLE_base_address)
 	continue;
 
-      if (!base.has_value ())
-	{
-	  /* We have no valid base address for the ranges
-	     data.  */
-	  complaint (_("Invalid .debug_rnglists data (no base address)"));
-	  return false;
-	}
-
       if (range_beginning > range_end)
 	{
 	  /* Inverted range entries are invalid.  */
@@ -13916,8 +13957,20 @@ dwarf2_rnglists_process (unsigned offset, struct dwarf2_cu *cu,
       if (range_beginning == range_end)
 	continue;
 
-      range_beginning += *base;
-      range_end += *base;
+      /* Only DW_RLE_offset_pair needs the base address added.  */
+      if (rlet == DW_RLE_offset_pair)
+	{
+	  if (!base.has_value ())
+	    {
+	      /* We have no valid base address for the DW_RLE_offset_pair.  */
+	      complaint (_("Invalid .debug_rnglists data (no base address for "
+			   "DW_RLE_offset_pair)"));
+	      return false;
+	    }
+
+	  range_beginning += *base;
+	  range_end += *base;
+	}
 
       /* A not-uncommon case of bad debug info.
 	 Don't pollute the addrmap with bad data.  */
@@ -13950,7 +14003,7 @@ dwarf2_rnglists_process (unsigned offset, struct dwarf2_cu *cu,
 
 template <typename Callback>
 static int
-dwarf2_ranges_process (unsigned offset, struct dwarf2_cu *cu,
+dwarf2_ranges_process (unsigned offset, struct dwarf2_cu *cu, dwarf_tag tag,
 		       Callback &&callback)
 {
   dwarf2_per_objfile *per_objfile = cu->per_objfile;
@@ -13966,7 +14019,7 @@ dwarf2_ranges_process (unsigned offset, struct dwarf2_cu *cu,
   CORE_ADDR baseaddr;
 
   if (cu_header->version >= 5)
-    return dwarf2_rnglists_process (offset, cu, callback);
+    return dwarf2_rnglists_process (offset, cu, tag, callback);
 
   base = cu->base_address;
 
@@ -14052,7 +14105,7 @@ dwarf2_ranges_process (unsigned offset, struct dwarf2_cu *cu,
 static int
 dwarf2_ranges_read (unsigned offset, CORE_ADDR *low_return,
 		    CORE_ADDR *high_return, struct dwarf2_cu *cu,
-		    dwarf2_psymtab *ranges_pst)
+		    dwarf2_psymtab *ranges_pst, dwarf_tag tag)
 {
   struct objfile *objfile = cu->per_objfile->objfile;
   struct gdbarch *gdbarch = objfile->arch ();
@@ -14062,7 +14115,7 @@ dwarf2_ranges_read (unsigned offset, CORE_ADDR *low_return,
   CORE_ADDR high = 0;
   int retval;
 
-  retval = dwarf2_ranges_process (offset, cu,
+  retval = dwarf2_ranges_process (offset, cu, tag,
     [&] (CORE_ADDR range_beginning, CORE_ADDR range_end)
     {
       if (ranges_pst != NULL)
@@ -14154,8 +14207,14 @@ dwarf2_get_pc_bounds (struct die_info *die, CORE_ADDR *lowpc,
 	{
 	  /* DW_AT_rnglists_base does not apply to DIEs from the DWO skeleton.
 	     We take advantage of the fact that DW_AT_ranges does not appear
-	     in DW_TAG_compile_unit of DWO files.  */
-	  int need_ranges_base = die->tag != DW_TAG_compile_unit;
+	     in DW_TAG_compile_unit of DWO files.
+
+	     Attributes of the form DW_FORM_rnglistx have already had their
+	     value changed by read_rnglist_index and already include
+	     DW_AT_rnglists_base, so don't need to add the ranges base,
+	     either.  */
+	  int need_ranges_base = (die->tag != DW_TAG_compile_unit
+				  && attr->form != DW_FORM_rnglistx);
 	  unsigned int ranges_offset = (DW_UNSND (attr)
 					+ (need_ranges_base
 					   ? cu->ranges_base
@@ -14163,7 +14222,8 @@ dwarf2_get_pc_bounds (struct die_info *die, CORE_ADDR *lowpc,
 
 	  /* Value of the DW_AT_ranges attribute is the offset in the
 	     .debug_ranges section.  */
-	  if (!dwarf2_ranges_read (ranges_offset, &low, &high, cu, pst))
+	  if (!dwarf2_ranges_read (ranges_offset, &low, &high, cu, pst,
+				   die->tag))
 	    return PC_BOUNDS_INVALID;
 	  /* Found discontinuous range of addresses.  */
 	  ret = PC_BOUNDS_RANGES;
@@ -14325,8 +14385,14 @@ dwarf2_record_block_ranges (struct die_info *die, struct block *block,
     {
       /* DW_AT_rnglists_base does not apply to DIEs from the DWO skeleton.
 	 We take advantage of the fact that DW_AT_ranges does not appear
-	 in DW_TAG_compile_unit of DWO files.  */
-      int need_ranges_base = die->tag != DW_TAG_compile_unit;
+	 in DW_TAG_compile_unit of DWO files.
+
+	 Attributes of the form DW_FORM_rnglistx have already had their
+	 value changed by read_rnglist_index and already include
+	 DW_AT_rnglists_base, so don't need to add the ranges base,
+	 either.  */
+      int need_ranges_base = (die->tag != DW_TAG_compile_unit
+			      && attr->form != DW_FORM_rnglistx);
 
       /* The value of the DW_AT_ranges attribute is the offset of the
          address range list in the .debug_ranges section.  */
@@ -14334,7 +14400,7 @@ dwarf2_record_block_ranges (struct die_info *die, struct block *block,
 			      + (need_ranges_base ? cu->ranges_base : 0));
 
       std::vector<blockrange> blockvec;
-      dwarf2_ranges_process (offset, cu,
+      dwarf2_ranges_process (offset, cu, die->tag,
 	[&] (CORE_ADDR start, CORE_ADDR end)
 	{
 	  start += baseaddr;
@@ -18154,8 +18220,13 @@ read_full_die_1 (const struct die_reader_specs *reader,
   auto maybe_addr_base = die->addr_base ();
   if (maybe_addr_base.has_value ())
     cu->addr_base = *maybe_addr_base;
+
+  attr = die->attr (DW_AT_rnglists_base);
+  if (attr != nullptr)
+    cu->ranges_base = DW_UNSND (attr);
+
   for (int index : indexes_that_need_reprocess)
-    read_attribute_reprocess (reader, &die->attrs[index]);
+    read_attribute_reprocess (reader, &die->attrs[index], die->tag);
   *diep = die;
   return info_ptr;
 }
@@ -18524,7 +18595,7 @@ partial_die_info::read (const struct die_reader_specs *reader,
          already been read at this point, so there is no need to wait until
 	 the loop terminates to do the reprocessing.  */
       if (need_reprocess)
-	read_attribute_reprocess (reader, &attr);
+	read_attribute_reprocess (reader, &attr, tag);
       /* Store the data if it is of an attribute we want to keep in a
          partial symbol table.  */
       switch (attr.name)
@@ -18670,10 +18741,16 @@ partial_die_info::read (const struct die_reader_specs *reader,
 
 	case DW_AT_ranges:
 	  {
-	    /* It would be nice to reuse dwarf2_get_pc_bounds here,
-	       but that requires a full DIE, so instead we just
-	       reimplement it.  */
-	    int need_ranges_base = tag != DW_TAG_compile_unit;
+	    /* DW_AT_rnglists_base does not apply to DIEs from the DWO
+	       skeleton.  We take advantage of the fact the DW_AT_ranges
+	       does not appear in DW_TAG_compile_unit of DWO files.
+
+	       Attributes of the form DW_FORM_rnglistx have already had
+               their value changed by read_rnglist_index and already
+	       include DW_AT_rnglists_base, so don't need to add the ranges
+	       base, either.  */
+	    int need_ranges_base = (tag != DW_TAG_compile_unit
+				    && attr.form != DW_FORM_rnglistx);
 	    unsigned int ranges_offset = (DW_UNSND (&attr)
 					  + (need_ranges_base
 					     ? cu->ranges_base
@@ -18682,7 +18759,7 @@ partial_die_info::read (const struct die_reader_specs *reader,
 	    /* Value of the DW_AT_ranges attribute is the offset in the
 	       .debug_ranges section.  */
 	    if (dwarf2_ranges_read (ranges_offset, &lowpc, &highpc, cu,
-				    nullptr))
+				    nullptr, tag))
 	      has_pc_info = 1;
 	  }
 	  break;
@@ -19006,11 +19083,11 @@ partial_die_info::fixup (struct dwarf2_cu *cu)
   fixup_called = 1;
 }
 
-/* Read the .debug_loclists header contents from the given SECTION in the
-   HEADER.  */
+/* Read the .debug_loclists or .debug_rnglists header (they are the same format)
+   contents from the given SECTION in the HEADER.  */
 static void
-read_loclist_header (struct loclist_header *header,
-		      struct dwarf2_section_info *section)
+read_loclists_rnglists_header (struct loclists_rnglists_header *header,
+			       struct dwarf2_section_info *section)
 {
   unsigned int bytes_read;
   bfd *abfd = section->get_bfd_owner ();
@@ -19063,8 +19140,8 @@ read_loclist_index (struct dwarf2_cu *cu, ULONGEST loclist_index)
   if (section->buffer == NULL)
     complaint (_("DW_FORM_loclistx used without .debug_loclists "
 	        "section [in module %s]"), objfile_name (objfile));
-  struct loclist_header header;
-  read_loclist_header (&header, section);
+  struct loclists_rnglists_header header;
+  read_loclists_rnglists_header (&header, section);
   if (loclist_index >= header.offset_entry_count)
     complaint (_("DW_FORM_loclistx pointing outside of "
 	        ".debug_loclists offset array [in module %s]"),
@@ -19083,13 +19160,68 @@ read_loclist_index (struct dwarf2_cu *cu, ULONGEST loclist_index)
     return bfd_get_64 (abfd, info_ptr) + loclist_base;
 }
 
+/* Given a DW_FORM_rnglistx value RNGLIST_INDEX, fetch the offset from the
+   array of offsets in the .debug_rnglists section.  */
+static CORE_ADDR
+read_rnglist_index (struct dwarf2_cu *cu, ULONGEST rnglist_index,
+		    dwarf_tag tag)
+{
+  struct dwarf2_per_objfile *dwarf2_per_objfile = cu->per_objfile;
+  struct objfile *objfile = dwarf2_per_objfile->objfile;
+  bfd *abfd = objfile->obfd;
+  ULONGEST rnglist_header_size =
+    (cu->header.initial_length_size == 4 ? RNGLIST_HEADER_SIZE32
+     : RNGLIST_HEADER_SIZE64);
+  ULONGEST rnglist_base =
+      (cu->dwo_unit != nullptr) ? rnglist_header_size : cu->ranges_base;
+  ULONGEST start_offset =
+    rnglist_base + rnglist_index * cu->header.offset_size;
+
+  /* Get rnglists section.  */
+  struct dwarf2_section_info *section = cu_debug_rnglists_section (cu, tag);
+
+  /* Read the rnglists section content.  */
+  section->read (objfile);
+  if (section->buffer == nullptr)
+    error (_("DW_FORM_rnglistx used without .debug_rnglists section "
+	     "[in module %s]"),
+	   objfile_name (objfile));
+
+  /* Verify the rnglist index is valid.  */
+  struct loclists_rnglists_header header;
+  read_loclists_rnglists_header (&header, section);
+  if (rnglist_index >= header.offset_entry_count)
+    error (_("DW_FORM_rnglistx index pointing outside of "
+	     ".debug_rnglists offset array [in module %s]"),
+	   objfile_name (objfile));
+
+  /* Validate that the offset is within the section's range.  */
+  if (start_offset >= section->size)
+    error (_("DW_FORM_rnglistx pointing outside of "
+             ".debug_rnglists section [in module %s]"),
+	   objfile_name (objfile));
+
+  /* Validate that reading won't go beyond the end of the section.  */
+  if (start_offset + cu->header.offset_size > rnglist_base + section->size)
+    error (_("Reading DW_FORM_rnglistx index beyond end of"
+	     ".debug_rnglists section [in module %s]"),
+	   objfile_name (objfile));
+
+  const gdb_byte *info_ptr = section->buffer + start_offset;
+
+  if (cu->header.offset_size == 4)
+    return read_4_bytes (abfd, info_ptr) + rnglist_base;
+  else
+    return read_8_bytes (abfd, info_ptr) + rnglist_base;
+}
+
 /* Process the attributes that had to be skipped in the first round. These
    attributes are the ones that need str_offsets_base or addr_base attributes.
    They could not have been processed in the first round, because at the time
    the values of str_offsets_base or addr_base may not have been known.  */
 static void
 read_attribute_reprocess (const struct die_reader_specs *reader,
-			  struct attribute *attr)
+			  struct attribute *attr, dwarf_tag tag)
 {
   struct dwarf2_cu *cu = reader->cu;
   switch (attr->form)
@@ -19101,6 +19233,9 @@ read_attribute_reprocess (const struct die_reader_specs *reader,
       case DW_FORM_loclistx:
 	 DW_UNSND (attr) = read_loclist_index (cu, DW_UNSND (attr));
 	 break;
+      case DW_FORM_rnglistx:
+        DW_UNSND (attr) = read_rnglist_index (cu, DW_UNSND (attr), tag);
+        break;
       case DW_FORM_strx:
       case DW_FORM_strx1:
       case DW_FORM_strx2:
@@ -19282,8 +19417,10 @@ read_attribute_value (const struct die_reader_specs *reader,
       DW_SND (attr) = read_signed_leb128 (abfd, info_ptr, &bytes_read);
       info_ptr += bytes_read;
       break;
-    case DW_FORM_udata:
     case DW_FORM_rnglistx:
+      *need_reprocess = true;
+      /* FALLTHROUGH */
+    case DW_FORM_udata:
       DW_UNSND (attr) = read_unsigned_leb128 (abfd, info_ptr, &bytes_read);
       info_ptr += bytes_read;
       break;
@@ -23372,6 +23509,34 @@ cu_debug_loc_section (struct dwarf2_cu *cu)
 				  : &per_objfile->per_bfd->loc);
 }
 
+/* Return the .debug_rnglists section to use for CU.  */
+static struct dwarf2_section_info *
+cu_debug_rnglists_section (struct dwarf2_cu *cu, dwarf_tag tag)
+{
+  if (cu->header.version < 5)
+    error (_(".debug_rnglists section cannot be used in DWARF %d"),
+	   cu->header.version);
+  struct dwarf2_per_objfile *dwarf2_per_objfile = cu->per_objfile;
+
+  /* Make sure we read the .debug_rnglists section from the file that
+     contains the DW_AT_ranges attribute we are reading.  Normally that
+     would be the .dwo file, if there is one.  However for DW_TAG_compile_unit
+     or DW_TAG_skeleton unit, we always want to read from objfile/linked
+     program.  */
+  if (cu->dwo_unit != nullptr
+      && tag != DW_TAG_compile_unit
+      && tag != DW_TAG_skeleton_unit)
+    {
+      struct dwo_sections *sections = &cu->dwo_unit->dwo_file->sections;
+
+      if (sections->rnglists.size > 0)
+	return &sections->rnglists;
+      else
+	error (_(".debug_rnglists section is missing from .dwo file."));
+    }
+  return &dwarf2_per_objfile->per_bfd->rnglists;
+}
+
 /* A helper function that fills in a dwarf2_loclist_baton.  */
 
 static void
diff --git a/gdb/testsuite/gdb.dwarf2/dw5-rnglist-test.cc b/gdb/testsuite/gdb.dwarf2/dw5-rnglist-test.cc
new file mode 100644
index 0000000000..81693f569d
--- /dev/null
+++ b/gdb/testsuite/gdb.dwarf2/dw5-rnglist-test.cc
@@ -0,0 +1,97 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2020 Free Software Foundation, Inc.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+#include <iostream>
+#include <vector>
+
+struct node {
+  int id;
+  node *left;
+  node *right;
+  bool visited;
+};
+
+node  node_array[50];
+unsigned int CUR_IDX = 0;
+
+node *
+make_node (int val)
+{
+  node *new_node = &(node_array[CUR_IDX++]);
+  new_node->left = NULL;
+  new_node->right = NULL;
+  new_node->id = val;
+  new_node->visited = false;
+
+  return new_node;
+}
+
+void
+tree_insert (node *root, int val)
+{
+  if (val < root->id)
+    {
+      if (root->left)
+        tree_insert (root->left, val);
+      else
+        root->left = make_node(val);
+    }
+  else if (val > root->id)
+    {
+      if (root->right)
+        tree_insert (root->right, val);
+      else
+        root->right = make_node(val);
+    }
+}
+
+void
+inorder (node *root)
+{
+  std::vector<node *> todo;
+  todo.push_back (root);
+  while (!todo.empty())
+    {
+      node *curr = todo.back();
+      todo.pop_back(); /* break-here */
+      if (curr->visited)
+        std::cout << curr->id << " ";
+      else
+        {
+          curr->visited = true;
+          if (curr->right)
+            todo.push_back (curr->right);
+          todo.push_back (curr);
+          if (curr->left)
+            todo.push_back (curr->left);
+        }
+    }
+}
+
+int
+main (int argc, char **argv)
+{
+  node *root = make_node (35);
+
+  tree_insert (root, 28);
+  tree_insert (root, 20);
+  tree_insert (root, 60);
+
+  inorder (root);
+
+  return 0;
+}
diff --git a/gdb/testsuite/gdb.dwarf2/dw5-rnglist-test.exp b/gdb/testsuite/gdb.dwarf2/dw5-rnglist-test.exp
new file mode 100644
index 0000000000..af6c34b5dd
--- /dev/null
+++ b/gdb/testsuite/gdb.dwarf2/dw5-rnglist-test.exp
@@ -0,0 +1,40 @@
+# Copyright 2020 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+# Check that GDB can find the variables in a lexical block with a
+# DW_FORM_rnglistx DW_AT_ranges field.  This test is intended for DWARF-5,
+# compiled with clang++.
+
+standard_testfile .cc
+
+# This test is intended for targets which support DWARF-5.
+# Since we pass an explicit -gdwarf-5 to the compiler,
+# we let that be the test of whether the target supports it.
+
+if { [prepare_for_testing "failed to prepare" "${testfile}" \
+          $srcfile {debug c++ additional_flags=-gdwarf-5 \
+                        additional_flags=-O0}] } {
+    return -1
+}
+
+if ![runto_main] {
+    return -1
+}
+
+gdb_breakpoint [gdb_get_line_number "break-here"]
+gdb_continue_to_breakpoint "break-here" ".* break-here .*"
+
+gdb_test "print curr" "\\\(node \\\*\\\) $hex <node_array>"
+gdb_test "print *curr" "= {id = 35, left = $hex <node_array\\+$decimal>, right = $hex <node_array\\+$decimal>, visited = false}"
-- 
2.27.0.389.gc38d7665816-goog