[PATCH v2 0/4] Fix reading DW_FORM_addrx with address size of 2

[PATCH v2 0/4] Fix reading DW_FORM_addrx with address size of 2

[Simon Marchi]

This is version 2 of this patch:

  https://inbox.sourceware.org/gdb-patches/af0e0cb7-b012-4e78-b567-c232eca8222d@polymtl.ca/T/#mb7baec6653460d149a84aa538dc6faa64c28d65b

I split out the DWARF assembler changes in its own patch, since it's now
a bit more substantial (patch 1).

I added a patch that validates the address sizes read from DWARF against
a set of known values (patch 2).

I added a patch that removes an unnecessary segment_collector_size
field (patch 3)..

Finally, patch 4 is the actual new version of the original patch.  The
main change is the way to computation is done, to try to be more robust
against various cases of overflow.

Simon Marchi (4):
  gdb/testsuite: add support for DWARF 5 .debug_addr sections to DWARF
    assembler
  gdb/dwarf: validate address sizes when reading DWARF headers
  gdb/dwarf: don't store segment_collector_size (sic)
  gdb/dwarf: fix reading DW_FORM_addrx with address size of 2

 gdb/dwarf2/aranges.c                         |   4 +-
 gdb/dwarf2/frame.c                           |  13 +-
 gdb/dwarf2/read.c                            |  54 +++++----
 gdb/dwarf2/read.h                            |   4 +
 gdb/dwarf2/types.h                           |   8 ++
 gdb/dwarf2/unit-head.c                       |   7 ++
 gdb/dwarf2/unit-head.h                       |   5 +
 gdb/testsuite/gdb.dwarf2/dw2-addr-size-2.c   |  22 ++++
 gdb/testsuite/gdb.dwarf2/dw2-addr-size-2.exp |  75 ++++++++++++
 gdb/testsuite/lib/dwarf.exp                  | 118 +++++++++++++++++--
 10 files changed, 274 insertions(+), 36 deletions(-)
 create mode 100644 gdb/testsuite/gdb.dwarf2/dw2-addr-size-2.c
 create mode 100644 gdb/testsuite/gdb.dwarf2/dw2-addr-size-2.exp


base-commit: 3e5100fe3b15e2b3ece60f57b4c1ecc0c50cae53
-- 
2.55.0

[PATCH v2 1/4] gdb/testsuite: add support for DWARF 5 .debug_addr sections to DWARF assembler

[Simon Marchi]

The DWARF assembler knows how to emit .debug_addr as they were in the
DWARF 4 GNU extensions days.  This patch adds proper DWARF 5 .debug_addr
support.

The only difference is that the GNU extensions .debug_addr does not have
a header.  It is just an array of addresses (concatenated from all the
contributions).  The DW_AT_GNU_addr_base attributes of the various units
point at different places in it.  The DWARF 5 version of the section has
a header, similar to other sections.

The changes are:

 - Add a version parameter to the debug_addr_label proc.

 - Make the debug_addr_label proc emit a header if it's version 5 or
   higher.  Otherwise, it's understood to be a GNU extensions
   .debug_addr section and does not emit a header.  A call to the
   debug_addr_label proc now marks the beginning of a .debug_addr
   contribution.

 - Make the debug_addr_label proc end the previous .debug_addr
   contribution, if there is one.  To end a contribution, we emit a
   label that is referred to in the unit_length computation.  Track this
   label name through a new _debug_addr_end_label namespace variable.

 - When using a form or operator that appends to .debug_addr, verify
   that a .debug_addr contribution is open.  This is just to help catch
   programmer mistakes where one would forget to call the
   debug_addr_label proc before using DW_FORM_addrx, for example.
   However, if building multiple CUs that use .debug_addr, it does not
   protect against forgetting to call the debug_addr_label proc to begin
   the contribution of the second CU.

 - Add support for DW_FORM_addrx and DW_AT_addr_base, synonymous for
   their GNU extensions counterpart.

Change-Id: I2226cc57d91317de111a2468269122ff87be5c70
---
 gdb/testsuite/lib/dwarf.exp | 118 ++++++++++++++++++++++++++++++++----
 1 file changed, 106 insertions(+), 12 deletions(-)

