This is the mail archive of the gdb-patches@sources.redhat.com mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [RFA] Don't SEGV on invalid dwarf2 frame info


Mark Kettenis writes:
 > Richard Henderson <rth@twiddle.net> writes:
 > 
 > > Elena, this is the patch I was thinking about.
 > > 
 > > For the audience, there is at least one bug in current cvs ld's .eh_frame
 > > optimization code that results in padding being added between sections.
 > > But we saw similar problems when we added support for .eh_frame generation
 > > within the assembler (and didn't .align sections), so the discussion in 
 > > the patch is a bit more broad than that.
 > > 
 > > Does this seem reasonable?
 > 
 > It does to me.  It's unfortunately that this is necessary, but it
 > makes things more robust, so please check this in.
 > 
 > Mark


This one seems to work for me (almost no changes, from the original, just
added makefile, and other few things, including a check on initial_length.)

hmm, wonder if this new output will break some tests... I am checking now.

GNU gdb 5.3.90_2003-07-07-cvs
Copyright 2003 Free Software Foundation, Inc.
GDB is free software, covered by the GNU General Public License, and you are
welcome to change it and/or distribute copies of it under certain conditions.
Type "show copying" to see the conditions.
There is absolutely no warranty for GDB.  Type "show warranty" for details.
This GDB was configured as "x86_64-unknown-linux-gnu"...
Setting up the environment for debugging gdb.
.gdbinit:5: Error in sourced command file:
Function "internal_error" not defined.
(gdb) r
Starting program: /home/ezannoni/gdb-6/toomany/gdb/testsuite/gdb.base/break 
During symbol reading...Corrupt data in /lib64/tls/libm.so.6:.eh_frame; align 4 workaround apparently succeeded...
During symbol reading...Corrupt data in /lib64/tls/libc.so.6:.eh_frame; align 4 workaround apparently succeeded...
720

Program exited normally.



elena

Index: Makefile.in
===================================================================
RCS file: /cvs/src/src/gdb/Makefile.in,v
retrieving revision 1.410.2.4
diff -u -p -r1.410.2.4 Makefile.in
--- Makefile.in	7 Jul 2003 15:57:29 -0000	1.410.2.4
+++ Makefile.in	10 Jul 2003 22:16:39 -0000
@@ -1703,7 +1703,7 @@ dwarf2loc.o: dwarf2loc.c $(defs_h) $(ui_
 dwarf2-frame.o: $(defs_h) $(dwarf2expr_h) $(elf_dwarf2_h) $(frame_h) \
 	$(frame_base_h) $(frame_unwind_h) $(gdbcore_h) $(gdbtypes_h) \
 	$(symtab_h) $(objfiles_h) $(regcache_h) $(gdb_assert_h) \
-	$(gdb_string_h) $(dwarf2_frame_h)
+	$(gdb_string_h) $(dwarf2_frame_h) $(complaints_h)
 dwarf2read.o: dwarf2read.c $(defs_h) $(bfd_h) $(symtab_h) $(gdbtypes_h) \
 	$(symfile_h) $(objfiles_h) $(elf_dwarf2_h) $(buildsym_h) \
 	$(demangle_h) $(expression_h) $(filenames_h) $(macrotab_h) \

Index: dwarf2-frame.c
===================================================================
RCS file: /cvs/uberbaum/gdb/dwarf2-frame.c,v
retrieving revision 1.7
diff -u -p -r1.7 dwarf2-frame.c
--- dwarf2-frame.c	8 Jun 2003 18:27:13 -0000	1.7
+++ dwarf2-frame.c	10 Jul 2003 22:17:44 -0000
@@ -36,6 +36,7 @@
 #include "gdb_assert.h"
 #include "gdb_string.h"
 
+#include "complaints.h"
 #include "dwarf2-frame.h"
 
 /* Call Frame Information (CFI).  */
@@ -1058,35 +1059,44 @@ add_fde (struct comp_unit *unit, struct 
 #define DW64_CIE_ID ~0
 #endif
 
-/* Read a CIE or FDE in BUF and decode it.  */
+static char *decode_frame_entry (struct comp_unit *unit, char *start,
+				 int eh_frame_p);
 
+/* Decode the next CIE or FDE.  Return NULL if invalid input, otherwise
+   the next byte to be processed.  */
 static char *
-decode_frame_entry (struct comp_unit *unit, char *buf, int eh_frame_p)
+decode_frame_entry_1 (struct comp_unit *unit, char *start, int eh_frame_p)
 {
+  char *buf;
   LONGEST length;
   unsigned int bytes_read;
-  int dwarf64_p = 0;
-  ULONGEST cie_id = DW_CIE_ID;
+  int dwarf64_p;
+  ULONGEST cie_id;
   ULONGEST cie_pointer;
-  char *start = buf;
   char *end;
 
+  buf = start;
   length = read_initial_length (unit->abfd, buf, &bytes_read);
   buf += bytes_read;
   end = buf + length;
 
+  /* Are we still within the section? */
+  if (end > unit->dwarf_frame_buffer + unit->dwarf_frame_size)
+    return NULL;
+
   if (length == 0)
     return end;
 
-  if (bytes_read == 12)
-    dwarf64_p = 1;
+  /* Distinguish between 32 and 64-bit encoded frame info.  */
+  dwarf64_p = (bytes_read == 12);
 
-  /* In a .eh_frame section, zero is used to distinguish CIEs from
-     FDEs.  */
+  /* In a .eh_frame section, zero is used to distinguish CIEs from FDEs.  */
   if (eh_frame_p)
     cie_id = 0;
   else if (dwarf64_p)
     cie_id = DW64_CIE_ID;
+  else
+    cie_id = DW_CIE_ID;
 
   if (dwarf64_p)
     {
@@ -1124,7 +1134,8 @@ decode_frame_entry (struct comp_unit *un
       cie->encoding = encoding_for_size (unit->addr_size);
 
       /* Check version number.  */
-      gdb_assert (read_1_byte (unit->abfd, buf) == DW_CIE_VERSION);
+      if (read_1_byte (unit->abfd, buf) != DW_CIE_VERSION)
+	return NULL;
       buf += 1;
 
       /* Interpret the interesting bits of the augmentation.  */
@@ -1159,6 +1170,8 @@ decode_frame_entry (struct comp_unit *un
 
 	  length = read_unsigned_leb128 (unit->abfd, buf, &bytes_read);
 	  buf += bytes_read;
+	  if (buf > end)
+	    return NULL;
 	  cie->initial_instructions = buf + length;
 	  augmentation++;
 	}
@@ -1211,16 +1224,20 @@ decode_frame_entry (struct comp_unit *un
       /* This is a FDE.  */
       struct dwarf2_fde *fde;
 
+      /* In an .eh_frame section, the CIE pointer is the delta between the
+	 address within the FDE where the CIE pointer is stored and the
+	 address of the CIE.  Convert it to an offset into the .eh_frame
+	 section.  */
       if (eh_frame_p)
 	{
-	  /* In an .eh_frame section, the CIE pointer is the delta
-             between the address within the FDE where the CIE pointer
-             is stored and the address of the CIE.  Convert it to an
-             offset into the .eh_frame section.  */
 	  cie_pointer = buf - unit->dwarf_frame_buffer - cie_pointer;
 	  cie_pointer -= (dwarf64_p ? 8 : 4);
 	}
 
+      /* In either case, validate the result is still within the section.  */
+      if (cie_pointer >= unit->dwarf_frame_size)
+	return NULL;
+
       fde = (struct dwarf2_fde *)
 	obstack_alloc (&unit->objfile->psymbol_obstack,
 		       sizeof (struct dwarf2_fde));
@@ -1252,6 +1269,8 @@ decode_frame_entry (struct comp_unit *un
 
 	  length = read_unsigned_leb128 (unit->abfd, buf, &bytes_read);
 	  buf += bytes_read + length;
+	  if (buf > end)
+	    return NULL;
 	}
 
       fde->instructions = buf;
@@ -1262,6 +1281,97 @@ decode_frame_entry (struct comp_unit *un
 
   return end;
 }
+
+/* Read a CIE or FDE in BUF and decode it.  */
+static char *
+decode_frame_entry (struct comp_unit *unit, char *start, int eh_frame_p)
+{
+  enum { NONE, ALIGN4, ALIGN8, FAIL } workaround = NONE;
+  char *ret;
+  const char *msg;
+  ptrdiff_t start_offset;
+
+  while (1)
+    {
+      ret = decode_frame_entry_1 (unit, start, eh_frame_p);
+
+      if (ret != NULL)
+	break;
+
+      /* We have corrupt input data of some form.  */
+
+      /* ??? Try, weakly, to work around compiler/assembler/linker bugs
+	 and mismatches wrt padding and alignment of debug sections.  */
+      /* Note that there is no requirement in the standard for any
+	 alignment at all in the frame unwind sections.  Testing for
+	 alignment before trying to interpret data would be incorrect.
+
+	 However, GCC traditionally arranged for frame sections to be
+	 sized such that the FDE length and CIE fields happen to be
+	 aligned (in theory, for performance).  This, unfortunately,
+	 was done with .align directives, which had the side effect of
+	 forcing the section to be aligned by the linker.
+
+	 This becomes a problem when you have some other producer that
+	 creates frame sections that are not as strictly aligned.  That
+	 produces a hole in the frame info that gets filled by the 
+	 linker with zeros.
+
+	 The GCC behaviour is arguably a bug, but it's effectively now
+	 part of the ABI, so we're now stuck with it, at least at the
+	 object file level.  A smart linker may decide, in the process
+	 of compressing duplicate CIE information, that it can rewrite
+	 the entire output section without this extra padding.  */
+
+      start_offset = start - unit->dwarf_frame_buffer;
+      if (workaround <= ALIGN4 && (start_offset & 3) != 0)
+	{
+	  start += 4 - (start_offset & 3);
+	  workaround = ALIGN4;
+	  continue;
+	}
+      if (workaround <= ALIGN8 && (start_offset & 7) != 0)
+	{
+	  start += 8 - (start_offset & 7);
+	  workaround = ALIGN8;
+	  continue;
+	}
+
+      /* Nothing left to try.  Arrange to return as if we've consumed
+	 the entire input section.  Hopefully we'll get valid info from
+	 the other of .debug_frame/.eh_frame.  */
+      workaround = FAIL;
+      ret = unit->dwarf_frame_buffer + unit->dwarf_frame_size;
+      break;
+    }
+
+  switch (workaround)
+    {
+    case NONE:
+      return ret;
+    case ALIGN4:
+      complaint (&symfile_complaints,
+		 "Corrupt data in %s:%s; align 4 workaround apparently succeeded",
+		 unit->dwarf_frame_section->owner->filename,
+		 unit->dwarf_frame_section->name);
+      break;
+    case ALIGN8:
+      complaint (&symfile_complaints,
+		 "Corrupt data in %s:%s; align 8 workaround apparently succeeded",
+		 unit->dwarf_frame_section->owner->filename,
+		 unit->dwarf_frame_section->name);
+      break;
+    case FAIL:
+      complaint (&symfile_complaints,
+		 "Corrupt data in %s:%s",
+		 unit->dwarf_frame_section->owner->filename,
+		 unit->dwarf_frame_section->name);
+      break;
+    }
+
+  return ret;
+}
+
 
 
 /* FIXME: kettenis/20030504: This still needs to be integrated with
@@ -1307,9 +1417,9 @@ dwarf2_build_frame_info (struct objfile 
       unit.dwarf_frame_section = dwarf_eh_frame_section;
 
       /* FIXME: kettenis/20030602: This is the DW_EH_PE_datarel base
-         that for the i386/amd64 target, which currently is the only
-         target in GCC that supports/uses the DW_EH_PE_datarel
-         encoding.  */
+	 that for the i386/amd64 target, which currently is the only
+	 target in GCC that supports/uses the DW_EH_PE_datarel
+	 encoding.  */
       got = bfd_get_section_by_name (unit.abfd, ".got");
       if (got)
 	unit.dbase = got->vma;

 


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]