[PATCH 0/2] Two .debug_aranges changes

[PATCH 0/2] Two .debug_aranges changes

[Tom Tromey]

I wanted to change the DWARF reader to pre-read the .debug_aranges
section (actually just read it a little earlier than is currently
done); and while doing this I realized I could move the aranges code out to its own file.  This series is the result.q

Tom

[PATCH 1/2] Pre-read .debug_aranges section

[Tom Tromey]

While working on background DWARF reading, I found a race case that I
tracked down to the handling of the .debug_aranges section.  Currently
the section data is only read in after the CUs have all been created.
However, there's no real reason to do this -- it seems fine to read it
a little earlier, when all the other necessary sections are read in.

This patch makes this change, and updates the
read_addrmap_from_aranges API to assert that the section is read in.

This patch slightly changes the read_addrmap_from_aranges API as well,
to reject an empty section.  This seems better to me than what the
current code does, which is try to read an empty section but then do
no work.

Regression tested on x86-64 Fedora 38.
---
 gdb/dwarf2/read-debug-names.c |  1 +
 gdb/dwarf2/read.c             | 11 ++++++-----
 2 files changed, 7 insertions(+), 5 deletions(-)

diff --git a/gdb/dwarf2/read-debug-names.c b/gdb/dwarf2/read-debug-names.c
index 2e5067efb3d..89f5df6df90 100644
--- a/gdb/dwarf2/read-debug-names.c
+++ b/gdb/dwarf2/read-debug-names.c
@@ -166,6 +166,7 @@ create_addrmap_from_aranges (dwarf2_per_objfile *per_objfile,
 
   addrmap_mutable mutable_map;
 
+  section->read (per_objfile->objfile);
   if (read_addrmap_from_aranges (per_objfile, section, &mutable_map))
     per_bfd->index_addrmap
       = new (&per_bfd->obstack) addrmap_fixed (&per_bfd->obstack,
diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c
index d4aec19d31d..13ac83395eb 100644
--- a/gdb/dwarf2/read.c
+++ b/gdb/dwarf2/read.c
@@ -1586,6 +1586,7 @@ dwarf2_per_bfd::map_info_sections (struct objfile *objfile)
   ranges.read (objfile);
   rnglists.read (objfile);
   addr.read (objfile);
+  debug_aranges.read (objfile);
 
   for (auto &section : types)
     section.read (objfile);
@@ -1843,6 +1844,11 @@ read_addrmap_from_aranges (dwarf2_per_objfile *per_objfile,
 			   dwarf2_section_info *section,
 			   addrmap *mutable_map)
 {
+  /* Caller must ensure this is read in.  */
+  gdb_assert (section->readin);
+  if (section->empty ())
+    return false;
+
   struct objfile *objfile = per_objfile->objfile;
   bfd *abfd = objfile->obfd.get ();
   struct gdbarch *gdbarch = objfile->arch ();
@@ -1870,13 +1876,8 @@ read_addrmap_from_aranges (dwarf2_per_objfile *per_objfile,
     }
 
   std::set<sect_offset> debug_info_offset_seen;
-
-  section->read (objfile);
-
   const bfd_endian dwarf5_byte_order = gdbarch_byte_order (gdbarch);
-
   const gdb_byte *addr = section->buffer;
-
   while (addr < section->buffer + section->size)
     {
       const gdb_byte *const entry_addr = addr;
-- 
2.41.0

[PATCH 2/2] Move read_addrmap_from_aranges to new file

[Tom Tromey]

In the interest of shrinking dwarf2/read.c a little more, this patch
moves the code that deciphers .debug_aranges into a new file.
---
 gdb/Makefile.in               |   2 +
 gdb/dwarf2/aranges.c          | 200 ++++++++++++++++++++++++++++++++++
 gdb/dwarf2/aranges.h          |  35 ++++++
 gdb/dwarf2/read-debug-names.c |   1 +
 gdb/dwarf2/read.c             | 179 +-----------------------------
 gdb/dwarf2/read.h             |   8 --
 6 files changed, 239 insertions(+), 186 deletions(-)
 create mode 100644 gdb/dwarf2/aranges.c
 create mode 100644 gdb/dwarf2/aranges.h

diff --git a/gdb/Makefile.in b/gdb/Makefile.in
index 9c0a0bff2cd..52b08692b52 100644
--- a/gdb/Makefile.in
+++ b/gdb/Makefile.in
@@ -1065,6 +1065,7 @@ COMMON_SFILES = \
 	dwarf2/abbrev.c \
 	dwarf2/abbrev-cache.c \
 	dwarf2/ada-imported.c \
+	dwarf2/aranges.c \
 	dwarf2/attribute.c \
 	dwarf2/comp-unit-head.c \
 	dwarf2/cooked-index.c \
@@ -1321,6 +1322,7 @@ HFILES_NO_SRCDIR = \
 	disasm-flags.h \
 	disasm.h \
 	dummy-frame.h \
+	dwarf2/aranges.h \
 	dwarf2/cooked-index.h \
 	dwarf2/cu.h \
 	dwarf2/frame-tailcall.h \
diff --git a/gdb/dwarf2/aranges.c b/gdb/dwarf2/aranges.c
new file mode 100644
index 00000000000..3298d43e486
--- /dev/null
+++ b/gdb/dwarf2/aranges.c
@@ -0,0 +1,200 @@
+/* DWARF aranges handling
+
+   Copyright (C) 1994-2023 Free Software Foundation, Inc.
+
+   This file is part of GDB.
+
+   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 "defs.h"
+#include "dwarf2/aranges.h"
+#include "dwarf2/read.h"
+
+/* See aranges.h.  */
+
+bool
+read_addrmap_from_aranges (dwarf2_per_objfile *per_objfile,
+			   dwarf2_section_info *section,
+			   addrmap *mutable_map)
+{
+  /* Caller must ensure this is read in.  */
+  gdb_assert (section->readin);
+  if (section->empty ())
+    return false;
+
+  struct objfile *objfile = per_objfile->objfile;
+  bfd *abfd = objfile->obfd.get ();
+  struct gdbarch *gdbarch = objfile->arch ();
+  dwarf2_per_bfd *per_bfd = per_objfile->per_bfd;
+
+  std::unordered_map<sect_offset,
+		     dwarf2_per_cu_data *,
+		     gdb::hash_enum<sect_offset>>
+    debug_info_offset_to_per_cu;
+  for (const auto &per_cu : per_bfd->all_units)
+    {
+      /* A TU will not need aranges, and skipping them here is an easy
+	 way of ignoring .debug_types -- and possibly seeing a
+	 duplicate section offset -- entirely.  The same applies to
+	 units coming from a dwz file.  */
+      if (per_cu->is_debug_types || per_cu->is_dwz)
+	continue;
+
+      const auto insertpair
+	= debug_info_offset_to_per_cu.emplace (per_cu->sect_off,
+					       per_cu.get ());
+
+      /* Assume no duplicate offsets in all_units.  */
+      gdb_assert (insertpair.second);
+    }
+
+  std::set<sect_offset> debug_info_offset_seen;
+  const bfd_endian dwarf5_byte_order = gdbarch_byte_order (gdbarch);
+  const gdb_byte *addr = section->buffer;
+  while (addr < section->buffer + section->size)
+    {
+      const gdb_byte *const entry_addr = addr;
+      unsigned int bytes_read;
+
+      const LONGEST entry_length = read_initial_length (abfd, addr,
+							&bytes_read);
+      addr += bytes_read;
+
+      const gdb_byte *const entry_end = addr + entry_length;
+      const bool dwarf5_is_dwarf64 = bytes_read != 4;
+      const uint8_t offset_size = dwarf5_is_dwarf64 ? 8 : 4;
+      if (addr + entry_length > section->buffer + section->size)
+	{
+	  warning (_("Section .debug_aranges in %s entry at offset %s "
+		     "length %s exceeds section length %s, "
+		     "ignoring .debug_aranges."),
+		   objfile_name (objfile),
+		   plongest (entry_addr - section->buffer),
+		   plongest (bytes_read + entry_length),
+		   pulongest (section->size));
+	  return false;
+	}
+
+      /* The version number.  */
+      const uint16_t version = read_2_bytes (abfd, addr);
+      addr += 2;
+      if (version != 2)
+	{
+	  warning (_("Section .debug_aranges in %s entry at offset %s "
+		     "has unsupported version %d, ignoring .debug_aranges."),
+		   objfile_name (objfile),
+		   plongest (entry_addr - section->buffer), version);
+	  return false;
+	}
+
+      const uint64_t debug_info_offset
+	= extract_unsigned_integer (addr, offset_size, dwarf5_byte_order);
+      addr += offset_size;
+      const auto per_cu_it
+	= debug_info_offset_to_per_cu.find (sect_offset (debug_info_offset));
+      if (per_cu_it == debug_info_offset_to_per_cu.cend ())
+	{
+	  warning (_("Section .debug_aranges in %s entry at offset %s "
+		     "debug_info_offset %s does not exists, "
+		     "ignoring .debug_aranges."),
+		   objfile_name (objfile),
+		   plongest (entry_addr - section->buffer),
+		   pulongest (debug_info_offset));
+	  return false;
+	}
+      const auto insertpair
+	= debug_info_offset_seen.insert (sect_offset (debug_info_offset));
+      if (!insertpair.second)
+	{
+	  warning (_("Section .debug_aranges in %s has duplicate "
+		     "debug_info_offset %s, ignoring .debug_aranges."),
+		   objfile_name (objfile),
+		   sect_offset_str (sect_offset (debug_info_offset)));
+	  return false;
+	}
+      dwarf2_per_cu_data *const per_cu = per_cu_it->second;
+
+      const uint8_t address_size = *addr++;
+      if (address_size < 1 || address_size > 8)
+	{
+	  warning (_("Section .debug_aranges in %s entry at offset %s "
+		     "address_size %u is invalid, ignoring .debug_aranges."),
+		   objfile_name (objfile),
+		   plongest (entry_addr - section->buffer), address_size);
+	  return false;
+	}
+
+      const uint8_t segment_selector_size = *addr++;
+      if (segment_selector_size != 0)
+	{
+	  warning (_("Section .debug_aranges in %s entry at offset %s "
+		     "segment_selector_size %u is not supported, "
+		     "ignoring .debug_aranges."),
+		   objfile_name (objfile),
+		   plongest (entry_addr - section->buffer),
+		   segment_selector_size);
+	  return false;
+	}
+
+      /* Must pad to an alignment boundary that is twice the address
+	 size.  It is undocumented by the DWARF standard but GCC does
+	 use it.  However, not every compiler does this.  We can see
+	 whether it has happened by looking at the total length of the
+	 contents of the aranges for this CU -- it if isn't a multiple
+	 of twice the address size, then we skip any leftover
+	 bytes.  */
+      addr += (entry_end - addr) % (2 * address_size);
+
+      while (addr < entry_end)
+	{
+	  if (addr + 2 * address_size > entry_end)
+	    {
+	      warning (_("Section .debug_aranges in %s entry at offset %s "
+			 "address list is not properly terminated, "
+			 "ignoring .debug_aranges."),
+		       objfile_name (objfile),
+		       plongest (entry_addr - section->buffer));
+	      return false;
+	    }
+	  ULONGEST start = extract_unsigned_integer (addr, address_size,
+						     dwarf5_byte_order);
+	  addr += address_size;
+	  ULONGEST length = extract_unsigned_integer (addr, address_size,
+						      dwarf5_byte_order);
+	  addr += address_size;
+	  if (start == 0 && length == 0)
+	    {
+	      /* This can happen on some targets with --gc-sections.
+		 This pair of values is also used to mark the end of
+		 the entries for a given CU, but we ignore it and
+		 instead handle termination using the check at the top
+		 of the loop.  */
+	      continue;
+	    }
+	  if (start == 0 && !per_bfd->has_section_at_zero)
+	    {
+	      /* Symbol was eliminated due to a COMDAT group.  */
+	      continue;
+	    }
+	  ULONGEST end = start + length;
+	  start = (ULONGEST) per_objfile->adjust ((unrelocated_addr) start);
+	  end = (ULONGEST) per_objfile->adjust ((unrelocated_addr) end);
+	  mutable_map->set_empty (start, end - 1, per_cu);
+	}
+
+      per_cu->addresses_seen = true;
+    }
+
+  return true;
+}
diff --git a/gdb/dwarf2/aranges.h b/gdb/dwarf2/aranges.h
new file mode 100644
index 00000000000..43e1cbd0930
--- /dev/null
+++ b/gdb/dwarf2/aranges.h
@@ -0,0 +1,35 @@
+/* DWARF aranges handling
+
+   Copyright (C) 1994-2023 Free Software Foundation, Inc.
+
+   This file is part of GDB.
+
+   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/>.  */
+
+#ifndef GDB_DWARF2_ARANGES_H
+#define GDB_DWARF2_ARANGES_H
+
+class dwarf2_per_objfile;
+class dwarf2_section_info;
+class addrmap;
+
+/* Read the address map data from DWARF-5 .debug_aranges, and use it
+   to populate given addrmap.  Returns true on success, false on
+   failure.  */
+
+extern bool read_addrmap_from_aranges (dwarf2_per_objfile *per_objfile,
+				       dwarf2_section_info *section,
+				       addrmap *mutable_map);
+
+#endif /* GDB_DWARF2_ARANGES_H */
diff --git a/gdb/dwarf2/read-debug-names.c b/gdb/dwarf2/read-debug-names.c
index 89f5df6df90..c1b62b38f93 100644
--- a/gdb/dwarf2/read-debug-names.c
+++ b/gdb/dwarf2/read-debug-names.c
@@ -19,6 +19,7 @@
 
 #include "defs.h"
 #include "read-debug-names.h"
+#include "dwarf2/aranges.h"
 
 #include "complaints.h"
 #include "cp-support.h"
diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c
index 13ac83395eb..1ca640df6be 100644
--- a/gdb/dwarf2/read.c
+++ b/gdb/dwarf2/read.c
@@ -31,6 +31,7 @@
 #include "defs.h"
 #include "dwarf2/read.h"
 #include "dwarf2/abbrev.h"
+#include "dwarf2/aranges.h"
 #include "dwarf2/attribute.h"
 #include "dwarf2/comp-unit-head.h"
 #include "dwarf2/cu.h"
@@ -1837,184 +1838,6 @@ create_cu_from_index_list (dwarf2_per_bfd *per_bfd,
   return the_cu;
 }
 
-/* See read.h.  */
-
-bool
-read_addrmap_from_aranges (dwarf2_per_objfile *per_objfile,
-			   dwarf2_section_info *section,
-			   addrmap *mutable_map)
-{
-  /* Caller must ensure this is read in.  */
-  gdb_assert (section->readin);
-  if (section->empty ())
-    return false;
-
-  struct objfile *objfile = per_objfile->objfile;
-  bfd *abfd = objfile->obfd.get ();
-  struct gdbarch *gdbarch = objfile->arch ();
-  dwarf2_per_bfd *per_bfd = per_objfile->per_bfd;
-
-  std::unordered_map<sect_offset,
-		     dwarf2_per_cu_data *,
-		     gdb::hash_enum<sect_offset>>
-    debug_info_offset_to_per_cu;
-  for (const auto &per_cu : per_bfd->all_units)
-    {
-      /* A TU will not need aranges, and skipping them here is an easy
-	 way of ignoring .debug_types -- and possibly seeing a
-	 duplicate section offset -- entirely.  The same applies to
-	 units coming from a dwz file.  */
-      if (per_cu->is_debug_types || per_cu->is_dwz)
-	continue;
-
-      const auto insertpair
-	= debug_info_offset_to_per_cu.emplace (per_cu->sect_off,
-					       per_cu.get ());
-
-      /* Assume no duplicate offsets in all_units.  */
-      gdb_assert (insertpair.second);
-    }
-
-  std::set<sect_offset> debug_info_offset_seen;
-  const bfd_endian dwarf5_byte_order = gdbarch_byte_order (gdbarch);
-  const gdb_byte *addr = section->buffer;
-  while (addr < section->buffer + section->size)
-    {
-      const gdb_byte *const entry_addr = addr;
-      unsigned int bytes_read;
-
-      const LONGEST entry_length = read_initial_length (abfd, addr,
-							&bytes_read);
-      addr += bytes_read;
-
-      const gdb_byte *const entry_end = addr + entry_length;
-      const bool dwarf5_is_dwarf64 = bytes_read != 4;
-      const uint8_t offset_size = dwarf5_is_dwarf64 ? 8 : 4;
-      if (addr + entry_length > section->buffer + section->size)
-	{
-	  warning (_("Section .debug_aranges in %s entry at offset %s "
-		     "length %s exceeds section length %s, "
-		     "ignoring .debug_aranges."),
-		   objfile_name (objfile),
-		   plongest (entry_addr - section->buffer),
-		   plongest (bytes_read + entry_length),
-		   pulongest (section->size));
-	  return false;
-	}
-
-      /* The version number.  */
-      const uint16_t version = read_2_bytes (abfd, addr);
-      addr += 2;
-      if (version != 2)
-	{
-	  warning (_("Section .debug_aranges in %s entry at offset %s "
-		     "has unsupported version %d, ignoring .debug_aranges."),
-		   objfile_name (objfile),
-		   plongest (entry_addr - section->buffer), version);
-	  return false;
-	}
-
-      const uint64_t debug_info_offset
-	= extract_unsigned_integer (addr, offset_size, dwarf5_byte_order);
-      addr += offset_size;
-      const auto per_cu_it
-	= debug_info_offset_to_per_cu.find (sect_offset (debug_info_offset));
-      if (per_cu_it == debug_info_offset_to_per_cu.cend ())
-	{
-	  warning (_("Section .debug_aranges in %s entry at offset %s "
-		     "debug_info_offset %s does not exists, "
-		     "ignoring .debug_aranges."),
-		   objfile_name (objfile),
-		   plongest (entry_addr - section->buffer),
-		   pulongest (debug_info_offset));
-	  return false;
-	}
-      const auto insertpair
-	= debug_info_offset_seen.insert (sect_offset (debug_info_offset));
-      if (!insertpair.second)
-	{
-	  warning (_("Section .debug_aranges in %s has duplicate "
-		     "debug_info_offset %s, ignoring .debug_aranges."),
-		   objfile_name (objfile),
-		   sect_offset_str (sect_offset (debug_info_offset)));
-	  return false;
-	}
-      dwarf2_per_cu_data *const per_cu = per_cu_it->second;
-
-      const uint8_t address_size = *addr++;
-      if (address_size < 1 || address_size > 8)
-	{
-	  warning (_("Section .debug_aranges in %s entry at offset %s "
-		     "address_size %u is invalid, ignoring .debug_aranges."),
-		   objfile_name (objfile),
-		   plongest (entry_addr - section->buffer), address_size);
-	  return false;
-	}
-
-      const uint8_t segment_selector_size = *addr++;
-      if (segment_selector_size != 0)
-	{
-	  warning (_("Section .debug_aranges in %s entry at offset %s "
-		     "segment_selector_size %u is not supported, "
-		     "ignoring .debug_aranges."),
-		   objfile_name (objfile),
-		   plongest (entry_addr - section->buffer),
-		   segment_selector_size);
-	  return false;
-	}
-
-      /* Must pad to an alignment boundary that is twice the address
-	 size.  It is undocumented by the DWARF standard but GCC does
-	 use it.  However, not every compiler does this.  We can see
-	 whether it has happened by looking at the total length of the
-	 contents of the aranges for this CU -- it if isn't a multiple
-	 of twice the address size, then we skip any leftover
-	 bytes.  */
-      addr += (entry_end - addr) % (2 * address_size);
-
-      while (addr < entry_end)
-	{
-	  if (addr + 2 * address_size > entry_end)
-	    {
-	      warning (_("Section .debug_aranges in %s entry at offset %s "
-			 "address list is not properly terminated, "
-			 "ignoring .debug_aranges."),
-		       objfile_name (objfile),
-		       plongest (entry_addr - section->buffer));
-	      return false;
-	    }
-	  ULONGEST start = extract_unsigned_integer (addr, address_size,
-						     dwarf5_byte_order);
-	  addr += address_size;
-	  ULONGEST length = extract_unsigned_integer (addr, address_size,
-						      dwarf5_byte_order);
-	  addr += address_size;
-	  if (start == 0 && length == 0)
-	    {
-	      /* This can happen on some targets with --gc-sections.
-		 This pair of values is also used to mark the end of
-		 the entries for a given CU, but we ignore it and
-		 instead handle termination using the check at the top
-		 of the loop.  */
-	      continue;
-	    }
-	  if (start == 0 && !per_bfd->has_section_at_zero)
-	    {
-	      /* Symbol was eliminated due to a COMDAT group.  */
-	      continue;
-	    }
-	  ULONGEST end = start + length;
-	  start = (ULONGEST) per_objfile->adjust ((unrelocated_addr) start);
-	  end = (ULONGEST) per_objfile->adjust ((unrelocated_addr) end);
-	  mutable_map->set_empty (start, end - 1, per_cu);
-	}
-
-      per_cu->addresses_seen = true;
-    }
-
-  return true;
-}
-
 /* die_reader_func for dw2_get_file_names.  */
 
 static void
diff --git a/gdb/dwarf2/read.h b/gdb/dwarf2/read.h
index 1d9432c5c11..2cf40a8c2cd 100644
--- a/gdb/dwarf2/read.h
+++ b/gdb/dwarf2/read.h
@@ -955,12 +955,4 @@ extern void finalize_all_units (dwarf2_per_bfd *per_bfd);
 
 extern htab_up create_quick_file_names_table (unsigned int nr_initial_entries);
 
-/* Read the address map data from DWARF-5 .debug_aranges, and use it
-   to populate given addrmap.  Returns true on success, false on
-   failure.  */
-
-extern bool read_addrmap_from_aranges (dwarf2_per_objfile *per_objfile,
-				       dwarf2_section_info *section,
-				       addrmap *mutable_map);
-
 #endif /* DWARF2READ_H */
-- 
2.41.0

Re: [PATCH 1/2] Pre-read .debug_aranges section

[Guinevere Larsen]

On 14/10/2023 22:56, Tom Tromey wrote:
> While working on background DWARF reading, I found a race case that I
> tracked down to the handling of the .debug_aranges section.  Currently
> the section data is only read in after the CUs have all been created.
> However, there's no real reason to do this -- it seems fine to read it
> a little earlier, when all the other necessary sections are read in.
>
> This patch makes this change, and updates the
> read_addrmap_from_aranges API to assert that the section is read in.

I tested this as well and see no issues either. My one comment is about 
describing the section as "read in".

It made it very confusing to read (I thought there was a word missing 
somewhere at first), and the description of the "readin" flag says:

     /* True if we have tried to read this section.  */

So I think it would be better to describe that assert as "assert that 
the section has been read" or some other way to make it explicit that 
"read" is the past tense.

With some small rewording around this bit, everything looks good to me, 
Reviewed-By: Guinevere Larsen <blarsen@redhat.com>

-- 
Cheers,
Guinevere Larsen
She/Her/Hers

>
> This patch slightly changes the read_addrmap_from_aranges API as well,
> to reject an empty section.  This seems better to me than what the
> current code does, which is try to read an empty section but then do
> no work.
>
> Regression tested on x86-64 Fedora 38.
> ---
>   gdb/dwarf2/read-debug-names.c |  1 +
>   gdb/dwarf2/read.c             | 11 ++++++-----
>   2 files changed, 7 insertions(+), 5 deletions(-)
>
> diff --git a/gdb/dwarf2/read-debug-names.c b/gdb/dwarf2/read-debug-names.c
> index 2e5067efb3d..89f5df6df90 100644
> --- a/gdb/dwarf2/read-debug-names.c
> +++ b/gdb/dwarf2/read-debug-names.c
> @@ -166,6 +166,7 @@ create_addrmap_from_aranges (dwarf2_per_objfile *per_objfile,
>   
>     addrmap_mutable mutable_map;
>   
> +  section->read (per_objfile->objfile);
>     if (read_addrmap_from_aranges (per_objfile, section, &mutable_map))
>       per_bfd->index_addrmap
>         = new (&per_bfd->obstack) addrmap_fixed (&per_bfd->obstack,
> diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c
> index d4aec19d31d..13ac83395eb 100644
> --- a/gdb/dwarf2/read.c
> +++ b/gdb/dwarf2/read.c
> @@ -1586,6 +1586,7 @@ dwarf2_per_bfd::map_info_sections (struct objfile *objfile)
>     ranges.read (objfile);
>     rnglists.read (objfile);
>     addr.read (objfile);
> +  debug_aranges.read (objfile);
>   
>     for (auto &section : types)
>       section.read (objfile);
> @@ -1843,6 +1844,11 @@ read_addrmap_from_aranges (dwarf2_per_objfile *per_objfile,
>   			   dwarf2_section_info *section,
>   			   addrmap *mutable_map)
>   {
> +  /* Caller must ensure this is read in.  */
> +  gdb_assert (section->readin);
> +  if (section->empty ())
> +    return false;
> +
>     struct objfile *objfile = per_objfile->objfile;
>     bfd *abfd = objfile->obfd.get ();
>     struct gdbarch *gdbarch = objfile->arch ();
> @@ -1870,13 +1876,8 @@ read_addrmap_from_aranges (dwarf2_per_objfile *per_objfile,
>       }
>   
>     std::set<sect_offset> debug_info_offset_seen;
> -
> -  section->read (objfile);
> -
>     const bfd_endian dwarf5_byte_order = gdbarch_byte_order (gdbarch);
> -
>     const gdb_byte *addr = section->buffer;
> -
>     while (addr < section->buffer + section->size)
>       {
>         const gdb_byte *const entry_addr = addr;

Re: [PATCH 2/2] Move read_addrmap_from_aranges to new file

[Guinevere Larsen]

On 14/10/2023 22:56, Tom Tromey wrote:
> In the interest of shrinking dwarf2/read.c a little more, this patch
> moves the code that deciphers .debug_aranges into a new file.
This is a good change, I just have a small question: why you only chose 
to move read_addrmap_from_aranges and not also move 
create_addrmap_from_aranges? I know the second function isn't in 
dwarf2/read.c, but I think it would make aranges.c more consistent.

-- 
Cheers,
Guinevere Larsen
She/Her/Hers

> ---
>   gdb/Makefile.in               |   2 +
>   gdb/dwarf2/aranges.c          | 200 ++++++++++++++++++++++++++++++++++
>   gdb/dwarf2/aranges.h          |  35 ++++++
>   gdb/dwarf2/read-debug-names.c |   1 +
>   gdb/dwarf2/read.c             | 179 +-----------------------------
>   gdb/dwarf2/read.h             |   8 --
>   6 files changed, 239 insertions(+), 186 deletions(-)
>   create mode 100644 gdb/dwarf2/aranges.c
>   create mode 100644 gdb/dwarf2/aranges.h
>
> diff --git a/gdb/Makefile.in b/gdb/Makefile.in
> index 9c0a0bff2cd..52b08692b52 100644
> --- a/gdb/Makefile.in
> +++ b/gdb/Makefile.in
> @@ -1065,6 +1065,7 @@ COMMON_SFILES = \
>   	dwarf2/abbrev.c \
>   	dwarf2/abbrev-cache.c \
>   	dwarf2/ada-imported.c \
> +	dwarf2/aranges.c \
>   	dwarf2/attribute.c \
>   	dwarf2/comp-unit-head.c \
>   	dwarf2/cooked-index.c \
> @@ -1321,6 +1322,7 @@ HFILES_NO_SRCDIR = \
>   	disasm-flags.h \
>   	disasm.h \
>   	dummy-frame.h \
> +	dwarf2/aranges.h \
>   	dwarf2/cooked-index.h \
>   	dwarf2/cu.h \
>   	dwarf2/frame-tailcall.h \
> diff --git a/gdb/dwarf2/aranges.c b/gdb/dwarf2/aranges.c
> new file mode 100644
> index 00000000000..3298d43e486
> --- /dev/null
> +++ b/gdb/dwarf2/aranges.c
> @@ -0,0 +1,200 @@
> +/* DWARF aranges handling
> +
> +   Copyright (C) 1994-2023 Free Software Foundation, Inc.
> +
> +   This file is part of GDB.
> +
> +   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 "defs.h"
> +#include "dwarf2/aranges.h"
> +#include "dwarf2/read.h"
> +
> +/* See aranges.h.  */
> +
> +bool
> +read_addrmap_from_aranges (dwarf2_per_objfile *per_objfile,
> +			   dwarf2_section_info *section,
> +			   addrmap *mutable_map)
> +{
> +  /* Caller must ensure this is read in.  */
> +  gdb_assert (section->readin);
> +  if (section->empty ())
> +    return false;
> +
> +  struct objfile *objfile = per_objfile->objfile;
> +  bfd *abfd = objfile->obfd.get ();
> +  struct gdbarch *gdbarch = objfile->arch ();
> +  dwarf2_per_bfd *per_bfd = per_objfile->per_bfd;
> +
> +  std::unordered_map<sect_offset,
> +		     dwarf2_per_cu_data *,
> +		     gdb::hash_enum<sect_offset>>
> +    debug_info_offset_to_per_cu;
> +  for (const auto &per_cu : per_bfd->all_units)
> +    {
> +      /* A TU will not need aranges, and skipping them here is an easy
> +	 way of ignoring .debug_types -- and possibly seeing a
> +	 duplicate section offset -- entirely.  The same applies to
> +	 units coming from a dwz file.  */
> +      if (per_cu->is_debug_types || per_cu->is_dwz)
> +	continue;
> +
> +      const auto insertpair
> +	= debug_info_offset_to_per_cu.emplace (per_cu->sect_off,
> +					       per_cu.get ());
> +
> +      /* Assume no duplicate offsets in all_units.  */
> +      gdb_assert (insertpair.second);
> +    }
> +
> +  std::set<sect_offset> debug_info_offset_seen;
> +  const bfd_endian dwarf5_byte_order = gdbarch_byte_order (gdbarch);
> +  const gdb_byte *addr = section->buffer;
> +  while (addr < section->buffer + section->size)
> +    {
> +      const gdb_byte *const entry_addr = addr;
> +      unsigned int bytes_read;
> +
> +      const LONGEST entry_length = read_initial_length (abfd, addr,
> +							&bytes_read);
> +      addr += bytes_read;
> +
> +      const gdb_byte *const entry_end = addr + entry_length;
> +      const bool dwarf5_is_dwarf64 = bytes_read != 4;
> +      const uint8_t offset_size = dwarf5_is_dwarf64 ? 8 : 4;
> +      if (addr + entry_length > section->buffer + section->size)
> +	{
> +	  warning (_("Section .debug_aranges in %s entry at offset %s "
> +		     "length %s exceeds section length %s, "
> +		     "ignoring .debug_aranges."),
> +		   objfile_name (objfile),
> +		   plongest (entry_addr - section->buffer),
> +		   plongest (bytes_read + entry_length),
> +		   pulongest (section->size));
> +	  return false;
> +	}
> +
> +      /* The version number.  */
> +      const uint16_t version = read_2_bytes (abfd, addr);
> +      addr += 2;
> +      if (version != 2)
> +	{
> +	  warning (_("Section .debug_aranges in %s entry at offset %s "
> +		     "has unsupported version %d, ignoring .debug_aranges."),
> +		   objfile_name (objfile),
> +		   plongest (entry_addr - section->buffer), version);
> +	  return false;
> +	}
> +
> +      const uint64_t debug_info_offset
> +	= extract_unsigned_integer (addr, offset_size, dwarf5_byte_order);
> +      addr += offset_size;
> +      const auto per_cu_it
> +	= debug_info_offset_to_per_cu.find (sect_offset (debug_info_offset));
> +      if (per_cu_it == debug_info_offset_to_per_cu.cend ())
> +	{
> +	  warning (_("Section .debug_aranges in %s entry at offset %s "
> +		     "debug_info_offset %s does not exists, "
> +		     "ignoring .debug_aranges."),
> +		   objfile_name (objfile),
> +		   plongest (entry_addr - section->buffer),
> +		   pulongest (debug_info_offset));
> +	  return false;
> +	}
> +      const auto insertpair
> +	= debug_info_offset_seen.insert (sect_offset (debug_info_offset));
> +      if (!insertpair.second)
> +	{
> +	  warning (_("Section .debug_aranges in %s has duplicate "
> +		     "debug_info_offset %s, ignoring .debug_aranges."),
> +		   objfile_name (objfile),
> +		   sect_offset_str (sect_offset (debug_info_offset)));
> +	  return false;
> +	}
> +      dwarf2_per_cu_data *const per_cu = per_cu_it->second;
> +
> +      const uint8_t address_size = *addr++;
> +      if (address_size < 1 || address_size > 8)
> +	{
> +	  warning (_("Section .debug_aranges in %s entry at offset %s "
> +		     "address_size %u is invalid, ignoring .debug_aranges."),
> +		   objfile_name (objfile),
> +		   plongest (entry_addr - section->buffer), address_size);
> +	  return false;
> +	}
> +
> +      const uint8_t segment_selector_size = *addr++;
> +      if (segment_selector_size != 0)
> +	{
> +	  warning (_("Section .debug_aranges in %s entry at offset %s "
> +		     "segment_selector_size %u is not supported, "
> +		     "ignoring .debug_aranges."),
> +		   objfile_name (objfile),
> +		   plongest (entry_addr - section->buffer),
> +		   segment_selector_size);
> +	  return false;
> +	}
> +
> +      /* Must pad to an alignment boundary that is twice the address
> +	 size.  It is undocumented by the DWARF standard but GCC does
> +	 use it.  However, not every compiler does this.  We can see
> +	 whether it has happened by looking at the total length of the
> +	 contents of the aranges for this CU -- it if isn't a multiple
> +	 of twice the address size, then we skip any leftover
> +	 bytes.  */
> +      addr += (entry_end - addr) % (2 * address_size);
> +
> +      while (addr < entry_end)
> +	{
> +	  if (addr + 2 * address_size > entry_end)
> +	    {
> +	      warning (_("Section .debug_aranges in %s entry at offset %s "
> +			 "address list is not properly terminated, "
> +			 "ignoring .debug_aranges."),
> +		       objfile_name (objfile),
> +		       plongest (entry_addr - section->buffer));
> +	      return false;
> +	    }
> +	  ULONGEST start = extract_unsigned_integer (addr, address_size,
> +						     dwarf5_byte_order);
> +	  addr += address_size;
> +	  ULONGEST length = extract_unsigned_integer (addr, address_size,
> +						      dwarf5_byte_order);
> +	  addr += address_size;
> +	  if (start == 0 && length == 0)
> +	    {
> +	      /* This can happen on some targets with --gc-sections.
> +		 This pair of values is also used to mark the end of
> +		 the entries for a given CU, but we ignore it and
> +		 instead handle termination using the check at the top
> +		 of the loop.  */
> +	      continue;
> +	    }
> +	  if (start == 0 && !per_bfd->has_section_at_zero)
> +	    {
> +	      /* Symbol was eliminated due to a COMDAT group.  */
> +	      continue;
> +	    }
> +	  ULONGEST end = start + length;
> +	  start = (ULONGEST) per_objfile->adjust ((unrelocated_addr) start);
> +	  end = (ULONGEST) per_objfile->adjust ((unrelocated_addr) end);
> +	  mutable_map->set_empty (start, end - 1, per_cu);
> +	}
> +
> +      per_cu->addresses_seen = true;
> +    }
> +
> +  return true;
> +}
> diff --git a/gdb/dwarf2/aranges.h b/gdb/dwarf2/aranges.h
> new file mode 100644
> index 00000000000..43e1cbd0930
> --- /dev/null
> +++ b/gdb/dwarf2/aranges.h
> @@ -0,0 +1,35 @@
> +/* DWARF aranges handling
> +
> +   Copyright (C) 1994-2023 Free Software Foundation, Inc.
> +
> +   This file is part of GDB.
> +
> +   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/>.  */
> +
> +#ifndef GDB_DWARF2_ARANGES_H
> +#define GDB_DWARF2_ARANGES_H
> +
> +class dwarf2_per_objfile;
> +class dwarf2_section_info;
> +class addrmap;
> +
> +/* Read the address map data from DWARF-5 .debug_aranges, and use it
> +   to populate given addrmap.  Returns true on success, false on
> +   failure.  */
> +
> +extern bool read_addrmap_from_aranges (dwarf2_per_objfile *per_objfile,
> +				       dwarf2_section_info *section,
> +				       addrmap *mutable_map);
> +
> +#endif /* GDB_DWARF2_ARANGES_H */
> diff --git a/gdb/dwarf2/read-debug-names.c b/gdb/dwarf2/read-debug-names.c
> index 89f5df6df90..c1b62b38f93 100644
> --- a/gdb/dwarf2/read-debug-names.c
> +++ b/gdb/dwarf2/read-debug-names.c
> @@ -19,6 +19,7 @@
>   
>   #include "defs.h"
>   #include "read-debug-names.h"
> +#include "dwarf2/aranges.h"
>   
>   #include "complaints.h"
>   #include "cp-support.h"
> diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c
> index 13ac83395eb..1ca640df6be 100644
> --- a/gdb/dwarf2/read.c
> +++ b/gdb/dwarf2/read.c
> @@ -31,6 +31,7 @@
>   #include "defs.h"
>   #include "dwarf2/read.h"
>   #include "dwarf2/abbrev.h"
> +#include "dwarf2/aranges.h"
>   #include "dwarf2/attribute.h"
>   #include "dwarf2/comp-unit-head.h"
>   #include "dwarf2/cu.h"
> @@ -1837,184 +1838,6 @@ create_cu_from_index_list (dwarf2_per_bfd *per_bfd,
>     return the_cu;
>   }
>   
> -/* See read.h.  */
> -
> -bool
> -read_addrmap_from_aranges (dwarf2_per_objfile *per_objfile,
> -			   dwarf2_section_info *section,
> -			   addrmap *mutable_map)
> -{
> -  /* Caller must ensure this is read in.  */
> -  gdb_assert (section->readin);
> -  if (section->empty ())
> -    return false;
> -
> -  struct objfile *objfile = per_objfile->objfile;
> -  bfd *abfd = objfile->obfd.get ();
> -  struct gdbarch *gdbarch = objfile->arch ();
> -  dwarf2_per_bfd *per_bfd = per_objfile->per_bfd;
> -
> -  std::unordered_map<sect_offset,
> -		     dwarf2_per_cu_data *,
> -		     gdb::hash_enum<sect_offset>>
> -    debug_info_offset_to_per_cu;
> -  for (const auto &per_cu : per_bfd->all_units)
> -    {
> -      /* A TU will not need aranges, and skipping them here is an easy
> -	 way of ignoring .debug_types -- and possibly seeing a
> -	 duplicate section offset -- entirely.  The same applies to
> -	 units coming from a dwz file.  */
> -      if (per_cu->is_debug_types || per_cu->is_dwz)
> -	continue;
> -
> -      const auto insertpair
> -	= debug_info_offset_to_per_cu.emplace (per_cu->sect_off,
> -					       per_cu.get ());
> -
> -      /* Assume no duplicate offsets in all_units.  */
> -      gdb_assert (insertpair.second);
> -    }
> -
> -  std::set<sect_offset> debug_info_offset_seen;
> -  const bfd_endian dwarf5_byte_order = gdbarch_byte_order (gdbarch);
> -  const gdb_byte *addr = section->buffer;
> -  while (addr < section->buffer + section->size)
> -    {
> -      const gdb_byte *const entry_addr = addr;
> -      unsigned int bytes_read;
> -
> -      const LONGEST entry_length = read_initial_length (abfd, addr,
> -							&bytes_read);
> -      addr += bytes_read;
> -
> -      const gdb_byte *const entry_end = addr + entry_length;
> -      const bool dwarf5_is_dwarf64 = bytes_read != 4;
> -      const uint8_t offset_size = dwarf5_is_dwarf64 ? 8 : 4;
> -      if (addr + entry_length > section->buffer + section->size)
> -	{
> -	  warning (_("Section .debug_aranges in %s entry at offset %s "
> -		     "length %s exceeds section length %s, "
> -		     "ignoring .debug_aranges."),
> -		   objfile_name (objfile),
> -		   plongest (entry_addr - section->buffer),
> -		   plongest (bytes_read + entry_length),
> -		   pulongest (section->size));
> -	  return false;
> -	}
> -
> -      /* The version number.  */
> -      const uint16_t version = read_2_bytes (abfd, addr);
> -      addr += 2;
> -      if (version != 2)
> -	{
> -	  warning (_("Section .debug_aranges in %s entry at offset %s "
> -		     "has unsupported version %d, ignoring .debug_aranges."),
> -		   objfile_name (objfile),
> -		   plongest (entry_addr - section->buffer), version);
> -	  return false;
> -	}
> -
> -      const uint64_t debug_info_offset
> -	= extract_unsigned_integer (addr, offset_size, dwarf5_byte_order);
> -      addr += offset_size;
> -      const auto per_cu_it
> -	= debug_info_offset_to_per_cu.find (sect_offset (debug_info_offset));
> -      if (per_cu_it == debug_info_offset_to_per_cu.cend ())
> -	{
> -	  warning (_("Section .debug_aranges in %s entry at offset %s "
> -		     "debug_info_offset %s does not exists, "
> -		     "ignoring .debug_aranges."),
> -		   objfile_name (objfile),
> -		   plongest (entry_addr - section->buffer),
> -		   pulongest (debug_info_offset));
> -	  return false;
> -	}
> -      const auto insertpair
> -	= debug_info_offset_seen.insert (sect_offset (debug_info_offset));
> -      if (!insertpair.second)
> -	{
> -	  warning (_("Section .debug_aranges in %s has duplicate "
> -		     "debug_info_offset %s, ignoring .debug_aranges."),
> -		   objfile_name (objfile),
> -		   sect_offset_str (sect_offset (debug_info_offset)));
> -	  return false;
> -	}
> -      dwarf2_per_cu_data *const per_cu = per_cu_it->second;
> -
> -      const uint8_t address_size = *addr++;
> -      if (address_size < 1 || address_size > 8)
> -	{
> -	  warning (_("Section .debug_aranges in %s entry at offset %s "
> -		     "address_size %u is invalid, ignoring .debug_aranges."),
> -		   objfile_name (objfile),
> -		   plongest (entry_addr - section->buffer), address_size);
> -	  return false;
> -	}
> -
> -      const uint8_t segment_selector_size = *addr++;
> -      if (segment_selector_size != 0)
> -	{
> -	  warning (_("Section .debug_aranges in %s entry at offset %s "
> -		     "segment_selector_size %u is not supported, "
> -		     "ignoring .debug_aranges."),
> -		   objfile_name (objfile),
> -		   plongest (entry_addr - section->buffer),
> -		   segment_selector_size);
> -	  return false;
> -	}
> -
> -      /* Must pad to an alignment boundary that is twice the address
> -	 size.  It is undocumented by the DWARF standard but GCC does
> -	 use it.  However, not every compiler does this.  We can see
> -	 whether it has happened by looking at the total length of the
> -	 contents of the aranges for this CU -- it if isn't a multiple
> -	 of twice the address size, then we skip any leftover
> -	 bytes.  */
> -      addr += (entry_end - addr) % (2 * address_size);
> -
> -      while (addr < entry_end)
> -	{
> -	  if (addr + 2 * address_size > entry_end)
> -	    {
> -	      warning (_("Section .debug_aranges in %s entry at offset %s "
> -			 "address list is not properly terminated, "
> -			 "ignoring .debug_aranges."),
> -		       objfile_name (objfile),
> -		       plongest (entry_addr - section->buffer));
> -	      return false;
> -	    }
> -	  ULONGEST start = extract_unsigned_integer (addr, address_size,
> -						     dwarf5_byte_order);
> -	  addr += address_size;
> -	  ULONGEST length = extract_unsigned_integer (addr, address_size,
> -						      dwarf5_byte_order);
> -	  addr += address_size;
> -	  if (start == 0 && length == 0)
> -	    {
> -	      /* This can happen on some targets with --gc-sections.
> -		 This pair of values is also used to mark the end of
> -		 the entries for a given CU, but we ignore it and
> -		 instead handle termination using the check at the top
> -		 of the loop.  */
> -	      continue;
> -	    }
> -	  if (start == 0 && !per_bfd->has_section_at_zero)
> -	    {
> -	      /* Symbol was eliminated due to a COMDAT group.  */
> -	      continue;
> -	    }
> -	  ULONGEST end = start + length;
> -	  start = (ULONGEST) per_objfile->adjust ((unrelocated_addr) start);
> -	  end = (ULONGEST) per_objfile->adjust ((unrelocated_addr) end);
> -	  mutable_map->set_empty (start, end - 1, per_cu);
> -	}
> -
> -      per_cu->addresses_seen = true;
> -    }
> -
> -  return true;
> -}
> -
>   /* die_reader_func for dw2_get_file_names.  */
>   
>   static void
> diff --git a/gdb/dwarf2/read.h b/gdb/dwarf2/read.h
> index 1d9432c5c11..2cf40a8c2cd 100644
> --- a/gdb/dwarf2/read.h
> +++ b/gdb/dwarf2/read.h
> @@ -955,12 +955,4 @@ extern void finalize_all_units (dwarf2_per_bfd *per_bfd);
>   
>   extern htab_up create_quick_file_names_table (unsigned int nr_initial_entries);
>   
> -/* Read the address map data from DWARF-5 .debug_aranges, and use it
> -   to populate given addrmap.  Returns true on success, false on
> -   failure.  */
> -
> -extern bool read_addrmap_from_aranges (dwarf2_per_objfile *per_objfile,
> -				       dwarf2_section_info *section,
> -				       addrmap *mutable_map);
> -
>   #endif /* DWARF2READ_H */

Re: [PATCH 2/2] Move read_addrmap_from_aranges to new file

[Tom Tromey]

>>>>> Guinevere Larsen <blarsen@redhat.com> writes:

> On 14/10/2023 22:56, Tom Tromey wrote:
>> In the interest of shrinking dwarf2/read.c a little more, this patch
>> moves the code that deciphers .debug_aranges into a new file.
> This is a good change, I just have a small question: why you only
> chose to move read_addrmap_from_aranges and not also move
> create_addrmap_from_aranges? I know the second function isn't in
> dwarf2/read.c, but I think it would make aranges.c more consistent.

create_addrmap_from_aranges is only used by read-debug-names.c, so it
seemed a little better to just leave it there as a static function.

Tom

Re: [PATCH 2/2] Move read_addrmap_from_aranges to new file

[Guinevere Larsen]

On 27/10/2023 00:58, Tom Tromey wrote:
>>>>>> Guinevere Larsen <blarsen@redhat.com> writes:
>> On 14/10/2023 22:56, Tom Tromey wrote:
>>> In the interest of shrinking dwarf2/read.c a little more, this patch
>>> moves the code that deciphers .debug_aranges into a new file.
>> This is a good change, I just have a small question: why you only
>> chose to move read_addrmap_from_aranges and not also move
>> create_addrmap_from_aranges? I know the second function isn't in
>> dwarf2/read.c, but I think it would make aranges.c more consistent.
> create_addrmap_from_aranges is only used by read-debug-names.c, so it
> seemed a little better to just leave it there as a static function.
>
> Tom
>
Ah, I see. Sounds good enough for me, Reviewed-By: Guinevere Larsen 
<blarsen@redhat.com>

-- 
Cheers,
Guinevere Larsen
She/Her/Hers

Re: [PATCH 1/2] Pre-read .debug_aranges section

[Tom Tromey]

>>>>> "Guinevere" == Guinevere Larsen <blarsen@redhat.com> writes:

Guinevere>     /* True if we have tried to read this section.  */

Guinevere> So I think it would be better to describe that assert as "assert that
Guinevere> the section has been read" or some other way to make it explicit that
Guinevere> "read" is the past tense.

I changed it to

  /* Caller must ensure that the section has already been read.  */

Guinevere> With some small rewording around this bit, everything looks good to
Guinevere> me, Reviewed-By: Guinevere Larsen <blarsen@redhat.com>

I'm going to check these in now.

Tom