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

Message ID 4F90CD36-759D-4BDA-BFEC-8DD86F44A0B7@arm.com
State New, archived
Headers

Commit Message

Alan Hayward May 23, 2017, 5:49 p.m. UTC
  > On 22 May 2017, at 18:15, Pedro Alves <palves@redhat.com> wrote:
> 
> On 05/22/2017 05:05 PM, Alan Hayward wrote:
> 
>>     {
>>       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);
>> +      val = extract_integer<LONGEST> ((const gdb_byte *) addr, len, byte_order);
>> +      regcache->raw_supply (regnum, val);
>>     }
>> }
>> 
> 
>>   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);
>> +      LONGEST val = regcache->raw_collect<LONGEST> (regnum);
>> +      store_integer ((gdb_byte *) addr, len, byte_order, val);
> 
> I wonder whether we can get rid of the LONGEST / host integer
> middleman and simplify things while at it.  For instance, what if we
> had a version of raw_collect that took the destination buffer length
> as parameter:
> 
>      regcache->raw_collect_integer (regnum, (gdb_byte *) addr, len);
> 
> that would copy bytes over into addr, and if the register is
> narrower than LEN, then it'd insert the necessary
> leading zeros (or 0xFFs if signed extension necessary),
> and if the registers is wider than LEN, then it'd skip
> copying enough significant bytes so that LEN fits.
> 
> Likewise for regcache->raw_supply.

Is it the case that gdb always does a store_integer after a raw_collect
of a (U)LONGEST?
And always an extract_integer before a raw_supply of a (U)LONGEST ?
(Both of these are tricky to grep for, because the code sequence is over
multiple lines)

I was going to mock up a new raw_collect_integer, but then got carried
away and wrote the full patch changes.
This version makes the MIPS files look neater.


> 
> 
>> --- a/gdb/regcache.h
>> +++ b/gdb/regcache.h
>> @@ -21,6 +21,7 @@
>> #define REGCACHE_H
>> 
>> #include "common-regcache.h"
>> +#include "defs.h"
> 
> Headers should not include defs.h.  Is there some .c file that
> misses including defs.h first thing?
> 

I think I needed it in an earlier version of my patch.
Removed.


New version with raw_collect_integer and raw_supply_integer.

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-23  Alan Hayward  <alan.hayward@arm.com>

	* gdb/defs.h (LongType): New templated type.
	(extract_integer): New declaration.
	(extract_signed_integer): Switched to inline.
	(extract_unsigned_integer): Likewise.
	(store_integer): New declaration
	(store_signed_integer): Switched to inline.
	(store_unsigned_integer): Likewise.
	* 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 and
	instantiations.
	(regcache::raw_collect_integer): Likewise
	* gdb/regcache.h (regcache::raw_supply): New declaration.
	(regcache::raw_collect): Likewise
  

Comments

Pedro Alves May 23, 2017, 6:30 p.m. UTC | #1
On 05/23/2017 06:49 PM, Alan Hayward wrote:
> 
>> On 22 May 2017, at 18:15, Pedro Alves <palves@redhat.com> wrote:
>> I wonder whether we can get rid of the LONGEST / host integer
>> middleman and simplify things while at it.  For instance, what if we
>> had a version of raw_collect that took the destination buffer length
>> as parameter:
>>
>>      regcache->raw_collect_integer (regnum, (gdb_byte *) addr, len);
>>
>> that would copy bytes over into addr, and if the register is
>> narrower than LEN, then it'd insert the necessary
>> leading zeros (or 0xFFs if signed extension necessary),
>> and if the registers is wider than LEN, then it'd skip
>> copying enough significant bytes so that LEN fits.
>>
>> Likewise for regcache->raw_supply.
> 
> Is it the case that gdb always does a store_integer after a raw_collect
> of a (U)LONGEST?
> And always an extract_integer before a raw_supply of a (U)LONGEST ?
> (Both of these are tricky to grep for, because the code sequence is over
> multiple lines)

