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: ia64 tdep patch


I have included a patch which contains responses to your comments thus far.

The ChangeLog is:

2003-10-23  Jeff Johnston  <jjohnstn@redhat.com>
	* ia64-tdep.c: (ia64_frame_cache): Add new prev_cfm field.
	(pseudo_regs): Add comment regarding register stack registers.
	(ia64_alloc_frame_cache):  Initialize new prev_cfm field to 0.
	(floatformat_valid): New static routine.
	(floatformat_ia64_ext): Add name field and set up is_valid routine
	to floatformat_valid().
	(examine_prologue):  For the previous cfm, use
	frame_unwind_register()
	if the cfm is not stored in a register-stack register.  Save the
	previous cfm value in the prev_cfm field.  Add debug output.
	(ia64_frame_this_id): Use frame_id_build_special() to also register
	the bsp.  Add debug output.
	(ia64_sigtramp_frame_this_id): Ditto.
	(ia64_frame_prev_register):  Look at cache saved_regs for a few more
	registers and also add some checks for framelessness before accepting
	current register values for fields such as return address.  For cfm,
	use the cached prev_cfm field if available.  Add comment to explain
	PSR logic.  Add debug output.
	(ia64_sigtramp_frame_init_saved_regs): Bump up base by 16 to get
	sp needed for calling lower level
	ia64_linux_sigcontext_register_address().  Also save the
	bsp and sp address as part of initialization.
	(ia64_sigtramp_frame_cache): Hard-code stack size as it can't be
	calculated.  Cache the bsp and cfm values.
	(ia64_sigtramp_frame_prev_register): Add logic to this routine out
	instead of using ia64_frame_prev_register() which doesn't expect most
	registers to be saved.  The saved values for bsp and sp
	can be taken from the cache.  Add debug output.
	(ia64_push_dummy_call): Use frame_id_build_special() to also register
	the bsp.

Ok?

-- Jeff J.

J. Johnston wrote:
J. Johnston wrote:

Kevin Buettner wrote:

On Oct 21, 7:03pm, J. Johnston wrote:


You're looking at my old ChangeLog.




I've looked at your most recent patch, but do not see the new ChangeLog
entry.  Would you mind posting it?


2003-10-20 Jeff Johnston <jjohnstn@redhat.com>


* ia64-tdep.c: (ia64_frame_cache): Add new prev_cfm field.
(ia64_alloc_frame_cache): Initialize new prev_cfm field to 0.
(floatformat_valid): New static routine.
(floatformat_ia64_ext): Add name field and set up is_valid routine
to floatformat_valid().
(examine_prologue): For the previous cfm, use frame_unwind_register()
if the cfm is not stored in a register-stack register. Save the
previous cfm value in the prev_cfm field. Add debug output.
(ia64_frame_this_id): Use frame_id_build_special() to also register
the bsp. Add debug output.
(ia64_sigtramp_frame_this_id): Ditto.
(ia64_frame_prev_register): Look at cache saved_regs for a few more
registers and also add some checks for framelessness before accepting
current register values for fields such as return address. For cfm,
use the cached prev_cfm field if available. Add debug output.
(ia64_sigtramp_frame_init_saved_regs): Bump up base by 16 to get
sp needed for calling lower level
ia64_linux_sigcontext_register_address(). Also save the
bsp and sp address as part of initialization.
(ia64_sigtramp_frame_cache): Hard-code stack size as it can't be
calculated. Cache the bsp and cfm values.
(ia64_sigtramp_frame_prev_register): Flesh this routine out instead of
using ia64_frame_prev_register(). The saved values for bsp and sp can
be taken from the cache. Add debug output.
(ia64_push_dummy_call): Use frame_id_build_special() to also register
the bsp.




The code (below) in ia64_sigtramp_frame_prev_register() which computes
PSR doesn't look right to me.  Could you check it?  (If it is right,
please explain it...)



