[delayed-symfile] handle incorrect aranges

[delayed-symfile] handle incorrect aranges

[Tom Tromey]

I'm checking this in on the delayed-symfile branch.

Jan, I think you probably ought to merge this.

It turns out that older versions of GCC could generate a bogus
.debug_aranges section.  E.g., I have an oldish library that has:

    Contents of the .debug_aranges section:

      Length:                   1516
      Version:                  2
      Offset into .debug_info:  0x0
      Pointer Size:             8
      Segment Size:             0

        Address            Length
        0000000000000000 0000000000000000 
        000000000004f730 0000000000000008 
        000000000004f740 0000000000000008 
    [...]

This is bogus because a (0,0) entry marks the end of the section.

The bug is that a section like this would cause gdb to hang.

This patch adds some more sanity checking while we read .debug_aranges.
If anything is amiss, we just discard what we've read and return.  This
is safe, it just means we're falling back to the old, slow path.

Tom

2009-08-13  Tom Tromey  <tromey@redhat.com>

	* dwarf2read.c (dwarf2_create_quick_addrmap): Sanity-check aranges
	section.

diff --git a/gdb/dwarf2read.c b/gdb/dwarf2read.c
index 69328f4..2272259 100644
--- a/gdb/dwarf2read.c
+++ b/gdb/dwarf2read.c
@@ -1421,9 +1421,21 @@ dwarf2_create_quick_addrmap (struct objfile *objfile)
       unsigned int bytes_read, segment_size, delta;
       LONGEST info_offset;
       struct dwarf2_cu cu;
+      char *end_ptr;
 
       cu_header.initial_length_size = 0;
+      end_ptr = aranges_ptr;
       aranges_ptr = read_comp_unit_head (&cu_header, aranges_ptr, abfd);
+      end_ptr += cu_header.initial_length_size + cu_header.length;
+
+      /* Sanity check.  */
+      if (end_ptr - aranges_ptr >= dwarf2_per_objfile->aranges.size)
+	{
+	  do_cleanups (old);
+	  complaint (&symfile_complaints,
+		     _("aranges entry runs off end of `.debug_aranges' section, ignored"));
+	  return;
+	}
 
       segment_size = read_1_byte (abfd, aranges_ptr);
       aranges_ptr += 1;
@@ -1454,6 +1466,18 @@ dwarf2_create_quick_addrmap (struct objfile *objfile)
 
 	  addrmap_set_empty (mutable_map, address, address + length, objfile);
 	}
