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

[Simon Marchi <simon.marchi@efficios.com>]

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