[patch/CRIS] Prologue scan bug fixes, return_value implementation

[patch/CRIS] Prologue scan bug fixes, return_value implementation

[Orjan Friberg]

[-- Attachment #1: Type: text/plain, Size: 879 bytes --]

The patch below improves the CRIS prologue scanner by, at least partially, 
handling the case when the PC is still in the prologue, which takes care of 14 
FAILs in recurse.exp and 2 FAILs in step-test.exp.  I've also added a 
return_value implementation which gets rid of 32 FAILs and 30 KFAILs in 
structs.exp.  (All tests in those test cases now PASS.)

Ok to commit to trunk and 6.1 branch?

2004-03-03  Orjan Friberg  <orjanf@axis.com>

	* cris-tdep.c (cris_scan_prologue): Save the frame pointer's offset
	when the frame pointer is pushed.  Don't set the frame pointer's
	address on the stack unless it's actually located there.
	Set the SRP's address on the stack correctly when the PC is still in
	the prologue.
	(cris_return_value): New function.
	(cris_gdbarch_init): Clear deprecated store_return_value,
	extract_return_value.


-- 
Orjan Friberg
Axis Communications


[-- Attachment #2: patch --]
[-- Type: text/plain, Size: 3884 bytes --]

Index: cris-tdep.c
===================================================================
RCS file: /cvs/src/src/gdb/cris-tdep.c,v
retrieving revision 1.102
diff -u -r1.102 cris-tdep.c
--- cris-tdep.c	27 Feb 2004 12:39:26 -0000	1.102
+++ cris-tdep.c	3 Mar 2004 16:38:16 -0000
@@ -792,6 +792,14 @@
 		  info->leaf_function = 0;
 		}
             }
+	  else if (insn_next == 0x8FEE)
+            {
+	      /* push $r8 */
+	      if (info)
+		{
+		  info->r8_offset = info->sp_offset;
+		}
+            }
         }
       else if (insn == 0x866E)
         {
@@ -799,7 +807,6 @@
 	  if (info)
 	    {
 	      info->uses_frame = 1;
-	      info->r8_offset = info->sp_offset;
 	    }
           continue;
         }
@@ -947,6 +954,8 @@
       frame_unwind_unsigned_register (next_frame, CRIS_FP_REGNUM, 
 				      &this_base);
       info->base = this_base;
+      info->saved_regs[CRIS_FP_REGNUM].addr = info->base;
+  
       /* The FP points at the last saved register.  Adjust the FP back
          to before the first saved register giving the SP.  */
       info->prev_sp = info->base + info->r8_offset;
@@ -961,8 +970,6 @@
       info->prev_sp = info->base + info->size;
     }
       
-  info->saved_regs[CRIS_FP_REGNUM].addr = info->base;
-
   /* Calculate the addresses for the saved registers on the stack.  */
   /* FIXME: The address calculation should really be done on the fly while
      we're analyzing the prologue (we only hold one regsave value as it is 
@@ -981,8 +988,17 @@
 
   if (!info->leaf_function)
     {
-      /* SRP saved on the stack.  */
-      info->saved_regs[SRP_REGNUM].addr = info->base + 4;
+      /* SRP saved on the stack.  But where?  */
+      if (info->r8_offset == 0)
+	{
+	  /* R8 not pushed yet.  */
+	  info->saved_regs[SRP_REGNUM].addr = info->base;
+	}
+      else
+	{
+	  /* R8 pushed, but SP may or may not be moved to R8 yet.  */
+	  info->saved_regs[SRP_REGNUM].addr = info->base + 4;
+	}
     }
 
   /* The PC is found in SRP (the actual register or located on the stack).  */
@@ -1340,6 +1356,28 @@
     error ("cris_extract_return_value: type length too large");
 }
 
+/* Handle the CRIS return value convention.  */
+
+static enum return_value_convention
+cris_return_value (struct gdbarch *gdbarch, struct type *type,
+		   struct regcache *regcache, void *readbuf,
+		   const void *writebuf)
+{
+  if (TYPE_CODE (type) == TYPE_CODE_STRUCT ||
+      TYPE_CODE (type) == TYPE_CODE_UNION ||
+      TYPE_LENGTH (type) > 8)
+    /* Structs, unions, and anything larger than 8 bytes (2 registers)
+       goes on the stack.  */
+    return RETURN_VALUE_STRUCT_CONVENTION;
+
+  if (readbuf)
+    cris_extract_return_value (type, regcache, readbuf);
+  if (writebuf)
+    cris_store_return_value (type, regcache, writebuf);
+
+  return RETURN_VALUE_REGISTER_CONVENTION;
+}
+
 /* Returns 1 if the given type will be passed by pointer rather than 
    directly.  */
 
