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]

[rfa/6.0] Better handle unspecified CFI values


Hello,

This patch is an attempt at improving GDB's behavior when GCC "treads the boundaries of the CFI specification".

It does the following:

- changes the rules REG_UNMODIFIED -> REG_SAME_VALUE and REG_UNSAVED -> REG_UNDEFINED so that they better match the corresponding CFI register states (I could commit this separatly). The other names confused me :-)

- it adds a new register rule - REG_UNSPECIFIED - which is used to differentiate a register that is missing CFI info from a register that CFI specified as "undefined" (nee UNSAVED).

- when unwinding, it treats REG_UNSPECIFIED registers like REG_SAME_VALUE but with the additional hack to map an unspecified SP_REGNUM onto the CFA.

- if it detects an unspecified CFI entry it complains
It isn't perfect though - since it doesn't know the full range of valid debug info register numbers it can't check every entry. Instead it checks the range provided by CFI for unspecified holes and then complains about that. The reality is that GCC at least gets that bit right (but consistently forgets the SP).


I'd like to commit the patch as is for the 6.0 branch. For the mainline though, I'd like to make the additional changes:

- delete the SP_REGNUM hack from the REG_UNDEFINED rule (it's no longer needed, I think)

- add a check/complaint for the SP v CFA problem.

Anyway, the end result is that on x86-64 and i386 store.exp now passes.

ok to commit?
Andrew
2003-09-05  Andrew Cagney  <cagney@redhat.com>

	* dwarf2-frame.c (enum dwarf2_reg_rule): New, replace anonymous
	enum.  Add REG_UNSPECIFIED, rename REG_UNSAVED to REG_UNDEFINED
	and REG_UNMODIFIED to REG_SAME_VALUE.
	(execute_cfa_program): Update.
	(dwarf2_frame_cache): Update.  Initialize table to
	REG_UNSPECIFIED, complain if CFI fails to specify a register's
	location.
	(dwarf2_frame_prev_register): Update.  Handle REG_UNSPECIFIED.

Index: dwarf2-frame.c
===================================================================
RCS file: /cvs/src/src/gdb/dwarf2-frame.c,v
retrieving revision 1.12
diff -u -r1.12 dwarf2-frame.c
--- dwarf2-frame.c	26 Aug 2003 03:07:29 -0000	1.12
+++ dwarf2-frame.c	6 Sep 2003 00:15:28 -0000
@@ -97,6 +97,18 @@
 
 /* Structure describing a frame state.  */
 
+enum dwarf2_reg_rule
+{
+  /* Initializing using MEMSET.  Make certain that 0 maps onto the
+     correct enum value.  */
+  REG_UNSPECIFIED = 0,
+  REG_UNDEFINED,
+  REG_SAVED_OFFSET,
+  REG_SAVED_REG,
+  REG_SAVED_EXP,
+  REG_SAME_VALUE
+};
+
 struct dwarf2_frame_state
 {
   /* Each register save state can be described in terms of a CFA slot,
@@ -111,13 +123,7 @@
 	unsigned char *exp;
       } loc;
       ULONGEST exp_len;
-      enum {
-	REG_UNSAVED,
-	REG_SAVED_OFFSET,
-	REG_SAVED_REG,
-	REG_SAVED_EXP,
-	REG_UNMODIFIED
-      } how;
+      enum dwarf2_reg_rule how;
     } *reg;
     int num_regs;
 
@@ -354,13 +360,13 @@
 	    case DW_CFA_undefined:
 	      insn_ptr = read_uleb128 (insn_ptr, insn_end, &reg);
 	      dwarf2_frame_state_alloc_regs (&fs->regs, reg + 1);
-	      fs->regs.reg[reg].how = REG_UNSAVED;
+	      fs->regs.reg[reg].how = REG_UNDEFINED;
 	      break;
 
 	    case DW_CFA_same_value:
 	      insn_ptr = read_uleb128 (insn_ptr, insn_end, &reg);
 	      dwarf2_frame_state_alloc_regs (&fs->regs, reg + 1);
-	      fs->regs.reg[reg].how = REG_UNMODIFIED;
+	      fs->regs.reg[reg].how = REG_SAME_VALUE;
 	      break;
 
 	    case DW_CFA_register:
@@ -460,11 +466,10 @@
 dwarf2_frame_cache (struct frame_info *next_frame, void **this_cache)
 {
   struct cleanup *old_chain;
-  int num_regs = NUM_REGS + NUM_PSEUDO_REGS;
+  const int num_regs = NUM_REGS + NUM_PSEUDO_REGS;
   struct dwarf2_frame_cache *cache;
   struct dwarf2_frame_state *fs;
   struct dwarf2_fde *fde;
-  int reg;
 
   if (*this_cache)
     return *this_cache;
@@ -535,34 +540,64 @@
       internal_error (__FILE__, __LINE__, "Unknown CFA rule.");
     }
 
-  /* Save the register info in the cache.  */
-  for (reg = 0; reg < fs->regs.num_regs; reg++)
-    {
-      int regnum;
-
-      /* Skip the return address column.  */
-      if (reg == fs->retaddr_column)
-	/* NOTE: cagney/2003-06-07: Is this right?  What if the
-           RETADDR_COLUM corresponds to a real register (and, worse,
-           that isn't the PC_REGNUM)?  I'm guessing that the PC_REGNUM
-           further down is trying to handle this.  That can't be right
-           though - PC_REGNUM may not be valid (it can be -ve).  I
-           think, instead when RETADDR_COLUM isn't a real register, it
-           should map itself onto frame_pc_unwind.  */
-	continue;
-
-      /* Use the GDB register number as index.  */
-      regnum = DWARF2_REG_TO_REGNUM (reg);
+  /* Initialize things so that all registers are marked as
+     unspecified.  */
+  {
+    int regnum;
+    for (regnum = 0; regnum < num_regs; regnum++)
+      cache->reg[regnum].how = REG_UNSPECIFIED;
+  }
 
-      if (regnum >= 0 && regnum < num_regs)
-	cache->reg[regnum] = fs->regs.reg[reg];
-    }
+  /* Go through the DWARF2 CFI generated table and save its register
+     location information in the cache.  */
+  {
+    int cfinum;
+    for (cfinum = 0; cfinum < fs->regs.num_regs; cfinum++)
+      {
+	int regnum;
+	
+	/* Skip the return address column.  */
+	if (cfinum == fs->retaddr_column)
+	  /* NOTE: cagney/2003-06-07: Is this right?  What if the
+	     RETADDR_COLUM corresponds to a real register (and, worse,
+	     that isn't the PC_REGNUM)?  I'm guessing that the
+	     PC_REGNUM further down is trying to handle this.  That
+	     can't be right though - PC_REGNUM may not be valid (it
+	     can be -ve).  I think, instead when RETADDR_COLUM isn't a
+	     real register, it should map itself onto frame_pc_unwind.  */
+	  continue;
+
+	/* Use the GDB register number as the destination index.  */
+	regnum = DWARF2_REG_TO_REGNUM (cfinum);
+
+	/* If there's no corresponding GDB register, ignore it.  */
+	if (regnum < 0 || regnum >= num_regs)
+	  continue;
+
+	/* NOTE: cagney/2003-09-05: CFI should specify the disposition
+	   of all debug info registers.  If it doesn't complain (but
+	   not too loudly).  It turns out that GCC, assumes that an
+	   unspecified register implies "same value" when CFI (draft
+	   7) specifies nothing at all.  Such a register could equally
+	   be interpreted as "undefined".  Also note that this check
+	   isn't sufficient - should the CFI fully specify a
+	   contigious subset of registers this check won't detect a
+	   problem.  Is an upper bound on the number of debug info
+	   registers available?  */
+	if (fs->regs.reg[cfinum].how == REG_UNSPECIFIED)
+	  complaint (&symfile_complaints,
+		     "Incomplete CFI data; unspecified registers at 0x%s",
+		     paddr (fs->pc));
+
+	cache->reg[regnum] = fs->regs.reg[cfinum];
+      }
+  }
 
   /* Store the location of the return addess.  If the return address
      column (adjusted) is not the same as gdb's PC_REGNUM, then this
      implies a copy from the ra column register.  */
   if (fs->retaddr_column < fs->regs.num_regs
-      && fs->regs.reg[fs->retaddr_column].how != REG_UNSAVED)
+      && fs->regs.reg[fs->retaddr_column].how != REG_UNDEFINED)
     {
       /* See comment above about a possibly -ve PC_REGNUM.  If this
          assertion fails, it's a problem with this code and not the
@@ -572,7 +607,7 @@
     }
   else
     {
-      reg = DWARF2_REG_TO_REGNUM (fs->retaddr_column);
+      int reg = DWARF2_REG_TO_REGNUM (fs->retaddr_column);
       if (reg != PC_REGNUM)
 	{
 	  /* See comment above about PC_REGNUM being -ve.  If this
@@ -611,7 +646,9 @@
 
   switch (cache->reg[regnum].how)
     {
-    case REG_UNSAVED:
+    case REG_UNDEFINED:
+      /* If CFI explicitly specified that the value isn't defined,
+	 mark it as optomized away - the value isn't available.  */
       *optimizedp = 1;
       *lvalp = not_lval;
       *addrp = 0;
@@ -636,6 +673,12 @@
              very real posibility that CFA is an offset from some
              other register, having nothing to do with the unwound SP
              value.  */
+	  /* FIXME: cagney/2003-09-05: I think I do now.  GDB was
+	     lumping the two states "unspecified" and "undefined"
+	     together.  Here SP_REGNUM was "unspecified", GCC assuming
+	     that in such a case CFA would be used.  This code should
+	     be deleted - this specific problem now handled as part of
+	     REG_UNSPECIFIED case further down.  */
 	  *optimizedp = 0;
 	  if (valuep)
 	    {
@@ -687,7 +730,53 @@
 	}
       break;
 
-    case REG_UNMODIFIED:
+    case REG_UNSPECIFIED:
+      /* GCC, in its infinite wisdom decided to not provide unwind
+	 information for registers that are "same value".  Since
+	 DWARF2 (3 draft 7) doesn't define such behavior, said
+	 registers are actually undefined (which is different to CFI
+	 "undefined").  Code above issues a complaint about this.
+	 Here just fudge the books, assume GCC, and that the value is
+	 more inner on the stack.  */
+      if (SP_REGNUM >= 0 && regnum == SP_REGNUM)
+	{
+	  /* Can things get worse?  Yep!  One of the registers GCC
+	     forgot to provide unwind information for was the stack
+	     pointer.  Outch!  GCC appears to assumes that the CFA
+	     address can be used - after all it points to the inner
+	     most address of the previous frame before the function
+	     call and that's always the same as the stack pointer on
+	     return, right?  Wrong.  See GCC's i386 STDCALL option for
+	     an ABI that has a different entry and return stack
+	     pointer.  */
+	  /* DWARF V3 Draft 7 p102: Typically, the CFA is defined to
+	     be the value of the stack pointer at the call site in the
+	     previous frame (which may be different from its value on
+	     entry to the current frame).  */
+	  /* DWARF V3 Draft 7 p103: The first column of the rules
+             defines the rule which computes the CFA value; it may be
+             either a register and a signed offset that are added
+             together or a DWARF expression that is evaluated.  */
+	  /* FIXME: cagney/2003-09-05: Complain about this via
+             complaint().  */
+	  *optimizedp = 0;
+	  *lvalp = not_lval;
+	  *addrp = 0;
+	  *realnump = -1;
+	  if (valuep)
+	    /* Store the value.  */
+	    store_typed_address (valuep, builtin_type_void_data_ptr,
+				 cache->cfa);
+	}
+      else
+	/* Assume that the register can be found in the next inner
+           most frame.  */
+	frame_register_unwind (next_frame, regnum,
+			       optimizedp, lvalp, addrp, realnump, valuep);
+      break;
+
+
+    case REG_SAME_VALUE:
       frame_register_unwind (next_frame, regnum,
 			     optimizedp, lvalp, addrp, realnump, valuep);
       break;

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