I'll explain my logic. As you know, the VRAP address is the return address. AFAICT by reading the ABI and insn set, there is no information about what the return address is set to when the branch is in slot 0 or 1 (i.e. is the return address the next bundle or the next slot?). The ip register isn't supposed to contain the slot number; it is encoded in the PSR register. When gdb gets the pc value, it forms it by unwinding the PSR and IP registers - getting the slot number from the PSR and the IP register address to form a virtual pc address. I did not want to get the slot number wrong if it was encoded in the return address so this is why I masked it off above. The PSR register is only used by gdb in unwinding the pc.




Thanks for the explanation.  Could you please add a brief comment to the
code.


Will do.



Actually, you were right, I was wrong. The VRAP for sigtramp is the IP register so it won't have the PSR info in it. The sigcontext stores the PSR so we don't have to reconstruct it in this case. I have removed the code.



Regarding this hunk of code in ia64_sigtramp_frame_prev_register()...

+ else if ((regnum >= IA64_GR32_REGNUM && regnum <= IA64_GR127_REGNUM) ||
+ (regnum >= V32_REGNUM && regnum <= V127_REGNUM))
+ {
+ CORE_ADDR addr = 0;
+ if (regnum >= V32_REGNUM)
+ regnum = IA64_GR32_REGNUM + (regnum - V32_REGNUM);
+ addr = cache->saved_regs[regnum];
+ if (addr != 0)
+ {
+ *lvalp = lval_memory;
+ *addrp = addr;
+ read_memory (addr, valuep, register_size (current_gdbarch, regnum));
+ }
+ }


Could you add a comment explaining why the normal method of computing
V32 (via ia64_pseudo_register_read()) is inadequate?



I don't know. I had this for safety reasons already in the ia64_frame_prev_register() because I didn't know if it might be called with the pseudo register number or not. This code was copied. Should it be removed in both places?




I don't know.  I've been studying the code and am wondering why the
V32 ... V127 pseudo regs were introduced at all.  Could you remind
me of the reason?


They are needed because r32 to r127 are not accessible via the PTRACE interface. They are accessed via the bsp. Without flagging them as pseudo-registers, the regcache code returns 0 for all these registers.


I tell the dwarf to convert references to r32-r127 to be V32-V127 in ia64_dwarf_reg_to_regnum(). I would guess that a parameter reference in a previous frame would cause this conversion to occur.


I have updated the comment for the pseudo-register enums regarding the fact that r32 - r127 are not accessible via the ptrace register get/set interfaces.



Hmm... doesn't this hunk of code also need to be concerned with
register renames?  (I.e, the rotating register stuff...)  I'm
wondering why the floating point registers need it, but the
GRs don't.


This code was copied from ia64_frame_prev_register() as it used to be called to do the underlying work.


The stuff at the end of examine_prologue() handles rotating GRs for
the normal case but doesn't for floating point registers.  I would
doubt very much that the signal trampoline uses rotating registers
so I probably should remove it for the floating-point case.




I think you're probably right.

It'd be nice though if we could arrange for the code which handles
rotating registers for floating point and general regs to appear
in the same function.  (This can happen in a different patch though.)

Kevin





Index: ia64-tdep.c
===================================================================
RCS file: /cvs/src/src/gdb/ia64-tdep.c,v
retrieving revision 1.99
diff -u -p -r1.99 ia64-tdep.c
--- ia64-tdep.c	20 Oct 2003 20:38:07 -0000	1.99
+++ ia64-tdep.c	23 Oct 2003 16:16:56 -0000
@@ -108,7 +108,7 @@ static int fp_regnum = IA64_VFP_REGNUM;
 static int lr_regnum = IA64_VRAP_REGNUM;
 
 /* NOTE: we treat the register stack registers r32-r127 as pseudo-registers because
-   they are in memory and must be calculated via the bsp register.  */
+   they may not be accessible via the ptrace register get/set interfaces.  */
 enum pseudo_regs { FIRST_PSEUDO_REGNUM = NUM_IA64_RAW_REGS, VBOF_REGNUM = IA64_NAT127_REGNUM + 1, V32_REGNUM, 
 		   V127_REGNUM = V32_REGNUM + 95, 
 		   VP0_REGNUM, VP16_REGNUM = VP0_REGNUM + 16, VP63_REGNUM = VP0_REGNUM + 63, LAST_PSEUDO_REGNUM };
