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

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

On Jul 5, 2011, at 5:14 PM, ext Joel Brobecker wrote:

>> 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 
> 
> Just a few more nits... (a lot of rules, I know, but we try to have
> a consistent development style).

eh eh I sort of expected it, so I put a number after update in the subject :)

> [...]
> Empty line between documentation comment and start of definition...

this doesn't seem to be much followed in this file... I am fixing it for the functions I touch...
but I suppose it doesn't apply for in function comments, as it is never used like that in this file.

> [...]
>> +      /* checks that we expected a CIE */
>> +      gdb_assert ((entry_type & eh_cie_type_id) != 0);
> 
> Same here as well. But I think you meant something like `We expect
> a CIE.'.

actually no at that point he found a CIE but he has to check that a CIE was actually expected.
I have reworded it to /* Check that a CIE was expected. */

another thing that I have noted os 0x0c characters in the file (new page in ascii), not sure why they are there.

anyway 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	5 Jul 2011 16:17:57 -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,9 @@
       char *augmentation;
       unsigned int cie_version;
 
+      /* Check that a CIE was expected. */
+      gdb_assert ((entry_type & eh_cie_type_id) != 0);
+
       /* Record the offset into the .debug_frame section of this CIE.  */
       cie_pointer = start - unit->dwarf_frame_buffer;
 
@@ -2027,6 +2043,9 @@
       /* This is a FDE.  */
       struct dwarf2_fde *fde;
 
+      /* Check that an FDE was expected. */
+      gdb_assert ((entry_type & eh_fde_type_id) != 0);
+
       /* 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 +2067,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 +2109,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 +2125,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 +2279,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 +2301,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.  */