This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [PATCH 3/11] Add MIPS_MAX_REGISTER_SIZE (2/4)
> On 25 May 2017, at 12:48, Pedro Alves <palves@redhat.com> wrote:
>
> 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.
>
Agreed.
I considered removing, but it adds lots of extra arguments everywhere.
Unit tests added as suggested in the other email.
Using store_unsigned_integer and extract_unsigned_integer means that
I don't have to worry about endian specifics in the test.
I could template the two tests, but it looks cleaner this way.
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-25 Alan Hayward <alan.hayward@arm.com>
* defs.h (copy_integer_to_size): New declaration.
* findvar.c (copy_integer_to_size): New function.
(do_cuint_test): New selftest function.
(do_cint_test): New selftest function.
(copy_integer_to_size_test): Likewise.
(_initialize_findvar): Likewise.
* mips-fbsd-tdep.c (mips_fbsd_supply_reg): Use raw_supply_integer.
(mips_fbsd_collect_reg): Use 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.
* regcache.c (regcache::raw_supply_integer): New function.
(regcache::raw_collect_integer): Likewise.
* regcache.h: (regcache::raw_supply_integer): New declaration.
(regcache::raw_collect_integer): 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..bf72b2dc7710ca21b16056b8f0f274044bf744ca 100644
--- a/gdb/findvar.c
+++ b/gdb/findvar.c
@@ -33,6 +33,7 @@
#include "objfiles.h"
#include "language.h"
#include "dwarf2loc.h"
+#include "selftest.h"
/* Basic byte-swapping routines. All 'extract' functions return a
host-format integer from a target-format integer at ADDR which is
@@ -249,7 +250,46 @@ 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)
+ {
+ gdb_byte extension = 0;
+ if (is_signed
+ && ((byte_order != BFD_ENDIAN_BIG && source[source_size - 1] & 0x80)
+ || (byte_order == BFD_ENDIAN_BIG && source[0] & 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
@@ -1005,3 +1045,78 @@ address_from_register (int regnum, struct frame_info *frame)
return result;
}
+#if GDB_SELF_TEST
+namespace selftests {
+namespace findvar_tests {
+
+/* Functions to test copy_integer_to_size. Store SOURCE_VAL with size
+ SOURCE_SIZE to a buffer, making sure no sign extending happens at this
+ stage. Copy buffer to a new buffer using copy_integer_to_size. Extract
+ copied value and compare to DEST_VAL. Do all of this for both LITTLE and
+ BIG target endians. */
+
+static void
+do_cuint_test (long dest_val, int dest_size, ULONGEST src_val, int src_size)
+{
+ for (int i = 0; i < 2 ; i++)
+ {
+ ULONGEST srcbuf_val = 0, destbuf_val = 0;
+ gdb_byte* srcbuf = (gdb_byte*) &srcbuf_val;
+ gdb_byte* destbuf = (gdb_byte*) &destbuf_val;
+ enum bfd_endian byte_order = i ? BFD_ENDIAN_BIG : BFD_ENDIAN_LITTLE;
+
+ store_unsigned_integer (srcbuf, src_size, byte_order, src_val);
+ copy_integer_to_size (destbuf, dest_size, srcbuf, src_size, false,
+ byte_order);
+ SELF_CHECK (dest_val == extract_unsigned_integer (destbuf, dest_size,
+ byte_order));
+ }
+}
+
+static void
+do_cint_test (long dest_val, int dest_size, LONGEST src_val, int src_size)
+{
+ for (int i = 0; i < 2 ; i++)
+ {
+ LONGEST srcbuf_val = 0, destbuf_val = 0;
+ gdb_byte* srcbuf = (gdb_byte*) &srcbuf_val;
+ gdb_byte* destbuf = (gdb_byte*) &destbuf_val;
+ enum bfd_endian byte_order = i ? BFD_ENDIAN_LITTLE : BFD_ENDIAN_BIG;
+
+ store_unsigned_integer (srcbuf, src_size, byte_order, src_val);
+ copy_integer_to_size (destbuf, dest_size, srcbuf, src_size, true,
+ byte_order);
+ SELF_CHECK (dest_val == extract_signed_integer (destbuf, dest_size,
+ byte_order));
+ }
+}
+
+static void
+copy_integer_to_size_test ()
+{
+ do_cuint_test (0x12345678, 8, 0x12345678, 4);
+ do_cuint_test (0x345678, 8, 0x12345678, 3);
+ do_cuint_test (0x5678, 2, 0x12345678, 3);
+ do_cuint_test (0xdeadbeef, 8, 0xdeadbeef, 4);
+ do_cuint_test (0xadbeef, 8, 0xdeadbeef, 3);
+ do_cuint_test (0xbeef, 2, 0xdeadbeef, 3);
+ do_cint_test (0x12345678, 8, 0x12345678, 4);
+ do_cint_test (0x345678, 8, 0x12345678, 3);
+ do_cint_test (0x5678, 2, 0x12345678, 3);
+ do_cint_test (0xffffffffdeadbeef, 8, 0xdeadbeef, 4);
+ do_cint_test (0xffffffffffadbeef, 8, 0xdeadbeef, 3);
+ do_cint_test (0xffffffffffffbeef, 2, 0xbeef, 3);
+}
+
+} // namespace findvar_test
+} // namespace selftests
+
+#endif
+
+void
+_initialize_findvar (void)
+{
+#if GDB_SELF_TEST
+ register_self_test (selftests::findvar_tests::copy_integer_to_size_test);
+#endif
+}
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..90462aea73ba810bd1af9ddb8c6d02e1f0e95991 100644
--- a/gdb/regcache.c
+++ b/gdb/regcache.c
@@ -1189,6 +1189,31 @@ regcache::raw_supply (int regnum, const void *buf)
}
}
+/* Supply register REGNUM to REGCACHE. Value to supply is an integer stored at
+ address ADDR, in target endian, with length ADDR_LEN and sign IS_SIGNED. If
+ the register size is greater than ADDR_LEN, then the integer will be sign or
+ zero extended. If the register size is smaller than the integer, then the
+ most significant bytes of the integer will be truncated. */
+
+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 +1257,29 @@ regcache::raw_collect (int regnum, void *buf) const
memcpy (buf, regbuf, size);
}
+/* Collect register REGNUM from REGCACHE. Store collected value as an integer
+ at address ADDR, in target endian, with length ADDR_LEN and sign IS_SIGNED.
+ If ADDR_LEN is greater than the register size, then the integer will be sign
+ or zero extended. If ADDR_LEN is smaller than the register size, then the
+ most significant bytes of the integer will be truncated. */
+
+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)
{