[RFA] arm_store_return_value, big-endian (take 2)

[RFA] arm_store_return_value, big-endian (take 2)

[Michael Snyder]

This fixes up offsets in arm_store_return_value for big-endian targets,
just as my first patch did for arm_extract_return_value.

This supercedes the patch by the same name, which seems to have
been bollixed by an incomprehensible but reproducable bug in
gnu patch.

 
-------------------------------------------------------------------------------
2002-11-06  Michael Snyder  <msnyder@redhat.com>

        * arm-tdep.c (arm_store_return_value): Handle offset of
        small types on big-endian machines.

Index: arm-tdep.c
===================================================================
RCS file: /cvs/src/src/gdb/arm-tdep.c,v
retrieving revision 1.74
diff -p -r1.74 arm-tdep.c
*** arm-tdep.c  1 Nov 2002 21:21:49 -0000       1.74
--- arm-tdep.c  7 Nov 2002 00:22:13 -0000
*************** arm_store_return_value (struct type *typ
*** 2417,2422 ****
--- 2417,2425 ----
          break;
        }
      }
+   else if (TYPE_LENGTH (type) < REGISTER_RAW_SIZE (A1_REGNUM))
+     write_register_bytes (REGISTER_RAW_SIZE (A1_REGNUM) - TYPE_LENGTH (type),
+                         valbuf, TYPE_LENGTH (type));
    else
      write_register_bytes (ARM_A1_REGNUM, valbuf, TYPE_LENGTH (type));
  }

Re: [RFA] arm_store_return_value, big-endian (take 2)

[Richard Earnshaw]

> This fixes up offsets in arm_store_return_value for big-endian targets, 
> just as my first patch did for arm_extract_return_value.
> 
> This supercedes the patch by the same name, which seems to have
> been bollixed by an incomprehensible but reproducable bug in
> gnu patch.
> 
> 2002-11-06  Michael Snyder  <msnyder@redhat.com>
> 
> 	* arm-tdep.c (arm_store_return_value): Handle offset of
> 	small types on big-endian machines.
> 
> Index: arm-tdep.c
> ===================================================================
> RCS file: /cvs/src/src/gdb/arm-tdep.c,v
> retrieving revision 1.74
> diff -p -r1.74 arm-tdep.c
> *** arm-tdep.c	1 Nov 2002 21:21:49 -0000	1.74
> --- arm-tdep.c	7 Nov 2002 00:22:13 -0000
> *************** arm_store_return_value (struct type *typ
> *** 2417,2422 ****
> --- 2417,2425 ----
>   	  break;
>   	}
>       }
> +   else if (TYPE_LENGTH (type) < REGISTER_RAW_SIZE (A1_REGNUM))
> +     write_register_bytes (REGISTER_RAW_SIZE (A1_REGNUM) - TYPE_LENGTH (type),
> + 			  valbuf, TYPE_LENGTH (type));
>     else
>       write_register_bytes (ARM_A1_REGNUM, valbuf, TYPE_LENGTH (type));
>   }
> 

Leaving asside the issue of the correctness of write_register_bytes (note 
to self, must finish of my register patches), I don't think this is 
correct -- in fact, I think it's also wrong for little-endian as well.

What should happen is that the smaller-than-word value should be 
zero/sign-extended to 32 bits and then the whole thing stored in A1_REGNUM.

R.

Re: [RFA] arm_store_return_value, big-endian (take 2)

[Michael Snyder]

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

Richard Earnshaw wrote:
> 
> Leaving asside the issue of the correctness of write_register_bytes (note
> to self, must finish of my register patches), I don't think this is
> correct -- in fact, I think it's also wrong for little-endian as well.
> 
> What should happen is that the smaller-than-word value should be
> zero/sign-extended to 32 bits and then the whole thing stored in A1_REGNUM.

Ah, thanks.  OK, how about this?

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

2002-11-06  Michael Snyder  <msnyder@redhat.com>

	* arm-tdep.c (arm_store_return_value): Handle offset of
	small types on big-endian machines.

Index: arm-tdep.c
===================================================================
RCS file: /cvs/src/src/gdb/arm-tdep.c,v
retrieving revision 1.74
diff -p -r1.74 arm-tdep.c
*** arm-tdep.c	1 Nov 2002 21:21:49 -0000	1.74
--- arm-tdep.c	8 Nov 2002 03:17:49 -0000
*************** arm_store_return_value (struct type *typ
*** 2417,2422 ****
--- 2417,2429 ----
  	  break;
  	}
      }
+   else if (TYPE_LENGTH (type) < REGISTER_RAW_SIZE (A1_REGNUM)
+ 	   && TARGET_BYTE_ORDER == BFD_ENDIAN_BIG)
+     {
+       LONGEST tmp = unpack_long (type, valbuf);
+ 
+       write_register (ARM_A1_REGNUM, tmp);
+     }
    else
      write_register_bytes (ARM_A1_REGNUM, valbuf, TYPE_LENGTH (type));
  }