+
+      /* Some older versions of GCC incorrectly started the arange
+	 with a (0,0) pair.  If we encounter any oddity while reading
+	 the section, just abandon the attempt; falling back to the
+	 slower code is always safe.  */
+      if (aranges_ptr != end_ptr)
+	{
+	  do_cleanups (old);
+	  complaint (&symfile_complaints,
+		     _("aranges entry ends early, ignored"));
+	  return;
+	}
     }
 
   objfile->quick_addrmap = addrmap_create_fixed (mutable_map,

Re: [delayed-symfile] handle incorrect aranges

[Jan Kratochvil]

On Thu, 13 Aug 2009 21:52:42 +0200, Tom Tromey wrote:
> Jan, I think you probably ought to merge this.

Could you please also fix the address_size == 0 problem bugreported inside RH?
Testcase attached.

readelf:
Contents of the .debug_aranges section:

  Length:                   8
  Version:                  2
  Offset into .debug_info:  0x0
  Pointer Size:             0
  Segment Size:             0

    Address    Length
Floating point exception


Testcase approval for [delayed-symfile] requested.


Thanks,
Jan


--- /dev/null
+++ b/gdb/testsuite/gdb.dwarf2/dw2-aranges.S
@@ -0,0 +1,140 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2004, 2007, 2008, 2009 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 .debug_aranges containing zero address_size.  */
+
+/* Dummy function to provide debug information for.  */
+
+	.text
+.Lbegin_text1:
+	.globl main
+	.type main, %function
+main:
+.Lbegin_main:
+	.int 0
+.Lend_main:
+	.size main, .-main
+.Lend_text1:
+
+/* Debug information */
+
+	.section .debug_info
+.Lcu1_begin:
+	/* CU header */
+	.4byte	.Lcu1_end - .Lcu1_start		/* Length of Compilation Unit */
+.Lcu1_start:
+	.2byte	2				/* DWARF Version */
+	.4byte	.Labbrev1_begin			/* Offset into abbrev section */
+	.byte	4				/* Pointer size */
+
+	/* CU die */
+	.uleb128 1				/* Abbrev: DW_TAG_compile_unit */
+	.4byte	.Lend_text1			/* DW_AT_high_pc */
+	.4byte	.Lbegin_text1			/* DW_AT_low_pc */
+	.ascii	"file1.txt\0"			/* DW_AT_name */
+	.ascii	"GNU C 3.3.3\0"			/* DW_AT_producer */
+	.byte	1				/* DW_AT_language (C) */
+
+	/* main */
+	.uleb128	2			/* Abbrev: DW_TAG_subprogram */
+	.byte		1			/* DW_AT_external */
+	.byte		1			/* DW_AT_decl_file */
+	.byte		2			/* DW_AT_decl_line */
+	.ascii		"main\0"		/* DW_AT_name */
+	.4byte		.Ltype_int-.Lcu1_begin	/* DW_AT_type */
+	.4byte		.Lbegin_main	/* DW_AT_low_pc */
+	.4byte		.Lend_main		/* DW_AT_high_pc */
+	.byte		1			/* DW_AT_frame_base: length */
+	.byte		0x55			/* DW_AT_frame_base: DW_OP_reg5 */
+
+.Ltype_int:
+	.uleb128	3			/* Abbrev: DW_TAG_base_type */
+	.ascii		"int\0"			/* DW_AT_name */
+	.byte		4			/* DW_AT_byte_size */
+	.byte		5			/* DW_AT_encoding */
+
+	.byte		0			/* End of children of CU */
+
+.Lcu1_end:
+
+/* Abbrev table */
+	.section .debug_abbrev
+.Labbrev1_begin:
+	.uleb128	1			/* Abbrev code */
+	.uleb128	0x11			/* DW_TAG_compile_unit */
+	.byte		1			/* has_children */
+	.uleb128	0x12			/* DW_AT_high_pc */
+	.uleb128	0x1			/* DW_FORM_addr */
+	.uleb128	0x11			/* DW_AT_low_pc */
+	.uleb128	0x1			/* DW_FORM_addr */
+	.uleb128	0x3			/* DW_AT_name */
+	.uleb128	0x8			/* DW_FORM_string */
+	.uleb128	0x25			/* DW_AT_producer */
+	.uleb128	0x8			/* DW_FORM_string */
+	.uleb128	0x13			/* DW_AT_language */
+	.uleb128	0xb			/* DW_FORM_data1 */
+	.byte		0x0			/* Terminator */
+	.byte		0x0			/* Terminator */
+
+	.uleb128	2			/* Abbrev code */
+	.uleb128	0x2e			/* DW_TAG_subprogram */
+	.byte		0			/* has_children */
+	.uleb128	0x3f			/* DW_AT_external */
+	.uleb128	0xc			/* DW_FORM_flag */
+	.uleb128	0x3a			/* DW_AT_decl_file */
+	.uleb128	0xb			/* DW_FORM_data1 */
+	.uleb128	0x3b			/* DW_AT_decl_line */
+	.uleb128	0xb			/* DW_FORM_data1 */
+	.uleb128	0x3			/* DW_AT_name */
+	.uleb128	0x8			/* DW_FORM_string */
+	.uleb128	0x49			/* DW_AT_type */
+	.uleb128	0x13			/* DW_FORM_ref4 */
+	.uleb128	0x11			/* DW_AT_low_pc */
+	.uleb128	0x1			/* DW_FORM_addr */
+	.uleb128	0x12			/* DW_AT_high_pc */
+	.uleb128	0x1			/* DW_FORM_addr */
+	.uleb128	0x40			/* DW_AT_frame_base */
+	.uleb128	0xa			/* DW_FORM_block1 */
+	.byte		0x0			/* Terminator */
+	.byte		0x0			/* Terminator */
+
+	.uleb128	3			/* Abbrev code */
+	.uleb128	0x24			/* DW_TAG_base_type */
+	.byte		0			/* has_children */
+	.uleb128	0x3			/* DW_AT_name */
+	.uleb128	0x8			/* DW_FORM_string */
+	.uleb128	0xb			/* DW_AT_byte_size */
+	.uleb128	0xb			/* DW_FORM_data1 */
+	.uleb128	0x3e			/* DW_AT_encoding */
+	.uleb128	0xb			/* DW_FORM_data1 */
+	.byte		0x0			/* Terminator */
+	.byte		0x0			/* Terminator */
+
+	.byte		0x0			/* Terminator */
+	.byte		0x0			/* Terminator */
+
+/* aranges table */
+	.section .debug_aranges
+	.long	.Laranges_end - 1f
+1:
+	.2byte	2				/* aranges Version */
+	.4byte	.Lcu1_begin - .debug_info	/* Offset into .debug_info section */
+	/* The GDB crasher is this zero value.  */
+	.byte		0			/* aranges address_size */
+	.byte		0			/* aranges segment_size */
+
+.Laranges_end:
--- /dev/null
+++ b/gdb/testsuite/gdb.dwarf2/dw2-aranges.exp
@@ -0,0 +1,40 @@
+# Copyright 2004, 2005, 2007, 2008, 2009 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 .debug_aranges containing zero address_size.
+
+# This test can only be run on targets which support DWARF-2 and use gas.
+# For now pick a sampling of likely targets.
+if {![istarget *-*-linux*]
+    && ![istarget *-*-gnu*]
+    && ![istarget *-*-elf*]
+    && ![istarget *-*-openbsd*]
+    && ![istarget arm-*-eabi*]
+    && ![istarget powerpc-*-eabi*]} {
+    return 0  
+}
+
+set testfile "dw2-aranges"
+set srcfile ${testfile}.S
+set binfile ${objdir}/${subdir}/${testfile}
+
+if  { [gdb_compile "${srcdir}/${subdir}/${srcfile}" "${binfile}" executable {nodebug}] != "" } {
+    return -1
+}
+
+clean_restart $testfile
+
+# Failed gdb_load would abort the testcase execution earlier.
+pass "file loaded"

Re: [delayed-symfile] handle incorrect aranges

[Tom Tromey]

>>>>> "Jan" == Jan Kratochvil <jan.kratochvil@redhat.com> writes:

Jan> Could you please also fix the address_size == 0 problem bugreported
Jan> inside RH?  Testcase attached.

Sure.

FWIW, I started with the original test case.  What happened there was
that the aranges reader stopped early, then treated some other bytes
from the middle of an aranges section as a new CU header.  This is how
we ended up with address_size==0.  If you 'readelf' the original
binaries you'll see there is nothing wrong except that bogus (0,0)
entry.

I'm checking in the appended.  It adds a check for the zero address
size, and it also adds a check that the length of an entry is non-zero,
a constraint mentioned in the DWARF standard (and which, I think,
addrmap probably does not handle well).

Finally, this patch fixes a small bug when computing the end of the
range.  addrmap uses end-inclusive, so we have to subtract one.

Jan> Testcase approval for [delayed-symfile] requested.

Yes, thank you very much.

I'll wait until you push before committing, so I can test against your
test case.

Tom

2009-08-14  Tom Tromey  <tromey@redhat.com>

	* dwarf2read.c (dwarf2_create_quick_addrmap): Ignore zero
	addr_size and entries with zero length.  Properly compute end of
	range.

diff --git a/gdb/dwarf2read.c b/gdb/dwarf2read.c
index 2272259..4bae7cb 100644
--- a/gdb/dwarf2read.c
+++ b/gdb/dwarf2read.c
@@ -1436,6 +1436,13 @@ dwarf2_create_quick_addrmap (struct objfile *objfile)
 		     _("aranges entry runs off end of `.debug_aranges' section, ignored"));
 	  return;
 	}
