[2/2] Move read_addrmap_from_aranges to new file

Message ID 20231014205755.996771-3-tom@tromey.com
State New
Headers
Series Two .debug_aranges changes |

Checks

Context Check Description
linaro-tcwg-bot/tcwg_gdb_build--master-aarch64 success Testing passed
linaro-tcwg-bot/tcwg_gdb_build--master-arm success Testing passed
linaro-tcwg-bot/tcwg_gdb_check--master-arm success Testing passed
linaro-tcwg-bot/tcwg_gdb_check--master-aarch64 success Testing passed

Commit Message

Tom Tromey Oct. 14, 2023, 8:56 p.m. UTC
  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
  

Comments

Guinevere Larsen Oct. 26, 2023, 9:56 a.m. UTC | #1
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.
  
Tom Tromey Oct. 26, 2023, 10:58 p.m. UTC | #2
>>>>> 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
  
Guinevere Larsen Oct. 27, 2023, 7:27 a.m. UTC | #3
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>
  

Patch

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