Re: [RFA] arm_store_return_value, big-endian (take 2)

[Richard Earnshaw]

> Richard Earnshaw wrote:
> > 
> > Leaving asside the issue of the correctness of write_register_bytes (note
> > to self, must finish of my register patches), I don't think this is
> > correct -- in fact, I think it's also wrong for little-endian as well.
> > 
> > What should happen is that the smaller-than-word value should be
> > zero/sign-extended to 32 bits and then the whole thing stored in A1_REGNUM.
> 
> Ah, thanks.  OK, how about this?
> 
> 2002-11-06  Michael Snyder  <msnyder@redhat.com>
> 
> 	* arm-tdep.c (arm_store_return_value): Handle offset of
> 	small types on big-endian machines.


And for little-endian?

R.

Re: [RFA] arm_store_return_value, big-endian (take 2)

[Michael Snyder]

Richard Earnshaw wrote:
> 
> > Richard Earnshaw wrote:
> > >
> > > Leaving asside the issue of the correctness of write_register_bytes (note
> > > to self, must finish of my register patches), I don't think this is
> > > correct -- in fact, I think it's also wrong for little-endian as well.
> > >
> > > What should happen is that the smaller-than-word value should be
> > > zero/sign-extended to 32 bits and then the whole thing stored in A1_REGNUM.
> >
> > Ah, thanks.  OK, how about this?
> >
> > 2002-11-06  Michael Snyder  <msnyder@redhat.com>
> >
> >       * arm-tdep.c (arm_store_return_value): Handle offset of
> >       small types on big-endian machines.
> 
> And for little-endian?

It already works for little-endian.  I've tested this with 
arm-sim, arm-sim/-mbig-endian, and arm-sim/-mthumb.

Re: [RFA] arm_store_return_value, big-endian (take 2)

[Richard Earnshaw]

> Richard Earnshaw wrote:
> > 
> > > Richard Earnshaw wrote:
> > > >
> > > > Leaving asside the issue of the correctness of write_register_bytes (note
> > > > to self, must finish of my register patches), I don't think this is
> > > > correct -- in fact, I think it's also wrong for little-endian as well.
> > > >
> > > > What should happen is that the smaller-than-word value should be
> > > > zero/sign-extended to 32 bits and then the whole thing stored in A1_REGNUM.
> > >
> > > Ah, thanks.  OK, how about this?
> > >
> > > 2002-11-06  Michael Snyder  <msnyder@redhat.com>
> > >
> > >       * arm-tdep.c (arm_store_return_value): Handle offset of
> > >       small types on big-endian machines.
> > 
> > And for little-endian?
> 
> It already works for little-endian.  I've tested this with 
> arm-sim, arm-sim/-mbig-endian, and arm-sim/-mthumb.

But it's not zero/sign extending properly for little-endian, so garbage is 
remaining in the top part of A1

R.

Re: [RFA] arm_store_return_value, big-endian (take 2)

[Michael Snyder]

Richard Earnshaw wrote:
> 
> > Richard Earnshaw wrote:
> > >
> > > > Richard Earnshaw wrote:
> > > > >
> > > > > Leaving asside the issue of the correctness of write_register_bytes (note
> > > > > to self, must finish of my register patches), I don't think this is
> > > > > correct -- in fact, I think it's also wrong for little-endian as well.
> > > > >
> > > > > What should happen is that the smaller-than-word value should be
> > > > > zero/sign-extended to 32 bits and then the whole thing stored in A1_REGNUM.
> > > >
> > > > Ah, thanks.  OK, how about this?
> > > >
> > > > 2002-11-06  Michael Snyder  <msnyder@redhat.com>
> > > >
> > > >       * arm-tdep.c (arm_store_return_value): Handle offset of
> > > >       small types on big-endian machines.
> > >
> > > And for little-endian?
> >
> > It already works for little-endian.  I've tested this with
> > arm-sim, arm-sim/-mbig-endian, and arm-sim/-mthumb.
> 
> But it's not zero/sign extending properly for little-endian, so garbage is
> remaining in the top part of A1

Ah; well, I didn't make it any worse!  ;-)
Can I leave that detail for someone else, and just
submit this minor improvement?

Re: [RFA] arm_store_return_value, big-endian (take 2)

[Richard Earnshaw]

> Richard Earnshaw wrote:
> > 
> > > Richard Earnshaw wrote:
> > > >
> > > > > Richard Earnshaw wrote:
> > > > > >
> > > > > > Leaving asside the issue of the correctness of write_register_bytes (note
> > > > > > to self, must finish of my register patches), I don't think this is
> > > > > > correct -- in fact, I think it's also wrong for little-endian as well.
> > > > > >
> > > > > > What should happen is that the smaller-than-word value should be
> > > > > > zero/sign-extended to 32 bits and then the whole thing stored in A1_REGNUM.
> > > > >
> > > > > Ah, thanks.  OK, how about this?
> > > > >
> > > > > 2002-11-06  Michael Snyder  <msnyder@redhat.com>
> > > > >
> > > > >       * arm-tdep.c (arm_store_return_value): Handle offset of
> > > > >       small types on big-endian machines.
> > > >
> > > > And for little-endian?
> > >
> > > It already works for little-endian.  I've tested this with
> > > arm-sim, arm-sim/-mbig-endian, and arm-sim/-mthumb.
> > 
> > But it's not zero/sign extending properly for little-endian, so garbage is
> > remaining in the top part of A1
> 
> Ah; well, I didn't make it any worse!  ;-)
> Can I leave that detail for someone else, and just
> submit this minor improvement?

