diff mbox

[3/11] Add MIPS_MAX_REGISTER_SIZE (2/4)

Message ID 5A105765-C70D-413C-BB35-50BAA5FD5865@arm.com
State New
Headers show

Commit Message

Alan Hayward May 24, 2017, 7:45 p.m. UTC
> On 24 May 2017, at 12:07, Pedro Alves <palves@redhat.com> wrote:

> 

> On 05/24/2017 11:20 AM, Alan Hayward wrote:

> 

>>>> Meanwhile raw_collect_integer doesn’t need a signed parameter:

>>> 

>>> Wouldn't we need to support ADDR_LEN larger than the register size?

>> 

>> This might be me misunderstanding gdb,

>> But I assumed that addr would always be shorter than the register size.

>> If addr is bigger than the register size then the most significant bits will

>> be chopped off (including the sign), which I think would be a bad idea?

> 

> Yeah, the case of a 32-bit register being given a 64-bit slot in a ptrace

> register buffer is actually not unheard of.  For example the

> segment registers on x86-64 (cs, ss, ds, etc.) are 32-bit in

> gdb's register cache, but Linux ptrace transfers them as 64-bit

> [see /usr/include/sys/reg.h].  I'm not exactly sure whether

> in such cases we end up needing to sign/zero extend when copying

> back, or whether the kernel ignores the upper bits.  I think that

> on x86 we just copy the lower 4 bytes and leave the upper ones as

> they were, so probably the latter.  The MIPS architecture is special

> around addresses being signed though, and given the existing code,

> I'd play it safe and keep the collect/store functions mirrors - if

> one truncates, the other fills/extends, and vice versa.  There's also

> 

>  /* Is the target using 64-bit raw integer registers but only

>     storing a left-aligned 32-bit value in each?  */

>  int mips64_transfers_32bit_regs_p;

> 

> which most probably doesn't apply in this case (FreeBSD, while

> I think that was originally added for remote), but it compounds in

> the weirdness.

> 

> Thanks,

> Pedro Alves

> 


Added copy_integer_to_size, and removed the templates.

Manually tested of copy_integer_to_size to make sure the signs and
endian parts all work.

Tested on a --enable-targets=all build using make check with board files
unix and native-gdbserver.
I do not have a MIPS machine to test on.
Ok to commit?

Alan.


2017-05-24  Alan Hayward  <alan.hayward@arm.com>

	* gdb/defs.h (copy_integer_to_size): New declaration.
	* gdb/findvar.c (extract_signed_integer): Removed function.
	(extract_unsigned_integer): Likewise.
	(store_signed_integer): Removed function.
	(store_unsigned_integer): Likewise.
	* mips-fbsd-tdep.c (mips_fbsd_supply_reg): Use raw_supply_integer.
	(mips_fbsd_collect_reg): Use templated raw_collect_integer.
	* mips-linux-tdep.c (supply_32bit_reg): Use raw_supply_integer.
	(mips64_fill_gregset): Use raw_collect_integer.
	(mips64_fill_fpregset): Use raw_supply_integer.
	* gdb/regcache.c (regcache::raw_supply_integer): New function.
	(regcache::raw_collect_integer): Likewise
	* gdb/regcache.h (regcache::raw_supply): New declaration.
	(regcache::raw_collect): Likewise

Comments

Pedro Alves May 25, 2017, 10:46 a.m. UTC | #1
On 05/24/2017 08:45 PM, Alan Hayward wrote:

> Added copy_integer_to_size, and removed the templates.
> 
> Manually tested of copy_integer_to_size to make sure the signs and
> endian parts all work.

Those manual tests would have been perfect candidates for some
unit tests.  All you'd need to do is add this at the bottom
of gdb/findvar.c:

#if GDB_SELF_TEST
namespace selftests {
namespace findvar_tests {

static void
run_test ()
{
  // Here, exercise the various code paths of copy_integer_to_size,
  // calling SELF_CHECK.
}

} // namespace findvar_test
} // namespace selftests

#endif