@@ -232,6 +232,7 @@ struct ia64_frame_cache
   CORE_ADDR saved_sp;	/* stack pointer for frame */
   CORE_ADDR bsp;	/* points at r32 for the current frame */
   CORE_ADDR cfm;	/* cfm value for current frame */
+  CORE_ADDR prev_cfm;   /* cfm value for previous frame */
   int   frameless;
   int   sof;		/* Size of frame  (decoded from cfm value) */
   int	sol;		/* Size of locals (decoded from cfm value) */
@@ -316,10 +317,18 @@ ia64_dwarf_reg_to_regnum (int reg)
   return reg;
 }
 
+static int
+floatformat_valid (fmt, from)
+     const struct floatformat *fmt;
+     const char *from;
+{
+  return 1;
+}
+
 const struct floatformat floatformat_ia64_ext =
 {
   floatformat_little, 82, 0, 1, 17, 65535, 0x1ffff, 18, 64,
-  floatformat_intbit_yes
+  floatformat_intbit_yes, "floatformat_ia64_ext", floatformat_valid
 };
 
 
@@ -1030,6 +1039,7 @@ ia64_alloc_frame_cache (void)
   cache->base = 0;
   cache->pc = 0;
   cache->cfm = 0;
+  cache->prev_cfm = 0;
   cache->sof = 0;
   cache->sol = 0;
   cache->sor = 0;