@@ -3792,9 +3830,7 @@
       internal_error (__FILE__, __LINE__, "cris_gdbarch_init: unknown byte order in info");
     }
 
-  /* FIXME: Should be replaced by a cris_return_value implementation.  */
-  set_gdbarch_store_return_value (gdbarch, cris_store_return_value);
-  set_gdbarch_extract_return_value (gdbarch, cris_extract_return_value);
+  set_gdbarch_return_value (gdbarch, cris_return_value);
   set_gdbarch_deprecated_reg_struct_has_addr (gdbarch, 
 					      cris_reg_struct_has_addr);
   set_gdbarch_use_struct_convention (gdbarch, always_use_struct_convention);
@@ -3883,8 +3919,6 @@
 
   set_gdbarch_breakpoint_from_pc (gdbarch, cris_breakpoint_from_pc);
   
-  /* Prologue analyzer may have to be able to parse an incomplete
-     prologue (PC in prologue, that is).  Check infrun.c.  */
   set_gdbarch_unwind_pc (gdbarch, cris_unwind_pc);
   set_gdbarch_unwind_sp (gdbarch, cris_unwind_sp);
   set_gdbarch_unwind_dummy_id (gdbarch, cris_unwind_dummy_id);

Re: [patch/CRIS] Prologue scan bug fixes, return_value implementation

[Daniel Jacobowitz]

On Wed, Mar 03, 2004 at 05:40:09PM +0100, Orjan Friberg wrote:
> The patch below improves the CRIS prologue scanner by, at least partially, 
> handling the case when the PC is still in the prologue, which takes care of 
> 14 FAILs in recurse.exp and 2 FAILs in step-test.exp.  I've also added a 
> return_value implementation which gets rid of 32 FAILs and 30 KFAILs in 
> structs.exp.  (All tests in those test cases now PASS.)
> 
> Ok to commit to trunk and 6.1 branch?

As Andrew mentioned, you're the listed maintainer of the CRIS port -
you don't need approval for changes to CRIS-specific files.  I think
that at this point it's appropriate for the branch also.

> +/* Handle the CRIS return value convention.  */
> +
> +static enum return_value_convention
> +cris_return_value (struct gdbarch *gdbarch, struct type *type,
> +		   struct regcache *regcache, void *readbuf,
> +		   const void *writebuf)
> +{
> +  if (TYPE_CODE (type) == TYPE_CODE_STRUCT ||
> +      TYPE_CODE (type) == TYPE_CODE_UNION ||
> +      TYPE_LENGTH (type) > 8)

Style nit: || at the beginning of the line, please.


-- 
Daniel Jacobowitz
MontaVista Software                         Debian GNU/Linux Developer

Re: [patch/CRIS] Prologue scan bug fixes, return_value implementation

[Orjan Friberg]

Daniel Jacobowitz wrote:
> 
> As Andrew mentioned, you're the listed maintainer of the CRIS port -
> you don't need approval for changes to CRIS-specific files.  I think
> that at this point it's appropriate for the branch also.

Ah, ok.  I'm being overly cautious.  Thanks for the clarification.

>>+  if (TYPE_CODE (type) == TYPE_CODE_STRUCT ||
>>+      TYPE_CODE (type) == TYPE_CODE_UNION ||
>>+      TYPE_LENGTH (type) > 8)
> 
> 
> Style nit: || at the beginning of the line, please.

Thanks for noticing; will fix before commit.

-- 
Orjan Friberg
Axis Communications

Re: [patch/CRIS] Prologue scan bug fixes, return_value implementation

[Andrew Cagney]

> Daniel Jacobowitz wrote:
> 
>>
>> As Andrew mentioned, you're the listed maintainer of the CRIS port -
>> you don't need approval for changes to CRIS-specific files.  I think
>> that at this point it's appropriate for the branch also.
> 
> 
> Ah, ok.  I'm being overly cautious.  Thanks for the clarification.

Yes.  Since the absolute worst possible outcome for such a commit is you 
breaking the CRIS architecture, it is entirely up to you to decide if 
its worth the risk :-)

Andrew

Re: [patch/CRIS] Prologue scan bug fixes, return_value implementation

[Orjan Friberg]

Andrew Cagney wrote:
> 
> Yes.  Since the absolute worst possible outcome for such a commit is you 
> breaking the CRIS architecture, it is entirely up to you to decide if 
> its worth the risk :-)

78 new PASS in gdb.base makes it worth the risk ;-).  (Committed to trunk and 
branch.)