void
_initialize_findvar (void)
{
#if GDB_SELF_TEST
  register_self_test (selftests::findvar_tests::run_test);
#endif
}


(and include selftest.h at the top of the file).

You'd run those tests with:
 $ make check TESTS="gdb.gdb/unittest.exp"

Or (my preferred when hacking):
 $ gdb --batch -q -ex "maint selftest"


> Tested on a --enable-targets=all build using make check with board files
> unix and native-gdbserver.
> I do not have a MIPS machine to test on.
> Ok to commit?


On 05/24/2017 08:45 PM, Alan Hayward wrote:
> 
> 
> 2017-05-24  Alan Hayward  <alan.hayward@arm.com>
> 
> 	* gdb/defs.h (copy_integer_to_size): New declaration.
> 	* gdb/findvar.c (extract_signed_integer): Removed function.
> 	(extract_unsigned_integer): Likewise.
> 	(store_signed_integer): Removed function.
> 	(store_unsigned_integer): Likewise.
> 	* mips-fbsd-tdep.c (mips_fbsd_supply_reg): Use raw_supply_integer.
> 	(mips_fbsd_collect_reg): Use templated raw_collect_integer.
> 	* mips-linux-tdep.c (supply_32bit_reg): Use raw_supply_integer.
> 	(mips64_fill_gregset): Use raw_collect_integer.
> 	(mips64_fill_fpregset): Use raw_supply_integer.
> 	* gdb/regcache.c (regcache::raw_supply_integer): New function.
> 	(regcache::raw_collect_integer): Likewise
> 	* gdb/regcache.h (regcache::raw_supply): New declaration.
> 	(regcache::raw_collect): Likewise

There are stale entries above.  Also, drop "gdb/" prefix, and add 
missing periods after "Likewise".