Given that to fix this for little-endian as well means that you just have 
to *remove* the endianness test from your patch, why is that so hard???!!!!

Also, you should add a comment at this point explaining that we want to 
extend the value into the whole register.

R.

Re: [RFA] arm_store_return_value, big-endian (take 2)

[Michael Snyder]

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

Richard Earnshaw wrote:
> 
> > Richard Earnshaw wrote:
> > >
> > > > Richard Earnshaw wrote:
> > > > >
> > > > > > Richard Earnshaw wrote:
> > > > > > >
> > > > > > > Leaving asside the issue of the correctness of write_register_bytes (note
> > > > > > > to self, must finish of my register patches), I don't think this is
> > > > > > > correct -- in fact, I think it's also wrong for little-endian as well.
> > > > > > >
> > > > > > > What should happen is that the smaller-than-word value should be
> > > > > > > zero/sign-extended to 32 bits and then the whole thing stored in A1_REGNUM.
> > > > > >
> > > > > > Ah, thanks.  OK, how about this?
> > > > > >
> > > > > > 2002-11-06  Michael Snyder  <msnyder@redhat.com>
> > > > > >
> > > > > >       * arm-tdep.c (arm_store_return_value): Handle offset of
> > > > > >       small types on big-endian machines.
> > > > >
> > > > > And for little-endian?
> > > >
> > > > It already works for little-endian.  I've tested this with
> > > > arm-sim, arm-sim/-mbig-endian, and arm-sim/-mthumb.
> > >
> > > But it's not zero/sign extending properly for little-endian, so garbage is
> > > remaining in the top part of A1
> >
> > Ah; well, I didn't make it any worse!  ;-)
> > Can I leave that detail for someone else, and just
> > submit this minor improvement?
> 
> Given that to fix this for little-endian as well means that you just have
> to *remove* the endianness test from your patch, why is that so hard???!!!!

Oh, I see.  Yes, it's no longer necessary.  But if it's not so hard, 
and you already see what's necessary, why not just do it instead of
ragging on me to do it?   ;-)

This what you have in mind?  Please just take it from here, I really
didn't intend to spend this much time and energy on it.

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

Index: arm-tdep.c
===================================================================
RCS file: /cvs/src/src/gdb/arm-tdep.c,v
retrieving revision 1.74
diff -p -r1.74 arm-tdep.c
*** arm-tdep.c	1 Nov 2002 21:21:49 -0000	1.74
--- arm-tdep.c	13 Nov 2002 19:48:21 -0000
*************** arm_store_return_value (struct type *typ
*** 2417,2424 ****
  	  break;
  	}
      }
!   else
!     write_register_bytes (ARM_A1_REGNUM, valbuf, TYPE_LENGTH (type));
  }
  
  /* Store the address of the place in which to copy the structure the
--- 2417,2424 ----
  	  break;
  	}
      }
!   else 
!     write_register (ARM_A1_REGNUM, unpack_long (type, valbuf);
  }
  
  /* Store the address of the place in which to copy the structure the

Re: [RFA] arm_store_return_value, big-endian (take 2)

[Richard Earnshaw]

> Oh, I see.  Yes, it's no longer necessary.  But if it's not so hard, 
> and you already see what's necessary, why not just do it instead of
> ragging on me to do it?   ;-)
> 

Because I don't have a big-endian world in which I can test it.

> This what you have in mind?  Please just take it from here, I really
> didn't intend to spend this much time and energy on it.

Nope, now you've broken larger-than-word objects.

> 
> Index: arm-tdep.c
> ===================================================================
> RCS file: /cvs/src/src/gdb/arm-tdep.c,v
> retrieving revision 1.74
> diff -p -r1.74 arm-tdep.c
> *** arm-tdep.c	1 Nov 2002 21:21:49 -0000	1.74
> --- arm-tdep.c	13 Nov 2002 19:48:21 -0000
> *************** arm_store_return_value (struct type *typ
> *** 2417,2424 ****
>   	  break;
>   	}
>       }
> !   else
> !     write_register_bytes (ARM_A1_REGNUM, valbuf, TYPE_LENGTH (type));
>   }
>   
>   /* Store the address of the place in which to copy the structure the
> --- 2417,2424 ----
>   	  break;
>   	}
>       }
> !   else 
> !     write_register (ARM_A1_REGNUM, unpack_long (type, valbuf);
>   }
>   
>   /* Store the address of the place in which to copy the structure the
>