The observation here is that we're transferring integer data between
two places that seemingly both use target formatting:

 target integer 1 -> host integer -> target integer 2

when the target integer 1 and 2 have the same sizes, then we
copy data directly without the host integer middle man.
But when they don't have the same size, we do the host integer
conversion steps.  I was questioning having that step in the
first place.  If the target integers have different sizes,
we'll either end up with zero/sign extension, or truncation.
It seems to me offhand that doing those directly in the destination
buffer shouldn't be difficult?

> 
> I was going to mock up a new raw_collect_integer, but then got carried
> away and wrote the full patch changes.
> This version makes the MIPS files look neater.

Hmm, I think you may have misunderstood.  The main point was to
avoid having to have T/LONGEST temporary at all here:

> +template<typename T>
> +typename std::enable_if<(std::is_same<T, LONGEST>::value
> +			 || std::is_same<T, ULONGEST>::value),
> +			void>::type
> +regcache::raw_supply_integer (int regnum, const gdb_byte *addr, int addr_len)
> +{
> +  enum bfd_endian byte_order = gdbarch_byte_order (m_descr->gdbarch);
> +  gdb_byte *regbuf;
> +  size_t regsize;
> +  T val;
> +
> +  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];
> +
> +  val = extract_integer<T> (addr, addr_len, byte_order);
> +  store_integer (regbuf, regsize, byte_order, val);
> +  m_register_status[regnum] = REG_VALID;

and maybe the need for all the templating.

I.e., in the cases at hand, both the regcache buffer an
the ADDR/LEN buffer are in target format, with the mismatch
being in integer width (i.e., may need zero/sign extension or
truncation), so it seems to me that we should be able to copy
data to/from the register buffer directly, without having to
convert to host format (the T / LONGEST) as an intermediate step.

So basically if we start with that you have, what we'd need
is a version of store_integer that takes a ADDR/LEN pair
instead of a T val.

Thanks,
Pedro Alves
  
Alan Hayward May 24, 2017, 9:07 a.m. UTC | #2
> On 23 May 2017, at 19:30, Pedro Alves <palves@redhat.com> wrote:

> 

> On 05/23/2017 06:49 PM, Alan Hayward wrote:

>> 

>>> On 22 May 2017, at 18:15, Pedro Alves <palves@redhat.com> wrote:

>>> I wonder whether we can get rid of the LONGEST / host integer

>>> middleman and simplify things while at it.  For instance, what if we

>>> had a version of raw_collect that took the destination buffer length

>>> as parameter:

>>> 

>>>     regcache->raw_collect_integer (regnum, (gdb_byte *) addr, len);

>>> 

>>> that would copy bytes over into addr, and if the register is

>>> narrower than LEN, then it'd insert the necessary

>>> leading zeros (or 0xFFs if signed extension necessary),

>>> and if the registers is wider than LEN, then it'd skip

>>> copying enough significant bytes so that LEN fits.

>>> 

>>> Likewise for regcache->raw_supply.

>> 

>> Is it the case that gdb always does a store_integer after a raw_collect

>> of a (U)LONGEST?

>> And always an extract_integer before a raw_supply of a (U)LONGEST ?

>> (Both of these are tricky to grep for, because the code sequence is over

>> multiple lines)

> 

> The observation here is that we're transferring integer data between

> two places that seemingly both use target formatting:

> 

> target integer 1 -> host integer -> target integer 2

> 

> when the target integer 1 and 2 have the same sizes, then we

> copy data directly without the host integer middle man.

> But when they don't have the same size, we do the host integer

> conversion steps.  I was questioning having that step in the

> first place.  If the target integers have different sizes,

> we'll either end up with zero/sign extension, or truncation.

> It seems to me offhand that doing those directly in the destination

> buffer shouldn't be difficult?

> 

>> 

>> I was going to mock up a new raw_collect_integer, but then got carried

>> away and wrote the full patch changes.

>> This version makes the MIPS files look neater.

> 

> Hmm, I think you may have misunderstood.  The main point was to

> avoid having to have T/LONGEST temporary at all here:

> 