> 
> 
> diff --git a/gdb/defs.h b/gdb/defs.h
> index a0b586f401eca205334e9f237081f4da97c83aa1..a1a97bb1e791d4f423788797d1f04c3e89877d90 100644
> --- a/gdb/defs.h
> +++ b/gdb/defs.h
> @@ -658,7 +658,10 @@ extern void store_unsigned_integer (gdb_byte *, int,
>  extern void store_typed_address (gdb_byte *buf, struct type *type,
>  				 CORE_ADDR addr);
> 
> -
> 
> +extern void copy_integer_to_size (gdb_byte *dest, int dest_size,
> +				  const gdb_byte *source, int source_size,
> +				  bool is_signed, enum bfd_endian byte_order);
> +
>  /* From valops.c */
> 
>  extern int watchdog;
> diff --git a/gdb/findvar.c b/gdb/findvar.c
> index ed4d5c1266c9de069981b306bc8229ae5fb02350..5a82e493f9ca6d9337a22defc4377235f36acba8 100644
> --- a/gdb/findvar.c
> +++ b/gdb/findvar.c
> @@ -249,7 +249,47 @@ store_typed_address (gdb_byte *buf, struct type *type, CORE_ADDR addr)
>    gdbarch_address_to_pointer (get_type_arch (type), type, buf, addr);
>  }
> 
> +/* Copy a value from SOURCE of size SOURCE_SIZE bytes to DEST of size DEST_SIZE
> +   bytes.  If SOURCE_SIZE is greater than DEST_SIZE, then truncate the most
> +   significant bytes.  If SOURCE_SIZE is less than DEST_SIZE then either sign
> +   or zero extended according to IS_SIGNED.  Values are stored in memory with
> +   endianess BYTE_ORDER.  */
> 
> +void
> +copy_integer_to_size (gdb_byte *dest, int dest_size, const gdb_byte *source,
> +		      int source_size, bool is_signed,
> +		      enum bfd_endian byte_order)
> +{
> +  signed int size_diff = dest_size - source_size;
> +
> +  /* Copy across everything from SOURCE that can fit into DEST.  */
> +
> +  if (byte_order == BFD_ENDIAN_BIG && size_diff > 0)
> +    memcpy (dest + size_diff, source, source_size);
> +  else if (byte_order == BFD_ENDIAN_BIG && size_diff < 0)
> +    memcpy (dest, source - size_diff, dest_size);
> +  else
> +    memcpy (dest, source, std::min (source_size, dest_size));
> +
> +  /* Fill the remaining space in DEST by either zero extending or sign
> +     extending.  */
> +
> +  if (size_diff > 0)
> +    {
> +      char extension = 0;

gdb_byte.

> +      if (is_signed)
> +	if ((byte_order == BFD_ENDIAN_BIG && source[0] & 0x80)
> +	    || (byte_order != BFD_ENDIAN_BIG
> +		&& source[source_size - 1] & 0x80))
> +	  extension = 0xff;
> +

Merge the ifs?  Like, e.g.:

  gdb_byte extension;

  if (is_signed
      && ((byte_order == BFD_ENDIAN_BIG && source[0] & 0x80)
	  || (byte_order != BFD_ENDIAN_BIG
	      && source[source_size - 1] & 0x80)))
    extension = 0xff;
  else
    extension = 0;


> +      /* Extend into MSBs of SOURCE.  */
> +      if (byte_order == BFD_ENDIAN_BIG)
> +	memset (dest, extension, size_diff);
> +      else
> +	memset (dest + source_size, extension, size_diff);
> +    }
> +}
> 
>  /* Return a `value' with the contents of (virtual or cooked) register
>     REGNUM as found in the specified FRAME.  The register's type is
> diff --git a/gdb/mips-fbsd-tdep.c b/gdb/mips-fbsd-tdep.c
> index 00fae0ec60ddc9e645d3236efe29f2f9e9ceab5c..13cf98585f96f1acfe6decbe320530d609bee646 100644
> --- a/gdb/mips-fbsd-tdep.c
> +++ b/gdb/mips-fbsd-tdep.c
> @@ -47,57 +47,24 @@
>     34th is a dummy for padding.  */
>  #define MIPS_FBSD_NUM_FPREGS	34
> 
> -/* Supply a single register.  If the source register size matches the
> -   size the regcache expects, this can use regcache_raw_supply().  If
> -   they are different, this copies the source register into a buffer
> -   that can be passed to regcache_raw_supply().  */
> +/* Supply a single register.  The register size might not match, so use
> +   regcache->raw_supply_integer ().  */
> 
>  static void
>  mips_fbsd_supply_reg (struct regcache *regcache, int regnum, const void *addr,
>  		      size_t len)
>  {
> -  struct gdbarch *gdbarch = get_regcache_arch (regcache);
> -
> -  if (register_size (gdbarch, regnum) == len)
> -    regcache_raw_supply (regcache, regnum, addr);
> -  else
> -    {
> -      enum bfd_endian byte_order = gdbarch_byte_order (gdbarch);
> -      gdb_byte buf[MAX_REGISTER_SIZE];
> -      LONGEST val;
> -
> -      val = extract_signed_integer ((const gdb_byte *) addr, len, byte_order);
> -      store_signed_integer (buf, register_size (gdbarch, regnum), byte_order,
> -			    val);
> -      regcache_raw_supply (regcache, regnum, buf);
> -    }
> +  regcache->raw_supply_integer (regnum, (const gdb_byte *) addr, len, true);
>  }

Nice!

> --- a/gdb/mips-linux-tdep.c
> +++ b/gdb/mips-linux-tdep.c
> @@ -116,13 +116,7 @@ mips_linux_get_longjmp_target (struct frame_info *frame, CORE_ADDR *pc)
>  static void
>  supply_32bit_reg (struct regcache *regcache, int regnum, const void *addr)
>  {
> -  struct gdbarch *gdbarch = get_regcache_arch (regcache);
> -  enum bfd_endian byte_order = gdbarch_byte_order (gdbarch);
> -  gdb_byte buf[MAX_REGISTER_SIZE];
> -  store_signed_integer (buf, register_size (gdbarch, regnum), byte_order,
> -			extract_signed_integer ((const gdb_byte *) addr, 4,
> -						byte_order));
> -  regcache_raw_supply (regcache, regnum, buf);
> +  regcache->raw_supply_integer (regnum, (const gdb_byte *) addr, 4, true);
>  }

