Re: [PATCH] Removal of uses of MAX_REGISTER_SIZE

Alan Hayward
2017-02-14T11:24:29Z

> On 8 Feb 2017, at 17:09, Yao Qi <qiyaoltc@gmail.com> wrote:
>
> Alan Hayward <Alan.Hayward@arm.com> writes:
>
>> @@ -1135,8 +1135,8 @@ register_changed_p (int regnum, struct regcache *prev_regs,
>> struct regcache *this_regs)
>> {
>> struct gdbarch *gdbarch = get_regcache_arch (this_regs);
>> - gdb_byte prev_buffer[MAX_REGISTER_SIZE];
>> - gdb_byte this_buffer[MAX_REGISTER_SIZE];
>> + std::vector<gdb_byte> prev_buffer (register_size (gdbarch, regnum));
>> + std::vector<gdb_byte> this_buffer (register_size (gdbarch, regnum));
>> enum register_status prev_status;
>> enum register_status this_status;
>>
>
> This function should be moved to regcache.c, because it is about
> comparing bytes of a certain register in both regcaches. Then, wen can
> compare raw registers from register_buffer, and pseudo registers from
> the values.
>
>> @@ -1146,13 +1146,13 @@ register_changed_p (int regnum, struct regcache *prev_regs,
>> return 1;
>>
>> /* Get register contents and compare. */
>> - prev_status = regcache_cooked_read (prev_regs, regnum, prev_buffer);
>> - this_status = regcache_cooked_read (this_regs, regnum, this_buffer);
>> + prev_status = regcache_cooked_read (prev_regs, regnum, prev_buffer.data ());
>> + this_status = regcache_cooked_read (this_regs, regnum, this_buffer.data ());
>>
>> if (this_status != prev_status)
>> return 1;
>> else if (this_status == REG_VALID)
>> - return memcmp (prev_buffer, this_buffer,
>> + return memcmp (prev_buffer.data (), this_buffer.data (),
>> register_size (gdbarch, regnum)) != 0;
>> else
>> return 0;
>
> --
> Yao (齐尧)

Ignore my previous reply to this comment. I've since noticed you can just use
regcache_cooked_read_value.

New patch below.

I removed the error case from mi-main becuase it was impossible for the code
to get here - register_changed_p would always return 0 or 1.

Tested on a build of all targets using make check with target boards unix and
native-gdbserver. This testing also included the two patches:
* M68K_MAX_REGISTER_SIZE and I386_MAX_REGISTER_SIZE changes
* stack.c changes

Ok?

Alan.

2017-02-13 Alan Hayward <alan.hayward@arm.com>

* gdb/mi/mi-main.c (mi_cmd_data_list_changed_registers): Use
regcache_register_changed_p
(register_changed_p): Remove.
* gdb/regcache.c (regcache_register_changed_p): New function.
* gdb/regcache.h (regcache_register_changed_p): New declaration.

diff --git a/gdb/mi/mi-main.c b/gdb/mi/mi-main.c
index 57c23ebf5d6b2d3b398aa40ebd9b3cb70c56125c..d22cb9b59d3c5d65609bf3762457bba90401fd8f 100644
--- a/gdb/mi/mi-main.c
+++ b/gdb/mi/mi-main.c
@@ -91,8 +91,6 @@ static void mi_execute_cli_command (const char *cmd, int args_p,
const char *args);
static void mi_execute_async_cli_command (char *cli_command,
char **argv, int argc);
-static int register_changed_p (int regnum, struct regcache *,
- struct regcache *);
static void output_register (struct frame_info *, int regnum, int format,
int skip_unavailable);

@@ -1098,11 +1096,7 @@ mi_cmd_data_list_changed_registers (char *command, char **argv, int argc)
if (gdbarch_register_name (gdbarch, regnum) == NULL
|| *(gdbarch_register_name (gdbarch, regnum)) == '\0')
continue;
- changed = register_changed_p (regnum, prev_regs, this_regs);
- if (changed < 0)
- error (_("-data-list-changed-registers: "
- "Unable to read register contents."));
- else if (changed)
+ if (regcache_register_changed_p (regnum, prev_regs, this_regs))
uiout->field_int (NULL, regnum);
}
}
@@ -1117,11 +1111,7 @@ mi_cmd_data_list_changed_registers (char *command, char **argv, int argc)
&& gdbarch_register_name (gdbarch, regnum) != NULL
&& *gdbarch_register_name (gdbarch, regnum) != '\000')
{
- changed = register_changed_p (regnum, prev_regs, this_regs);
- if (changed < 0)
- error (_("-data-list-changed-registers: "
- "Unable to read register contents."));
- else if (changed)
+ if (regcache_register_changed_p (regnum, prev_regs, this_regs))
uiout->field_int (NULL, regnum);
}
else
@@ -1130,34 +1120,6 @@ mi_cmd_data_list_changed_registers (char *command, char **argv, int argc)
do_cleanups (cleanup);
}