>> +template<typename T>

>> +typename std::enable_if<(std::is_same<T, LONGEST>::value

>> +			 || std::is_same<T, ULONGEST>::value),

>> +			void>::type

>> +regcache::raw_supply_integer (int regnum, const gdb_byte *addr, int addr_len)

>> +{

>> +  enum bfd_endian byte_order = gdbarch_byte_order (m_descr->gdbarch);

>> +  gdb_byte *regbuf;

>> +  size_t regsize;

>> +  T val;

>> +

>> +  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];

>> +

>> +  val = extract_integer<T> (addr, addr_len, byte_order);

>> +  store_integer (regbuf, regsize, byte_order, val);

>> +  m_register_status[regnum] = REG_VALID;

> 

> and maybe the need for all the templating.

> 


Would still need to have eater signed/unsigned versions of the functions,
or maybe have “bool signed” parameter.


> I.e., in the cases at hand, both the regcache buffer an

> the ADDR/LEN buffer are in target format, with the mismatch

> being in integer width (i.e., may need zero/sign extension or

> truncation), so it seems to me that we should be able to copy

> data to/from the register buffer directly, without having to

> convert to host format (the T / LONGEST) as an intermediate step.

> 

> So basically if we start with that you have, what we'd need

> is a version of store_integer that takes a ADDR/LEN pair

> instead of a T val.

> 


Are you suggesting something like this:
(Warning - this snippet may not even compile, and I’m not sure on the endian
logic)


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);
  void *regbuf;
  size_t regsize;

  gdb_assert (regnum >= 0 && regnum < m_descr->nr_raw_registers);
  gdb_assert (!m_readonly_p);
  gdb_assert (addr != 0);
  gdb_assert (addr_len <= regsize);

  regbuf = register_buffer (regnum);
  regsize = m_descr->sizeof_register[regnum];

  copy_and_fill_to_size (regbuf, addr, addr_len, regsize, is_signed, byte_order);
  m_register_status[regnum] = REG_VALID;
}

/* Copy COPY_LEN bytes from SOURCE to DEST, then sign extend or zero extend
   to FILL_LEN bytes.  */
void copy_and_fill_to_size (const gdb_byte *dest, const gdb_byte *source,
			    int copy_len, int fill_len, bool is_signed,
			    enum bfd_endian byte_order)
{
  signed int len_diff = fill_len - copy_len;
  gdb_assert (len_diff >= 0);

  if (byte_order == BFD_ENDIAN_BIG)
    memcpy (dest+len_diff, source, copy_len);
  else
    memcpy (dest, source, copy_len);

  if (len_diff > 0)
    {
      if (signed)
      	// TODO: sign extend from copy_len to fill_len
      else
	{
	  if (byte_order == BFD_ENDIAN_BIG)
      	    memset (dest, 0, len_diff);
      	  else
      	    memset (dest+copy_len, 0, len_diff);
	}
    }
}


Meanwhile raw_collect_integer doesn’t need a signed parameter:

void
regcache::raw_collect_integer (int regnum, gdb_byte *addr, int addr_len) 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);
  gdb_assert (addr_len <= regsize);

  regbuf = register_buffer (regnum);
  regsize = m_descr->sizeof_register[regnum];

  if (byte_order == BFD_ENDIAN_BIG)
    regbuf += regsize - addr_len;

  memcpy (addr, regbuf, addr_len);
}



> Thanks,

> Pedro Alves

>
  
Pedro Alves May 24, 2017, 9:20 a.m. UTC | #3
On 05/24/2017 10:07 AM, Alan Hayward wrote:

>> Hmm, I think you may have misunderstood.  The main point was to
>> avoid having to have T/LONGEST temporary at all here:

...

>>
>> and maybe the need for all the templating.
>>
> 
> Would still need to have eater signed/unsigned versions of the functions,
> or maybe have “bool signed” parameter.

Parameter sounds fine to me.

> Are you suggesting something like this:
> (Warning - this snippet may not even compile, and I’m not sure on the endian
> logic)