Nice.  :-)
[snip several "nice"s]

> diff --git a/gdb/regcache.h b/gdb/regcache.h
> index 4dcfccbac70f0f962bf5e5596d035fda42322795..409482d17c0542c7a53620d88d33fa9706fa72c5 100644
> --- a/gdb/regcache.h
> +++ b/gdb/regcache.h
> @@ -294,8 +294,14 @@ public:
> 
>    void raw_collect (int regnum, void *buf) const;
> 
> +  void raw_collect_integer (int regnum, gdb_byte *addr, int addr_len,
> +			    bool is_signed) const;
> +
>    void raw_supply (int regnum, const void *buf);
> 
> +  void raw_supply_integer (int regnum, const gdb_byte *addr, int addr_len,
> +			   bool is_signed);
> +
>    void raw_supply_zeroed (int regnum);
> 
>    void raw_copy (int regnum, struct regcache *src_regcache);
> diff --git a/gdb/regcache.c b/gdb/regcache.c
> index 660558f7ff10f9d8346b6e08422e16c38c3c4d7d..ec6446b897922a8f9f44bbf94b7f1d198b0a6d4b 100644
> --- a/gdb/regcache.c
> +++ b/gdb/regcache.c
> @@ -1189,6 +1189,28 @@ regcache::raw_supply (int regnum, const void *buf)
>      }
>  }
> 
> +/* Supply register REGNUM with an integer, whose contents are stored in ADDR,
> +   with length ADDR_LEN and sign IS_SIGNED, to REGCACHE.  */

It'd be good to say something about extending/truncating here, as
well as mention that the ADDR contents are assumed to be in
target byte order.

