From: Alan Hayward <Alan.Hayward@arm.com>
To: Yao Qi <qiyaoltc@gmail.com>
Cc: Pedro Alves <palves@redhat.com>,
Joel Brobecker <brobecker@adacore.com>,
"gdb-patches@sourceware.org" <gdb-patches@sourceware.org>,
nd <nd@arm.com>
Subject: Re: [PATCH] Removal of uses of MAX_REGISTER_SIZE
Date: Wed, 08 Feb 2017 14:17:00 -0000 [thread overview]
Message-ID: <34564AB1-2F18-45E3-BD61-EF123F6BF9B5@arm.com> (raw)
In-Reply-To: <20170208104713.GB32023@E107787-LIN>
(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);
next prev parent reply other threads:[~2017-02-08 14:17 UTC|newest]
Thread overview: 28+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-01-24 10:31 Alan Hayward
2017-01-27 11:49 ` Pedro Alves
2017-01-27 12:11 ` Pedro Alves
2017-01-27 16:46 ` Alan Hayward
2017-02-01 15:49 ` Pedro Alves
2017-02-01 12:45 ` Yao Qi
2017-02-01 15:48 ` Pedro Alves
2017-02-02 9:40 ` Joel Brobecker
2017-02-03 9:59 ` Alan Hayward
2017-02-03 10:28 ` Yao Qi
2017-02-03 11:00 ` Pedro Alves
2017-02-03 11:25 ` Alan Hayward
2017-02-03 16:50 ` Yao Qi
2017-02-06 9:33 ` Alan Hayward
[not found] ` <20170206152635.GE11916@E107787-LIN>
2017-02-07 16:33 ` Alan Hayward
2017-02-08 10:47 ` Yao Qi
2017-02-08 14:17 ` Alan Hayward [this message]
2017-02-08 12:06 ` Yao Qi
2017-02-08 12:24 ` Yao Qi
2017-02-08 14:44 ` Alan Hayward
2017-02-18 23:19 ` Yao Qi
2017-02-20 11:19 ` Alan Hayward
2017-02-08 15:32 ` Yao Qi
2017-02-08 17:10 ` Yao Qi
2017-02-09 13:26 ` Alan Hayward
2017-02-14 11:24 ` Alan Hayward
2017-02-08 17:36 ` Yao Qi
2017-02-13 11:59 ` Alan Hayward
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=34564AB1-2F18-45E3-BD61-EF123F6BF9B5@arm.com \
--to=alan.hayward@arm.com \
--cc=brobecker@adacore.com \
--cc=gdb-patches@sourceware.org \
--cc=nd@arm.com \
--cc=palves@redhat.com \
--cc=qiyaoltc@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).