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

[Caroline Tice <cmtice@google.com>]

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

I think your description of how ranges work is correct.  I've made all
your requested changes (I think).  Attached is the latest patch for
review. :-)

-- Caroline
cmtice@google.com

gdb/ChangeLog:

2020-07-09  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 dwo_sections): Add rnglists field.
        (read_attribut_reprocess): Add tag parameter.
        (dwarf2_ranges_read): Add tag parameter.
        (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; 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), 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_read.
        (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-09  Caroline Tice  <cmtice@google.com>

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

On Fri, Jul 3, 2020 at 10:11 PM Simon Marchi <simark@simark.ca> wrote:
>
> On 2020-07-03 6:47 p.m., Caroline Tice wrote:
> > "
> >
> > I have made most of your requested changes, which are in the attached
> > patch.  I disagree with one of your comments, and I had a question
> > about another:
> >
> > First the disagreement.  You said:
> >
> >>> @@ -18684,7 +18764,8 @@ partial_die_info::read (const struct die_reader_specs *reader,
> >>>           /* 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;
> >>> +         int need_ranges_base = (tag != DW_TAG_compile_unit
> >>> +                                 && attr.form != DW_FORM_rnglistx);
> >>
> >> It looks like this fix is unrelated from the rest of the patch, which deals
> >> about reading rnglists from dwo files.  It's better to have on fix per patch.
> >> So I think this fix would deserve its own patch, with a commit message that
> >> clearly explains the issue and the fix.
> >>
> > This is actually a required fix for dealing with rnglists.  Without
> > it, when the DW_AT_ranges uses the form
> > DW_FORM_rnglistx, the wrong value is calculated and the entire rnglist
> > is ignored (if you turn on complaints this will happen).  So without
> > this fix, I can't test any of the other code for fixing rnglists, with
> > or without .dwo files.
>
> Ah ok, I must have understood it wrong then.  I find all of this really confusing.
>
> I'll summarize what I think I understand, please let me know if I say anything wrong.
> We are dealing with two similar ways of dealing with address ranges, one is the
> pre-standard GNU version here, which was using DW_AT_GNU_ranges_base:
>
>   https://gcc.gnu.org/wiki/DebugFission
>
> and the version in DWARF 5.
>
> In the pre-standard version, only one range section was present, and it was in the
> main linked file.  DW_AT_ranges attributes in the dwo, the form DW_FORM_sec_offset,
> are actually pointing at the section in the main file, at offset
>
>   CU's DW_AT_GNU_ranges_base value (found in the skeleton, in the main file) + DIE's DW_AT_ranges
>
> If there is a DW_AT_ranges attribute in the skeleton CU, in the main linked file,
> then it is absolute, it is _not_ added to the DW_AT_GNU_ranges_base value.  Hence
> the `needs_range_base` thingy.  The `ranges_offset` variable in this function is
> the direct offset to the range list to read.
>
> In DWARF 5, we have a ranges section in the main linked file and one in the dwo.  DW_AT_ranges
> attributes refer to the ranges section of the file they are in.  The DW_AT_rnglists_base
> points to the beginning of the range table for that CU.  The actual value of DW_AT_ranges
> attribute (when using the DW_FORM_rnglistx form) is an index in the offset table.  However,
> due to the reprocessing you added in this patch, the attribute value is now in fact the
> offset with `DW_AT_rnglists_base` already added.  And _that's_ the reason why
> DW_FORM_rnglistx attributes should not have the range base applied.
>
> I think the comment above in the code is misleading and needs to be updated, this one:
>
>           /* 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.  */
>
> In commit 18a8505e38fc ("Dwarf 5: Handle debug_str_offsets and indexed attributes
> that have base offsets."), the it was changed like so:
>
> -         /* DW_AT_ranges_base does not apply to DIEs from the DWO skeleton.
> +         /* DW_AT_rnglists_base does not apply to DIEs from the DWO skeleton.
>
> I think it was a wrong change, because it mixes two different things.  DW_AT_rnglists_base
> is a DWARF 5 thing, and the range base not applying to the values from the skeleton is a
> pre-standard thing.  I think it should say something like:
>
>           /* DW_AT_GNU_ranges_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.
>
>              Attributes of form DW_AT_rnglistx have already had their value changed
>              by read_rnglist_index and already include DW_AT_rnglists_base, so don't
>              add the ranges base either.  */
>
> >
> >> I'm also not sure I understand it: when the form is DW_FORM_rnglistx, don't
> >> we want to sometimes apply the base coming from DW_AT_rnglists_base?
> >
> > It all depends on what form is being used. If the DW_AT_ranges is
> > using DW_FORM_sec_offset (which is what
> > GCC generates), then you do need to add the base value.But if you
> > are using the DW_FORM_rnglisx (which is what clang generates), the
> > base value has already been considered/incorporated into the attribute
> > so adding it in (again) causes the value to be incorrect (again, turn
> > on complaints and see what happens if you try to add the base into
> > something with DW_FORM_rnglistx).  From the DWARF5 spec: "[A] rnglist
> > is either represented as: An index into the .debug_rnglists section
> > (DW_FORM_rnglistx). The ...operand identifies an offset location
> > relative to the base of that section...[or] An offset into the
> > .debug_rnglists section (DW_FORM_sec_offset).  The operand consists of
> > a byte offset from the beginning of the .debug_rnglists section."
> > Note the DW_FORM_rnglistx is already relative to the base.
>
> Ok.  What was not clear to me in the first pass was that read_rnglist_index
> already has modified the attribute value to include DW_AT_rnglists_base.
>
> Now, about values of class rnglist and form DW_FORM_sec_offset in DWARF 5.
> The spec says this about them:
>
>   - An offset into the .debug_rnglists section (DW_FORM_sec_offset).
>   The operand consists of a byte offset from the beginning of the
>   .debug_rnglists section. It is relocatable in a relocatable object file,
>   and relocated in an executable or shared object file. In the 32-bit
>   DWARF format, this offset is a 4-byte unsigned value; in the 64-bit
>   DWARF format, it is an 8-byte unsigned value (see Section 7.4 on
>   page 196).
>
> Since it says that the value is already relocated in an executable, does it
> mean we should not add the base for these, in DWARF 5?
>
> > Now, my question:  I'm not quite clear as to whether you really want
> > me to change the code in
> > cu_debug_rnglists_section or not.   I wrote the code mostly by copying
> > the code for loclists as closely as possible, and making whatever
> > changes to that were required to make it correct for rnglists.  Is it
> > better to have the code more consistent with the loclists code?If
> > you really want me to make the changes there you suggest, then I will;
>
> I think the situation is a bit different than for `cu_debug_loc_section`:
> when you have a dwo, I don't think there is the possibility of having an
> attribute referring to a location list in the skeleton.  So if you have a
> dwo, you'll always want the loc section from the dwo.  So there, the pattern
>
>   if (there is a dwo)
>     return the dwo section;
>
>   return the main file section;
>
> makes more sense.  But in the case of range lists, we have attributes
> in both the skeleton and the dwo that require range lists, and the right
> section to return depends on where the attribute is.  That's why, when I
> see this pattern:
>
>   if (there is a dwo)
>     return the dwo section;
>
>   return the main file section;
>
> that pattern, I find it looks wrong, because the right section to return
> does not depend (only) on whether there is dwo.  As we saw, it only works
> "by chance" because this is only called once for the skeleton (when
> reading DW_AT_ranges) and the `cu->dwo_unit` pointer happens not to be set
> up yet.  It is also one refactor away from breaking, if for some reason we
> reorder some code and the `cu->dwo_unit` pointer gets initialized earlier.
>
> > I just wanted confirmation that you really want that change (i.e.
> > adding the dwarf tag and checking vs. DW_TAG_compile_unit, & calling
> > it from dwarf2_rnglists_process).
>
> I think that checking DW_TAG_compile_unit is still a bit of a hack - ideally,
> the caller could just tell us if the DIE comes from a dwo or not.  But at
> least it's logical and based on some DWARF knowledge.  It would certainly
> require a comment to explain it, because it wouldn't be obvious.
>
> > @@ -1379,11 +1382,16 @@ 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 *);
> > +                            struct dwarf2_cu *, dwarf2_psymtab *,
> > +                            dwarf_tag);
>
> This declaration is unneeded and can simply be removed.  Could you push an
> obvious patch that does this?.
>
>
> > @@ -13917,8 +13969,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 ranges
> > +              data.  */
> > +           complaint (_("Invalid .debug_rnglists data (no base address)"));
>
> The comment fits on a single line.  But it can probably be changed to be more
> specific to DW_RLE_offset_pair.  Maybe the complaint could be more specific too,
> and mention DW_RLE_offset_pair.  It would be valid to have .debug_rnglists without
> a base address (I think?), but here it's invalid because there is not base address
> _and_ we have encountered a DW_RLE_offset_pair.
>
> > @@ -19094,6 +19167,57 @@ 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)
> > +{
> > +  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 ? LOCLIST_HEADER_SIZE32
> > +     : LOCLIST_HEADER_SIZE64);
>
> Should this really refer to loclist header sizes?  If they are the same
> sizes as range list table headers, I suppose it's just a coincidence.
>
> > +  ULONGEST rnglist_base =
> > +      (cu->dwo_unit) ? rnglist_header_size : cu->ranges_base;
>
> cu->dwo_unit != nullptr
>
> > +  ULONGEST start_offset =
> > +    rnglist_base + rnglist_index * cu->header.offset_size;
> > +
> > +  /* Get rnglists section.  */
> > +  struct dwarf2_section_info *section = cu_debug_rnglists_section (cu);
> > +  if (section == nullptr)
> > +    error (_("Cannot find .debug_rnglists section [in module %s]"),
> > +        objfile_name(objfile));
>
> Space before paren.
>
> > +
> > +  /* Read the rnglists section content.  */
> > +  section->read (objfile);
> > +  if (section->buffer == NULL)
>
> In new code, I suggest using nullptr instead of NULL, just for consistency.
>
> > +    error (_("DW_FORM_rnglistx used without .debug_rnglists section "
> > +          "[in module %s]"),
> > +       objfile_name (objfile));
> > +
> > +  /* Verify the rnglist header is valid (same as loclist header).  */
> > +  struct loclist_header header;
> > +  read_loclist_header (&header, section);
>
> Ok well, if we are going to take advantage that they are the same, the name
> of the function and types should reflect it.  `read_loclist_header` should
> become `read_loclists_rnglists_header` (using the plural to match the section
> names).
>
> > +  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));
>
> Space before paren.
>
> > +
> > +  /* 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));
> > +
> > +  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;
>
> An idea for a subsequent cleanup, we could have a `read_offset` function
> that does
>
>   if (cu->header.offset_size == 4)
>     return read_4_bytes (abfd, info_ptr);
>   else
>     return read_8_bytes (abfd, info_ptr);
>
> There are a few spots that could use it.
>
> Simon

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

From f88972d75ec7d6b5efc596ed8f316d141fbafffa Mon Sep 17 00:00:00 2001
From: Caroline Tice <cmtice@google.com>
Date: Wed, 1 Jul 2020 12:39:08 -0700
Subject: [PATCH v4] 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                             | 259 +++++++++++++++---
 gdb/testsuite/gdb.dwarf2/dw5-rnglist-test.cc  |  97 +++++++
 gdb/testsuite/gdb.dwarf2/dw5-rnglist-test.exp |  40 +++
 3 files changed, 352 insertions(+), 44 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 405b5fb334..b1f54fb8bf 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" },
@@ -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
@@ -12448,6 +12458,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;
@@ -13792,7 +13807,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;
@@ -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;
+
+  rnglists_section->read (objfile);
+  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 ();
 
@@ -13820,8 +13851,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)
@@ -13843,6 +13874,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)
 	    {
@@ -13861,6 +13897,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;
@@ -13889,6 +13938,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;
@@ -13898,14 +13960,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.  */
@@ -13917,8 +13971,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.  */
@@ -13951,7 +14017,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;
@@ -13967,7 +14033,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;
 
@@ -14053,7 +14119,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 ();
@@ -14063,7 +14129,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)
@@ -14155,8 +14221,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
@@ -14164,7 +14236,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;
@@ -14326,8 +14399,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.  */
@@ -14335,7 +14414,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;
@@ -18165,8 +18244,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;
 }
@@ -18535,7 +18619,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)
@@ -18681,10 +18765,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
@@ -18693,7 +18783,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;
@@ -19017,11 +19107,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 loclist_header *header,
+			       struct dwarf2_section_info *section)
 {
   unsigned int bytes_read;
   bfd *abfd = section->get_bfd_owner ();
@@ -19075,7 +19165,7 @@ read_loclist_index (struct dwarf2_cu *cu, ULONGEST loclist_index)
     complaint (_("DW_FORM_loclistx used without .debug_loclists "
 	        "section [in module %s]"), objfile_name (objfile));
   struct loclist_header header;
-  read_loclist_header (&header, section);
+  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]"),
@@ -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
+   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));
+
+  /* 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 header is valid (same as loclist header).  */
+  struct loclist_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));
+
+  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)
@@ -19112,6 +19254,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:
@@ -19293,8 +19438,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;
@@ -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;
+}
+
 /* 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.383.g050319c2ae-goog