> +
> +void
> +regcache::raw_supply_integer (int regnum, const gdb_byte *addr, int addr_len,
> +			      bool is_signed)
> +{

> +/* Collect register REGNUM from regcache to an integer, whose contents are
> +   stored in ADDR, with length ADDR_LEN and sign IS_SIGNED.  */

Ditto.

> +
> +void
> +regcache::raw_collect_integer (int regnum, gdb_byte *addr, int addr_len,
> +			       bool is_signed) const
> +{

Otherwise this looks good to me.  It'd look great with unit tests.  :-)

Yao, what do you think?

Thanks,
Pedro Alves
Yao Qi May 25, 2017, 11:43 a.m. UTC | #2
Pedro Alves <palves@redhat.com> writes:

> Those manual tests would have been perfect candidates for some
> unit tests.  All you'd need to do is add this at the bottom
> of gdb/findvar.c:
>
> #if GDB_SELF_TEST
> namespace selftests {
> namespace findvar_tests {
>
> static void
> run_test ()
> {
>   // Here, exercise the various code paths of copy_integer_to_size,
>   // calling SELF_CHECK.
> }
>
> } // namespace findvar_test
> } // namespace selftests
>
> #endif
>
> void
> _initialize_findvar (void)
> {
> #if GDB_SELF_TEST
>   register_self_test (selftests::findvar_tests::run_test);
> #endif
> }
>
>

It is great to have a unit test, and that is what I want to suggest.

>> +/* Supply a single register.  The register size might not match, so use
>> +   regcache->raw_supply_integer ().  */
>> 
>>  static void
>>  mips_fbsd_supply_reg (struct regcache *regcache, int regnum, const void *addr,
>>  		      size_t len)
>>  {
>> -  struct gdbarch *gdbarch = get_regcache_arch (regcache);
>> -
>> -  if (register_size (gdbarch, regnum) == len)
>> -    regcache_raw_supply (regcache, regnum, addr);
>> -  else
>> -    {
>> -      enum bfd_endian byte_order = gdbarch_byte_order (gdbarch);
>> -      gdb_byte buf[MAX_REGISTER_SIZE];
>> -      LONGEST val;
>> -
>> -      val = extract_signed_integer ((const gdb_byte *) addr, len, byte_order);
>> -      store_signed_integer (buf, register_size (gdbarch, regnum), byte_order,
>> -			    val);
>> -      regcache_raw_supply (regcache, regnum, buf);
>> -    }
>> +  regcache->raw_supply_integer (regnum, (const gdb_byte *) addr, len, true);
>>  }
>
> Nice!
>

Can we completely remove mips_fbsd_supply_reg? and use
regcache->raw_supply_integer instead?

>> --- a/gdb/mips-linux-tdep.c
>> +++ b/gdb/mips-linux-tdep.c
>> @@ -116,13 +116,7 @@ mips_linux_get_longjmp_target (struct
>> frame_info *frame, CORE_ADDR *pc)
>>  static void
>>  supply_32bit_reg (struct regcache *regcache, int regnum, const void *addr)
>>  {
>> -  struct gdbarch *gdbarch = get_regcache_arch (regcache);
>> -  enum bfd_endian byte_order = gdbarch_byte_order (gdbarch);
>> -  gdb_byte buf[MAX_REGISTER_SIZE];
>> -  store_signed_integer (buf, register_size (gdbarch, regnum), byte_order,
>> -			extract_signed_integer ((const gdb_byte *) addr, 4,
>> -						byte_order));
>> -  regcache_raw_supply (regcache, regnum, buf);
>> +  regcache->raw_supply_integer (regnum, (const gdb_byte *) addr, 4, true);
>>  }
>
> Nice.  :-)
> [snip several "nice"s]
>

Likewise, remove supply_32bit_reg and use regcache->raw_supply_integer.
Pedro Alves May 25, 2017, 11:48 a.m. UTC | #3
On 05/25/2017 12:43 PM, Yao Qi wrote:
>>> >>  static void
>>> >>  supply_32bit_reg (struct regcache *regcache, int regnum, const void *addr)
>>> >>  {
>>> >> -  struct gdbarch *gdbarch = get_regcache_arch (regcache);
>>> >> -  enum bfd_endian byte_order = gdbarch_byte_order (gdbarch);
>>> >> -  gdb_byte buf[MAX_REGISTER_SIZE];
>>> >> -  store_signed_integer (buf, register_size (gdbarch, regnum), byte_order,
>>> >> -			extract_signed_integer ((const gdb_byte *) addr, 4,
>>> >> -						byte_order));
>>> >> -  regcache_raw_supply (regcache, regnum, buf);
>>> >> +  regcache->raw_supply_integer (regnum, (const gdb_byte *) addr, 4, true);
>>> >>  }
>> >
>> > Nice.  :-)
>> > [snip several "nice"s]
>> >
> Likewise, remove supply_32bit_reg and use regcache->raw_supply_integer.

There are multiple calls to it, so inlining it would require writing
the "4, true" arguments at all call sites.
Leaving supply_32bit_reg (etc.) as small wrapper functions helps
readability, IMHO.

Thanks,
Pedro Alves
diff mbox

Patch

diff --git a/gdb/defs.h b/gdb/defs.h
index a0b586f401eca205334e9f237081f4da97c83aa1..a1a97bb1e791d4f423788797d1f04c3e89877d90 100644
--- a/gdb/defs.h
+++ b/gdb/defs.h
@@ -658,7 +658,10 @@  extern void store_unsigned_integer (gdb_byte *, int,
 extern void store_typed_address (gdb_byte *buf, struct type *type,
 				 CORE_ADDR addr);

-

+extern void copy_integer_to_size (gdb_byte *dest, int dest_size,
+				  const gdb_byte *source, int source_size,
+				  bool is_signed, enum bfd_endian byte_order);
+
 /* From valops.c */

 extern int watchdog;
diff --git a/gdb/findvar.c b/gdb/findvar.c
index ed4d5c1266c9de069981b306bc8229ae5fb02350..5a82e493f9ca6d9337a22defc4377235f36acba8 100644
--- a/gdb/findvar.c
+++ b/gdb/findvar.c
@@ -249,7 +249,47 @@  store_typed_address (gdb_byte *buf, struct type *type, CORE_ADDR addr)
   gdbarch_address_to_pointer (get_type_arch (type), type, buf, addr);
 }

