* [PATCH 2/2] GDB: S12Z: new function s12z_extract_return_value
@ 2018-11-15 19:02 John Darrington
0 siblings, 0 replies; 5+ messages in thread
From: John Darrington @ 2018-11-15 19:02 UTC (permalink / raw)
To: gdb-patches; +Cc: John Darrington
Make gdb aware of the return values of functions which
return in registers.
gdb/ChangeLog:
* s12z-tdep.c (s12z_extract_return_value): New function.
(inv_reg_perm) New array.
(s12z_return_value): Populate readbuf if non-null.
---
gdb/s12z-tdep.c | 62 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
1 file changed, 61 insertions(+), 1 deletion(-)
diff --git a/gdb/s12z-tdep.c b/gdb/s12z-tdep.c
index bd0bd7c001..3da88523f7 100644
--- a/gdb/s12z-tdep.c
+++ b/gdb/s12z-tdep.c
@@ -37,7 +37,9 @@
#define N_PHYSICAL_REGISTERS (S12Z_N_REGISTERS - 2)
-/* A permutation of all the physical registers. */
+/* A permutation of all the physical registers. Indexing this array
+ with an integer from gdb's internal representation will return the
+ register enum. */
static const int reg_perm[N_PHYSICAL_REGISTERS] =
{
REG_D0,
@@ -55,6 +57,16 @@ static const int reg_perm[N_PHYSICAL_REGISTERS] =
REG_CCW
};
+/* The inverse of the above permutation. Indexing this
+ array with a register enum (e.g. REG_D2) will return the register
+ number in gdb's internal representation. */
+static const int inv_reg_perm[N_PHYSICAL_REGISTERS] =
+ {
+ 2, 3, 4, 5, /* d2, d3, d4, d5 */
+ 0, 1, /* d0, d1 */
+ 6, 7, /* d6, d7 */
+ 8, 9, 10, 11, 12 /* x, y, s, p, ccw */
+ };
/* Return the name of the register REGNUM. */
static const char *
@@ -467,11 +479,59 @@ s12z_print_registers_info (struct gdbarch *gdbarch,
\f
+
+static void
+s12z_extract_return_value (struct type *type, struct regcache *regcache,
+ void *valbuf)
+{
+ int reg = -1;
+
+ gdb_byte buf[4];
+
+ switch (TYPE_LENGTH (type))
+ {
+ case 0: /* Nothing to do */
+ return;
+
+ case 1:
+ reg = REG_D0;
+ break;
+
+ case 2:
+ reg = REG_D2;
+ break;
+
+ case 3:
+ reg = REG_X;
+ break;
+
+ case 4:
+ reg = REG_D6;
+ break;
+
+ default:
+ error (_("bad size for return value"));
+ return;
+ }
+
+ regcache->cooked_read (inv_reg_perm[reg], buf);
+ memcpy (valbuf, buf, TYPE_LENGTH (type));
+}
+
static enum return_value_convention
s12z_return_value (struct gdbarch *gdbarch, struct value *function,
struct type *type, struct regcache *regcache,
gdb_byte *readbuf, const gdb_byte *writebuf)
{
+ if (TYPE_CODE (type) == TYPE_CODE_STRUCT
+ || TYPE_CODE (type) == TYPE_CODE_UNION
+ || TYPE_CODE (type) == TYPE_CODE_ARRAY
+ || TYPE_LENGTH (type) > 4)
+ return RETURN_VALUE_STRUCT_CONVENTION;
+
+ if (readbuf)
+ s12z_extract_return_value (type, regcache, readbuf);
+
return RETURN_VALUE_REGISTER_CONVENTION;
}
--
2.11.0
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 2/2] GDB: S12Z: new function s12z_extract_return_value
2018-11-18 9:13 ` John Darrington
@ 2018-11-18 18:28 ` Kevin Buettner
0 siblings, 0 replies; 5+ messages in thread
From: Kevin Buettner @ 2018-11-18 18:28 UTC (permalink / raw)
To: gdb-patches
On Sun, 18 Nov 2018 10:13:37 +0100
John Darrington <john@darrington.wattle.id.au> wrote:
> Hi Kevin,
>
> Thanks for the review.
>
> On Sat, Nov 17, 2018 at 11:35:50PM -0700, Kevin Buettner wrote:
> >
> > gdb/ChangeLog:
> > * s12z-tdep.c (s12z_extract_return_value): New function.
> > (inv_reg_perm) New array.
> > (s12z_return_value): Populate readbuf if non-null.
>
> Make sure that this is indented correctly when it eventually goes
> in the ChangeLog file.
>
> Is it documented anywhere what "correctly" means?
https://www.gnu.org/prep/standards/standards.html#Change-Logs
https://www.gnu.org/software/emacs/manual/html_node/emacs/Format-of-ChangeLog.html
Though with regards to indentation, neither of those links adequately
describe the ChangeLog entries that we've been using for many years
now.
When you do the commit / push, your ChangeLog entry should look something
like this:
YYYY-MM-DD John Darrington <john@darrington.wattle.id.au>
* s12z-tdep.c (s12z_extract_return_value): New function.
(inv_reg_perm) New array.
(s12z_return_value): Populate readbuf if non-null.
In particular, a single tab precedes the non-header (and non-empty)
portions of the ChangeLog entry.
Kevin
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 2/2] GDB: S12Z: new function s12z_extract_return_value
2018-11-18 6:35 ` Kevin Buettner
@ 2018-11-18 9:13 ` John Darrington
2018-11-18 18:28 ` Kevin Buettner
0 siblings, 1 reply; 5+ messages in thread
From: John Darrington @ 2018-11-18 9:13 UTC (permalink / raw)
To: Kevin Buettner; +Cc: gdb-patches, John Darrington
Hi Kevin,
Thanks for the review.
On Sat, Nov 17, 2018 at 11:35:50PM -0700, Kevin Buettner wrote:
>
> gdb/ChangeLog:
> * s12z-tdep.c (s12z_extract_return_value): New function.
> (inv_reg_perm) New array.
> (s12z_return_value): Populate readbuf if non-null.
Make sure that this is indented correctly when it eventually goes
in the ChangeLog file.
Is it documented anywhere what "correctly" means?
My two cents on all of the above...
I think you'll have a lot less grief with this architecture port if
you don't try to use the numbering defined in include/opcode/s12z.h.
Create a new numbering with new constants for GDB's purposes ordered
as shown in the reg_perm array. Then use these constants in place of
the various REG_ constants that are currently in s12z-tdep.c. If you
still want to be able to access registers[], it may make sense to
have an array which maps GDB's constants to those in include/opcode.
I'm beginning to think that you're right here. I may change it in the
way you propose in a future patch.
Also, if you want CCH and CCL to be show in "info registers" and/or
allow the user to display and set them, these can be implemented via
the use of pseudo-registers.
A the moment, I don't think it's worth bothering about. CCH and CCL are
merely the high and low bytes of a 16bit register CCW. The names CCH
and CCL only exist because of two (rarely used) instructions in the ISA.
> /* Return the name of the register REGNUM. */
> static const char *
> @@ -467,11 +479,59 @@ s12z_print_registers_info (struct gdbarch *gdbarch,
>
> \f
>
> +
> +static void
> +s12z_extract_return_value (struct type *type, struct regcache *regcache,
> + void *valbuf)
> +{
> + int reg = -1;
> +
> + gdb_byte buf[4];
> +
> + switch (TYPE_LENGTH (type))
> + {
> + case 0: /* Nothing to do */
> + return;
> +
> + case 1:
> + reg = REG_D0;
> + break;
> +
> + case 2:
> + reg = REG_D2;
> + break;
> +
> + case 3:
> + reg = REG_X;
> + break;
> +
> + case 4:
> + reg = REG_D6;
> + break;
> +
> + default:
> + error (_("bad size for return value"));
> + return;
> + }
> +
> + regcache->cooked_read (inv_reg_perm[reg], buf);
> + memcpy (valbuf, buf, TYPE_LENGTH (type));
Is there any reason not to just pass valbuf in place of buf to
cooked_read? Doing so will get rid of the memcpy.
Probably not. Thanks for noticing this.
J'
--
Avoid eavesdropping. Send strong encrypted email.
PGP Public key ID: 1024D/2DE827B3
fingerprint = 8797 A26D 0854 2EAB 0285 A290 8A67 719C 2DE8 27B3
See http://sks-keyservers.net or any PGP keyserver for public key.
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 2/2] GDB: S12Z: new function s12z_extract_return_value
2018-11-12 9:18 ` [PATCH 2/2] GDB: S12Z: new function s12z_extract_return_value John Darrington
@ 2018-11-18 6:35 ` Kevin Buettner
2018-11-18 9:13 ` John Darrington
0 siblings, 1 reply; 5+ messages in thread
From: Kevin Buettner @ 2018-11-18 6:35 UTC (permalink / raw)
To: gdb-patches; +Cc: John Darrington
Hi John,
See my comments inline with your patch, below.
On Mon, 12 Nov 2018 10:17:21 +0100
John Darrington <john@darrington.wattle.id.au> wrote:
> Make gdb aware of the return values of functions which
> return in registers.
>
> gdb/ChangeLog:
> * s12z-tdep.c (s12z_extract_return_value): New function.
> (inv_reg_perm) New array.
> (s12z_return_value): Populate readbuf if non-null.
Make sure that this is indented correctly when it eventually goes
in the ChangeLog file.
> diff --git a/gdb/s12z-tdep.c b/gdb/s12z-tdep.c
> index bd0bd7c001..3da88523f7 100644
> --- a/gdb/s12z-tdep.c
> +++ b/gdb/s12z-tdep.c
> @@ -37,7 +37,9 @@
> #define N_PHYSICAL_REGISTERS (S12Z_N_REGISTERS - 2)
>
>
> -/* A permutation of all the physical registers. */
> +/* A permutation of all the physical registers. Indexing this array
> + with an integer from gdb's internal representation will return the
> + register enum. */
> static const int reg_perm[N_PHYSICAL_REGISTERS] =
> {
> REG_D0,
> @@ -55,6 +57,16 @@ static const int reg_perm[N_PHYSICAL_REGISTERS] =
> REG_CCW
> };
>
> +/* The inverse of the above permutation. Indexing this
> + array with a register enum (e.g. REG_D2) will return the register
> + number in gdb's internal representation. */
> +static const int inv_reg_perm[N_PHYSICAL_REGISTERS] =
> + {
> + 2, 3, 4, 5, /* d2, d3, d4, d5 */
> + 0, 1, /* d0, d1 */
> + 6, 7, /* d6, d7 */
> + 8, 9, 10, 11, 12 /* x, y, s, p, ccw */
> + };
My two cents on all of the above...
I think you'll have a lot less grief with this architecture port if
you don't try to use the numbering defined in include/opcode/s12z.h.
Create a new numbering with new constants for GDB's purposes ordered
as shown in the reg_perm array. Then use these constants in place of
the various REG_ constants that are currently in s12z-tdep.c. If you
still want to be able to access registers[], it may make sense to
have an array which maps GDB's constants to those in include/opcode.
Also, if you want CCH and CCL to be show in "info registers" and/or
allow the user to display and set them, these can be implemented via
the use of pseudo-registers.
> /* Return the name of the register REGNUM. */
> static const char *
> @@ -467,11 +479,59 @@ s12z_print_registers_info (struct gdbarch *gdbarch,
>
> \f
>
> +
> +static void
> +s12z_extract_return_value (struct type *type, struct regcache *regcache,
> + void *valbuf)
> +{
> + int reg = -1;
> +
> + gdb_byte buf[4];
> +
> + switch (TYPE_LENGTH (type))
> + {
> + case 0: /* Nothing to do */
> + return;
> +
> + case 1:
> + reg = REG_D0;
> + break;
> +
> + case 2:
> + reg = REG_D2;
> + break;
> +
> + case 3:
> + reg = REG_X;
> + break;
> +
> + case 4:
> + reg = REG_D6;
> + break;
> +
> + default:
> + error (_("bad size for return value"));
> + return;
> + }
> +
> + regcache->cooked_read (inv_reg_perm[reg], buf);
> + memcpy (valbuf, buf, TYPE_LENGTH (type));
Is there any reason not to just pass valbuf in place of buf to
cooked_read? Doing so will get rid of the memcpy.
Kevin
^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH 2/2] GDB: S12Z: new function s12z_extract_return_value
2018-11-12 9:18 [PATCH 1/2] GDB: S12Z: Add assertion John Darrington
@ 2018-11-12 9:18 ` John Darrington
2018-11-18 6:35 ` Kevin Buettner
0 siblings, 1 reply; 5+ messages in thread
From: John Darrington @ 2018-11-12 9:18 UTC (permalink / raw)
To: gdb-patches; +Cc: John Darrington
Make gdb aware of the return values of functions which
return in registers.
gdb/ChangeLog:
* s12z-tdep.c (s12z_extract_return_value): New function.
(inv_reg_perm) New array.
(s12z_return_value): Populate readbuf if non-null.
---
gdb/s12z-tdep.c | 62 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
1 file changed, 61 insertions(+), 1 deletion(-)
diff --git a/gdb/s12z-tdep.c b/gdb/s12z-tdep.c
index bd0bd7c001..3da88523f7 100644
--- a/gdb/s12z-tdep.c
+++ b/gdb/s12z-tdep.c
@@ -37,7 +37,9 @@
#define N_PHYSICAL_REGISTERS (S12Z_N_REGISTERS - 2)
-/* A permutation of all the physical registers. */
+/* A permutation of all the physical registers. Indexing this array
+ with an integer from gdb's internal representation will return the
+ register enum. */
static const int reg_perm[N_PHYSICAL_REGISTERS] =
{
REG_D0,
@@ -55,6 +57,16 @@ static const int reg_perm[N_PHYSICAL_REGISTERS] =
REG_CCW
};
+/* The inverse of the above permutation. Indexing this
+ array with a register enum (e.g. REG_D2) will return the register
+ number in gdb's internal representation. */
+static const int inv_reg_perm[N_PHYSICAL_REGISTERS] =
+ {
+ 2, 3, 4, 5, /* d2, d3, d4, d5 */
+ 0, 1, /* d0, d1 */
+ 6, 7, /* d6, d7 */
+ 8, 9, 10, 11, 12 /* x, y, s, p, ccw */
+ };
/* Return the name of the register REGNUM. */
static const char *
@@ -467,11 +479,59 @@ s12z_print_registers_info (struct gdbarch *gdbarch,
\f
+
+static void
+s12z_extract_return_value (struct type *type, struct regcache *regcache,
+ void *valbuf)
+{
+ int reg = -1;
+
+ gdb_byte buf[4];
+
+ switch (TYPE_LENGTH (type))
+ {
+ case 0: /* Nothing to do */
+ return;
+
+ case 1:
+ reg = REG_D0;
+ break;
+
+ case 2:
+ reg = REG_D2;
+ break;
+
+ case 3:
+ reg = REG_X;
+ break;
+
+ case 4:
+ reg = REG_D6;
+ break;
+
+ default:
+ error (_("bad size for return value"));
+ return;
+ }
+
+ regcache->cooked_read (inv_reg_perm[reg], buf);
+ memcpy (valbuf, buf, TYPE_LENGTH (type));
+}
+
static enum return_value_convention
s12z_return_value (struct gdbarch *gdbarch, struct value *function,
struct type *type, struct regcache *regcache,
gdb_byte *readbuf, const gdb_byte *writebuf)
{
+ if (TYPE_CODE (type) == TYPE_CODE_STRUCT
+ || TYPE_CODE (type) == TYPE_CODE_UNION
+ || TYPE_CODE (type) == TYPE_CODE_ARRAY
+ || TYPE_LENGTH (type) > 4)
+ return RETURN_VALUE_STRUCT_CONVENTION;
+
+ if (readbuf)
+ s12z_extract_return_value (type, regcache, readbuf);
+
return RETURN_VALUE_REGISTER_CONVENTION;
}
--
2.11.0
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2018-11-18 18:28 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-11-15 19:02 [PATCH 2/2] GDB: S12Z: new function s12z_extract_return_value John Darrington
-- strict thread matches above, loose matches on Subject: below --
2018-11-12 9:18 [PATCH 1/2] GDB: S12Z: Add assertion John Darrington
2018-11-12 9:18 ` [PATCH 2/2] GDB: S12Z: new function s12z_extract_return_value John Darrington
2018-11-18 6:35 ` Kevin Buettner
2018-11-18 9:13 ` John Darrington
2018-11-18 18:28 ` Kevin Buettner
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).