-static int
-register_changed_p (int regnum, struct regcache *prev_regs,
- struct regcache *this_regs)
-{
- struct gdbarch *gdbarch = get_regcache_arch (this_regs);
- gdb_byte prev_buffer[MAX_REGISTER_SIZE];
- gdb_byte this_buffer[MAX_REGISTER_SIZE];
- enum register_status prev_status;
- enum register_status this_status;
-
- /* First time through or after gdbarch change consider all registers
- as changed. */
- if (!prev_regs || get_regcache_arch (prev_regs) != gdbarch)
- return 1;
-
- /* Get register contents and compare. */
- prev_status = regcache_cooked_read (prev_regs, regnum, prev_buffer);
- this_status = regcache_cooked_read (this_regs, regnum, this_buffer);
-
- if (this_status != prev_status)
- return 1;
- else if (this_status == REG_VALID)
- return memcmp (prev_buffer, this_buffer,
- register_size (gdbarch, regnum)) != 0;
- else
- return 0;
-}
-
/* Return a list of register number and value pairs. The valid
arguments expected are: a letter indicating the format in which to
display the registers contents. This can be one of: x
diff --git a/gdb/regcache.h b/gdb/regcache.h
index e5a7cf553279b8cc0d546ec1b8274cbf97e246d5..174aa0166f419b40b0509b1be5c6dce3c2373fd0 100644
--- a/gdb/regcache.h
+++ b/gdb/regcache.h
@@ -228,4 +228,11 @@ extern void regcache_cpy (struct regcache *dest, struct regcache *src);
extern void registers_changed (void);
extern void registers_changed_ptid (ptid_t);

+/* Return true if the register value of regnum differs between prev_regs and
+ this_regs. prev_regs can be null, but this_regs must be set. */
+
+extern bool regcache_register_changed_p (int regnum,
+ struct regcache *prev_regs,
+ struct regcache *this_regs);
+
#endif /* REGCACHE_H */
diff --git a/gdb/regcache.c b/gdb/regcache.c
index 9d28aa2c2114e0f1c52758bb2fbe9669a329c13e..96cc94ee7cfeb60d3b6541ca0b49871e6087aa8a 100644
--- a/gdb/regcache.c
+++ b/gdb/regcache.c
@@ -1251,6 +1251,74 @@ regcache_write_pc (struct regcache *regcache, CORE_ADDR pc)
}

+/* Return true if the register value of regnum differs between prev_regs and
+ this_regs. prev_regs can be null, but this_regs must be set. */
+
+bool
+regcache_register_changed_p (int regnum, struct regcache *prev_regs,
+ struct regcache *this_regs)
+{
+ struct gdbarch *gdbarch;
+ gdb_assert (regnum >= 0);
+ gdb_assert (this_regs);
+
+ /* if there are no previous registers consider all registers as changed. */
+ if (!prev_regs)
+ return true;
+
+ gdb_assert (regnum < prev_regs->descr->nr_cooked_registers);
+ gdb_assert (regnum < this_regs->descr->nr_cooked_registers);
+ gdbarch = get_regcache_arch (this_regs);
+
+ /* If arches don't match then consider all registers as changed. */
+ if (gdbarch != get_regcache_arch (prev_regs))
+ return true;
+
+ if (regnum < this_regs->descr->nr_raw_registers)
+ {
+ enum register_status prev_status, this_status;
+
+ prev_status = (enum register_status) prev_regs->register_status[regnum];
+ this_status = (enum register_status) this_regs->register_status[regnum];
+
+ if (this_status != prev_status)
+ return true;
+ else if (this_status == REG_VALID)
+ return memcmp (register_buffer (prev_regs, regnum),
+ register_buffer (this_regs, regnum),
+ register_size (gdbarch, regnum)) != 0;
+ else
+ return false;
+ }
+ else
+ {
+ struct value *prev_value, *this_value;
+ bool ret;
+
+ prev_value = regcache_cooked_read_value (prev_regs, regnum);
+ this_value = regcache_cooked_read_value (this_regs, regnum);
+
+ if (value_optimized_out (prev_value)
+ || value_optimized_out (this_value))
+ ret = value_optimized_out (prev_value)
+ != value_optimized_out (this_value);
+ else if (!value_entirely_available (prev_value)
+ || !value_entirely_available (this_value))
+ ret = value_entirely_available (prev_value)
+ != value_entirely_available (this_value);
+ else
+ ret = memcmp (value_contents_raw (prev_value),
+ value_contents_raw (this_value),
+ register_size (gdbarch, regnum)) != 0;
+
+ release_value (prev_value);
+ value_free (prev_value);
+ release_value (this_value);
+ value_free (this_value);
+ return ret;
+ }
+}
+
static void
reg_flush_command (char *command, int from_tty)
{