-- 
Orjan Friberg
Axis Communications

[patch/CRIS] Prologue scan bug fixes, return_value implementation

[Orjan Friberg]

[-- Attachment #1: Type: text/plain, Size: 879 bytes --]

The patch below improves the CRIS prologue scanner by, at least partially, 
handling the case when the PC is still in the prologue, which takes care of 14 
FAILs in recurse.exp and 2 FAILs in step-test.exp.  I've also added a 
return_value implementation which gets rid of 32 FAILs and 30 KFAILs in 
structs.exp.  (All tests in those test cases now PASS.)

Ok to commit to trunk and 6.1 branch?

2004-03-03  Orjan Friberg  <orjanf@axis.com>

	* cris-tdep.c (cris_scan_prologue): Save the frame pointer's offset
	when the frame pointer is pushed.  Don't set the frame pointer's
	address on the stack unless it's actually located there.
	Set the SRP's address on the stack correctly when the PC is still in
	the prologue.
	(cris_return_value): New function.
	(cris_gdbarch_init): Clear deprecated store_return_value,
	extract_return_value.


-- 
Orjan Friberg
Axis Communications


[-- Attachment #2: patch --]
[-- Type: text/plain, Size: 3884 bytes --]

Index: cris-tdep.c
===================================================================
RCS file: /cvs/src/src/gdb/cris-tdep.c,v
retrieving revision 1.102
diff -u -r1.102 cris-tdep.c
--- cris-tdep.c	27 Feb 2004 12:39:26 -0000	1.102
+++ cris-tdep.c	3 Mar 2004 16:38:16 -0000
@@ -792,6 +792,14 @@
 		  info->leaf_function = 0;
 		}
             }
+	  else if (insn_next == 0x8FEE)
+            {
+	      /* push $r8 */
+	      if (info)
+		{
+		  info->r8_offset = info->sp_offset;
+		}
+            }
         }
       else if (insn == 0x866E)
         {
@@ -799,7 +807,6 @@
 	  if (info)
 	    {
 	      info->uses_frame = 1;
-	      info->r8_offset = info->sp_offset;
 	    }
           continue;
         }
@@ -947,6 +954,8 @@
       frame_unwind_unsigned_register (next_frame, CRIS_FP_REGNUM, 
 				      &this_base);
       info->base = this_base;
+      info->saved_regs[CRIS_FP_REGNUM].addr = info->base;
+  
       /* The FP points at the last saved register.  Adjust the FP back
          to before the first saved register giving the SP.  */
       info->prev_sp = info->base + info->r8_offset;
@@ -961,8 +970,6 @@
       info->prev_sp = info->base + info->size;
     }
       
-  info->saved_regs[CRIS_FP_REGNUM].addr = info->base;
-
   /* Calculate the addresses for the saved registers on the stack.  */
   /* FIXME: The address calculation should really be done on the fly while
      we're analyzing the prologue (we only hold one regsave value as it is 
@@ -981,8 +988,17 @@
 
   if (!info->leaf_function)
     {
-      /* SRP saved on the stack.  */
-      info->saved_regs[SRP_REGNUM].addr = info->base + 4;
+      /* SRP saved on the stack.  But where?  */
+      if (info->r8_offset == 0)
+	{
+	  /* R8 not pushed yet.  */
+	  info->saved_regs[SRP_REGNUM].addr = info->base;
+	}
+      else
+	{
+	  /* R8 pushed, but SP may or may not be moved to R8 yet.  */
+	  info->saved_regs[SRP_REGNUM].addr = info->base + 4;
+	}
     }
 
   /* The PC is found in SRP (the actual register or located on the stack).  */
@@ -1340,6 +1356,28 @@
     error ("cris_extract_return_value: type length too large");
 }
 
+/* Handle the CRIS return value convention.  */
+
+static enum return_value_convention
+cris_return_value (struct gdbarch *gdbarch, struct type *type,
+		   struct regcache *regcache, void *readbuf,
+		   const void *writebuf)
+{
+  if (TYPE_CODE (type) == TYPE_CODE_STRUCT ||
+      TYPE_CODE (type) == TYPE_CODE_UNION ||
+      TYPE_LENGTH (type) > 8)
+    /* Structs, unions, and anything larger than 8 bytes (2 registers)
+       goes on the stack.  */
+    return RETURN_VALUE_STRUCT_CONVENTION;
+
+  if (readbuf)
+    cris_extract_return_value (type, regcache, readbuf);
+  if (writebuf)
+    cris_store_return_value (type, regcache, writebuf);
+
+  return RETURN_VALUE_REGISTER_CONVENTION;
+}
+
 /* Returns 1 if the given type will be passed by pointer rather than 
    directly.  */
 
