From: Guinevere Larsen <blarsen@redhat.com>
To: Tom Tromey <tom@tromey.com>, gdb-patches@sourceware.org
Subject: Re: [PATCH 2/2] Move read_addrmap_from_aranges to new file
Date: Thu, 26 Oct 2023 11:56:43 +0200 [thread overview]
Message-ID: <e16a3731-5655-4b9d-05b6-0252c82e8f7e@redhat.com> (raw)
In-Reply-To: <20231014205755.996771-3-tom@tromey.com>
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 */
next prev parent reply other threads:[~2023-10-26 9:56 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-10-14 20:56 [PATCH 0/2] Two .debug_aranges changes Tom Tromey
2023-10-14 20:56 ` [PATCH 1/2] Pre-read .debug_aranges section Tom Tromey
2023-10-26 9:54 ` Guinevere Larsen
2023-10-29 16:34 ` Tom Tromey
2023-10-14 20:56 ` [PATCH 2/2] Move read_addrmap_from_aranges to new file Tom Tromey
2023-10-26 9:56 ` Guinevere Larsen [this message]
2023-10-26 22:58 ` Tom Tromey
2023-10-27 7:27 ` Guinevere Larsen
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=e16a3731-5655-4b9d-05b6-0252c82e8f7e@redhat.com \
--to=blarsen@redhat.com \
--cc=gdb-patches@sourceware.org \
--cc=tom@tromey.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).