Yes, exactly.  WDYT?

> Meanwhile raw_collect_integer doesn’t need a signed parameter:

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

Thanks,
Pedro Alves
  
Pedro Alves May 24, 2017, 9:29 a.m. UTC | #4
On 05/24/2017 10:07 AM, Alan Hayward wrote:
> /* Copy COPY_LEN bytes from SOURCE to DEST, then sign extend or zero extend
>    to FILL_LEN bytes.  */
> void copy_and_fill_to_size (const gdb_byte *dest, const gdb_byte *source,
> 			    int copy_len, int fill_len, bool is_signed,
> 			    enum bfd_endian byte_order)
> {
>   signed int len_diff = fill_len - copy_len;
>   gdb_assert (len_diff >= 0);
> 
>   if (byte_order == BFD_ENDIAN_BIG)
>     memcpy (dest+len_diff, source, copy_len);
>   else
>     memcpy (dest, source, copy_len);

Note here I was thinking you'd need to handle truncation as well.
I.e., only copy fill_len bytes when fill_len is narrower than
copy_len.  So I'd probably rename copy_len/fill_len to dest_len/source_len
to match 'dest' and 'source', and name the function something
else that doesn't have "fill" in it.

Thanks,
Pedro Alves
  
Alan Hayward May 24, 2017, 10:20 a.m. UTC | #5
> On 24 May 2017, at 10:20, Pedro Alves <palves@redhat.com> wrote:

> 

> On 05/24/2017 10:07 AM, Alan Hayward wrote:

> 

>>> Hmm, I think you may have misunderstood.  The main point was to

>>> avoid having to have T/LONGEST temporary at all here:

> 

> ...

> 

>>> 

>>> and maybe the need for all the templating.

>>> 

>> 

>> Would still need to have eater signed/unsigned versions of the functions,

>> or maybe have “bool signed” parameter.

> 

> Parameter sounds fine to me.

> 

>> Are you suggesting something like this:

>> (Warning - this snippet may not even compile, and I’m not sure on the endian

>> logic)

> 

> Yes, exactly.  WDYT?


I’m happy in principle with it (after fix ups)

> 

>> 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?


> 

> On 24 May 2017, at 10:29, Pedro Alves <palves@redhat.com> wrote:

> 

> On 05/24/2017 10:07 AM, Alan Hayward wrote:

>> /* Copy COPY_LEN bytes from SOURCE to DEST, then sign extend or zero extend

>>   to FILL_LEN bytes.  */

>> void copy_and_fill_to_size (const gdb_byte *dest, const gdb_byte *source,

>> 			    int copy_len, int fill_len, bool is_signed,

>> 			    enum bfd_endian byte_order)

>> {

>>  signed int len_diff = fill_len - copy_len;

>>  gdb_assert (len_diff >= 0);

>> 

>>  if (byte_order == BFD_ENDIAN_BIG)

>>    memcpy (dest+len_diff, source, copy_len);

>>  else

>>    memcpy (dest, source, copy_len);

> 

> Note here I was thinking you'd need to handle truncation as well.

> I.e., only copy fill_len bytes when fill_len is narrower than

> copy_len.  So I'd probably rename copy_len/fill_len to dest_len/source_len

> to match 'dest' and 'source', and name the function something

> else that doesn't have "fill" in it.


Agreed (regardless of the outcome for the previous question).


Alan.
  
Pedro Alves May 24, 2017, 11:07 a.m. UTC | #6
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
  

Patch

diff --git a/gdb/defs.h b/gdb/defs.h
index a0b586f401eca205334e9f237081f4da97c83aa1..010fe55a4760ebc7a420115e7590199fcab899a0 100644
--- a/gdb/defs.h
+++ b/gdb/defs.h
@@ -627,6 +627,11 @@  enum symbol_needs_kind
   SYMBOL_NEEDS_FRAME
 };

+template<typename T>
+using LongType = typename std::enable_if<(std::is_same<T, LONGEST>::value
+					  || std::is_same<T, ULONGEST>::value),
+					 T>::type;
+
 /* Dynamic target-system-dependent parameters for GDB.  */
 #include "gdbarch.h"

@@ -637,11 +642,69 @@  enum { MAX_REGISTER_SIZE = 64 };

 /* In findvar.c.  */

-extern LONGEST extract_signed_integer (const gdb_byte *, int,
-				       enum bfd_endian);
+/* All 'extract' functions return a host-format integer from a target-format
+   integer at ADDR which is LEN bytes long.  */

-extern ULONGEST extract_unsigned_integer (const gdb_byte *, int,
-					  enum bfd_endian);
+template<typename T>
+LongType<T>
+extract_integer (const gdb_byte *addr, int len, enum bfd_endian byte_order)
+{
+  T retval = 0;
+  const unsigned char *p;
+  const unsigned char *startaddr = addr;
+  const unsigned char *endaddr = startaddr + len;
+
+  if (len > (int) sizeof (T))
+    error (_("\
+That operation is not available on integers of more than %d bytes."),
+	   (int) sizeof (T));
+
+  /* Start at the most significant end of the integer, and work towards
+     the least significant.  */
+  if (byte_order == BFD_ENDIAN_BIG)
+    {
+      p = startaddr;
+      if (std::is_signed<T>::value)
+	{
+	  /* Do the sign extension once at the start.  */
+	  retval = ((LONGEST) * p ^ 0x80) - 0x80;
+	  ++p;
+	}
+      for (; p < endaddr; ++p)
+	retval = (retval << 8) | *p;
+    }
+  else
+    {
+      p = endaddr - 1;
+      if (std::is_signed<T>::value)
+	{
+	  /* Do the sign extension once at the start.  */
+	  retval = ((LONGEST) * p ^ 0x80) - 0x80;
+	  --p;
+	}
+      for (; p >= startaddr; --p)
+	retval = (retval << 8) | *p;
+    }
+  return retval;
+}
+
+template LongType<ULONGEST> extract_integer<ULONGEST>
+  (const gdb_byte *addr, int len, enum bfd_endian byte_order);
+
+template LongType<LONGEST> extract_integer<LONGEST>
+  (const gdb_byte *addr, int len, enum bfd_endian byte_order);
+
+inline ULONGEST extract_unsigned_integer (const gdb_byte *addr, int len,
+					  enum bfd_endian byte_order)
+{
+  return extract_integer<ULONGEST> (addr, len, byte_order);
+}
+
+inline LONGEST extract_signed_integer (const gdb_byte *addr, int len,
+				       enum bfd_endian byte_order)
+{
+  return extract_integer<LONGEST> (addr, len, byte_order);
+}

 extern int extract_long_unsigned_integer (const gdb_byte *, int,
 					  enum bfd_endian, LONGEST *);
@@ -649,11 +712,58 @@  extern int extract_long_unsigned_integer (const gdb_byte *, int,
 extern CORE_ADDR extract_typed_address (const gdb_byte *buf,
 					struct type *type);

-extern void store_signed_integer (gdb_byte *, int,
-				  enum bfd_endian, LONGEST);
+/* All 'store' functions accept a host-format integer and store a
+   target-format integer at ADDR which is LEN bytes long.  */

-extern void store_unsigned_integer (gdb_byte *, int,
-				    enum bfd_endian, ULONGEST);
+template<typename T>
+typename std::enable_if<(std::is_same<T, LONGEST>::value
+			 || std::is_same<T, ULONGEST>::value),
+			void>::type
+store_integer (gdb_byte *addr, int len, enum bfd_endian byte_order,
+	       T val)
+{
+  gdb_byte *p;
+  gdb_byte *startaddr = addr;
+  gdb_byte *endaddr = startaddr + len;
+
+  /* Start at the least significant end of the integer, and work towards
+     the most significant.  */
+  if (byte_order == BFD_ENDIAN_BIG)
+    {
+      for (p = endaddr - 1; p >= startaddr; --p)
+	{
+	  *p = val & 0xff;
+	  val >>= 8;
+	}
+    }
+  else
+    {
+      for (p = startaddr; p < endaddr; ++p)
+	{
+	  *p = val & 0xff;
+	  val >>= 8;
+	}
+    }
+}
+
+template void store_integer (gdb_byte *addr, int len,
+				       enum bfd_endian byte_order,
+				       LongType<ULONGEST> val);
+template void store_integer (gdb_byte *addr, int len,
+				      enum bfd_endian byte_order,
+				      LongType<LONGEST> val);
+
+inline void store_signed_integer (gdb_byte *addr, int len,
+				  enum bfd_endian byte_order, LONGEST val)
+{
+  store_integer (addr, len, byte_order, val);
+}
+
+inline void store_unsigned_integer (gdb_byte *addr, int len,
+				    enum bfd_endian byte_order, ULONGEST val)
+{
+  store_integer (addr, len, byte_order, val);
+}

 extern void store_typed_address (gdb_byte *buf, struct type *type,
 				 CORE_ADDR addr);
diff --git a/gdb/findvar.c b/gdb/findvar.c
index ed4d5c1266c9de069981b306bc8229ae5fb02350..614d1291eac5a7438a1bf3f6dd0b5d012a0dfaa7 100644
--- a/gdb/findvar.c
+++ b/gdb/findvar.c
@@ -34,9 +34,7 @@ 
 #include "language.h"
 #include "dwarf2loc.h"

-/* Basic byte-swapping routines.  All 'extract' functions return a
-   host-format integer from a target-format integer at ADDR which is
-   LEN bytes long.  */
+/* Basic byte-swapping routines.  */

 #if TARGET_CHAR_BIT != 8 || HOST_CHAR_BIT != 8
   /* 8 bit characters are a pretty safe assumption these days, so we
@@ -46,70 +44,6 @@ 
 you lose
 #endif

-LONGEST
-extract_signed_integer (const gdb_byte *addr, int len,
-			enum bfd_endian byte_order)
-{
-  LONGEST retval;
-  const unsigned char *p;
-  const unsigned char *startaddr = addr;
-  const unsigned char *endaddr = startaddr + len;
-
-  if (len > (int) sizeof (LONGEST))
-    error (_("\
-That operation is not available on integers of more than %d bytes."),
-	   (int) sizeof (LONGEST));
-
-  /* Start at the most significant end of the integer, and work towards
-     the least significant.  */
-  if (byte_order == BFD_ENDIAN_BIG)
-    {
-      p = startaddr;
-      /* Do the sign extension once at the start.  */
-      retval = ((LONGEST) * p ^ 0x80) - 0x80;
-      for (++p; p < endaddr; ++p)
-	retval = (retval << 8) | *p;
-    }
-  else
-    {
-      p = endaddr - 1;
-      /* Do the sign extension once at the start.  */
-      retval = ((LONGEST) * p ^ 0x80) - 0x80;
-      for (--p; p >= startaddr; --p)
-	retval = (retval << 8) | *p;
-    }
-  return retval;
-}
-
-ULONGEST
-extract_unsigned_integer (const gdb_byte *addr, int len,
-			  enum bfd_endian byte_order)
-{
-  ULONGEST retval;
-  const unsigned char *p;
-  const unsigned char *startaddr = addr;
-  const unsigned char *endaddr = startaddr + len;
-
-  if (len > (int) sizeof (ULONGEST))
-    error (_("\
-That operation is not available on integers of more than %d bytes."),
-	   (int) sizeof (ULONGEST));
-
-  /* Start at the most significant end of the integer, and work towards
-     the least significant.  */
-  retval = 0;
-  if (byte_order == BFD_ENDIAN_BIG)
-    {
-      for (p = startaddr; p < endaddr; ++p)
-	retval = (retval << 8) | *p;
-    }
-  else
-    {
-      for (p = endaddr - 1; p >= startaddr; --p)
-	retval = (retval << 8) | *p;
-    }
-  return retval;
-}

 /* Sometimes a long long unsigned integer can be extracted as a
    LONGEST value.  This is done so that we can print these values
@@ -177,65 +111,6 @@  extract_typed_address (const gdb_byte *buf, struct type *type)
   return gdbarch_pointer_to_address (get_type_arch (type), type, buf);
 }

-/* All 'store' functions accept a host-format integer and store a
-   target-format integer at ADDR which is LEN bytes long.  */
-
-void
-store_signed_integer (gdb_byte *addr, int len,
-		      enum bfd_endian byte_order, LONGEST val)
-{
-  gdb_byte *p;
-  gdb_byte *startaddr = addr;
-  gdb_byte *endaddr = startaddr + len;
-
-  /* Start at the least significant end of the integer, and work towards
-     the most significant.  */
-  if (byte_order == BFD_ENDIAN_BIG)
-    {
-      for (p = endaddr - 1; p >= startaddr; --p)
-	{
-	  *p = val & 0xff;
-	  val >>= 8;
-	}
-    }
-  else
-    {
-      for (p = startaddr; p < endaddr; ++p)
-	{
-	  *p = val & 0xff;
-	  val >>= 8;
-	}
-    }
-}
-
-void
-store_unsigned_integer (gdb_byte *addr, int len,
-			enum bfd_endian byte_order, ULONGEST val)
-{
-  unsigned char *p;
-  unsigned char *startaddr = (unsigned char *) addr;
-  unsigned char *endaddr = startaddr + len;
-
-  /* Start at the least significant end of the integer, and work towards
-     the most significant.  */
-  if (byte_order == BFD_ENDIAN_BIG)
-    {
-      for (p = endaddr - 1; p >= startaddr; --p)
-	{
-	  *p = val & 0xff;
-	  val >>= 8;
-	}
-    }
-  else
-    {
-      for (p = startaddr; p < endaddr; ++p)
-	{
-	  *p = val & 0xff;
-	  val >>= 8;
-	}
-    }
-}
-
 /* Store the address ADDR as a pointer of type TYPE at BUF, in target
    form.  */
 void
diff --git a/gdb/mips-fbsd-tdep.c b/gdb/mips-fbsd-tdep.c
index 00fae0ec60ddc9e645d3236efe29f2f9e9ceab5c..7278fb71e3ad1f18d35cd696832d99bf90198620 100644
--- a/gdb/mips-fbsd-tdep.c
+++ b/gdb/mips-fbsd-tdep.c
@@ -48,9 +48,8 @@ 
 #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().  */
+   size the regcache expects, this can use regcache->raw_supply ().  If
+   they are different, this can use regcache->raw_supply_integer ().  */

 static void
 mips_fbsd_supply_reg (struct regcache *regcache, int regnum, const void *addr,
@@ -59,25 +58,16 @@  mips_fbsd_supply_reg (struct regcache *regcache, int regnum, const void *addr,
   struct gdbarch *gdbarch = get_regcache_arch (regcache);

   if (register_size (gdbarch, regnum) == len)
-    regcache_raw_supply (regcache, regnum, addr);
+    regcache->raw_supply (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<LONGEST> (regnum, (const gdb_byte *) addr,
+					   len);
 }

 /* 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.  */
+   regcache->raw_collect ().  If they are different, this can use
+   regcache->raw_collect_integer ().  */

 static void
 mips_fbsd_collect_reg (const struct regcache *regcache, int regnum, void *addr,
@@ -86,18 +76,9 @@  mips_fbsd_collect_reg (const struct regcache *regcache, int regnum, void *addr,
   struct gdbarch *gdbarch = get_regcache_arch (regcache);

   if (register_size (gdbarch, regnum) == len)
-    regcache_raw_collect (regcache, regnum, addr);
+    regcache->raw_collect (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<LONGEST> (regnum, (gdb_byte *) addr, len);
 }

 /* 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..b30fe294a0bbbb815418e65149a88f59eb15befb 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<LONGEST> (regnum, (const gdb_byte *) addr, 4);
 }

 /* 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<LONGEST> (regno, (gdb_byte *) dst, 8);
     }
 }

@@ -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<LONGEST> (regno, to, 4);
     }
   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<LONGEST> (regno, to, 4);
     }
   else if (regno == -1)
     {
diff --git a/gdb/regcache.h b/gdb/regcache.h
index 4dcfccbac70f0f962bf5e5596d035fda42322795..c47401948270b1c75d9d3b02cbbc3256db85f84e 100644
--- a/gdb/regcache.h
+++ b/gdb/regcache.h
@@ -294,8 +294,20 @@  public:

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

+  template<typename T>
+  typename std::enable_if<(std::is_same<T, LONGEST>::value
+			 || std::is_same<T, ULONGEST>::value),
+			void>::type
+  raw_collect_integer (int regnum, gdb_byte *addr, int addr_len) const;
+
   void raw_supply (int regnum, const void *buf);

+  template<typename T>
+  typename std::enable_if<(std::is_same<T, LONGEST>::value
+			   || std::is_same<T, ULONGEST>::value),
+			   void>::type
+  raw_supply_integer (int regnum, const gdb_byte *addr, int addr_len);
+
   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..cc9ab74b087cbda2d79264674e9b463e1528363f 100644
--- a/gdb/regcache.c
+++ b/gdb/regcache.c
@@ -1189,6 +1189,39 @@  regcache::raw_supply (int regnum, const void *buf)
     }
 }

+/* Supply register REGNUM, whose contents are stored in ADDR, with length
+   ADDR_LEN to a (U)LONGEST, then store to REGCACHE, taking BYTE_ORDER into
+   account.  */
+
+template<typename T>
+typename std::enable_if<(std::is_same<T, LONGEST>::value
+			 || std::is_same<T, ULONGEST>::value),
+			void>::type
+regcache::raw_supply_integer (int regnum, const gdb_byte *addr, int addr_len)
+{
+  enum bfd_endian byte_order = gdbarch_byte_order (m_descr->gdbarch);
+  gdb_byte *regbuf;
+  size_t regsize;
+  T val;
+
+  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];
+
+  val = extract_integer<T> (addr, addr_len, byte_order);
+  store_integer (regbuf, regsize, byte_order, val);
+  m_register_status[regnum] = REG_VALID;
+}
+
+template void regcache::raw_supply_integer<ULONGEST> (int regnum,
+						      const gdb_byte *addr,
+						      int addr_len);
+template void regcache::raw_supply_integer<LONGEST> (int regnum,
+						     const gdb_byte *addr,
+						     int addr_len);
+
 /* 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 +1265,37 @@  regcache::raw_collect (int regnum, void *buf) const
   memcpy (buf, regbuf, size);
 }

+/* Collect register REGNUM from REGCACHE into a (U)LONGEST, then store ADDR_LEN
+   bytes into buffer ADDR, taking BYTE_ORDER into account.  */
+
+template<typename T>
+typename std::enable_if<(std::is_same<T, LONGEST>::value
+			 || std::is_same<T, ULONGEST>::value),
+			void>::type
+regcache::raw_collect_integer (int regnum, gdb_byte *addr, int addr_len) const
+{
+  enum bfd_endian byte_order = gdbarch_byte_order (m_descr->gdbarch);
+  const gdb_byte *regbuf;
+  size_t regsize;
+  T val;
+
+  gdb_assert (regnum >= 0 && regnum < m_descr->nr_raw_registers);
+
+  regbuf = register_buffer (regnum);
+  regsize = m_descr->sizeof_register[regnum];
+
+  val = extract_integer<T> (regbuf, regsize, byte_order);
+  store_integer (addr, addr_len, byte_order, val);
+}
+
+template void regcache::raw_collect_integer<ULONGEST> (int regnum,
+						       gdb_byte *addr,
+						       int addr_len) const;
+template void regcache::raw_collect_integer<LONGEST> (int regnum,
+						      gdb_byte *addr,
+						      int addr_len) const;
+
+
 void
 regcache::raw_copy (int regnum, struct regcache *src_regcache)
 {