[PATCH] Remove MAX_REGISTER_SIZE from regcache.c
Alan Hayward
Alan.Hayward@arm.com
Wed Apr 5 14:53:00 GMT 2017
> On 24 Mar 2017, at 10:27, Alan Hayward <Alan.Hayward@arm.com> wrote:
>
>>
>> On 24 Mar 2017, at 08:49, Yao Qi <qiyaoltc@gmail.com> wrote:
>>
>> Alan Hayward <Alan.Hayward@arm.com> writes:
>>
>>> @@ -126,6 +129,8 @@ init_regcache_descr (struct gdbarch *gdbarch)
>>> descr->register_offset[i] = offset;
>>> offset += descr->sizeof_register[i];
>>> gdb_assert (MAX_REGISTER_SIZE >= descr->sizeof_register[i]);
>>
>> Do we still need to keep MAX_REGISTER_SIZE? or you plan to remove it later.
>>
>
> I planned to keep it here until I remove MAX_REGISTER_SIZE.
> Whilst the define is still in use, we should really keep this check here.
>
>
>>> + descr->max_register_size = std::max (descr->max_register_size,
>>> + descr->sizeof_register[i]);
>>> }
>>> /* Set the real size of the raw register cache buffer. */
>>> descr->sizeof_raw_registers = offset;
>>
>>> @@ -1465,17 +1473,19 @@ regcache_dump (struct regcache *regcache, struct ui_file *file,
>>> fprintf_unfiltered (file, "Cooked value");
>>> else
>>> {
>>> - enum register_status status;
>>> + struct value *value = regcache_cooked_read_value (regcache,
>>> + regnum);
>>>
>>> - status = regcache_cooked_read (regcache, regnum, buf);
>>> - if (status == REG_UNKNOWN)
>>> - fprintf_unfiltered (file, "<invalid>");
>>
>> "<invalid>" is lost after your patch.
>
> Yes. There seems to be no way of getting an UNKNOWN status when reading using values.
>
> Looking into the code, only msp430 and nds32 seem to explicitly set status to UNKNOWN.
> Everything else looks like it will error instead of setting UNKNOWN
>
> I could add "if (regcache_register_status (regcache, regnum) == REG_UNKNOWN)"
> before calling regcache_cooked_read_value. But I think that’s not what we want, as we
> want to always try reading from the target.
>
Looking at this a little further, it looks that in reality mps430 will
never return REG_UNKNOWN. Also, nds32 has two checks at the top that should
never fire (and cause a REG_UNKNOWN), and a single return where every other
architecture has an error.
In the patch below, I've updated both those files to make them more like
other architectures. Nothing else has changed.
Tested on a --enable-targets=all build using make check with board files
unix and native-gdbserver.
I do not have a mps430 or nds32 machine to test on.
Ok to commit?
Alan.
2017-04-05 Alan Hayward <alan.hayward@arm.com>
* gdb/regcache.c (struct regcache_descr): Add max_register_size
(init_regcache_descr): Calculate maximum register size.
(max_register_size): New function.
(regcache_save): Avoid buffer use.
(regcache_restore): Use vec instead of array.
(regcache_dump): Avoid buffer use.
* gdb/regcache.h (struct regcache_descr): Add max_register_size
(max_register_size): New function.
* gdb/msp430-tdep.c (msp430_pseudo_register_read): Never return
REG_UNKNOWN.
* gdb/nds32-tdep.c (nds32_pseudo_register_read): Likewise.
diff --git a/gdb/msp430-tdep.c b/gdb/msp430-tdep.c
index 75329dfcc5ed94fff19639db4db21dd0874d0e96..d9eebf0cc2647a079db2f822145d0fb74ea301e4 100644
--- a/gdb/msp430-tdep.c
+++ b/gdb/msp430-tdep.c
@@ -221,10 +221,9 @@ msp430_pseudo_register_read (struct gdbarch *gdbarch,
struct regcache *regcache,
int regnum, gdb_byte *buffer)
{
- enum register_status status = REG_UNKNOWN;
-
if (MSP430_NUM_REGS <= regnum && regnum < MSP430_NUM_TOTAL_REGS)
{
+ enum register_status status;
ULONGEST val;
enum bfd_endian byte_order = gdbarch_byte_order (gdbarch);
int regsize = register_size (gdbarch, regnum);
@@ -234,11 +233,10 @@ msp430_pseudo_register_read (struct gdbarch *gdbarch,
if (status == REG_VALID)
store_unsigned_integer (buffer, regsize, byte_order, val);
+ return status;
}
else
gdb_assert_not_reached ("invalid pseudo register number");
-
- return status;
}
/* Implement the "pseudo_register_write" gdbarch method. */
diff --git a/gdb/nds32-tdep.c b/gdb/nds32-tdep.c
index 05c48aa27d84bc0286712f0143a9447a79ae066b..e9955d60a8132d5bc3af234dff0c86c8e59d4855 100644
--- a/gdb/nds32-tdep.c
+++ b/gdb/nds32-tdep.c
@@ -445,11 +445,11 @@ nds32_pseudo_register_read (struct gdbarch *gdbarch,
struct gdbarch_tdep *tdep = gdbarch_tdep (gdbarch);
gdb_byte reg_buf[8];
int offset, fdr_regnum;
- enum register_status status = REG_UNKNOWN;
+ enum register_status status;
/* Sanity check. */
- if (tdep->fpu_freg == -1 || tdep->use_pseudo_fsrs == 0)
- return status;
+ gdb_assert (tdep->fpu_freg != -1);
+ gdb_assert (tdep->use_pseudo_fsrs > 0);
regnum -= gdbarch_num_regs (gdbarch);
@@ -466,9 +466,11 @@ nds32_pseudo_register_read (struct gdbarch *gdbarch,
status = regcache_raw_read (regcache, fdr_regnum, reg_buf);
if (status == REG_VALID)
memcpy (buf, reg_buf + offset, 4);
+
+ return status;
}
- return status;
+ gdb_assert_not_reached ("invalid pseudo register number");
}
/* Implement the "pseudo_register_write" gdbarch method. */
diff --git a/gdb/regcache.h b/gdb/regcache.h
index 1dd4127818ba1f6fa5b9e5f2d326843833d42945..f201f0c084f40ccf276a7e1ba19050cbc11208ad 100644
--- a/gdb/regcache.h
+++ b/gdb/regcache.h
@@ -215,6 +215,8 @@ extern struct type *register_type (struct gdbarch *gdbarch, int regnum);
extern int register_size (struct gdbarch *gdbarch, int regnum);
+/* Return the size of the largest register. */
+extern long max_register_size (struct gdbarch *gdbarch);
/* Save/restore a register cache. The set of registers saved /
restored into the DST regcache determined by the save_reggroup /
diff --git a/gdb/regcache.c b/gdb/regcache.c
index 8bbd3a655c007d649b91ec0e3d2374553990923f..cc621cf248c44d6159b68e01eb130fa5bf176fb3 100644
--- a/gdb/regcache.c
+++ b/gdb/regcache.c
@@ -73,6 +73,9 @@ struct regcache_descr
/* Cached table containing the type of each register. */
struct type **register_type;
+
+ /* Size of the largest register. */
+ long max_register_size;
};
static void *
@@ -126,6 +129,8 @@ init_regcache_descr (struct gdbarch *gdbarch)
descr->register_offset[i] = offset;
offset += descr->sizeof_register[i];
gdb_assert (MAX_REGISTER_SIZE >= descr->sizeof_register[i]);
+ descr->max_register_size = std::max (descr->max_register_size,
+ descr->sizeof_register[i]);
}
/* Set the real size of the raw register cache buffer. */
descr->sizeof_raw_registers = offset;
@@ -136,6 +141,8 @@ init_regcache_descr (struct gdbarch *gdbarch)
descr->register_offset[i] = offset;
offset += descr->sizeof_register[i];
gdb_assert (MAX_REGISTER_SIZE >= descr->sizeof_register[i]);
+ descr->max_register_size = std::max (descr->max_register_size,
+ descr->sizeof_register[i]);
}
/* Set the real size of the readonly register cache buffer. */
descr->sizeof_cooked_registers = offset;
@@ -187,6 +194,13 @@ regcache_register_size (const struct regcache *regcache, int n)
return register_size (get_regcache_arch (regcache), n);
}
+long
+max_register_size (struct gdbarch *gdbarch)
+{
+ struct regcache_descr *descr = regcache_descr (gdbarch);
+ return descr->max_register_size;
+}
+
/* The register cache for storing raw register values. */
struct regcache
@@ -337,7 +351,6 @@ regcache_save (struct regcache *dst, regcache_cooked_read_ftype *cooked_read,
void *src)
{
struct gdbarch *gdbarch = dst->descr->gdbarch;
- gdb_byte buf[MAX_REGISTER_SIZE];
int regnum;
/* The DST should be `read-only', if it wasn't then the save would
@@ -356,17 +369,13 @@ regcache_save (struct regcache *dst, regcache_cooked_read_ftype *cooked_read,
{
if (gdbarch_register_reggroup_p (gdbarch, regnum, save_reggroup))
{
- enum register_status status = cooked_read (src, regnum, buf);
+ gdb_byte *dst_buf = register_buffer (dst, regnum);
+ enum register_status status = cooked_read (src, regnum, dst_buf);
- if (status == REG_VALID)
- memcpy (register_buffer (dst, regnum), buf,
- register_size (gdbarch, regnum));
- else
+ if (status != REG_VALID)
{
gdb_assert (status != REG_UNKNOWN);
-
- memset (register_buffer (dst, regnum), 0,
- register_size (gdbarch, regnum));
+ memset (dst_buf, 0, register_size (gdbarch, regnum));
}
dst->register_status[regnum] = status;
}
@@ -379,7 +388,7 @@ regcache_restore (struct regcache *dst,
void *cooked_read_context)
{
struct gdbarch *gdbarch = dst->descr->gdbarch;
- gdb_byte buf[MAX_REGISTER_SIZE];
+ std::vector<gdb_byte> buf (max_register_size (gdbarch));
int regnum;
/* The dst had better not be read-only. If it is, the `restore'
@@ -395,9 +404,9 @@ regcache_restore (struct regcache *dst,
{
enum register_status status;
- status = cooked_read (cooked_read_context, regnum, buf);
+ status = cooked_read (cooked_read_context, regnum, buf.data ());
if (status == REG_VALID)
- regcache_cooked_write (dst, regnum, buf);
+ regcache_cooked_write (dst, regnum, buf.data ());
}
}
}
@@ -1339,7 +1348,6 @@ regcache_dump (struct regcache *regcache, struct ui_file *file,
int footnote_register_offset = 0;
int footnote_register_type_name_null = 0;
long register_offset = 0;
- gdb_byte buf[MAX_REGISTER_SIZE];
#if 0
fprintf_unfiltered (file, "nr_raw_registers %d\n",
@@ -1466,8 +1474,8 @@ regcache_dump (struct regcache *regcache, struct ui_file *file,
fprintf_unfiltered (file, "<unavailable>");
else
{
- regcache_raw_read (regcache, regnum, buf);
- print_hex_chars (file, buf,
+ regcache_raw_update (regcache, regnum);
+ print_hex_chars (file, register_buffer (regcache, regnum),
regcache->descr->sizeof_register[regnum],
gdbarch_byte_order (gdbarch));
}
@@ -1480,17 +1488,19 @@ regcache_dump (struct regcache *regcache, struct ui_file *file,
fprintf_unfiltered (file, "Cooked value");
else
{
- enum register_status status;
+ struct value *value = regcache_cooked_read_value (regcache,
+ regnum);
- status = regcache_cooked_read (regcache, regnum, buf);
- if (status == REG_UNKNOWN)
- fprintf_unfiltered (file, "<invalid>");
- else if (status == REG_UNAVAILABLE)
+ if (value_optimized_out (value)
+ || !value_entirely_available (value))
fprintf_unfiltered (file, "<unavailable>");
else
- print_hex_chars (file, buf,
+ print_hex_chars (file, value_contents_all (value),
regcache->descr->sizeof_register[regnum],
gdbarch_byte_order (gdbarch));
+
+ release_value (value);
+ value_free (value);
}
}
More information about the Gdb-patches
mailing list