diff --git a/gdb/testsuite/lib/dwarf.exp b/gdb/testsuite/lib/dwarf.exp
index 839c51742650..351903242f98 100644
--- a/gdb/testsuite/lib/dwarf.exp
+++ b/gdb/testsuite/lib/dwarf.exp
@@ -580,10 +580,14 @@ namespace eval Dwarf {
     # The address size for debug ranges section.
     variable _debug_ranges_64_bit
 
-    # The index into the .debug_addr section (used for fission
-    # generation).
+    # The index of the next entry in the currently open .debug_addr
+    # contribution, or the empty string if no contribution is open.
     variable _debug_addr_index
 
+    # The label marking the end of the currently open .debug_addr
+    # contribution, or the empty string if no contribution is open.
+    variable _debug_addr_end_label
+
     # Flag, true if the current CU is contains fission information,
     # otherwise false.
     variable _cu_is_fission
@@ -808,10 +812,13 @@ namespace eval Dwarf {
 		_op .${_cu_addr_size}byte $value
 	    }
 
+	    DW_FORM_addrx -
 	    DW_FORM_GNU_addr_index {
 		variable _debug_addr_index
 		variable _cu_addr_size
 
+		_debug_addr_check_open
+
 		_op .uleb128 ${_debug_addr_index}
 		incr _debug_addr_index
 
@@ -948,6 +955,7 @@ namespace eval Dwarf {
 	    DW_AT_name {
 		return DW_FORM_string
 	    }
+	    DW_AT_addr_base -
 	    DW_AT_GNU_addr_base {
 		return DW_FORM_sec_offset
 	    }
@@ -1323,6 +1331,8 @@ namespace eval Dwarf {
 	variable _debug_addr_index
 	variable _cu_addr_size
 
+	_debug_addr_check_open
+
 	_op .uleb128 ${_debug_addr_index}
 	incr _debug_addr_index
 
@@ -1658,20 +1668,99 @@ namespace eval Dwarf {
 	uplevel $_level $body
     }
 
-    # Return a label that references the current position in the
-    # .debug_addr table.  When a user is creating split DWARF they
-    # will define two CUs, the first will be the split DWARF content,
-    # and the second will be the non-split stub CU.  The split DWARF
-    # CU fills in the .debug_addr section, but the non-split CU
-    # includes a reference to the start of the section.  The label
-    # returned by this proc provides that reference.
-    proc debug_addr_label {} {
+    # Verify that a .debug_addr contribution is open, that is that the
+    # debug_addr_label proc was called.
+    proc _debug_addr_check_open {} {
 	variable _debug_addr_index
 
-	set lbl [new_label "debug_addr_idx_${_debug_addr_index}_"]
+	if { $_debug_addr_index == "" } {
+	    error "no .debug_addr contribution is open, call the addr proc first"
+	}
+    }
+
+    # Terminate the currently open .debug_addr contribution, if any, by
+    # defining its end label.
+    proc _debug_addr_end_contribution {} {
+	variable _debug_addr_end_label
+
+	if { $_debug_addr_end_label == "" } {
+	    return
+	}
+
+	_defer_output .debug_addr {
+	    define_label $_debug_addr_end_label
+	}
+
+	set _debug_addr_end_label ""
+    }
+
+    # Start this unit's contribution to the .debug_addr section and return a
+    # label referencing its first entry.
+    #
+    # When a user is creating split DWARF they will define two CUs, the first will
+    # be the split DWARF content, and the second will be the non-split stub
+    # CU.  The split DWARF CU fills in the .debug_addr section with addresses,
+    # but the non-split CU includes a reference to the start of the address
+    # array section (through DW_AT_addr_base for DWARF 5, DW_AT_GNU_addr_base
+    # for DWARF 4 GNU extensions).
+    #
+    # This proc must be called from within a cu or tu body, before any
+    # use of DW_FORM_addrx, or any operator that appends an address to
+    # .debug_addr.
+    #
+    # The `version` option gives the version of the .debug_addr section to
+    # emit.  If the version is >= 5, start the contribution with a header.
+    # The offset and address sizes are obtained from the current unit.
+    #
+    # If the version is < 5, then the contribution is understood to be in the
+    # format of DWARF 4 GNU extension, which does not use a header (in which
+    # case the version isn't written anywhere).
+    #
+    # Calling this proc again terminates the previous contribution, starts a
+    # new one, and restarts entry indices at zero.
+    proc debug_addr_label { {options {}} } {
+	variable _debug_addr_index
+	variable _debug_addr_end_label
+	variable _cu_addr_size
+	variable _cu_offset_size
+
+	parse_options {
+	    {version 4}
+	}
+
+	_debug_addr_end_contribution
+
+	if { $version >= 5 } {
+	    set _debug_addr_end_label [new_label "debug_addr_end_"]
+	    set post_unit_len_label [new_label "debug_addr_post_unit_len_"]
+
+	    _defer_output .debug_addr {
+		if { $_cu_offset_size == 8 } {
+		    _op .4byte 0xffffffff "unit length 1/2"
+		    _op .8byte \
+			"$_debug_addr_end_label - $post_unit_len_label" \
+			"unit length 2/2"
+		} else {
+		    _op .4byte \
+			"$_debug_addr_end_label - $post_unit_len_label" \
+			"unit length"
+		}
+
+		define_label $post_unit_len_label
+
+		_op .2byte $version "version"
+		_op .byte $_cu_addr_size "address size"
+		_op .byte 0 "segment selector size"
+	    }
+	}
+
+	set lbl [new_label "debug_addr_base_"]
 	_defer_output .debug_addr {
 	    define_label $lbl
 	}
+
+	set _debug_addr_index 0
+
 	return $lbl
     }
 
@@ -3988,6 +4077,7 @@ namespace eval Dwarf {
 	variable _line_header_end_label
 	variable _debug_ranges_64_bit
 	variable _debug_addr_index
+	variable _debug_addr_end_label
 	variable _level
 	variable _dwo_abbrev_num
 
@@ -4022,7 +4112,8 @@ namespace eval Dwarf {
 
 	set _line_count 0
 	set _debug_ranges_64_bit [is_64_target]
-	set _debug_addr_index 0
+	set _debug_addr_index ""
+	set _debug_addr_end_label ""
 	set _dwo_abbrev_num 1
 
 	# Dummy CU at the start to ensure that the first CU in $body is not
@@ -4053,6 +4144,9 @@ namespace eval Dwarf {
 	    }
 	}
 
+	# Terminate the last .debug_addr contribution, if any.
+	_debug_addr_end_contribution
+
 	_write_deferred_output
 
 	_section .note.GNU-stack {
-- 
2.55.0

[PATCH v2 2/4] gdb/dwarf: validate address sizes when reading DWARF headers

[Simon Marchi]

There is currently very little validation done on the addr_size fields
read from the various DWARF section headers.  One could write some DWARF
debug info with strange address sizes (like 0 or 47), and it's not
always clear how GDB will react.  Instead of wondering how each site
that uses the address size will behave, I propose to do some early
validation on the address size fields, so that the rest of the code does
not have to worry about unexpected values.

I chose to make the valid values 2, 4 and 8.  This is based on the fact
that we have a few sites where we do:

      switch (unit->addr_size)
	{
	case 8:
	  return bfd_get_signed_64 (unit->abfd, buf);
	case 4:
	  return bfd_get_signed_32 (unit->abfd, buf);
	case 2:
	  return bfd_get_signed_16 (unit->abfd, buf);
	default:
	  abort ();
	}

LLVM's getSupportedAddressSizes function lists the same size.

Add the dwarf2_addr_size_is_supported function, and use it at a few
places where we read an address size from a header.

 - unit-head.c, where we read unit headers from .debug_info

 - read.c, where we read .debug_loclists and .debug_rnglists headers

 - aranges.c, where we read .debug_aranges headers.  It replaces a more
   lax check.

 - frame.c, where we read .debug_frame headers

 - dwarf_decode_line_header in line-header.c just skips over the address
   size, I did not add a check there.

Change-Id: I1b3bd220981347bdf347647183efd1165040875d
---
 gdb/dwarf2/aranges.c   | 4 ++--
 gdb/dwarf2/frame.c     | 5 +++++
 gdb/dwarf2/read.c      | 7 +++++++
 gdb/dwarf2/types.h     | 8 ++++++++
 gdb/dwarf2/unit-head.c | 7 +++++++
 5 files changed, 29 insertions(+), 2 deletions(-)

diff --git a/gdb/dwarf2/aranges.c b/gdb/dwarf2/aranges.c
index b085497844b5..9b5143a051f8 100644
--- a/gdb/dwarf2/aranges.c
+++ b/gdb/dwarf2/aranges.c
@@ -137,11 +137,11 @@ read_addrmap_from_aranges (dwarf2_per_objfile *per_objfile,
       dwarf2_per_cu *const per_cu = per_cu_it->second;
 
       const uint8_t address_size = *addr++;
-      if (address_size < 1 || address_size > 8)
+      if (!dwarf2_addr_size_is_supported (address_size))
 	{
 	  warn->warn
 	    (_("Section .debug_aranges in %ps entry at offset %s "
-	       "address_size %u is invalid, ignoring .debug_aranges."),
+	       "address_size %u is not supported, ignoring .debug_aranges."),
 	     styled_string (file_name_style.style (),
 			    objfile_name (objfile)),
 	     plongest (entry_addr - section->buffer), address_size);
diff --git a/gdb/dwarf2/frame.c b/gdb/dwarf2/frame.c
index 6f0601a0146e..110b0d61faef 100644
--- a/gdb/dwarf2/frame.c
+++ b/gdb/dwarf2/frame.c
@@ -1770,6 +1770,11 @@ decode_frame_entry_1 (struct gdbarch *gdbarch,
 	  /* FIXME: check that this is the same as from the CU header.  */
 	  cie->addr_size = read_1_byte (unit->abfd, buf);
 	  ++buf;
+
+	  if (!dwarf2_addr_size_is_supported (cie->addr_size))
+	    error (_("Unsupported address size in CIE "
+		     "(is %u, should be 2, 4 or 8)."), cie->addr_size);
+
 	  cie->segment_size = read_1_byte (unit->abfd, buf);
 	  ++buf;
 	}
diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c
index 19448795e53e..c1454a664bef 100644
--- a/gdb/dwarf2/read.c
+++ b/gdb/dwarf2/read.c
@@ -14091,6 +14091,13 @@ read_loclists_rnglists_header (struct loclists_rnglists_header *header,
   header->addr_size = read_1_byte (abfd, info_ptr);
   info_ptr += 1;
 
+  if (!dwarf2_addr_size_is_supported (header->addr_size))
+    error (_(DWARF_ERROR_PREFIX
+	     "unsupported address size in %s header "
+	     "(is %u, should be 2, 4 or 8) [in module %s]"),
+	   section->get_name (), header->addr_size,
+	   section->get_file_name ());
+
   header->segment_collector_size = read_1_byte (abfd, info_ptr);
   info_ptr += 1;
 
diff --git a/gdb/dwarf2/types.h b/gdb/dwarf2/types.h
index cb8ce33940ca..d1ef2fb6fa82 100644
--- a/gdb/dwarf2/types.h
+++ b/gdb/dwarf2/types.h
@@ -39,4 +39,12 @@ sect_offset_str (sect_offset offset)
   return hex_string (to_underlying (offset));
 }
 
+/* Return true if ADDR_SIZE is an address size GDB knows how to handle.  */
+
+static inline bool
+dwarf2_addr_size_is_supported (unsigned int addr_size)
+{
+  return addr_size == 2 || addr_size == 4 || addr_size == 8;
+}
+
 #endif /* GDB_DWARF2_TYPES_H */
diff --git a/gdb/dwarf2/unit-head.c b/gdb/dwarf2/unit-head.c
index 1771e43da97f..b6a30a4c31ff 100644
--- a/gdb/dwarf2/unit-head.c
+++ b/gdb/dwarf2/unit-head.c
@@ -110,6 +110,13 @@ read_unit_head (struct unit_head *header, const gdb_byte *info_ptr,
       header->addr_size = read_1_byte (abfd, info_ptr);
       info_ptr += 1;
     }
+
+  if (!dwarf2_addr_size_is_supported (header->addr_size))
+    error (_(DWARF_ERROR_PREFIX
+	     "unsupported address size in unit header "
+	     "(is %u, should be 2, 4 or 8) [in module %s]"),
+	   header->addr_size, filename);
+
   signed_addr = bfd_get_sign_extend_vma (abfd);
   if (signed_addr < 0)
     internal_error (_("read_unit_head: dwarf from non elf file"));
-- 
2.55.0

[PATCH v2 3/4] gdb/dwarf: don't store segment_collector_size (sic)

[Simon Marchi]

I noticed that structure loclists_rnglists_header stored the segment
selector size (which seems to be misspelled as "collector").  Nothing
uses that, and it has even been removed from the DWARF 6 draft (it stays
as a "reserved" field).  Remove the structure field and simply skip it.

There are other functions (read_addrmap_from_aranges and
dwarf_decode_line_header) that do read it, only to validate that it's 0,
but it didn't seem useful to me to add such a check here (we could even
remove these other ones).

Change-Id: If73f510398bef2cb1215ce7ec2e4e083d5a50697
---
 gdb/dwarf2/read.c | 6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c
index c1454a664bef..2f38dfdaf970 100644
--- a/gdb/dwarf2/read.c
+++ b/gdb/dwarf2/read.c
@@ -238,10 +238,6 @@ struct loclists_rnglists_header
      the target system.  */
   unsigned char addr_size;
 
-  /* A 1-byte unsigned integer containing the size in bytes of a segment selector
-     on the target system.  */
-  unsigned char segment_collector_size;
-
   /* A 4-byte count of the number of offsets that follow the header.  */
   unsigned int offset_entry_count;
 };
@@ -14098,7 +14094,7 @@ read_loclists_rnglists_header (struct loclists_rnglists_header *header,
 	   section->get_name (), header->addr_size,
 	   section->get_file_name ());
 
-  header->segment_collector_size = read_1_byte (abfd, info_ptr);
+  /* Skip segment_selector_size.  */
   info_ptr += 1;
 
   header->offset_entry_count = read_4_bytes (abfd, info_ptr);
-- 
2.55.0

[PATCH v2 4/4] gdb/dwarf: fix reading DW_FORM_addrx with address size of 2

[Simon Marchi]

From: Simon Marchi <simon.marchi@polymtl.ca>

While investigating AVR binaries for bug 34638, I stumbled on a crash of
GDB when loading an AVR binary generated by clang:

    $ cat repro.c
    volatile int sink;

    static void
    helper (int v)
    {
        sink = v;
    }

    int
    main (void)
    {
        helper (42);
        return 0;
    }

    $ clang --target=avr -mmcu=atmega328p -g -O1 -o repro repro.c
    /usr/bin/avr-ld: warning: _clear_bss.o: missing .note.GNU-stack section implies executable stack
    /usr/bin/avr-ld: NOTE: This behaviour is deprecated and will be removed in a future version of the linker

    $ ./gdb -q -nx --data-directory data-directory repro
    Reading symbols from repro...
    (gdb) =================================================================
    ==2749200==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x7bacecc1ca81 at pc 0x560d4a5964d6 bp 0x7b8ce9ffc410 sp 0x7b8ce9ffc400
    READ of size 1 at 0x7bacecc1ca81 thread T1
        #0 0x560d4a5964d5 in bfd_getl64 /home/simark/src/binutils-gdb/bfd/libbfd.c:903
        #1 0x560d48b83034 in read_addr_index_1 /home/simark/src/binutils-gdb/gdb/dwarf2/read.c:14676
        #2 0x560d48b83160 in read_addr_index /home/simark/src/binutils-gdb/gdb/dwarf2/read.c:14684
        #3 0x560d48b802de in cutu_reader::read_attribute_reprocess(attribute*, dwarf_tag) /home/simark/src/binutils-gdb/gdb/dwarf2/read.c:14257
        #4 0x560d48b7c7fe in cutu_reader::read_toplevel_die(gdb::array_view<attribute*>) /home/simark/src/binutils-gdb/gdb/dwarf2/read.c:13811
        #5 0x560d48b3292a in cutu_reader::cutu_reader(dwarf2_per_cu&, dwarf2_per_objfile&, dwarf2_cu*, bool, std::optional<language>, abbrev_table_cache&) /home/simark/src/binutils-gdb/gdb/dwarf2/read.c:2833
        #6 0x560d48bc47ba in std::__detail::_MakeUniq<cutu_reader>::__single_object std::make_unique<cutu_reader, dwarf2_per_cu&, dwarf2_per_objfile&, decltype(nullptr), bool, std::nullopt_t const&, abbrev_table_cache&>(dwarf2_per_cu&, dwarf2_per_objfile&, decltype(nullptr)&&, bool&&, std::nullopt_t const&, abbrev_table_cache&) /usr/include/c++/16/bits/unique_ptr.h:1105
        #7 0x560d48b33caf in cooked_index_worker_debug_info::process_unit(dwarf2_per_cu*, dwarf2_per_objfile*, cooked_index_worker_result*) /home/simark/src/binutils-gdb/gdb/dwarf2/read.c:3165
        #8 0x560d48bb72eb in cooked_index_worker_debug_info::parallel_indexing_worker::process_one(dwarf2_per_cu&)::{lambda()#1}::operator()() const /home/simark/src/binutils-gdb/gdb/dwarf2/read.c:3065

The problem is that the CU generated by Clang has an address size of 2:

    Compilation Unit @ offset 0x5f4:
     Length:        0x66 (32-bit)
     Version:       5
     Unit Type:     DW_UT_compile (1)
     Abbrev Offset: 0x5a2
     Pointer Size:  2

But read_addr_index_1 only knows how to read addresses of size 4 and 8:

    if (addr_size == 4)
      return (unrelocated_addr) bfd_get_32 (abfd, info_ptr);
    else
      return (unrelocated_addr) bfd_get_64 (abfd, info_ptr);

This ends up reading 8 bytes instead of two which either returns a wrong
value, reads past the end of the section, or both.

Use extract_unsigned_integer with the CU's address size instead of
hardcoding sizes.

This also shows that the bounds check just above is not sufficient.  It
only verifies that the entry starts inside the section, but not that it
fits in it entirely.  Adjust it to account for the size of the
entry.  While at it, re-write the computation to be safer against other
kinds of overflow, like if the DW_AT_addr_base pointed outside the
.debug_addr section.

I added a dwarf2_per_bfd::byte_order helper method to conveniently get
the endianness from a bfd, it could probably get reused elsewhere.

Add a test using the DWARF assembler.  Without the fix, it either fails
with an ASan abort if GDB is built with that, or one of the "info
address" commands simply returns the wrong answer.

Change-Id: I1212178394bc13c4a049c21684245ebb54c5d6f6
---
 gdb/dwarf2/frame.c                           |  8 ++-
 gdb/dwarf2/read.c                            | 41 +++++++----
 gdb/dwarf2/read.h                            |  4 ++
 gdb/dwarf2/unit-head.h                       |  5 ++
 gdb/testsuite/gdb.dwarf2/dw2-addr-size-2.c   | 22 ++++++
 gdb/testsuite/gdb.dwarf2/dw2-addr-size-2.exp | 75 ++++++++++++++++++++
 6 files changed, 138 insertions(+), 17 deletions(-)
 create mode 100644 gdb/testsuite/gdb.dwarf2/dw2-addr-size-2.c
 create mode 100644 gdb/testsuite/gdb.dwarf2/dw2-addr-size-2.exp

diff --git a/gdb/dwarf2/frame.c b/gdb/dwarf2/frame.c
index 110b0d61faef..95e9f1e7c34d 100644
--- a/gdb/dwarf2/frame.c
+++ b/gdb/dwarf2/frame.c
@@ -84,7 +84,9 @@ struct dwarf2_cie
   /* Encoding of addresses.  */
   gdb_byte encoding;
 
-  /* Target address size in bytes.  */
+  /* Target address size in bytes.
+
+     Contains one of the values accepted by dwarf2_addr_size_is_supported.  */
   int addr_size;
 
   /* Target pointer size in bytes.  */
@@ -848,7 +850,9 @@ struct dwarf2_frame_cache
   /* Return address register.  */
   struct dwarf2_frame_state_reg retaddr_reg;
 
-  /* Target address size in bytes.  */
+  /* Target address size in bytes.
+
+     Contains one of the values accepted by dwarf2_addr_size_is_supported.  */
   int addr_size;
 
   /* The dwarf2_per_objfile from which this frame description came.  */
diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c
index 2f38dfdaf970..a461fa56d52b 100644
--- a/gdb/dwarf2/read.c
+++ b/gdb/dwarf2/read.c
@@ -235,7 +235,9 @@ struct loclists_rnglists_header
   short version;
 
   /* A 1-byte unsigned integer containing the size in bytes of an address on
-     the target system.  */
+     the target system.
+
+     Contains one of the values accepted by dwarf2_addr_size_is_supported.  */
   unsigned char addr_size;
 
   /* A 4-byte count of the number of offsets that follow the header.  */
@@ -14658,25 +14660,34 @@ read_addr_index_1 (dwarf2_per_objfile *per_objfile, unsigned int addr_index,
 		   std::optional<ULONGEST> addr_base, int addr_size)
 {
   struct objfile *objfile = per_objfile->objfile;
-  bfd *abfd = objfile->obfd.get ();
-  const gdb_byte *info_ptr;
+  dwarf2_per_bfd *per_bfd = per_objfile->per_bfd;
   ULONGEST addr_base_or_zero = addr_base.has_value () ? *addr_base : 0;
 
-  per_objfile->per_bfd->addr.read (objfile);
-  if (per_objfile->per_bfd->addr.buffer == NULL)
+  per_bfd->addr.read (objfile);
+  if (per_bfd->addr.buffer == NULL)
     error (_("DW_FORM_addr_index used without .debug_addr section [in module %s]"),
 	   objfile_name (objfile));
-  if (addr_base_or_zero + addr_index * addr_size
-      >= per_objfile->per_bfd->addr.size)
-    error (_("DW_FORM_addr_index pointing outside of "
-	     ".debug_addr section [in module %s]"),
+
+  /* Check that the DW_AT_addr_base value makes sense.  */
+  if (addr_base_or_zero > per_bfd->addr.size)
+    error (_("DW_AT_addr_base points outside of .debug_addr section "
+	     "[in module %s]"),
 	   objfile_name (objfile));
-  info_ptr = (per_objfile->per_bfd->addr.buffer + addr_base_or_zero
-	      + addr_index * addr_size);
-  if (addr_size == 4)
-    return (unrelocated_addr) bfd_get_32 (abfd, info_ptr);
-  else
-    return (unrelocated_addr) bfd_get_64 (abfd, info_ptr);
+
+  ULONGEST bytes_avail = per_bfd->addr.size - addr_base_or_zero;
+  ULONGEST entry_offset = static_cast<ULONGEST> (addr_index) * addr_size;
+
+  /* Check that the whole entry fits inside the section.  */
+  if (entry_offset + addr_size > bytes_avail)
+    error (_("DW_FORM_addr_index points outside of .debug_addr section "
+	     "[in module %s]"),
+	   objfile_name (objfile));
+
+  const gdb_byte *addr_ptr
+    = per_bfd->addr.buffer + addr_base_or_zero + entry_offset;
+
+  return (unrelocated_addr) extract_unsigned_integer (addr_ptr, addr_size,
+						      per_bfd->byte_order ());
 }
 
 /* Given index ADDR_INDEX in .debug_addr, fetch the value.  */
diff --git a/gdb/dwarf2/read.h b/gdb/dwarf2/read.h
index 15dd2abf3a1e..4f4f493e88dd 100644
--- a/gdb/dwarf2/read.h
+++ b/gdb/dwarf2/read.h
@@ -582,6 +582,10 @@ struct dwarf2_per_bfd
   const char *filename () const
   { return bfd_get_filename (this->obfd); }
 
+  /* Return the endianness of the BFD.  */
+  bfd_endian byte_order () const
+  { return bfd_big_endian (this->obfd) ? BFD_ENDIAN_BIG : BFD_ENDIAN_LITTLE; }
+
   /* Return the unit given its index.  */
   dwarf2_per_cu &get_unit (int index) const
   {
diff --git a/gdb/dwarf2/unit-head.h b/gdb/dwarf2/unit-head.h
index 057cc8950997..d142d1603968 100644
--- a/gdb/dwarf2/unit-head.h
+++ b/gdb/dwarf2/unit-head.h
@@ -42,7 +42,12 @@ struct unit_head
   unsigned int m_length = 0;
 public:
   unsigned char version = 0;
+
+  /* Size of an address on the target system.
+
+     Contains one of the values accepted by dwarf2_addr_size_is_supported.  */
   unsigned char addr_size = 0;
+
   unsigned char signed_addr_p = 0;
   sect_offset abbrev_sect_off {};
 
diff --git a/gdb/testsuite/gdb.dwarf2/dw2-addr-size-2.c b/gdb/testsuite/gdb.dwarf2/dw2-addr-size-2.c
new file mode 100644
index 000000000000..6a0e311ef418
--- /dev/null
+++ b/gdb/testsuite/gdb.dwarf2/dw2-addr-size-2.c
@@ -0,0 +1,22 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2026 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/>.  */
+
+int
+main (void)
+{
+  return 0;
+}
diff --git a/gdb/testsuite/gdb.dwarf2/dw2-addr-size-2.exp b/gdb/testsuite/gdb.dwarf2/dw2-addr-size-2.exp
new file mode 100644
index 000000000000..170092e723ef
--- /dev/null
+++ b/gdb/testsuite/gdb.dwarf2/dw2-addr-size-2.exp
@@ -0,0 +1,75 @@
+# Copyright 2026 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/>.
+
+# Test a CU with an address size that is neither 4 nor 8.
+#
+# In particular, test reading DW_FORM_addrx attributes from a CU whose address
+# size is 2, which used to crash GDB. This is seen in binaries produced by
+# Clang for AVR.
+
+load_lib dwarf.exp
+
+require dwarf2_support
+
+standard_testfile .c -dw.S
+
+set func_one_addr 0x1234
+set func_two_addr 0x1238
+
+set asm_file [standard_output_file $srcfile2]
+Dwarf::assemble $asm_file {
+    cu {
+	version 5
+	addr_size 2
+    } {
+	# Start this CU's contribution to .debug_addr and capture a label to
+	# its first entry.
+	set addr_base_lbl [debug_addr_label { version 5 }]
+
+	compile_unit {
+	    DW_AT_name dw2-addr-size-2.c
+	    DW_AT_comp_dir /tmp
+	    DW_AT_addr_base $addr_base_lbl
+	    DW_AT_low_pc $::func_one_addr DW_FORM_addrx
+	    DW_AT_high_pc 8 DW_FORM_data1
+	} {
+	    subprogram {
+		DW_AT_name func_one
+		DW_AT_low_pc $::func_one_addr DW_FORM_addrx
+		DW_AT_high_pc 4 DW_FORM_data1
+		DW_AT_external 1 flag
+	    } {
+	    }
+	    subprogram {
+		DW_AT_name func_two
+		DW_AT_low_pc $::func_two_addr DW_FORM_addrx
+		DW_AT_high_pc 4 DW_FORM_data1
+		DW_AT_external 1 flag
+	    } {
+	    }
+	}
+    }
+}
+
+if { [prepare_for_testing "failed to prepare" ${testfile} \
+	  [list $srcfile $asm_file] {nodebug}] } {
+    return -1
+}
+
+gdb_test "info address func_one" \
+    "Symbol \"func_one\" is a function at address $func_one_addr\\."
+
+gdb_test "info address func_two" \
+    "Symbol \"func_two\" is a function at address $func_two_addr\\."
-- 
2.55.0

Re: [PATCH v2 4/4] gdb/dwarf: fix reading DW_FORM_addrx with address size of 2

[Tom Tromey]

>>>>> "Simon" == Simon Marchi <simon.marchi@efficios.com> writes:

Simon> @@ -14658,25 +14660,34 @@ read_addr_index_1 (dwarf2_per_objfile *per_objfile, unsigned int addr_index,

Simon> +  ULONGEST entry_offset = static_cast<ULONGEST> (addr_index) * addr_size;

Since the only use is in a cast to another type, perhaps the type should
just be ULONGEST in the signature.

Tom

Re: [PATCH v2 0/4] Fix reading DW_FORM_addrx with address size of 2

[Tom Tromey]

>>>>> "Simon" == Simon Marchi <simon.marchi@efficios.com> writes:

Simon> This is version 2 of this patch:
Simon>   https://inbox.sourceware.org/gdb-patches/af0e0cb7-b012-4e78-b567-c232eca8222d@polymtl.ca/T/#mb7baec6653460d149a84aa538dc6faa64c28d65b

I sent one comment but LGTM.
Approved-By: Tom Tromey <tom@tromey.com>

Tom

Re: [PATCH v2 4/4] gdb/dwarf: fix reading DW_FORM_addrx with address size of 2

[Simon Marchi]

On 9/23/26 11:13 AM, Tom Tromey wrote:
>>>>>> "Simon" == Simon Marchi <simon.marchi@efficios.com> writes:
> 
> Simon> @@ -14658,25 +14660,34 @@ read_addr_index_1 (dwarf2_per_objfile *per_objfile, unsigned int addr_index,
> 
> Simon> +  ULONGEST entry_offset = static_cast<ULONGEST> (addr_index) * addr_size;
> 
> Since the only use is in a cast to another type, perhaps the type should
> just be ULONGEST in the signature.

I did this change.  All the callers start with a 64 address index vlaue
anyway, so we might as well just pass it as 64 bits all the way through.

Pushed with that changed, thanks.

Simon