Re: [PATCH] Removal of uses of MAX_REGISTER_SIZE

Alan Hayward
2017-02-08T14:17:25Z

(Resend, in plain text format)

> On 8 Feb 2017, at 10:47, Yao Qi <qiyaoltc@gmail.com> wrote:
>
> On 17-02-07 16:33:19, Alan Hayward wrote:
>
> Hi Alan,
> We end up having multiple different ways removing MAX_REGISTER_SIZE, and
> each change is quite independent. I'll split it in my review, and you can
> to post a patch set in the next version.
>
>> diff --git a/gdb/i386-tdep.c b/gdb/i386-tdep.c
>> index 8a4d59f6fdae8ec785462d0ceedcd6501b955cf0..081a16c6896ce7aee4db3b0be45fbbdd2c23dbdb 100644
>> --- a/gdb/i386-tdep.c
>> +++ b/gdb/i386-tdep.c
>> @@ -3250,7 +3250,7 @@ i386_pseudo_register_read_into_value (struct gdbarch *gdbarch,
>> int regnum,
>> struct value *result_value)
>> {
>> - gdb_byte raw_buf[MAX_REGISTER_SIZE];
>> + gdb_byte raw_buf[I386_MAX_REGISTER_SIZE];
>> enum register_status status;
>> gdb_byte *buf = value_contents_raw (result_value);
>>
>> @@ -3455,7 +3455,7 @@ void
>> i386_pseudo_register_write (struct gdbarch *gdbarch, struct regcache *regcache,
>> int regnum, const gdb_byte *buf)
>> {
>> - gdb_byte raw_buf[MAX_REGISTER_SIZE];
>> + gdb_byte raw_buf[I386_MAX_REGISTER_SIZE];
>>
>> if (i386_mmx_regnum_p (gdbarch, regnum))
>> {
>> @@ -5037,7 +5037,7 @@ i386_process_record (struct gdbarch *gdbarch, struct regcache *regcache,
>> uint32_t opcode;
>> uint8_t opcode8;
>> ULONGEST addr;
>> - gdb_byte buf[MAX_REGISTER_SIZE];
>> + gdb_byte buf[I386_MAX_REGISTER_SIZE];
>> struct i386_record_s ir;
>> struct gdbarch_tdep *tdep = gdbarch_tdep (gdbarch);
>> uint8_t rex_w = -1;
>> diff --git a/gdb/i387-tdep.c b/gdb/i387-tdep.c
>> index adbe72133089bc371108d5dd79bf8d8e61ba259c..fcd5ad248d6b737b9f27e294ce166a118e4bdcad 100644
>> --- a/gdb/i387-tdep.c
>> +++ b/gdb/i387-tdep.c
>> @@ -899,7 +899,7 @@ i387_supply_xsave (struct regcache *regcache, int regnum,
>> const gdb_byte *regs = (const gdb_byte *) xsave;
>> int i;
>> unsigned int clear_bv;
>> - static const gdb_byte zero[MAX_REGISTER_SIZE] = { 0 };
>> + static const gdb_byte zero[I386_MAX_REGISTER_SIZE] = { 0 };
>> enum
>> {
>> none = 0x0,
>> diff --git a/gdb/m68k-linux-nat.c b/gdb/m68k-linux-nat.c
>> index 6944c74eb198381135fda3ddf01b9da3a63e62d5..e5182caf39197f759c85c2321e4d66c428f5911e 100644
>> --- a/gdb/m68k-linux-nat.c
>> +++ b/gdb/m68k-linux-nat.c
>> @@ -105,7 +105,7 @@ fetch_register (struct regcache *regcache, int regno)
>> struct gdbarch *gdbarch = get_regcache_arch (regcache);
>> long regaddr, val;
>> int i;
>> - gdb_byte buf[MAX_REGISTER_SIZE];
>> + gdb_byte buf[M68K_MAX_REGISTER_SIZE];
>
> Nit, we can even reduce the size of 'buf' to sizeof (long), because the
> code read/write register by PTRACE_PEEKUSER/PTRACE_POKEUSER which is
> word-wide operation.

That wouldn't work.
The code calls memcpy on the buffer using buf[i] up to max size of register_size (gdbarch, regno).

>
>> int tid;
>>
>> /* Overload thread id onto process id. */
>> @@ -160,7 +160,7 @@ store_register (const struct regcache *regcache, int regno)
>> long regaddr, val;
>> int i;
>> int tid;
>> - gdb_byte buf[MAX_REGISTER_SIZE];
>> + gdb_byte buf[M68K_MAX_REGISTER_SIZE];
>>
>> /* Overload thread id onto process id. */
>> tid = ptid_get_lwp (inferior_ptid);
>
> This part is OK.
>
> --
> Yao (齐尧)

The full M68K_MAX_REGISTER_SIZE/I386_MAX_REGISTER_SIZE changes as a single patch:

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

* i386-tdep.c (i386_pseudo_register_read_into_value): Use
I386_MAX_REGISTER_SIZE.
(i386_pseudo_register_write): Likewise.
(i386_process_record): Likewise.
* i387-tdep.c (i387_supply_xsave): Likewise.
* m68k-linux-nat.c (fetch_register): Use M68K_MAX_REGISTER_SIZE.
(store_register): Likewise.

Alan.

diff --git a/gdb/i386-tdep.c b/gdb/i386-tdep.c
index 8a4d59f6fdae8ec785462d0ceedcd6501b955cf0..081a16c6896ce7aee4db3b0be45fbbdd2c23dbdb 100644
--- a/gdb/i386-tdep.c
+++ b/gdb/i386-tdep.c
@@ -3250,7 +3250,7 @@ i386_pseudo_register_read_into_value (struct gdbarch *gdbarch,
int regnum,
struct value *result_value)
{
- gdb_byte raw_buf[MAX_REGISTER_SIZE];
+ gdb_byte raw_buf[I386_MAX_REGISTER_SIZE];
enum register_status status;
gdb_byte *buf = value_contents_raw (result_value);

@@ -3455,7 +3455,7 @@ void
i386_pseudo_register_write (struct gdbarch *gdbarch, struct regcache *regcache,
int regnum, const gdb_byte *buf)
{
- gdb_byte raw_buf[MAX_REGISTER_SIZE];
+ gdb_byte raw_buf[I386_MAX_REGISTER_SIZE];

if (i386_mmx_regnum_p (gdbarch, regnum))
{
@@ -5037,7 +5037,7 @@ i386_process_record (struct gdbarch *gdbarch, struct regcache *regcache,
uint32_t opcode;
uint8_t opcode8;
ULONGEST addr;
- gdb_byte buf[MAX_REGISTER_SIZE];
+ gdb_byte buf[I386_MAX_REGISTER_SIZE];
struct i386_record_s ir;
struct gdbarch_tdep *tdep = gdbarch_tdep (gdbarch);
uint8_t rex_w = -1;
diff --git a/gdb/i387-tdep.c b/gdb/i387-tdep.c
index adbe72133089bc371108d5dd79bf8d8e61ba259c..fcd5ad248d6b737b9f27e294ce166a118e4bdcad 100644
--- a/gdb/i387-tdep.c
+++ b/gdb/i387-tdep.c
@@ -899,7 +899,7 @@ i387_supply_xsave (struct regcache *regcache, int regnum,
const gdb_byte *regs = (const gdb_byte *) xsave;
int i;
unsigned int clear_bv;
- static const gdb_byte zero[MAX_REGISTER_SIZE] = { 0 };
+ static const gdb_byte zero[I386_MAX_REGISTER_SIZE] = { 0 };
enum
{
none = 0x0,
diff --git a/gdb/m68k-linux-nat.c b/gdb/m68k-linux-nat.c
index 6944c74eb198381135fda3ddf01b9da3a63e62d5..e5182caf39197f759c85c2321e4d66c428f5911e 100644
--- a/gdb/m68k-linux-nat.c
+++ b/gdb/m68k-linux-nat.c
@@ -105,7 +105,7 @@ fetch_register (struct regcache *regcache, int regno)
struct gdbarch *gdbarch = get_regcache_arch (regcache);
long regaddr, val;
int i;
- gdb_byte buf[MAX_REGISTER_SIZE];
+ gdb_byte buf[M68K_MAX_REGISTER_SIZE];
int tid;

/* Overload thread id onto process id. */
@@ -160,7 +160,7 @@ store_register (const struct regcache *regcache, int regno)
long regaddr, val;
int i;
int tid;
- gdb_byte buf[MAX_REGISTER_SIZE];
+ gdb_byte buf[M68K_MAX_REGISTER_SIZE];

/* Overload thread id onto process id. */
tid = ptid_get_lwp (inferior_ptid);