@@ -3792,9 +3830,7 @@
       internal_error (__FILE__, __LINE__, "cris_gdbarch_init: unknown byte order in info");
     }
 
-  /* FIXME: Should be replaced by a cris_return_value implementation.  */
-  set_gdbarch_store_return_value (gdbarch, cris_store_return_value);
-  set_gdbarch_extract_return_value (gdbarch, cris_extract_return_value);
+  set_gdbarch_return_value (gdbarch, cris_return_value);
   set_gdbarch_deprecated_reg_struct_has_addr (gdbarch, 
 					      cris_reg_struct_has_addr);
   set_gdbarch_use_struct_convention (gdbarch, always_use_struct_convention);
@@ -3883,8 +3919,6 @@
 
   set_gdbarch_breakpoint_from_pc (gdbarch, cris_breakpoint_from_pc);
   
-  /* Prologue analyzer may have to be able to parse an incomplete
-     prologue (PC in prologue, that is).  Check infrun.c.  */
   set_gdbarch_unwind_pc (gdbarch, cris_unwind_pc);
   set_gdbarch_unwind_sp (gdbarch, cris_unwind_sp);
   set_gdbarch_unwind_dummy_id (gdbarch, cris_unwind_dummy_id);

Re: [patch/CRIS] Prologue scan bug fixes, return_value implementation

[Orjan Friberg]

Daniel Jacobowitz wrote:
> 
> As Andrew mentioned, you're the listed maintainer of the CRIS port -
> you don't need approval for changes to CRIS-specific files.  I think
> that at this point it's appropriate for the branch also.

Ah, ok.  I'm being overly cautious.  Thanks for the clarification.

>>+  if (TYPE_CODE (type) == TYPE_CODE_STRUCT ||
>>+      TYPE_CODE (type) == TYPE_CODE_UNION ||
>>+      TYPE_LENGTH (type) > 8)
> 
> 
> Style nit: || at the beginning of the line, please.

Thanks for noticing; will fix before commit.

-- 
Orjan Friberg
Axis Communications

Re: [patch/CRIS] Prologue scan bug fixes, return_value implementation

[Andrew Cagney]

> Daniel Jacobowitz wrote:
> 
>>
>> As Andrew mentioned, you're the listed maintainer of the CRIS port -
>> you don't need approval for changes to CRIS-specific files.  I think
>> that at this point it's appropriate for the branch also.
> 
> 
> Ah, ok.  I'm being overly cautious.  Thanks for the clarification.

Yes.  Since the absolute worst possible outcome for such a commit is you 
breaking the CRIS architecture, it is entirely up to you to decide if 
its worth the risk :-)

Andrew

Re: [patch/CRIS] Prologue scan bug fixes, return_value implementation

[Orjan Friberg]

Andrew Cagney wrote:
> 
> Yes.  Since the absolute worst possible outcome for such a commit is you 
> breaking the CRIS architecture, it is entirely up to you to decide if 
> its worth the risk :-)

78 new PASS in gdb.base makes it worth the risk ;-).  (Committed to trunk and 
branch.)

-- 
Orjan Friberg
Axis Communications

Re: [patch/CRIS] Prologue scan bug fixes, return_value implementation

[Daniel Jacobowitz]

On Wed, Mar 03, 2004 at 05:40:09PM +0100, Orjan Friberg wrote:
> The patch below improves the CRIS prologue scanner by, at least partially, 
> handling the case when the PC is still in the prologue, which takes care of 
> 14 FAILs in recurse.exp and 2 FAILs in step-test.exp.  I've also added a 
> return_value implementation which gets rid of 32 FAILs and 30 KFAILs in 
> structs.exp.  (All tests in those test cases now PASS.)
> 
> Ok to commit to trunk and 6.1 branch?

As Andrew mentioned, you're the listed maintainer of the CRIS port -
you don't need approval for changes to CRIS-specific files.  I think
that at this point it's appropriate for the branch also.

> +/* Handle the CRIS return value convention.  */
> +
> +static enum return_value_convention
> +cris_return_value (struct gdbarch *gdbarch, struct type *type,
> +		   struct regcache *regcache, void *readbuf,
> +		   const void *writebuf)
> +{
> +  if (TYPE_CODE (type) == TYPE_CODE_STRUCT ||
> +      TYPE_CODE (type) == TYPE_CODE_UNION ||
> +      TYPE_LENGTH (type) > 8)

Style nit: || at the beginning of the line, please.


-- 
Daniel Jacobowitz
MontaVista Software                         Debian GNU/Linux Developer