ARM PATCH fix extract_return_value and store_return_value

ARM PATCH fix extract_return_value and store_return_value

[Richard Earnshaw]

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


This patch cleans up the way the ARM tdep code handles extracting and 
storing return values.  Not only does it convert the methods to the new 
regcache abstraction but it also fixes (at least I hope it fixes) the 
big-endian problems that Michael was reporting on a big-endian ARM.

Michael, I've only run this through a little-endian testsuite.  Could you 
let me know if it solves the problems you were reporting.

R.

2002-12-14  Richard Earnshaw  <rearnsha@arm.com>

	* arm-tdep.c (convert_from_extended): New argument to hold the
	type of floating point result we want to convert to.  Make input 
	argument const.  Fix all callers.
	(convert_to_extended): Similarly.
	(arm_extract_return_value): Now takes a regcache argument.  Change
	code to use regcache accessor functions.  Correctly extract 
	smaller-than-word results on big-endian machines.
	(arm_store_return_value): Now takes a regcache argument.  Change
	code to use regcache accessor functions.  Correctly zero/sign extend
	smaller than word results before storing into r0.
	(arm_gdbarch_init): Register new-style extract_return_value and
	store_return_value functions.



[-- Attachment #2: gdb-retval.patch --]
[-- Type: application/x-patch , Size: 10781 bytes --]

Re: ARM PATCH fix extract_return_value and store_return_value

[Michael Snyder]

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

Richard Earnshaw wrote:
> 
> This patch cleans up the way the ARM tdep code handles extracting and
> storing return values.  Not only does it convert the methods to the new
> regcache abstraction but it also fixes (at least I hope it fixes) the
> big-endian problems that Michael was reporting on a big-endian ARM.
> 
> Michael, I've only run this through a little-endian testsuite.  Could you
> let me know if it solves the problems you were reporting.
> 
> R.
> 
> 2002-12-14  Richard Earnshaw  <rearnsha@arm.com>
> 
>         * arm-tdep.c (convert_from_extended): New argument to hold the
>         type of floating point result we want to convert to.  Make input
>         argument const.  Fix all callers.
>         (convert_to_extended): Similarly.
>         (arm_extract_return_value): Now takes a regcache argument.  Change
>         code to use regcache accessor functions.  Correctly extract
>         smaller-than-word results on big-endian machines.
>         (arm_store_return_value): Now takes a regcache argument.  Change
>         code to use regcache accessor functions.  Correctly zero/sign extend
>         smaller than word results before storing into r0.
>         (arm_gdbarch_init): Register new-style extract_return_value and
>         store_return_value functions.

Hi Richard, 

I do appologize for the delay -- I haven't had occasion to build an
arm simulator until now.  Now that I have, I can report that these
changes do fix two fails for big-endian running callfuncs.exp.
One of the fails was returning a one-byte struct, the other
a two-byte struct.   There were no other fails in callfuncs.exp.

Alas, I do not remember what problems I was reporting earlier.
I'll try to look them up in the morning.  Meanwhile I wanted to
share the positive results, and to say I see no reason not to 
commit your change.  As is, they conflict with some of Elena's
vector changes, but I've massaged them into closer conformance
with a more recent revision.  Here's my merged patch (not entirely
up to date, but more recent than what appears here).

[-- Attachment #2: local.patch --]
[-- Type: text/plain, Size: 11908 bytes --]

Index: arm-tdep.c
===================================================================
RCS file: /cvs/cvsfiles/devo/gdb/arm-tdep.c,v
retrieving revision 1.125.2.1
diff -p -r1.125.2.1 arm-tdep.c
*** arm-tdep.c	2002/12/19 03:08:11	1.125.2.1
--- arm-tdep.c	2003/01/24 02:06:03
*************** static void set_disassembly_flavor_sfunc
*** 151,157 ****
  					 struct cmd_list_element *);
  static void set_disassembly_flavor (void);
  
! static void convert_from_extended (void *ptr, void *dbl);
  
  /* Define other aspects of the stack frame.  We keep the offsets of
     all saved registers, 'cause we need 'em a lot!  We also keep the
--- 151,160 ----
  					 struct cmd_list_element *);
  static void set_disassembly_flavor (void);
  
! static void convert_from_extended (const struct floatformat *, const void *,
! 				   void *);
! static void convert_to_extended (const struct floatformat *, void *,
! 				 const void *);
  
  /* Define other aspects of the stack frame.  We keep the offsets of
     all saved registers, 'cause we need 'em a lot!  We also keep the
*************** arm_register_sim_regno (int regnum)
*** 2112,2118 ****
     little-endian systems.  */
  
  static void