+      if (cu_header.addr_size == 0)
+	{
+	  do_cleanups (old);
+	  complaint (&symfile_complaints,
+		     _("aranges entry has zero addr_size, ignored"));
+	  return;
+	}
 
       segment_size = read_1_byte (abfd, aranges_ptr);
       aranges_ptr += 1;
@@ -1462,9 +1469,18 @@ dwarf2_create_quick_addrmap (struct objfile *objfile)
 	  if (address == 0 && length == 0)
 	    break;
 
+	  if (length == 0)
+	    {
+	      do_cleanups (old);
+	      complaint (&symfile_complaints,
+			 _("aranges entry has zero length, ignored"));
+	      return;
+	    }
+
 	  address += baseaddr;
 
-	  addrmap_set_empty (mutable_map, address, address + length, objfile);
+	  addrmap_set_empty (mutable_map, address, address + length - 1,
+			     objfile);
 	}
 
       /* Some older versions of GCC incorrectly started the arange

Re: [delayed-symfile] handle incorrect aranges

[Jan Kratochvil]

On Fri, 14 Aug 2009 19:13:02 +0200, Tom Tromey wrote:
> FWIW, I started with the original test case.  What happened there was
> that the aranges reader stopped early, then treated some other bytes
> from the middle of an aranges section as a new CU header.  This is how
> we ended up with address_size==0.  If you 'readelf' the original
> binaries you'll see there is nothing wrong except that bogus (0,0)
> entry.

Oh, understood now.


> I'll wait until you push before committing, so I can test against your
> test case.

Checked the testcase in as: 77fa7778a37b0d28a7e4e5235f074a10ecf1815d
Fedora 11: https://admin.fedoraproject.org/updates/gdb-6.8.50.20090302-37.fc11


Thanks,
Jan