[binutils-gdb] [gdb/symtab] Fix parsing of .debug_str_offsets header

[binutils-gdb] [gdb/symtab] Fix parsing of .debug_str_offsets header

[Tom de Vries]

https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=65067f1c2c842f001017638c4fe53613d6656263

commit 65067f1c2c842f001017638c4fe53613d6656263
Author: Tom de Vries <tdevries@suse.de>
Date:   Mon Jun 27 12:47:26 2022 +0200

    [gdb/symtab] Fix parsing of .debug_str_offsets header
    
    When running test-case gdb.dwarf2/fission-mix.exp with target board dwarf64
    and gcc-12 (defaulting to DWARF5), I run into:
    ...
    (gdb) break func2^M
    Offset from DW_FORM_GNU_str_index or DW_FORM_strx pointing outside of \
      .debug_str.dwo section in CU at offset 0x0 [in module fission-mix]^M
    (gdb) FAIL: gdb.dwarf2/fission-mix.exp: break func2
    ...
    
    The .debug_str_offsets section has version 5, so as per the standard it has
    it's own header, with initial length and version:
    ...
    Contents of the .debug_str_offsets.dwo section (loaded from fission-mix2.dwo):
    
        Length: 0x1c
        Version: 0x5
           Index   Offset [String]
               0        0 build/gdb/testsuite
               1       33 GNU C17
               2       8f src/gdb/testsuite/gdb.dwarf2/fission-mix-2.c
    ...
    
    But when trying to read the string offset at index 0 in the table (which
    is 0), we start reading at offset 8, which points in the header, at the last
    4 bytes of the initial length (it's 12 bytes because of 64-bit dwarf), as well
    at the 2-byte version field and 2 bytes of padding, so we get:
    ...
    (gdb) p /x str_offset
    $1 = 0x500000000
    ...
    which indeed is an offset that doesn't fit in the .debug_str section.
    
    The offset 8 is based on reader->cu->header.addr_size:
    ...
    static const char *
    read_dwo_str_index (const struct die_reader_specs *reader, ULONGEST str_index)
    {
     ULONGEST str_offsets_base = reader->cu->header.version >= 5
                                 ? reader->cu->header.addr_size : 0;
    ...
    which doesn't in look in agreement with the standard.
    
    Note that this happens to give the right answer for 32-bit dwarf and
    addr_size == 8, because then we have header size ==
    (initial length (4) + version (2) + padding (2)) == 8.
    
    Conversely, for 32-bit dwarf and addr_size == 4 (target board unix/-m32)
    we run into a similar problem.  It just happens to not trigger the warning,
    instead we get the wrong strings, like "func2" for DW_AT_producer and
    "build/gdb/testsuite" for DW_AT_name of the DW_TAG_compile_unit DIE.
    
    Fix this by parsing the .debug_str_offsets header in read_dwo_str_index.
    
    Add a FIXME that we should not parse this for every call.
    
    Tested on x86_64-linux.

Diff:
---
 gdb/dwarf2/read.c | 64 +++++++++++++++++++++++++++++++++++++++++++++++++------
 1 file changed, 57 insertions(+), 7 deletions(-)

diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c
index 80bb2929435..d5088395fb1 100644
--- a/gdb/dwarf2/read.c
+++ b/gdb/dwarf2/read.c
@@ -19384,7 +19384,8 @@ static const char *
 read_str_index (struct dwarf2_cu *cu,
 		struct dwarf2_section_info *str_section,
 		struct dwarf2_section_info *str_offsets_section,
-		ULONGEST str_offsets_base, ULONGEST str_index)
+		ULONGEST str_offsets_base, ULONGEST str_index,
+		unsigned offset_size)
 {
   dwarf2_per_objfile *per_objfile = cu->per_objfile;
   struct objfile *objfile = per_objfile->objfile;
@@ -19408,8 +19409,8 @@ read_str_index (struct dwarf2_cu *cu,
 	   sect_offset_str (cu->header.sect_off), objf_name);
   info_ptr = (str_offsets_section->buffer
 	      + str_offsets_base
-	      + str_index * cu->header.offset_size);
-  if (cu->header.offset_size == 4)
+	      + str_index * offset_size);
+  if (offset_size == 4)
     str_offset = bfd_get_32 (abfd, info_ptr);
   else
     str_offset = bfd_get_64 (abfd, info_ptr);
@@ -19425,12 +19426,60 @@ read_str_index (struct dwarf2_cu *cu,
 static const char *
 read_dwo_str_index (const struct die_reader_specs *reader, ULONGEST str_index)
 {
-  ULONGEST str_offsets_base = reader->cu->header.version >= 5
-			      ? reader->cu->header.addr_size : 0;
+  unsigned offset_size;
+  ULONGEST str_offsets_base;
+  if (reader->cu->header.version >= 5)
+    {
+      /* We have a DWARF5 CU with a reference to a .debug_str_offsets section,
+         so assume the .debug_str_offsets section is DWARF5 as well, and
+	 parse the header.  FIXME: Parse the header only once.  */
+      unsigned int bytes_read = 0;
+      bfd *abfd = reader->dwo_file->sections.str_offsets.get_bfd_owner ();
+      const gdb_byte *p = reader->dwo_file->sections.str_offsets.buffer;
+
+      /* Header: Initial length.  */
+      read_initial_length (abfd, p + bytes_read, &bytes_read);
+
+      /* Determine offset_size based on the .debug_str_offsets header.  */
+      const bool dwarf5_is_dwarf64 = bytes_read != 4;
+      offset_size = dwarf5_is_dwarf64 ? 8 : 4;
+
+      /* Header: Version.  */
+      unsigned version = read_2_bytes (abfd, p + bytes_read);
+      bytes_read += 2;
+
+      if (version <= 4)
+	{
+	  /* We'd like one warning here about ignoring the section, but
+	     because we parse the header more than once (see FIXME above)
+	     we'd have many warnings, so use a complaint instead, which at
+	     least has a limit. */
+	  complaint (_("Section .debug_str_offsets in %s has unsupported"
+		       " version %d, use empty string."),
+		   reader->dwo_file->dwo_name, version);
+	  return "";
+	}
+
+      /* Header: Padding.  */
+      bytes_read += 2;
+
+      str_offsets_base = bytes_read;
+    }
+  else
+    {
+      /* We have a pre-DWARF5 CU with a reference to a .debug_str_offsets
+	 section, assume the .debug_str_offsets section is pre-DWARF5 as
+	 well, which doesn't have a header.  */
+      str_offsets_base = 0;
+
+      /* Determine offset_size based on the .debug_info header.  */
+      offset_size = reader->cu->header.offset_size;
+  }
+
   return read_str_index (reader->cu,
 			 &reader->dwo_file->sections.str,
 			 &reader->dwo_file->sections.str_offsets,
-			 str_offsets_base, str_index);
+			 str_offsets_base, str_index, offset_size);
 }
 
 /* Given a DW_FORM_GNU_str_index from a Fission stub, fetch the string.  */
@@ -19452,7 +19501,8 @@ read_stub_str_index (struct dwarf2_cu *cu, ULONGEST str_index)
   return read_str_index (cu,
 			 &cu->per_objfile->per_bfd->str,
 			 &cu->per_objfile->per_bfd->str_offsets,
-			 *cu->str_offsets_base, str_index);
+			 *cu->str_offsets_base, str_index,
+			 cu->header.offset_size);
 }
 
 /* Return the length of an LEB128 number in BUF.  */