! convert_from_extended (void *ptr, void *dbl)
  {
    DOUBLEST d;
    if (TARGET_BYTE_ORDER == BFD_ENDIAN_BIG)
--- 2115,2122 ----
     little-endian systems.  */
  
  static void
! convert_from_extended (const struct floatformat *fmt, const void *ptr,
! 		       void *dbl)
  {
    DOUBLEST d;
    if (TARGET_BYTE_ORDER == BFD_ENDIAN_BIG)
*************** convert_from_extended (void *ptr, void *
*** 2120,2133 ****
    else
      floatformat_to_doublest (&floatformat_arm_ext_littlebyte_bigword,
  			     ptr, &d);
!   floatformat_from_doublest (TARGET_DOUBLE_FORMAT, &d, dbl);
  }
  
  static void
! convert_to_extended (void *dbl, void *ptr)
  {
    DOUBLEST d;
!   floatformat_to_doublest (TARGET_DOUBLE_FORMAT, ptr, &d);
    if (TARGET_BYTE_ORDER == BFD_ENDIAN_BIG)
      floatformat_from_doublest (&floatformat_arm_ext_big, &d, dbl);
    else
--- 2124,2137 ----
    else
      floatformat_to_doublest (&floatformat_arm_ext_littlebyte_bigword,
  			     ptr, &d);
!   floatformat_from_doublest (fmt, &d, dbl);
  }
  
  static void
! convert_to_extended (const struct floatformat *fmt, void *dbl, const void *ptr)
  {
    DOUBLEST d;
!   floatformat_to_doublest (fmt, ptr, &d);
    if (TARGET_BYTE_ORDER == BFD_ENDIAN_BIG)
      floatformat_from_doublest (&floatformat_arm_ext_big, &d, dbl);
    else
*************** arm_breakpoint_from_pc (CORE_ADDR *pcptr
*** 2665,2689 ****
  
  static void
  arm_extract_return_value (struct type *type,
! 			  char regbuf[REGISTER_BYTES],
! 			  char *valbuf)
  {
    struct gdbarch_tdep *tdep = gdbarch_tdep (current_gdbarch);
    if (TYPE_CODE_FLT == TYPE_CODE (type))
      {
-       struct gdbarch_tdep *tdep = gdbarch_tdep (current_gdbarch);
- 
        switch (tdep->fp_model)
  	{
  	case ARM_FLOAT_FPA:
! 	  convert_from_extended (&regbuf[REGISTER_BYTE (ARM_F0_REGNUM)],
! 				 valbuf);
  	  break;
  
  	case ARM_FLOAT_SOFT:
  	case ARM_FLOAT_SOFT_VFP:
! 	  memcpy (valbuf, &regbuf[REGISTER_BYTE (ARM_A1_REGNUM)],
! 		  TYPE_LENGTH (type));
  	  break;
  
  	default:
--- 2669,2707 ----
  
  static void
  arm_extract_return_value (struct type *type,
! 			  struct regcache *regs,
! 			  void *dst)
  {
    struct gdbarch_tdep *tdep = gdbarch_tdep (current_gdbarch);
+   bfd_byte *tmpbuf;
+   bfd_byte *valbuf = dst;
+   ULONGEST tmp;
+   int regno;
+   int len;
+ 
+   tmpbuf = alloca (MAX_REGISTER_RAW_SIZE);
    if (TYPE_CODE_FLT == TYPE_CODE (type))
      {
        switch (tdep->fp_model)
  	{
  	case ARM_FLOAT_FPA:
! 	  {
! 	    /* The value is in register F0 in internal format.  We need to
! 	       extract the raw value and then convert it to the desired
! 	       internal type.  */
! 
! 	    regcache_cooked_read (regs, ARM_F0_REGNUM, tmpbuf);
! 	    convert_from_extended (floatformat_from_type (type), tmpbuf,
! 				   valbuf);
! 	  }
  	  break;
  
  	case ARM_FLOAT_SOFT:
  	case ARM_FLOAT_SOFT_VFP:
! 	  regcache_cooked_read (regs, ARM_A1_REGNUM, valbuf);
! 	  if (TYPE_LENGTH (type) > 4)
! 	    regcache_cooked_read (regs, ARM_A1_REGNUM + 1,
! 				  valbuf + INT_REGISTER_RAW_SIZE);
  	  break;
  
  	default:
*************** arm_extract_return_value (struct type *t
*** 2697,2708 ****
        && TYPE_VECTOR (type)
        && TYPE_LENGTH (type) == COP0_REGISTER_SIZE)
      {
!       memcpy (valbuf, &regbuf[REGISTER_BYTE (COP0REGNUM)],
! 	      TYPE_LENGTH (type));
      }
    else
!     memcpy (valbuf, &regbuf[REGISTER_BYTE (ARM_A1_REGNUM)],
! 	    TYPE_LENGTH (type));
  }
  
  /* Extract from an array REGBUF containing the (raw) register state
--- 2715,2778 ----
        && TYPE_VECTOR (type)
        && TYPE_LENGTH (type) == COP0_REGISTER_SIZE)
      {
!       len = TYPE_LENGTH (type);
!       regno = COP0REGNUM;
! 
!       while (len > 0)
! 	{
! 	  /* See comments below for TYPE_CODE_INT.  */
! 	  regcache_cooked_read_unsigned (regs, regno++, &tmp);
! 	  store_unsigned_integer (valbuf, 
! 				  (len > COP0_REGISTER_SIZE
! 				   ? COP0REGISTER_SIZE : len),
! 				  tmp);
! 	  len -= COP0_REGISTER_SIZE;
! 	  valbuf += COP0_REGISTER_SIZE;
! 	}
!     }
!   else if (TYPE_CODE (type) == TYPE_CODE_INT
! 	   || TYPE_CODE (type) == TYPE_CODE_CHAR
! 	   || TYPE_CODE (type) == TYPE_CODE_BOOL
! 	   || TYPE_CODE (type) == TYPE_CODE_PTR
! 	   || TYPE_CODE (type) == TYPE_CODE_REF
! 	   || TYPE_CODE (type) == TYPE_CODE_ENUM)
!     {
!       /* If the the type is a plain integer, then the access is
! 	 straight-forward.  Otherwise we have to play around a bit more.  */
!       len = TYPE_LENGTH (type);
!       regno = ARM_A1_REGNUM;
! 
!       while (len > 0)
! 	{
! 	  /* By using store_unsigned_integer we avoid having to do
! 	     anything special for small big-endian values.  */
! 	  regcache_cooked_read_unsigned (regs, regno++, &tmp);
! 	  store_unsigned_integer (valbuf, 
! 				  (len > INT_REGISTER_RAW_SIZE
! 				   ? INT_REGISTER_RAW_SIZE : len),
! 				  tmp);
! 	  len -= INT_REGISTER_RAW_SIZE;
! 	  valbuf += INT_REGISTER_RAW_SIZE;
! 	}
      }
    else
!     {
!       /* For a structure or union the behaviour is as if the value had
!          been stored to word-aligned memory and then loaded into 
!          registers with 32-bit load instruction(s).  */
! 
!       len = TYPE_LENGTH (type);
!       regno = ARM_A1_REGNUM;
! 
!       while (len > 0)
! 	{
! 	  regcache_cooked_read (regs, regno++, tmpbuf);
! 	  memcpy (valbuf, tmpbuf,
! 		  len > INT_REGISTER_RAW_SIZE ? INT_REGISTER_RAW_SIZE : len);
! 	  len -= INT_REGISTER_RAW_SIZE;
! 	  valbuf += INT_REGISTER_RAW_SIZE;
! 	}
!     }
  }
  
  /* Extract from an array REGBUF containing the (raw) register state
*************** arm_use_struct_convention (int gcc_p, st
*** 2815,2841 ****
     TYPE, given in virtual format.  */
  
  static void
! arm_store_return_value (struct type *type, char *valbuf)
  {
    struct gdbarch_tdep *tdep = gdbarch_tdep (current_gdbarch);
    if (TYPE_CODE (type) == TYPE_CODE_FLT)
      {
-       struct gdbarch_tdep *tdep = gdbarch_tdep (current_gdbarch);
-       char buf[ARM_MAX_REGISTER_RAW_SIZE];
- 
        switch (tdep->fp_model)
  	{
  	case ARM_FLOAT_FPA:
  
! 	  convert_to_extended (valbuf, buf);
! 	  deprecated_write_register_bytes (REGISTER_BYTE (ARM_F0_REGNUM), buf,
! 					   FP_REGISTER_RAW_SIZE);
  	  break;
  
  	case ARM_FLOAT_SOFT:
  	case ARM_FLOAT_SOFT_VFP:
! 	  deprecated_write_register_bytes (ARM_A1_REGNUM, valbuf,
! 					   TYPE_LENGTH (type));
  	  break;
  
  	default:
--- 2885,2917 ----
     TYPE, given in virtual format.  */
  
  static void
! arm_store_return_value (struct type *type, 
! 			struct regcache *regs,
! 			const void *src)
  {
    struct gdbarch_tdep *tdep = gdbarch_tdep (current_gdbarch);
+   const bfd_byte *valbuf = src;
+   char *buf;
+   int regno;
+   int len;
+ 
+   buf = alloca (MAX_REGISTER_RAW_SIZE);
    if (TYPE_CODE (type) == TYPE_CODE_FLT)
      {
        switch (tdep->fp_model)
  	{
  	case ARM_FLOAT_FPA:
  
! 	  convert_to_extended (floatformat_from_type (type), buf, valbuf);
! 	  regcache_cooked_write (regs, ARM_F0_REGNUM, buf);
  	  break;
  
  	case ARM_FLOAT_SOFT:
  	case ARM_FLOAT_SOFT_VFP:
! 	  regcache_cooked_write (regs, ARM_A1_REGNUM, valbuf);
! 	  if (TYPE_LENGTH (type) > 4)
! 	    regcache_cooked_write (regs, ARM_A1_REGNUM + 1, 
! 				   valbuf + INT_REGISTER_RAW_SIZE);
  	  break;
  
  	default:
*************** arm_store_return_value (struct type *typ
*** 2848,2859 ****
    else if (tdep->abi_uses_simd_p
  	   && TYPE_VECTOR (type)
  	   && TYPE_LENGTH (type) == COP0_REGISTER_SIZE)
      {
!       deprecated_write_register_gen (COP0REGNUM, valbuf);
      }
    else
!     deprecated_write_register_bytes (ARM_A1_REGNUM, valbuf,
! 				     TYPE_LENGTH (type));
  }
  
  /* Store the address of the place in which to copy the structure the
--- 2924,2989 ----
    else if (tdep->abi_uses_simd_p
  	   && TYPE_VECTOR (type)
  	   && TYPE_LENGTH (type) == COP0_REGISTER_SIZE)
+     {
+       len = TYPE_LENGTH (type);
+       regno = COP0REGNUM;
+ 
+       while (len > 0)
+ 	{
+ 	  regcache_cooked_write (regs, regno++, valbuf);
+ 	  len -= COP0_REGISTER_SIZE;
+ 	  valbuf += COP0_REGISTER_SIZE;
+ 	}
+     }
+   else if (TYPE_CODE (type) == TYPE_CODE_INT
+ 	   || TYPE_CODE (type) == TYPE_CODE_CHAR
+ 	   || TYPE_CODE (type) == TYPE_CODE_BOOL
+ 	   || TYPE_CODE (type) == TYPE_CODE_PTR
+ 	   || TYPE_CODE (type) == TYPE_CODE_REF
+ 	   || TYPE_CODE (type) == TYPE_CODE_ENUM)
      {
!       if (TYPE_LENGTH (type) <= 4)
! 	{
! 	  /* Values of one word or less are zero/sign-extended and
! 	     returned in r0.  */
! 	  LONGEST val = unpack_long (type, valbuf);
! 
! 	  store_signed_integer (buf, INT_REGISTER_RAW_SIZE, val);
! 	  regcache_cooked_write (regs, ARM_A1_REGNUM, buf);
! 	}
!       else
! 	{
! 	  /* Integral values greater than one word are stored in consecutive
! 	     registers starting with r0.  This will always be a multiple of
! 	     the regiser size.  */
! 	  len = TYPE_LENGTH (type);
! 	  regno = ARM_A1_REGNUM;
! 
! 	  while (len > 0)
! 	    {
! 	      regcache_cooked_write (regs, regno++, valbuf);
! 	      len -= INT_REGISTER_RAW_SIZE;
! 	      valbuf += INT_REGISTER_RAW_SIZE;
! 	    }
! 	}
      }
    else
!     {
!       /* For a structure or union the behaviour is as if the value had
!          been stored to word-aligned memory and then loaded into 
!          registers with 32-bit load instruction(s).  */
!       len = TYPE_LENGTH (type);
!       regno = ARM_A1_REGNUM;
! 
!       while (len > 0)
! 	{
! 	  memcpy (buf, valbuf,
! 		  len > INT_REGISTER_RAW_SIZE ? INT_REGISTER_RAW_SIZE : len);
! 	  regcache_cooked_write (regs, regno++, buf);
! 	  len -= INT_REGISTER_RAW_SIZE;
! 	  valbuf += INT_REGISTER_RAW_SIZE;
! 	}
!     }
  }
  
  /* Store the address of the place in which to copy the structure the
*************** arm_gdbarch_init (struct gdbarch_info in
*** 3365,3372 ****
    set_gdbarch_register_name (gdbarch, arm_register_name);
  
    /* Returning results.  */
!   set_gdbarch_deprecated_extract_return_value (gdbarch, arm_extract_return_value);
!   set_gdbarch_deprecated_store_return_value (gdbarch, arm_store_return_value);
    set_gdbarch_store_struct_return (gdbarch, arm_store_struct_return);
    set_gdbarch_use_struct_convention (gdbarch, arm_use_struct_convention);
    set_gdbarch_extract_struct_value_address (gdbarch,
--- 3495,3502 ----
    set_gdbarch_register_name (gdbarch, arm_register_name);
  
    /* Returning results.  */
!   set_gdbarch_extract_return_value (gdbarch, arm_extract_return_value);
!   set_gdbarch_store_return_value (gdbarch, arm_store_return_value);
    set_gdbarch_store_struct_return (gdbarch, arm_store_struct_return);
    set_gdbarch_use_struct_convention (gdbarch, arm_use_struct_convention);
    set_gdbarch_extract_struct_value_address (gdbarch,

Re: ARM PATCH fix extract_return_value and store_return_value

[Richard Earnshaw]

> > 2002-12-14  Richard Earnshaw  <rearnsha@arm.com>
> > 
> >         * arm-tdep.c (convert_from_extended): New argument to hold the
> >         type of floating point result we want to convert to.  Make input
> >         argument const.  Fix all callers.
> >         (convert_to_extended): Similarly.
> >         (arm_extract_return_value): Now takes a regcache argument.  Change
> >         code to use regcache accessor functions.  Correctly extract
> >         smaller-than-word results on big-endian machines.
> >         (arm_store_return_value): Now takes a regcache argument.  Change
> >         code to use regcache accessor functions.  Correctly zero/sign extend
> >         smaller than word results before storing into r0.
> >         (arm_gdbarch_init): Register new-style extract_return_value and
> >         store_return_value functions.
> 
> Hi Richard, 
> 
> I can report that these
> changes do fix two fails for big-endian running callfuncs.exp.
> One of the fails was returning a one-byte struct, the other
> a two-byte struct.   There were no other fails in callfuncs.exp.

Excellent.  Thanks for doing the tests.

> As is, they conflict with some of Elena's
> vector changes, but I've massaged them into closer conformance
> with a more recent revision.  Here's my merged patch (not entirely
> up to date, but more recent than what appears here).

I installed the patches to the public tree back in December, so these 
conflicts must relate to some internal version you are testing on.

R.

Re: ARM PATCH fix extract_return_value and store_return_value

[Michael Snyder]

Richard Earnshaw wrote:
> 
> > > 2002-12-14  Richard Earnshaw  <rearnsha@arm.com>
> > >
> > >         * arm-tdep.c (convert_from_extended): New argument to hold the
> > >         type of floating point result we want to convert to.  Make input
> > >         argument const.  Fix all callers.
> > >         (convert_to_extended): Similarly.
> > >         (arm_extract_return_value): Now takes a regcache argument.  Change
> > >         code to use regcache accessor functions.  Correctly extract
> > >         smaller-than-word results on big-endian machines.
> > >         (arm_store_return_value): Now takes a regcache argument.  Change
> > >         code to use regcache accessor functions.  Correctly zero/sign extend
> > >         smaller than word results before storing into r0.
> > >         (arm_gdbarch_init): Register new-style extract_return_value and
> > >         store_return_value functions.
> >
> > Hi Richard,
> >
> > I can report that these
> > changes do fix two fails for big-endian running callfuncs.exp.
> > One of the fails was returning a one-byte struct, the other
> > a two-byte struct.   There were no other fails in callfuncs.exp.
> 
> Excellent.  Thanks for doing the tests.
> 
> > As is, they conflict with some of Elena's
> > vector changes, but I've massaged them into closer conformance
> > with a more recent revision.  Here's my merged patch (not entirely
> > up to date, but more recent than what appears here).
> 
> I installed the patches to the public tree back in December, so these
> conflicts must relate to some internal version you are testing on.

D'oh!  You're right, I'm working with sources that we're just now
getting ready to contribute.  Well, we'll work out the conflicts then.

Meantime, archaeological evidence indicates that we've covered the
cases that were troubling me before, so I guess we can close this
issue at least until Red Hat contributes the iMWXT port.

Thanks so much for your help on this!

Michael