Re: [PATCH,gdb,Update3]: ensures that cie ptr of an fda is a cie

[Fawzi Mohamed <fawzi.mohamed@nokia.com>]

On Jul 5, 2011, at 6:25 PM, ext Tom Tromey wrote:

>>>>>> "Fawzi" == Fawzi Mohamed <fawzi.mohamed@nokia.com> writes:
> [...]
> I think enum constant names should be all caps.
> That is the usual style.

Ok thanks.

> Fawzi> +      gdb_assert ((entry_type & eh_cie_type_id)!=0);
> 
> It seems to me that this code is attempting to validate user input.  If
> so, an assertion is incorrect -- instead it should call either error or
> warning and in the latter case also arrange for the bad data to be
> ignored.

What I do is to detect corrupt eh_sections. I have to say that I didn't see eh_section as user input, but for a debugger it makes sense.
I have used an error, because (at least in my case) this was triggered by another gdb bug, so I guess that it is seldom enough that recovery is not required (for recovery I guess one should nuke cie_table and fde_table, and then artificially advance or try to reach the error handler, but I am not 100% sure).

The error message has a dot at the end there doesn't seem to be a consensus on that, but I hope it is ok.

So here is the updated patch:

2011-07-04  Fawzi Mohamed  <fawzi.mohamed@nokia.com>

	* dwarf2-frame.c (decode_frame_entry, decode_frame_entry_1): Ensure
	that CIE pointer of an FDE really points to a CIE .
Index: gdb/dwarf2-frame.c
===================================================================
RCS file: /cvs/src/src/gdb/dwarf2-frame.c,v
retrieving revision 1.125
diff -f -d -u -r1.125 dwarf2-frame.c
--- gdb/dwarf2-frame.c	4 Jul 2011 16:30:09 -0000	1.125
+++ gdb/dwarf2-frame.c	6 Jul 2011 10:04:52 -0000
@@ -1801,17 +1801,30 @@
#define DW64_CIE_ID ~0
#endif

+/* Defines the type of eh_frames that are expected to be decoded: CIE, FDE
+   or any of them. */
+
+enum eh_frame_type
+{
+  EH_CIE_TYPE_ID = 1 << 0,
+  EH_FDE_TYPE_ID = 1 << 1,
+  EH_CIE_OR_FDE_TYPE_ID = EH_CIE_TYPE_ID + EH_FDE_TYPE_ID
+};
+
static gdb_byte *decode_frame_entry (struct comp_unit *unit, gdb_byte *start,
				     int eh_frame_p,
                                     struct dwarf2_cie_table *cie_table,
-                                     struct dwarf2_fde_table *fde_table);
+                                     struct dwarf2_fde_table *fde_table,
+                                     enum eh_frame_type entry_type);
+
+/* Decode the next CIE or FDE, entry_type specifies the expected type.
+   Return NULL if invalid input, otherwise the next byte to be processed.  */

-/* Decode the next CIE or FDE.  Return NULL if invalid input, otherwise
-   the next byte to be processed.  */
static gdb_byte *
decode_frame_entry_1 (struct comp_unit *unit, gdb_byte *start, int eh_frame_p,
                      struct dwarf2_cie_table *cie_table,
-                      struct dwarf2_fde_table *fde_table)
+                      struct dwarf2_fde_table *fde_table,
+                      enum eh_frame_type entry_type)
{
  struct gdbarch *gdbarch = get_objfile_arch (unit->objfile);
  gdb_byte *buf, *end;
@@ -1862,6 +1875,10 @@
      char *augmentation;
      unsigned int cie_version;

+      /* Check that a CIE was expected. */
+      if ((entry_type & EH_CIE_TYPE_ID) == 0)
+	error (_("Found a CIE when not expecting it, broken eh_info"));
+
      /* Record the offset into the .debug_frame section of this CIE.  */
      cie_pointer = start - unit->dwarf_frame_buffer;

@@ -2027,6 +2044,10 @@
      /* This is a FDE.  */
      struct dwarf2_fde *fde;

+      /* Check that an FDE was expected. */
+      if ((entry_type & EH_FDE_TYPE_ID) == 0)
+	error (_("Found an FDE when not expecting it, broken eh_info."));
+
      /* 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
@@ -2048,7 +2069,8 @@
      if (fde->cie == NULL)
	{
	  decode_frame_entry (unit, unit->dwarf_frame_buffer + cie_pointer,
-			      eh_frame_p, cie_table, fde_table);
+			      eh_frame_p, cie_table, fde_table,
+			      EH_CIE_TYPE_ID);
	  fde->cie = find_cie (cie_table, cie_pointer);
	}

@@ -2089,11 +2111,14 @@
  return end;
}

-/* Read a CIE or FDE in BUF and decode it.  */
+/* Read a CIE or FDE in BUF and decode it. Entry_type specifies if we expect
+   an FDE or a CIE. */
+
static gdb_byte *
decode_frame_entry (struct comp_unit *unit, gdb_byte *start, int eh_frame_p,
                    struct dwarf2_cie_table *cie_table,
-                    struct dwarf2_fde_table *fde_table)
+                    struct dwarf2_fde_table *fde_table,
+                    enum eh_frame_type entry_type)
{
  enum { NONE, ALIGN4, ALIGN8, FAIL } workaround = NONE;
  gdb_byte *ret;
@@ -2102,7 +2127,7 @@
  while (1)
    {
      ret = decode_frame_entry_1 (unit, start, eh_frame_p,
-                                  cie_table, fde_table);
+				  cie_table, fde_table,entry_type);
      if (ret != NULL)
	break;

@@ -2256,7 +2281,8 @@
          frame_ptr = unit->dwarf_frame_buffer;
          while (frame_ptr < unit->dwarf_frame_buffer + unit->dwarf_frame_size)
            frame_ptr = decode_frame_entry (unit, frame_ptr, 1,
-                                            &cie_table, &fde_table);
+                                            &cie_table, &fde_table,
+                                            EH_CIE_OR_FDE_TYPE_ID);

          if (cie_table.num_entries != 0)
            {
@@ -2277,7 +2303,8 @@
      frame_ptr = unit->dwarf_frame_buffer;
      while (frame_ptr < unit->dwarf_frame_buffer + unit->dwarf_frame_size)
	frame_ptr = decode_frame_entry (unit, frame_ptr, 0,
-                                        &cie_table, &fde_table);
+                                        &cie_table, &fde_table,
+                                        EH_CIE_OR_FDE_TYPE_ID);
    }

  /* Discard the cie_table, it is no longer needed.  */