Re: ARM PATCH fix extract_return_value and store_return_value

Michael Snyder
2003-01-24T03:10:22Z

[-- 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,