+/* Copy a value from SOURCE of size SOURCE_SIZE bytes to DEST of size DEST_SIZE
+   bytes.  If SOURCE_SIZE is greater than DEST_SIZE, then truncate the most
+   significant bytes.  If SOURCE_SIZE is less than DEST_SIZE then either sign
+   or zero extended according to IS_SIGNED.  Values are stored in memory with
+   endianess BYTE_ORDER.  */

+void
+copy_integer_to_size (gdb_byte *dest, int dest_size, const gdb_byte *source,
+		      int source_size, bool is_signed,
+		      enum bfd_endian byte_order)
+{
+  signed int size_diff = dest_size - source_size;
+
+  /* Copy across everything from SOURCE that can fit into DEST.  */
+
+  if (byte_order == BFD_ENDIAN_BIG && size_diff > 0)
+    memcpy (dest + size_diff, source, source_size);
+  else if (byte_order == BFD_ENDIAN_BIG && size_diff < 0)
+    memcpy (dest, source - size_diff, dest_size);
+  else
+    memcpy (dest, source, std::min (source_size, dest_size));
+
+  /* Fill the remaining space in DEST by either zero extending or sign
+     extending.  */
+
+  if (size_diff > 0)
+    {
+      char extension = 0;
+      if (is_signed)
+	if ((byte_order == BFD_ENDIAN_BIG && source[0] & 0x80)
+	    || (byte_order != BFD_ENDIAN_BIG
+		&& source[source_size - 1] & 0x80))
+	  extension = 0xff;
+
+      /* Extend into MSBs of SOURCE.  */
+      if (byte_order == BFD_ENDIAN_BIG)
+	memset (dest, extension, size_diff);
+      else
+	memset (dest + source_size, extension, size_diff);
+    }
+}

 /* Return a `value' with the contents of (virtual or cooked) register
    REGNUM as found in the specified FRAME.  The register's type is
diff --git a/gdb/mips-fbsd-tdep.c b/gdb/mips-fbsd-tdep.c
index 00fae0ec60ddc9e645d3236efe29f2f9e9ceab5c..13cf98585f96f1acfe6decbe320530d609bee646 100644
--- a/gdb/mips-fbsd-tdep.c
+++ b/gdb/mips-fbsd-tdep.c
@@ -47,57 +47,24 @@ 
    34th is a dummy for padding.  */
 #define MIPS_FBSD_NUM_FPREGS	34

-/* Supply a single register.  If the source register size matches the
-   size the regcache expects, this can use regcache_raw_supply().  If
-   they are different, this copies the source register into a buffer
-   that can be passed to regcache_raw_supply().  */
+/* Supply a single register.  The register size might not match, so use
+   regcache->raw_supply_integer ().  */

 static void
 mips_fbsd_supply_reg (struct regcache *regcache, int regnum, const void *addr,
 		      size_t len)
 {
-  struct gdbarch *gdbarch = get_regcache_arch (regcache);
-
-  if (register_size (gdbarch, regnum) == len)
-    regcache_raw_supply (regcache, regnum, addr);
-  else
-    {
-      enum bfd_endian byte_order = gdbarch_byte_order (gdbarch);
-      gdb_byte buf[MAX_REGISTER_SIZE];
-      LONGEST val;
-
-      val = extract_signed_integer ((const gdb_byte *) addr, len, byte_order);
-      store_signed_integer (buf, register_size (gdbarch, regnum), byte_order,
-			    val);
-      regcache_raw_supply (regcache, regnum, buf);
-    }
+  regcache->raw_supply_integer (regnum, (const gdb_byte *) addr, len, true);
 }