@@ -1450,9 +1460,20 @@ examine_prologue (CORE_ADDR pc, CORE_ADD
 
       /* For the previous argument registers we require the previous bof.  
 	 If we can't find the previous cfm, then we can do nothing.  */
+      cfm = 0;
       if (cache->saved_regs[IA64_CFM_REGNUM] != 0)
 	{
 	  cfm = read_memory_integer (cache->saved_regs[IA64_CFM_REGNUM], 8);
+	}
+      else if (cfm_reg != 0)
+	{
+	  frame_unwind_register (next_frame, cfm_reg, buf);
+	  cfm = extract_unsigned_integer (buf, 8);
+	}
+      cache->prev_cfm = cfm;
+      
+      if (cfm != 0)
+	{
 	  sor = ((cfm >> 14) & 0xf) * 8;
 	  sof = (cfm & 0x7f);
 	  sol = (cfm >> 7) & 0x7f;
@@ -1564,7 +1585,11 @@ ia64_frame_this_id (struct frame_info *n
   if (cache->base == 0)
     return;
 
-  (*this_id) = frame_id_build (cache->base, cache->pc);
+  (*this_id) = frame_id_build_special (cache->base, cache->pc, cache->bsp);
+  if (gdbarch_debug >= 1)
+    fprintf_unfiltered (gdb_stdlog,
+			"regular frame id: code %lx, stack %lx, special %lx, next_frame %p\n",
+			this_id->code_addr, this_id->stack_addr, cache->bsp, next_frame);
 }
 
 static void
@@ -1628,18 +1653,20 @@ ia64_frame_prev_register (struct frame_i
     }
   else if (regnum == IA64_CFM_REGNUM)
     {
-      CORE_ADDR addr = 0;
-
-      if (cache->frameless)
+      CORE_ADDR addr = cache->saved_regs[IA64_CFM_REGNUM];
+      
+      if (addr != 0)
 	{
-	  CORE_ADDR cfm = 0;
-	  frame_unwind_register (next_frame, IA64_PFS_REGNUM, valuep);
+	  *lvalp = lval_memory;
+	  *addrp = addr;
+	  read_memory (addr, valuep, register_size (current_gdbarch, regnum));
 	}
-      else
+      else if (cache->prev_cfm)
+	store_unsigned_integer (valuep, register_size (current_gdbarch, regnum), cache->prev_cfm);
+      else if (cache->frameless)
 	{
-	  addr = cache->saved_regs[IA64_CFM_REGNUM];
-	  if (addr != 0)
-	    read_memory (addr, valuep, register_size (current_gdbarch, regnum));
+	  CORE_ADDR cfm = 0;
+	  frame_unwind_register (next_frame, IA64_PFS_REGNUM, valuep);
 	}
     }
   else if (regnum == IA64_VFP_REGNUM)
@@ -1727,53 +1754,68 @@ ia64_frame_prev_register (struct frame_i
   else if (regnum == IA64_IP_REGNUM)
     {
       CORE_ADDR pc = 0;
+      CORE_ADDR addr = cache->saved_regs[IA64_VRAP_REGNUM];
 
-      if (cache->frameless)
+      if (addr != 0)
 	{
-	  frame_unwind_register (next_frame, IA64_BR0_REGNUM, buf);
+	  *lvalp = lval_memory;
+	  *addrp = addr;
+	  read_memory (addr, buf, register_size (current_gdbarch, IA64_IP_REGNUM));
 	  pc = extract_unsigned_integer (buf, 8);
 	}
-      else
+      else if (cache->frameless)
 	{
-	  CORE_ADDR addr = cache->saved_regs[IA64_VRAP_REGNUM];
-	  if (addr != 0)
-	    {
-	      read_memory (addr, buf, register_size (current_gdbarch, IA64_IP_REGNUM));
-	      pc = extract_unsigned_integer (buf, 8);
-	    }
+	  frame_unwind_register (next_frame, IA64_BR0_REGNUM, buf);
+	  pc = extract_unsigned_integer (buf, 8);
 	}
       pc &= ~0xf;
       store_unsigned_integer (valuep, 8, pc);
     }
   else if (regnum == IA64_PSR_REGNUM)
     {
+      /* We don't know how to get the complete previous PSR, but we need it for
+	 the slot information when we unwind the pc (pc is formed of IP register
+	 plus slot information from PSR).  To get the previous slot information, 
+	 we mask it off the return address.  */
       ULONGEST slot_num = 0;
       CORE_ADDR pc= 0;
       CORE_ADDR psr = 0;
+      CORE_ADDR addr = cache->saved_regs[IA64_VRAP_REGNUM];
 
       frame_unwind_register (next_frame, IA64_PSR_REGNUM, buf);
       psr = extract_unsigned_integer (buf, 8);
 
-      if (cache->frameless)
+      if (addr != 0)
 	{
-	  CORE_ADDR pc;
-	  frame_unwind_register (next_frame, IA64_BR0_REGNUM, buf);
+	  *lvalp = lval_memory;
+	  *addrp = addr;
+	  read_memory (addr, buf, register_size (current_gdbarch, IA64_IP_REGNUM));
 	  pc = extract_unsigned_integer (buf, 8);
 	}
-      else
+      else if (cache->frameless)
 	{
-	  CORE_ADDR addr = cache->saved_regs[IA64_VRAP_REGNUM];
-	  if (addr != 0)
-	    {
-	      read_memory (addr, buf, register_size (current_gdbarch, IA64_IP_REGNUM));
-	      pc = extract_unsigned_integer (buf, 8);
-	    }
+	  CORE_ADDR pc;
+	  frame_unwind_register (next_frame, IA64_BR0_REGNUM, buf);
+	  pc = extract_unsigned_integer (buf, 8);
 	}
       psr &= ~(3LL << 41);
       slot_num = pc & 0x3LL;
       psr |= (CORE_ADDR)slot_num << 41;
       store_unsigned_integer (valuep, 8, psr);
     }
+  else if (regnum == IA64_BR0_REGNUM)
+    {
+      CORE_ADDR br0 = 0;
+      CORE_ADDR addr = cache->saved_regs[IA64_BR0_REGNUM];
+      if (addr != 0)
+	{
+	  *lvalp = lval_memory;
+	  *addrp = addr;
+	  read_memory (addr, buf, register_size (current_gdbarch, IA64_BR0_REGNUM));
+	  br0 = extract_unsigned_integer (buf, 8);
+	}
+      store_unsigned_integer (valuep, 8, br0);
+    }
  else if ((regnum >= IA64_GR32_REGNUM && regnum <= IA64_GR127_REGNUM) ||
 	   (regnum >= V32_REGNUM && regnum <= V127_REGNUM))
     {
@@ -1839,6 +1881,12 @@ ia64_frame_prev_register (struct frame_i
       else 
 	frame_unwind_register (next_frame, regnum, valuep);
     }
+
+  if (gdbarch_debug >= 1)
+    fprintf_unfiltered (gdb_stdlog,
+			"regular prev register <%d> <%s> is %lx\n", regnum, 
+			(((unsigned) regnum <= IA64_NAT127_REGNUM)
+			 ? ia64_register_names[regnum] : "r??"), extract_unsigned_integer (valuep, 8));
 }
  
 static const struct frame_unwind ia64_frame_unwind =
@@ -1869,10 +1917,8 @@ ia64_sigtramp_frame_init_saved_regs (str
 	SIGCONTEXT_REGISTER_ADDRESS (cache->base, IA64_CFM_REGNUM);
       cache->saved_regs[IA64_PSR_REGNUM] = 
 	SIGCONTEXT_REGISTER_ADDRESS (cache->base, IA64_PSR_REGNUM);
-#if 0
       cache->saved_regs[IA64_BSP_REGNUM] = 
-	SIGCONTEXT_REGISTER_ADDRESS (frame->frame, IA64_BSP_REGNUM);
-#endif
+	SIGCONTEXT_REGISTER_ADDRESS (cache->base, IA64_BSP_REGNUM);
       cache->saved_regs[IA64_RNAT_REGNUM] = 
 	SIGCONTEXT_REGISTER_ADDRESS (cache->base, IA64_RNAT_REGNUM);
       cache->saved_regs[IA64_CCV_REGNUM] = 
@@ -1886,9 +1932,8 @@ ia64_sigtramp_frame_init_saved_regs (str
       cache->saved_regs[IA64_LC_REGNUM] = 
 	SIGCONTEXT_REGISTER_ADDRESS (cache->base, IA64_LC_REGNUM);
       for (regno = IA64_GR1_REGNUM; regno <= IA64_GR31_REGNUM; regno++)
-	if (regno != sp_regnum)
-	  cache->saved_regs[regno] =
-	    SIGCONTEXT_REGISTER_ADDRESS (cache->base, regno);
+	cache->saved_regs[regno] =
+	  SIGCONTEXT_REGISTER_ADDRESS (cache->base, regno);
       for (regno = IA64_BR0_REGNUM; regno <= IA64_BR7_REGNUM; regno++)
 	cache->saved_regs[regno] =
 	  SIGCONTEXT_REGISTER_ADDRESS (cache->base, regno);
@@ -1912,7 +1957,16 @@ ia64_sigtramp_frame_cache (struct frame_
   cache = ia64_alloc_frame_cache ();
 
   frame_unwind_register (next_frame, sp_regnum, buf);
-  cache->base = extract_unsigned_integer (buf, 8) + cache->mem_stack_frame_size;
+  /* Note that frame size is hard-coded below.  We cannot calculate it
+     via prologue examination.  */
+  cache->base = extract_unsigned_integer (buf, 8) + 16;
+
+  frame_unwind_register (next_frame, IA64_BSP_REGNUM, buf);
+  cache->bsp = extract_unsigned_integer (buf, 8);
+
+  frame_unwind_register (next_frame, IA64_CFM_REGNUM, buf);
+  cache->cfm = extract_unsigned_integer (buf, 8);
+  cache->sof = cache->cfm & 0x7f;
 
   ia64_sigtramp_frame_init_saved_regs (cache);
 
@@ -1927,7 +1981,11 @@ ia64_sigtramp_frame_this_id (struct fram
   struct ia64_frame_cache *cache =
     ia64_sigtramp_frame_cache (next_frame, this_cache);
 
-  (*this_id) = frame_id_build (cache->base, frame_pc_unwind (next_frame));
+  (*this_id) = frame_id_build_special (cache->base, frame_pc_unwind (next_frame), cache->bsp);
+  if (gdbarch_debug >= 1)
+    fprintf_unfiltered (gdb_stdlog,
+			"sigtramp frame id: code %lx, stack %lx, special %lx, next_frame %p\n",
+			this_id->code_addr, this_id->stack_addr, cache->bsp, next_frame);
 }
 
 static void
@@ -1937,11 +1995,75 @@ ia64_sigtramp_frame_prev_register (struc
 				   enum lval_type *lvalp, CORE_ADDR *addrp,
 				   int *realnump, void *valuep)
 {
-  /* Make sure we've initialized the cache.  */
-  ia64_sigtramp_frame_cache (next_frame, this_cache);
+  char dummy_valp[MAX_REGISTER_SIZE];
+  char buf[MAX_REGISTER_SIZE];
+
+  struct ia64_frame_cache *cache =
+    ia64_sigtramp_frame_cache (next_frame, this_cache);
+
+  gdb_assert (regnum >= 0);
+
+  if (!target_has_registers)
+    error ("No registers.");
+
+  *optimizedp = 0;
+  *addrp = 0;
+  *lvalp = not_lval;
+  *realnump = -1;
+
+  /* Rather than check each time if valuep is non-null, supply a dummy buffer
+     when valuep is not supplied.  */
+  if (!valuep)
+    valuep = dummy_valp;
+  
+  memset (valuep, 0, register_size (current_gdbarch, regnum));
+ 
+  if (regnum == IA64_IP_REGNUM)
+    {
+      CORE_ADDR pc = 0;
+      CORE_ADDR addr = cache->saved_regs[IA64_VRAP_REGNUM];
+
+      if (addr != 0)
+	{
+	  *lvalp = lval_memory;
+	  *addrp = addr;
+	  read_memory (addr, buf, register_size (current_gdbarch, IA64_IP_REGNUM));
+	  pc = extract_unsigned_integer (buf, 8);
+	}
+      pc &= ~0xf;
+      store_unsigned_integer (valuep, 8, pc);
+    }
+ else if ((regnum >= IA64_GR32_REGNUM && regnum <= IA64_GR127_REGNUM) ||
+	   (regnum >= V32_REGNUM && regnum <= V127_REGNUM))
+    {
+      CORE_ADDR addr = 0;
+      if (regnum >= V32_REGNUM)
+	regnum = IA64_GR32_REGNUM + (regnum - V32_REGNUM);
+      addr = cache->saved_regs[regnum];
+      if (addr != 0)
+	{
+	  *lvalp = lval_memory;
+	  *addrp = addr;
+	  read_memory (addr, valuep, register_size (current_gdbarch, regnum));
+	}
+    }
+  else
+    {
+      /* All other registers not listed above.  */
+      CORE_ADDR addr = cache->saved_regs[regnum];
+      if (addr != 0)
+	{
+	  *lvalp = lval_memory;
+	  *addrp = addr;
+	  read_memory (addr, valuep, register_size (current_gdbarch, regnum));
+	}
+    }
 
-  ia64_frame_prev_register (next_frame, this_cache, regnum,
-			    optimizedp, lvalp, addrp, realnump, valuep);
+  if (gdbarch_debug >= 1)
+    fprintf_unfiltered (gdb_stdlog,
+			"sigtramp prev register <%s> is %lx\n",
+			(((unsigned) regnum <= IA64_NAT127_REGNUM)
+			 ? ia64_register_names[regnum] : "r??"), extract_unsigned_integer (valuep, 8));
 }
 
 static const struct frame_unwind ia64_sigtramp_frame_unwind =
@@ -2473,12 +2595,20 @@ static struct frame_id
 ia64_unwind_dummy_id (struct gdbarch *gdbarch, struct frame_info *next_frame)
 {
   char buf[8];
-  CORE_ADDR sp;
+  CORE_ADDR sp, bsp;
 
   frame_unwind_register (next_frame, sp_regnum, buf);
   sp = extract_unsigned_integer (buf, 8);
 
-  return frame_id_build (sp, frame_pc_unwind (next_frame));
+  frame_unwind_register (next_frame, IA64_BSP_REGNUM, buf);
+  bsp = extract_unsigned_integer (buf, 8);
+
+  if (gdbarch_debug >= 1)
+    fprintf_unfiltered (gdb_stdlog,
+			"dummy frame id: code %lx, stack %lx, special %lx\n",
+			frame_pc_unwind (next_frame), sp, bsp);
+
+  return frame_id_build_special (sp, frame_pc_unwind (next_frame), bsp);
 }
 
 static CORE_ADDR 

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