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


Kevin Buettner wrote:
On Oct 17, 3:58pm, J. Johnston wrote:


The attached ia64 patch fixes a few problems, most notably
backtracing through signal handlers.


I think these changes are mostly okay, but...

In your ChangeLog entry, you say:


	* ia64-tdep.c: Change all references of DEPRECATED_REGISTER_RAW_SIZE
	to use register_size() instead.


Yet, later on, in the patch, I see that you're reintroducing a use of
DEPRECATED_REGISTER_RAW_SIZE:

  +  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, DEPRECATED_REGISTER_RAW_SIZE (IA64_BR0_REGNUM));
  +	  br0 = extract_unsigned_integer (buf, 8);
  +	}
  +      store_unsigned_integer (valuep, 8, br0);
  +    }

Also, regarding:

/* We want to calculate the previous bsp as the end of the previous register stack frame.
This corresponds to what the hardware bsp register will be if we pop the frame
back which is why we might have been called. We know the beginning of the current
- frame is cache->bsp - cache->sof. This value in the previous frame points to
+ frame is cache->bsp - cache->sof. This value in the previous frame points to
the start of the output registers. We can calculate the end of that frame by adding
the size of output (sof (size of frame) - sol (size of locals)). */
ia64_frame_prev_register (next_frame, this_cache, IA64_CFM_REGNUM,
&cfm_optim, &cfm_lval, &cfm_addr, &cfm_realnum, cfm_valuep);
prev_cfm = extract_unsigned_integer (cfm_valuep, 8);
-
+ bsp = rse_address_add (cache->bsp, -(cache->sof));
prev_bsp = rse_address_add (bsp, (prev_cfm & 0x7f) - ((prev_cfm >> 7) & 0x7f));
-
+


The first white space change is okay, but the latter two are not.  I'd
really prefer to see whitespace changes occur via a separate patch anyway.

So, if you don't mind, could you please submit separate patches for:

  1) whitespace changes
  2) DEPRECATED_... elimination
  3) the CFM / backtracing changes.

Patches (1) and (2) are preapproved -- just make sure that you don't
introduce extra white space on an otherwise blank line.  For (1) and
(2), please post the patches that you end up committing.

Once (1) and (2) are split out, I'd like another chance to review what's
left (patch 3).


Ok, done. A patch for 1) and 2) has been checked in. See the new attached patch.


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.


With regard to (3), one of the questions that I already have concerns
the following line in ia64_sigtramp_frame_init_saved_regs():

+ CORE_ADDR sp = cache->base + 16;

Could you explain what this is about?  (Preferably with a comment in the
code?)


I've simplified it. It is the mem stack size that should have been added to the base. I have added a comment that it cannot be calculated via prologue examination.


Thanks,

Kevin

P.S. If you'd prefer to reverse things and submit patch 3 first, that'd
be okay too.

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	20 Oct 2003 21:40:48 -0000
@@ -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,20 +1754,19 @@ 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);
@@ -1750,30 +1776,42 @@ ia64_frame_prev_register (struct frame_i
       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 +1877,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 +1913,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 +1928,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 +1953,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 +1977,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 +1991,107 @@ 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);
 
-  ia64_frame_prev_register (next_frame, this_cache, regnum,
-			    optimizedp, lvalp, addrp, realnump, valuep);
+  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_PSR_REGNUM)
+    {
+      ULONGEST slot_num = 0;
+      CORE_ADDR pc= 0;
+      CORE_ADDR psr = 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);
+	}
+      psr &= ~(3LL << 41);
+      slot_num = pc & 0x3LL;
+      psr |= (CORE_ADDR)slot_num << 41;
+      store_unsigned_integer (valuep, 8, psr);
+    }
+ 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
+    {
+      CORE_ADDR addr = 0;
+      if (IA64_FR32_REGNUM <= regnum && regnum <= IA64_FR127_REGNUM)
+	{
+	  /* Fetch floating point register rename base from current
+	     frame marker for this frame.  */
+	  int rrb_fr = (cache->cfm >> 25) & 0x7f;
+
+	  /* Adjust the floating point register number to account for
+	     register rotation.  */
+	  regnum = IA64_FR32_REGNUM
+	         + ((regnum - IA64_FR32_REGNUM) + rrb_fr) % 96;
+	}
+
+      /* If we have stored a memory address, access the register.  */
+      addr = cache->saved_regs[regnum];
+      if (addr != 0)
+	{
+	  *lvalp = lval_memory;
+	  *addrp = addr;
+	  read_memory (addr, valuep, register_size (current_gdbarch, regnum));
+	}
+    }
+
+  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 +2623,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]