-/* Collect a single register.  If the destination register size
-   matches the size the regcache expects, this can use
-   regcache_raw_supply().  If they are different, this fetches the
-   register via regcache_raw_supply() into a buffer and then copies it
-   into the final destination.  */
+/* Collect a single register.  The register size might not match, so use
+   regcache->raw_collect_integer ().  */

 static void
 mips_fbsd_collect_reg (const struct regcache *regcache, int regnum, void *addr,
 		       size_t len)
 {
-  struct gdbarch *gdbarch = get_regcache_arch (regcache);
-
-  if (register_size (gdbarch, regnum) == len)
-    regcache_raw_collect (regcache, regnum, addr);
-  else
-    {
-      enum bfd_endian byte_order = gdbarch_byte_order (gdbarch);
-      gdb_byte buf[MAX_REGISTER_SIZE];
-      LONGEST val;
-
-      regcache_raw_collect (regcache, regnum, buf);
-      val = extract_signed_integer (buf, register_size (gdbarch, regnum),
-				    byte_order);
-      store_signed_integer ((gdb_byte *) addr, len, byte_order, val);
-    }
+  regcache->raw_collect_integer (regnum, (gdb_byte *) addr, len, true);
 }

 /* Supply the floating-point registers stored in FPREGS to REGCACHE.
diff --git a/gdb/mips-linux-tdep.c b/gdb/mips-linux-tdep.c
index 48a582a16c934abe6e8f87c46a6009649c606d49..ccfdcdf98bc1e528cd768efaaaffaa3405708f71 100644
--- a/gdb/mips-linux-tdep.c
+++ b/gdb/mips-linux-tdep.c
@@ -116,13 +116,7 @@  mips_linux_get_longjmp_target (struct frame_info *frame, CORE_ADDR *pc)
 static void
 supply_32bit_reg (struct regcache *regcache, int regnum, const void *addr)
 {
-  struct gdbarch *gdbarch = get_regcache_arch (regcache);
-  enum bfd_endian byte_order = gdbarch_byte_order (gdbarch);
-  gdb_byte buf[MAX_REGISTER_SIZE];
-  store_signed_integer (buf, register_size (gdbarch, regnum), byte_order,
-			extract_signed_integer ((const gdb_byte *) addr, 4,
-						byte_order));
-  regcache_raw_supply (regcache, regnum, buf);
+  regcache->raw_supply_integer (regnum, (const gdb_byte *) addr, 4, true);
 }

 /* Unpack an elf_gregset_t into GDB's register cache.  */
@@ -417,7 +411,6 @@  mips64_fill_gregset (const struct regcache *regcache,
 		     mips64_elf_gregset_t *gregsetp, int regno)
 {
   struct gdbarch *gdbarch = get_regcache_arch (regcache);
-  enum bfd_endian byte_order = gdbarch_byte_order (gdbarch);
   int regaddr, regi;
   mips64_elf_greg_t *regp = *gregsetp;
   void *dst;
@@ -460,14 +453,8 @@  mips64_fill_gregset (const struct regcache *regcache,

   if (regaddr != -1)
     {
-      gdb_byte buf[MAX_REGISTER_SIZE];
-      LONGEST val;
-
-      regcache_raw_collect (regcache, regno, buf);
-      val = extract_signed_integer (buf, register_size (gdbarch, regno),
-				    byte_order);
       dst = regp + regaddr;
-      store_signed_integer ((gdb_byte *) dst, 8, byte_order, val);
+      regcache->raw_collect_integer (regno, (gdb_byte *) dst, 8, true);
     }
 }

@@ -564,25 +551,13 @@  mips64_fill_fpregset (const struct regcache *regcache,
     }
   else if (regno == mips_regnum (gdbarch)->fp_control_status)
     {
-      gdb_byte buf[MAX_REGISTER_SIZE];
-      LONGEST val;
-
-      regcache_raw_collect (regcache, regno, buf);
-      val = extract_signed_integer (buf, register_size (gdbarch, regno),
-				    byte_order);
       to = (gdb_byte *) (*fpregsetp + 32);
-      store_signed_integer (to, 4, byte_order, val);
+      regcache->raw_collect_integer (regno, to, 4, true);
     }
   else if (regno == mips_regnum (gdbarch)->fp_implementation_revision)
     {
-      gdb_byte buf[MAX_REGISTER_SIZE];
-      LONGEST val;
-
-      regcache_raw_collect (regcache, regno, buf);
-      val = extract_signed_integer (buf, register_size (gdbarch, regno),
-				    byte_order);
       to = (gdb_byte *) (*fpregsetp + 32) + 4;
-      store_signed_integer (to, 4, byte_order, val);
+      regcache->raw_collect_integer (regno, to, 4, true);
     }
   else if (regno == -1)
     {
diff --git a/gdb/regcache.h b/gdb/regcache.h
index 4dcfccbac70f0f962bf5e5596d035fda42322795..409482d17c0542c7a53620d88d33fa9706fa72c5 100644
--- a/gdb/regcache.h
+++ b/gdb/regcache.h
@@ -294,8 +294,14 @@  public:

   void raw_collect (int regnum, void *buf) const;

+  void raw_collect_integer (int regnum, gdb_byte *addr, int addr_len,
+			    bool is_signed) const;
+
   void raw_supply (int regnum, const void *buf);

+  void raw_supply_integer (int regnum, const gdb_byte *addr, int addr_len,
+			   bool is_signed);
+
   void raw_supply_zeroed (int regnum);

   void raw_copy (int regnum, struct regcache *src_regcache);
diff --git a/gdb/regcache.c b/gdb/regcache.c
index 660558f7ff10f9d8346b6e08422e16c38c3c4d7d..ec6446b897922a8f9f44bbf94b7f1d198b0a6d4b 100644
--- a/gdb/regcache.c
+++ b/gdb/regcache.c
@@ -1189,6 +1189,28 @@  regcache::raw_supply (int regnum, const void *buf)
     }
 }

+/* Supply register REGNUM with an integer, whose contents are stored in ADDR,
+   with length ADDR_LEN and sign IS_SIGNED, to REGCACHE.  */
+
+void
+regcache::raw_supply_integer (int regnum, const gdb_byte *addr, int addr_len,
+			      bool is_signed)
+{
+  enum bfd_endian byte_order = gdbarch_byte_order (m_descr->gdbarch);
+  gdb_byte *regbuf;
+  size_t regsize;
+
+  gdb_assert (regnum >= 0 && regnum < m_descr->nr_raw_registers);
+  gdb_assert (!m_readonly_p);
+
+  regbuf = register_buffer (regnum);
+  regsize = m_descr->sizeof_register[regnum];
+
+  copy_integer_to_size (regbuf, regsize, addr, addr_len, is_signed,
+			byte_order);
+  m_register_status[regnum] = REG_VALID;
+}
+
 /* Supply register REGNUM with zeroed value to REGCACHE.  This is not the same
    as calling raw_supply with NULL (which will set the state to
    unavailable).  */
@@ -1232,6 +1254,26 @@  regcache::raw_collect (int regnum, void *buf) const
   memcpy (buf, regbuf, size);
 }

+/* Collect register REGNUM from regcache to an integer, whose contents are
+   stored in ADDR, with length ADDR_LEN and sign IS_SIGNED.  */
+
+void
+regcache::raw_collect_integer (int regnum, gdb_byte *addr, int addr_len,
+			       bool is_signed) const
+{
+  enum bfd_endian byte_order = gdbarch_byte_order (m_descr->gdbarch);
+  const gdb_byte *regbuf;
+  size_t regsize;
+
+  gdb_assert (regnum >= 0 && regnum < m_descr->nr_raw_registers);
+
+  regbuf = register_buffer (regnum);
+  regsize = m_descr->sizeof_register[regnum];
+
+  copy_integer_to_size (addr, addr_len, regbuf, regsize, is_signed,
+			byte_order);
+}
+
 void
 regcache::raw_copy (int regnum, struct regcache *src_regcache)
 {