RX: Convert target-description
Commit Message
gdb/ChangeLog
2019-08-21 Yoshinori Sato <ysato@users.sourceforge.jp>
* gdb/rx-tdep.c (rx_register_names): New.
(rx_register_name): Use rx_register_names.
(rx_register_name): Add check range.
(rx_register_g_packet_guesses): New.
(rx_gdbarch_init): Convert target-descriptions.
(_initialize_rx_tdep): Add initialize_tdesc_rx.
* gdb/features/Makefile: Add rx.xml.
* gdb/features/rx.xml: New.
---
gdb/features/Makefile | 2 ++
gdb/features/rx.xml | 70 ++++++++++++++++++++++++++++++++++++
gdb/rx-tdep.c | 99 ++++++++++++++++++++++++++++++---------------------
3 files changed, 130 insertions(+), 41 deletions(-)
create mode 100644 gdb/features/rx.xml
Comments
Thanks for the patch.
* Yoshinori Sato <ysato@users.sourceforge.jp> [2019-08-21 12:33:01 +0900]:
GDB patches usually include a brief description to the patch before
the changelog entry in the commit message, even if its just one
sentence like:
Convert the RX target to make use of target descriptions.
> gdb/ChangeLog
>
> 2019-08-21 Yoshinori Sato <ysato@users.sourceforge.jp>
>
> * gdb/rx-tdep.c (rx_register_names): New.
> (rx_register_name): Use rx_register_names.
> (rx_register_name): Add check range.
> (rx_register_g_packet_guesses): New.
> (rx_gdbarch_init): Convert target-descriptions.
> (_initialize_rx_tdep): Add initialize_tdesc_rx.
> * gdb/features/Makefile: Add rx.xml.
> * gdb/features/rx.xml: New.
You should generate and commit the gdb/features/rx.c file as well.
These files are not automatically generated by the build system and we
rely on people regenerating them when the xml file changes.
There's a few minor coding standard issues I've pointed out below.
> ---
> gdb/features/Makefile | 2 ++
> gdb/features/rx.xml | 70 ++++++++++++++++++++++++++++++++++++
> gdb/rx-tdep.c | 99 ++++++++++++++++++++++++++++++---------------------
> 3 files changed, 130 insertions(+), 41 deletions(-)
> create mode 100644 gdb/features/rx.xml
>
> diff --git a/gdb/features/Makefile b/gdb/features/Makefile
> index 0c84faf405..2b65d46df0 100644
> --- a/gdb/features/Makefile
> +++ b/gdb/features/Makefile
> @@ -161,6 +161,7 @@ XMLTOC = \
> rs6000/powerpc-vsx64.xml \
> rs6000/powerpc-vsx64l.xml \
> rs6000/rs6000.xml \
> + rx.xml \
> s390-linux32.xml \
> s390-linux32v1.xml \
> s390-linux32v2.xml \
> @@ -238,6 +239,7 @@ FEATURE_XMLFILES = aarch64-core.xml \
> riscv/64bit-cpu.xml \
> riscv/64bit-csr.xml \
> riscv/64bit-fpu.xml \
> + rx.xml \
> tic6x-c6xp.xml \
> tic6x-core.xml \
> tic6x-gp.xml
> diff --git a/gdb/features/rx.xml b/gdb/features/rx.xml
> new file mode 100644
> index 0000000000..b5aa9ac4a8
> --- /dev/null
> +++ b/gdb/features/rx.xml
> @@ -0,0 +1,70 @@
> +<?xml version="1.0"?>
> +<!-- Copyright (C) 2019 Free Software Foundation, Inc.
> +
> + Copying and distribution of this file, with or without modification,
> + are permitted in any medium without royalty provided the copyright
> + notice and this notice are preserved. -->
> +
> +<!DOCTYPE feature SYSTEM "gdb-target.dtd">
> +<feature name="org.gnu.gdb.rx.core">
> + <reg name="r0" bitsize="32" type="data_ptr"/>
> + <reg name="r1" bitsize="32" type="uint32"/>
> + <reg name="r2" bitsize="32" type="uint32"/>
> + <reg name="r3" bitsize="32" type="uint32"/>
> + <reg name="r4" bitsize="32" type="uint32"/>
> + <reg name="r5" bitsize="32" type="uint32"/>
> + <reg name="r6" bitsize="32" type="uint32"/>
> + <reg name="r7" bitsize="32" type="uint32"/>
> + <reg name="r8" bitsize="32" type="uint32"/>
> + <reg name="r9" bitsize="32" type="uint32"/>
> + <reg name="r10" bitsize="32" type="uint32"/>
> + <reg name="r11" bitsize="32" type="uint32"/>
> + <reg name="r12" bitsize="32" type="uint32"/>
> + <reg name="r13" bitsize="32" type="uint32"/>
> + <reg name="r14" bitsize="32" type="uint32"/>
> + <reg name="r15" bitsize="32" type="uint32"/>
> +
> + <flags id="psw_flags" size="4">
> + <field name="C" start="0" end="0"/>
> + <field name="Z" start="1" end="1"/>
> + <field name="S" start="2" end="2"/>
> + <field name="O" start="3" end="3"/>
> + <field name="I" start="16" end="16"/>
> + <field name="U" start="17" end="17"/>
> + <field name="PM" start="20" end="20"/>
> + <field name="IPL" start="24" end="27"/>
> + </flags>
> +
> + <flags id="fpsw_flags" size="4">
> + <field name="RM" start="0" end="1"/>
> + <field name="CV" start="2" end="2"/>
> + <field name="CO" start="3" end="3"/>
> + <field name="CZ" start="4" end="4"/>
> + <field name="CU" start="5" end="5"/>
> + <field name="CX" start="6" end="6"/>
> + <field name="CE" start="7" end="7"/>
> + <field name="DN" start="8" end="8"/>
> + <field name="EV" start="10" end="10"/>
> + <field name="EO" start="11" end="11"/>
> + <field name="EZ" start="12" end="12"/>
> + <field name="EU" start="13" end="13"/>
> + <field name="EX" start="14" end="14"/>
> + <field name="FV" start="26" end="26"/>
> + <field name="FO" start="27" end="27"/>
> + <field name="FZ" start="28" end="28"/>
> + <field name="FU" start="29" end="29"/>
> + <field name="FX" start="30" end="30"/>
> + <field name="FS" start="31" end="31"/>
> + </flags>
> +
> + <reg name="usp" bitsize="32" type="data_ptr"/>
> + <reg name="isp" bitsize="32" type="data_ptr"/>
> + <reg name="psw" bitsize="32" type="psw_flags"/>
> + <reg name="pc" bitsize="32" type="code_ptr"/>
> + <reg name="intb" bitsize="32" type="data_ptr"/>
> + <reg name="bpsw" bitsize="32" type="psw_flags"/>
> + <reg name="bpc" bitsize="32" type="code_ptr"/>
> + <reg name="fintv" bitsize="32" type="code_ptr"/>
> + <reg name="fpsw" bitsize="32" type="fpsw_flags"/>
> + <reg name="acc" bitsize="64" type="uint64"/>
> +</feature>
> diff --git a/gdb/rx-tdep.c b/gdb/rx-tdep.c
> index 4cbf919db9..b3398f122a 100644
> --- a/gdb/rx-tdep.c
> +++ b/gdb/rx-tdep.c
> @@ -33,11 +33,15 @@
> #include "value.h"
> #include "gdbcore.h"
> #include "dwarf2-frame.h"
> +#include "remote.h"
> +#include "target-descriptions.h"
>
> #include "elf/rx.h"
> #include "elf-bfd.h"
> #include <algorithm>
>
> +#include "features/rx.c"
> +
> /* Certain important register numbers. */
> enum
> {
> @@ -114,40 +118,21 @@ struct rx_prologue
> int reg_offset[RX_NUM_REGS];
> };
>
> +static const char *const rx_register_names[] = {
> + "r0", "r1", "r2", "r3", "r4", "r5", "r6", "r7",
> + "r8", "r9", "r10", "r11", "r12", "r13", "r14", "r15",
> + "usp", "isp", "psw", "pc", "intb", "bpsw","bpc","fintv",
> + "fpsw", "acc",
> +};
All file level variables and functions should have a header comment.
> +
> /* Implement the "register_name" gdbarch method. */
> static const char *
> rx_register_name (struct gdbarch *gdbarch, int regnr)
> {
> - static const char *const reg_names[] = {
> - "r0",
> - "r1",
> - "r2",
> - "r3",
> - "r4",
> - "r5",
> - "r6",
> - "r7",
> - "r8",
> - "r9",
> - "r10",
> - "r11",
> - "r12",
> - "r13",
> - "r14",
> - "r15",
> - "usp",
> - "isp",
> - "psw",
> - "pc",
> - "intb",
> - "bpsw",
> - "bpc",
> - "fintv",
> - "fpsw",
> - "acc"
> - };
> -
> - return reg_names[regnr];
> + if (regnr >= 0 && regnr < RX_NUM_REGS)
> + return rx_register_names[regnr];
> + else
> + return NULL;
> }
>
> /* Construct the flags type for PSW and BPSW. */
> @@ -1037,6 +1022,14 @@ rx_dwarf_reg_to_regnum (struct gdbarch *gdbarch, int reg)
> return -1;
> }
>
> +static void
> +rx_register_g_packet_guesses (struct gdbarch *gdbarch)
> +{
> + register_remote_g_packet_guess (gdbarch,
> + 4 * RX_NUM_REGS,
> + tdesc_rx);
> +}
Header comment again.
> +
> /* Allocate and initialize a gdbarch object. */
> static struct gdbarch *
> rx_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
> @@ -1044,6 +1037,8 @@ rx_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
> struct gdbarch *gdbarch;
> struct gdbarch_tdep *tdep;
> int elf_flags;
> + struct tdesc_arch_data *tdesc_data = NULL;
> + const struct target_desc *tdesc = info.target_desc;
>
> /* Extract the elf_flags if available. */
> if (info.abfd != NULL
> @@ -1065,8 +1060,33 @@ rx_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
> return arches->gdbarch;
> }
>
> - /* None found, create a new architecture from the information
> - provided. */
> + if (tdesc == NULL)
> + tdesc = tdesc_rx;
> +
> + /* Check any target description for validity. */
> + if (tdesc_has_registers (tdesc))
> + {
> + const struct tdesc_feature *feature;
> + int valid_p = 0;
This should be a bool.
> + int i = 0;
This can move down into the if block scope.
> + feature = tdesc_find_feature (tdesc, "org.gnu.gdb.rx.core");
> +
> + if (feature != NULL)
> + {
> + tdesc_data = tdesc_data_alloc ();
> + valid_p = 1;
> + for (i = 0; i < RX_NUM_REGS; i++)
> + valid_p &= tdesc_numbered_register (feature, tdesc_data, i,
> + rx_register_names[i]);
> + }
> +
> + if (!valid_p)
> + {
> + tdesc_data_cleanup (tdesc_data);
> + return NULL;
> + }
> + }
> +
> tdep = XCNEW (struct gdbarch_tdep);
> gdbarch = gdbarch_alloc (&info, tdep);
> tdep->elf_flags = elf_flags;
> @@ -1083,15 +1103,6 @@ rx_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
> set_gdbarch_sw_breakpoint_from_kind (gdbarch, rx_breakpoint::bp_from_kind);
> set_gdbarch_skip_prologue (gdbarch, rx_skip_prologue);
>
> - /* Target builtin data types. */
> - set_gdbarch_char_signed (gdbarch, 0);
> - set_gdbarch_short_bit (gdbarch, 16);
> - set_gdbarch_int_bit (gdbarch, 32);
> - set_gdbarch_long_bit (gdbarch, 32);
> - set_gdbarch_long_long_bit (gdbarch, 64);
> - set_gdbarch_ptr_bit (gdbarch, 32);
> - set_gdbarch_float_bit (gdbarch, 32);
> - set_gdbarch_float_format (gdbarch, floatformats_ieee_single);
I don't understand why these are being deleted. This doesn't feel
like it should be related to conversion to target descriptions.
> if (elf_flags & E_FLAG_RX_64BIT_DOUBLES)
> {
> set_gdbarch_double_bit (gdbarch, 64);
> @@ -1115,6 +1126,8 @@ rx_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
> dwarf2_append_unwinders (gdbarch);
> frame_unwind_append_unwinder (gdbarch, &rx_frame_unwind);
>
> + rx_register_g_packet_guesses (gdbarch);
> +
> /* Methods setting up a dummy call, and extracting the return value from
> a call. */
> set_gdbarch_push_dummy_call (gdbarch, rx_push_dummy_call);
> @@ -1123,6 +1136,9 @@ rx_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
> /* Virtual tables. */
> set_gdbarch_vbit_in_delta (gdbarch, 1);
>
> + if (tdesc_data != NULL)
> + tdesc_use_registers (gdbarch, tdesc, tdesc_data);
I'm confused by the 'tdesc_data != NULL' check here, my reading of
things is that either:
1. Target doesn't provide a target description, we fall back to the
built in tdesc_rx, this will be valid, tdesc_data will not be NULL.
2. The target provides a description, but its invalid, we bail out
earlier after the validity check, we never reach this code.
3. The target provides a description which is valid, tdesc_data will
not be NULL.
I'd prefer to see a 'gdb_assert (tdesc_data != NULL)' immediately
after the earlier 'if (!valid_p)' block, and then after that just
assume tdesc_data is not NULL.
Actually, while we're moving code around, is there any reason that
this call to tdesc_use_registers couldn't move up to be just after the
valid_p check? Given its closely related code?
> +
> return gdbarch;
> }
>
> @@ -1132,4 +1148,5 @@ void
> _initialize_rx_tdep (void)
> {
> register_gdbarch_init (bfd_arch_rx, rx_gdbarch_init);
> + initialize_tdesc_rx ();
> }
> --
> 2.11.0
>
Thanks,
Andrew
On Wed, 21 Aug 2019 18:24:33 +0900,
Andrew Burgess wrote:
>
> Thanks for the patch.
>
> * Yoshinori Sato <ysato@users.sourceforge.jp> [2019-08-21 12:33:01 +0900]:
>
> GDB patches usually include a brief description to the patch before
> the changelog entry in the commit message, even if its just one
> sentence like:
>
> Convert the RX target to make use of target descriptions.
OK.
> > gdb/ChangeLog
> >
> > 2019-08-21 Yoshinori Sato <ysato@users.sourceforge.jp>
> >
> > * gdb/rx-tdep.c (rx_register_names): New.
> > (rx_register_name): Use rx_register_names.
> > (rx_register_name): Add check range.
> > (rx_register_g_packet_guesses): New.
> > (rx_gdbarch_init): Convert target-descriptions.
> > (_initialize_rx_tdep): Add initialize_tdesc_rx.
> > * gdb/features/Makefile: Add rx.xml.
> > * gdb/features/rx.xml: New.
>
> You should generate and commit the gdb/features/rx.c file as well.
> These files are not automatically generated by the build system and we
> rely on people regenerating them when the xml file changes.
OK.
I forgat add it file.
Added it.
> There's a few minor coding standard issues I've pointed out below.
>
> > ---
> > gdb/features/Makefile | 2 ++
> > gdb/features/rx.xml | 70 ++++++++++++++++++++++++++++++++++++
> > gdb/rx-tdep.c | 99 ++++++++++++++++++++++++++++++---------------------
> > 3 files changed, 130 insertions(+), 41 deletions(-)
> > create mode 100644 gdb/features/rx.xml
> >
> > diff --git a/gdb/features/Makefile b/gdb/features/Makefile
> > index 0c84faf405..2b65d46df0 100644
> > --- a/gdb/features/Makefile
> > +++ b/gdb/features/Makefile
> > @@ -161,6 +161,7 @@ XMLTOC = \
> > rs6000/powerpc-vsx64.xml \
> > rs6000/powerpc-vsx64l.xml \
> > rs6000/rs6000.xml \
> > + rx.xml \
> > s390-linux32.xml \
> > s390-linux32v1.xml \
> > s390-linux32v2.xml \
> > @@ -238,6 +239,7 @@ FEATURE_XMLFILES = aarch64-core.xml \
> > riscv/64bit-cpu.xml \
> > riscv/64bit-csr.xml \
> > riscv/64bit-fpu.xml \
> > + rx.xml \
> > tic6x-c6xp.xml \
> > tic6x-core.xml \
> > tic6x-gp.xml
> > diff --git a/gdb/features/rx.xml b/gdb/features/rx.xml
> > new file mode 100644
> > index 0000000000..b5aa9ac4a8
> > --- /dev/null
> > +++ b/gdb/features/rx.xml
> > @@ -0,0 +1,70 @@
> > +<?xml version="1.0"?>
> > +<!-- Copyright (C) 2019 Free Software Foundation, Inc.
> > +
> > + Copying and distribution of this file, with or without modification,
> > + are permitted in any medium without royalty provided the copyright
> > + notice and this notice are preserved. -->
> > +
> > +<!DOCTYPE feature SYSTEM "gdb-target.dtd">
> > +<feature name="org.gnu.gdb.rx.core">
> > + <reg name="r0" bitsize="32" type="data_ptr"/>
> > + <reg name="r1" bitsize="32" type="uint32"/>
> > + <reg name="r2" bitsize="32" type="uint32"/>
> > + <reg name="r3" bitsize="32" type="uint32"/>
> > + <reg name="r4" bitsize="32" type="uint32"/>
> > + <reg name="r5" bitsize="32" type="uint32"/>
> > + <reg name="r6" bitsize="32" type="uint32"/>
> > + <reg name="r7" bitsize="32" type="uint32"/>
> > + <reg name="r8" bitsize="32" type="uint32"/>
> > + <reg name="r9" bitsize="32" type="uint32"/>
> > + <reg name="r10" bitsize="32" type="uint32"/>
> > + <reg name="r11" bitsize="32" type="uint32"/>
> > + <reg name="r12" bitsize="32" type="uint32"/>
> > + <reg name="r13" bitsize="32" type="uint32"/>
> > + <reg name="r14" bitsize="32" type="uint32"/>
> > + <reg name="r15" bitsize="32" type="uint32"/>
> > +
> > + <flags id="psw_flags" size="4">
> > + <field name="C" start="0" end="0"/>
> > + <field name="Z" start="1" end="1"/>
> > + <field name="S" start="2" end="2"/>
> > + <field name="O" start="3" end="3"/>
> > + <field name="I" start="16" end="16"/>
> > + <field name="U" start="17" end="17"/>
> > + <field name="PM" start="20" end="20"/>
> > + <field name="IPL" start="24" end="27"/>
> > + </flags>
> > +
> > + <flags id="fpsw_flags" size="4">
> > + <field name="RM" start="0" end="1"/>
> > + <field name="CV" start="2" end="2"/>
> > + <field name="CO" start="3" end="3"/>
> > + <field name="CZ" start="4" end="4"/>
> > + <field name="CU" start="5" end="5"/>
> > + <field name="CX" start="6" end="6"/>
> > + <field name="CE" start="7" end="7"/>
> > + <field name="DN" start="8" end="8"/>
> > + <field name="EV" start="10" end="10"/>
> > + <field name="EO" start="11" end="11"/>
> > + <field name="EZ" start="12" end="12"/>
> > + <field name="EU" start="13" end="13"/>
> > + <field name="EX" start="14" end="14"/>
> > + <field name="FV" start="26" end="26"/>
> > + <field name="FO" start="27" end="27"/>
> > + <field name="FZ" start="28" end="28"/>
> > + <field name="FU" start="29" end="29"/>
> > + <field name="FX" start="30" end="30"/>
> > + <field name="FS" start="31" end="31"/>
> > + </flags>
> > +
> > + <reg name="usp" bitsize="32" type="data_ptr"/>
> > + <reg name="isp" bitsize="32" type="data_ptr"/>
> > + <reg name="psw" bitsize="32" type="psw_flags"/>
> > + <reg name="pc" bitsize="32" type="code_ptr"/>
> > + <reg name="intb" bitsize="32" type="data_ptr"/>
> > + <reg name="bpsw" bitsize="32" type="psw_flags"/>
> > + <reg name="bpc" bitsize="32" type="code_ptr"/>
> > + <reg name="fintv" bitsize="32" type="code_ptr"/>
> > + <reg name="fpsw" bitsize="32" type="fpsw_flags"/>
> > + <reg name="acc" bitsize="64" type="uint64"/>
> > +</feature>
> > diff --git a/gdb/rx-tdep.c b/gdb/rx-tdep.c
> > index 4cbf919db9..b3398f122a 100644
> > --- a/gdb/rx-tdep.c
> > +++ b/gdb/rx-tdep.c
> > @@ -33,11 +33,15 @@
> > #include "value.h"
> > #include "gdbcore.h"
> > #include "dwarf2-frame.h"
> > +#include "remote.h"
> > +#include "target-descriptions.h"
> >
> > #include "elf/rx.h"
> > #include "elf-bfd.h"
> > #include <algorithm>
> >
> > +#include "features/rx.c"
> > +
> > /* Certain important register numbers. */
> > enum
> > {
> > @@ -114,40 +118,21 @@ struct rx_prologue
> > int reg_offset[RX_NUM_REGS];
> > };
> >
> > +static const char *const rx_register_names[] = {
> > + "r0", "r1", "r2", "r3", "r4", "r5", "r6", "r7",
> > + "r8", "r9", "r10", "r11", "r12", "r13", "r14", "r15",
> > + "usp", "isp", "psw", "pc", "intb", "bpsw","bpc","fintv",
> > + "fpsw", "acc",
> > +};
>
> All file level variables and functions should have a header comment.
OK.
Add comment.
> > +
> > /* Implement the "register_name" gdbarch method. */
> > static const char *
> > rx_register_name (struct gdbarch *gdbarch, int regnr)
> > {
> > - static const char *const reg_names[] = {
> > - "r0",
> > - "r1",
> > - "r2",
> > - "r3",
> > - "r4",
> > - "r5",
> > - "r6",
> > - "r7",
> > - "r8",
> > - "r9",
> > - "r10",
> > - "r11",
> > - "r12",
> > - "r13",
> > - "r14",
> > - "r15",
> > - "usp",
> > - "isp",
> > - "psw",
> > - "pc",
> > - "intb",
> > - "bpsw",
> > - "bpc",
> > - "fintv",
> > - "fpsw",
> > - "acc"
> > - };
> > -
> > - return reg_names[regnr];
> > + if (regnr >= 0 && regnr < RX_NUM_REGS)
> > + return rx_register_names[regnr];
> > + else
> > + return NULL;
> > }
> >
> > /* Construct the flags type for PSW and BPSW. */
> > @@ -1037,6 +1022,14 @@ rx_dwarf_reg_to_regnum (struct gdbarch *gdbarch, int reg)
> > return -1;
> > }
> >
> > +static void
> > +rx_register_g_packet_guesses (struct gdbarch *gdbarch)
> > +{
> > + register_remote_g_packet_guess (gdbarch,
> > + 4 * RX_NUM_REGS,
> > + tdesc_rx);
> > +}
>
> Header comment again.
>
> > +
> > /* Allocate and initialize a gdbarch object. */
> > static struct gdbarch *
> > rx_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
> > @@ -1044,6 +1037,8 @@ rx_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
> > struct gdbarch *gdbarch;
> > struct gdbarch_tdep *tdep;
> > int elf_flags;
> > + struct tdesc_arch_data *tdesc_data = NULL;
> > + const struct target_desc *tdesc = info.target_desc;
> >
> > /* Extract the elf_flags if available. */
> > if (info.abfd != NULL
> > @@ -1065,8 +1060,33 @@ rx_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
> > return arches->gdbarch;
> > }
> >
> > - /* None found, create a new architecture from the information
> > - provided. */
> > + if (tdesc == NULL)
> > + tdesc = tdesc_rx;
> > +
> > + /* Check any target description for validity. */
> > + if (tdesc_has_registers (tdesc))
> > + {
> > + const struct tdesc_feature *feature;
> > + int valid_p = 0;
>
> This should be a bool.
OK.
> > + int i = 0;
>
> This can move down into the if block scope.
OK.
> > + feature = tdesc_find_feature (tdesc, "org.gnu.gdb.rx.core");
> > +
> > + if (feature != NULL)
> > + {
> > + tdesc_data = tdesc_data_alloc ();
> > + valid_p = 1;
> > + for (i = 0; i < RX_NUM_REGS; i++)
> > + valid_p &= tdesc_numbered_register (feature, tdesc_data, i,
> > + rx_register_names[i]);
> > + }
> > +
> > + if (!valid_p)
> > + {
> > + tdesc_data_cleanup (tdesc_data);
> > + return NULL;
> > + }
> > + }
> > +
> > tdep = XCNEW (struct gdbarch_tdep);
> > gdbarch = gdbarch_alloc (&info, tdep);
> > tdep->elf_flags = elf_flags;
> > @@ -1083,15 +1103,6 @@ rx_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
> > set_gdbarch_sw_breakpoint_from_kind (gdbarch, rx_breakpoint::bp_from_kind);
> > set_gdbarch_skip_prologue (gdbarch, rx_skip_prologue);
> >
> > - /* Target builtin data types. */
> > - set_gdbarch_char_signed (gdbarch, 0);
> > - set_gdbarch_short_bit (gdbarch, 16);
> > - set_gdbarch_int_bit (gdbarch, 32);
> > - set_gdbarch_long_bit (gdbarch, 32);
> > - set_gdbarch_long_long_bit (gdbarch, 64);
> > - set_gdbarch_ptr_bit (gdbarch, 32);
> > - set_gdbarch_float_bit (gdbarch, 32);
> > - set_gdbarch_float_format (gdbarch, floatformats_ieee_single);
>
> I don't understand why these are being deleted. This doesn't feel
> like it should be related to conversion to target descriptions.
OK. Revert it.
> > if (elf_flags & E_FLAG_RX_64BIT_DOUBLES)
> > {
> > set_gdbarch_double_bit (gdbarch, 64);
> > @@ -1115,6 +1126,8 @@ rx_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
> > dwarf2_append_unwinders (gdbarch);
> > frame_unwind_append_unwinder (gdbarch, &rx_frame_unwind);
> >
> > + rx_register_g_packet_guesses (gdbarch);
> > +
> > /* Methods setting up a dummy call, and extracting the return value from
> > a call. */
> > set_gdbarch_push_dummy_call (gdbarch, rx_push_dummy_call);
> > @@ -1123,6 +1136,9 @@ rx_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
> > /* Virtual tables. */
> > set_gdbarch_vbit_in_delta (gdbarch, 1);
> >
> > + if (tdesc_data != NULL)
> > + tdesc_use_registers (gdbarch, tdesc, tdesc_data);
>
> I'm confused by the 'tdesc_data != NULL' check here, my reading of
> things is that either:
>
> 1. Target doesn't provide a target description, we fall back to the
> built in tdesc_rx, this will be valid, tdesc_data will not be NULL.
>
> 2. The target provides a description, but its invalid, we bail out
> earlier after the validity check, we never reach this code.
>
> 3. The target provides a description which is valid, tdesc_data will
> not be NULL.
>
> I'd prefer to see a 'gdb_assert (tdesc_data != NULL)' immediately
> after the earlier 'if (!valid_p)' block, and then after that just
> assume tdesc_data is not NULL.
OK.
> Actually, while we're moving code around, is there any reason that
> this call to tdesc_use_registers couldn't move up to be just after the
> valid_p check? Given its closely related code?
Since target-description can always be used, I moved to the front.
> > +
> > return gdbarch;
> > }
> >
> > @@ -1132,4 +1148,5 @@ void
> > _initialize_rx_tdep (void)
> > {
> > register_gdbarch_init (bfd_arch_rx, rx_gdbarch_init);
> > + initialize_tdesc_rx ();
> > }
> > --
> > 2.11.0
> >
>
> Thanks,
> Andrew
I'll sent v2 patch.
Thanks.
@@ -161,6 +161,7 @@ XMLTOC = \
rs6000/powerpc-vsx64.xml \
rs6000/powerpc-vsx64l.xml \
rs6000/rs6000.xml \
+ rx.xml \
s390-linux32.xml \
s390-linux32v1.xml \
s390-linux32v2.xml \
@@ -238,6 +239,7 @@ FEATURE_XMLFILES = aarch64-core.xml \
riscv/64bit-cpu.xml \
riscv/64bit-csr.xml \
riscv/64bit-fpu.xml \
+ rx.xml \
tic6x-c6xp.xml \
tic6x-core.xml \
tic6x-gp.xml
new file mode 100644
@@ -0,0 +1,70 @@
+<?xml version="1.0"?>
+<!-- Copyright (C) 2019 Free Software Foundation, Inc.
+
+ Copying and distribution of this file, with or without modification,
+ are permitted in any medium without royalty provided the copyright
+ notice and this notice are preserved. -->
+
+<!DOCTYPE feature SYSTEM "gdb-target.dtd">
+<feature name="org.gnu.gdb.rx.core">
+ <reg name="r0" bitsize="32" type="data_ptr"/>
+ <reg name="r1" bitsize="32" type="uint32"/>
+ <reg name="r2" bitsize="32" type="uint32"/>
+ <reg name="r3" bitsize="32" type="uint32"/>
+ <reg name="r4" bitsize="32" type="uint32"/>
+ <reg name="r5" bitsize="32" type="uint32"/>
+ <reg name="r6" bitsize="32" type="uint32"/>
+ <reg name="r7" bitsize="32" type="uint32"/>
+ <reg name="r8" bitsize="32" type="uint32"/>
+ <reg name="r9" bitsize="32" type="uint32"/>
+ <reg name="r10" bitsize="32" type="uint32"/>
+ <reg name="r11" bitsize="32" type="uint32"/>
+ <reg name="r12" bitsize="32" type="uint32"/>
+ <reg name="r13" bitsize="32" type="uint32"/>
+ <reg name="r14" bitsize="32" type="uint32"/>
+ <reg name="r15" bitsize="32" type="uint32"/>
+
+ <flags id="psw_flags" size="4">
+ <field name="C" start="0" end="0"/>
+ <field name="Z" start="1" end="1"/>
+ <field name="S" start="2" end="2"/>
+ <field name="O" start="3" end="3"/>
+ <field name="I" start="16" end="16"/>
+ <field name="U" start="17" end="17"/>
+ <field name="PM" start="20" end="20"/>
+ <field name="IPL" start="24" end="27"/>
+ </flags>
+
+ <flags id="fpsw_flags" size="4">
+ <field name="RM" start="0" end="1"/>
+ <field name="CV" start="2" end="2"/>
+ <field name="CO" start="3" end="3"/>
+ <field name="CZ" start="4" end="4"/>
+ <field name="CU" start="5" end="5"/>
+ <field name="CX" start="6" end="6"/>
+ <field name="CE" start="7" end="7"/>
+ <field name="DN" start="8" end="8"/>
+ <field name="EV" start="10" end="10"/>
+ <field name="EO" start="11" end="11"/>
+ <field name="EZ" start="12" end="12"/>
+ <field name="EU" start="13" end="13"/>
+ <field name="EX" start="14" end="14"/>
+ <field name="FV" start="26" end="26"/>
+ <field name="FO" start="27" end="27"/>
+ <field name="FZ" start="28" end="28"/>
+ <field name="FU" start="29" end="29"/>
+ <field name="FX" start="30" end="30"/>
+ <field name="FS" start="31" end="31"/>
+ </flags>
+
+ <reg name="usp" bitsize="32" type="data_ptr"/>
+ <reg name="isp" bitsize="32" type="data_ptr"/>
+ <reg name="psw" bitsize="32" type="psw_flags"/>
+ <reg name="pc" bitsize="32" type="code_ptr"/>
+ <reg name="intb" bitsize="32" type="data_ptr"/>
+ <reg name="bpsw" bitsize="32" type="psw_flags"/>
+ <reg name="bpc" bitsize="32" type="code_ptr"/>
+ <reg name="fintv" bitsize="32" type="code_ptr"/>
+ <reg name="fpsw" bitsize="32" type="fpsw_flags"/>
+ <reg name="acc" bitsize="64" type="uint64"/>
+</feature>
@@ -33,11 +33,15 @@
#include "value.h"
#include "gdbcore.h"
#include "dwarf2-frame.h"
+#include "remote.h"
+#include "target-descriptions.h"
#include "elf/rx.h"
#include "elf-bfd.h"
#include <algorithm>
+#include "features/rx.c"
+
/* Certain important register numbers. */
enum
{
@@ -114,40 +118,21 @@ struct rx_prologue
int reg_offset[RX_NUM_REGS];
};
+static const char *const rx_register_names[] = {
+ "r0", "r1", "r2", "r3", "r4", "r5", "r6", "r7",
+ "r8", "r9", "r10", "r11", "r12", "r13", "r14", "r15",
+ "usp", "isp", "psw", "pc", "intb", "bpsw","bpc","fintv",
+ "fpsw", "acc",
+};
+
/* Implement the "register_name" gdbarch method. */
static const char *
rx_register_name (struct gdbarch *gdbarch, int regnr)
{
- static const char *const reg_names[] = {
- "r0",
- "r1",
- "r2",
- "r3",
- "r4",
- "r5",
- "r6",
- "r7",
- "r8",
- "r9",
- "r10",
- "r11",
- "r12",
- "r13",
- "r14",
- "r15",
- "usp",
- "isp",
- "psw",
- "pc",
- "intb",
- "bpsw",
- "bpc",
- "fintv",
- "fpsw",
- "acc"
- };
-
- return reg_names[regnr];
+ if (regnr >= 0 && regnr < RX_NUM_REGS)
+ return rx_register_names[regnr];
+ else
+ return NULL;
}
/* Construct the flags type for PSW and BPSW. */
@@ -1037,6 +1022,14 @@ rx_dwarf_reg_to_regnum (struct gdbarch *gdbarch, int reg)
return -1;
}
+static void
+rx_register_g_packet_guesses (struct gdbarch *gdbarch)
+{
+ register_remote_g_packet_guess (gdbarch,
+ 4 * RX_NUM_REGS,
+ tdesc_rx);
+}
+
/* Allocate and initialize a gdbarch object. */
static struct gdbarch *
rx_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
@@ -1044,6 +1037,8 @@ rx_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
struct gdbarch *gdbarch;
struct gdbarch_tdep *tdep;
int elf_flags;
+ struct tdesc_arch_data *tdesc_data = NULL;
+ const struct target_desc *tdesc = info.target_desc;
/* Extract the elf_flags if available. */
if (info.abfd != NULL
@@ -1065,8 +1060,33 @@ rx_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
return arches->gdbarch;
}
- /* None found, create a new architecture from the information
- provided. */
+ if (tdesc == NULL)
+ tdesc = tdesc_rx;
+
+ /* Check any target description for validity. */
+ if (tdesc_has_registers (tdesc))
+ {
+ const struct tdesc_feature *feature;
+ int valid_p = 0;
+ int i = 0;
+ feature = tdesc_find_feature (tdesc, "org.gnu.gdb.rx.core");
+
+ if (feature != NULL)
+ {
+ tdesc_data = tdesc_data_alloc ();
+ valid_p = 1;
+ for (i = 0; i < RX_NUM_REGS; i++)
+ valid_p &= tdesc_numbered_register (feature, tdesc_data, i,
+ rx_register_names[i]);
+ }
+
+ if (!valid_p)
+ {
+ tdesc_data_cleanup (tdesc_data);
+ return NULL;
+ }
+ }
+
tdep = XCNEW (struct gdbarch_tdep);
gdbarch = gdbarch_alloc (&info, tdep);
tdep->elf_flags = elf_flags;
@@ -1083,15 +1103,6 @@ rx_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
set_gdbarch_sw_breakpoint_from_kind (gdbarch, rx_breakpoint::bp_from_kind);
set_gdbarch_skip_prologue (gdbarch, rx_skip_prologue);
- /* Target builtin data types. */
- set_gdbarch_char_signed (gdbarch, 0);
- set_gdbarch_short_bit (gdbarch, 16);
- set_gdbarch_int_bit (gdbarch, 32);
- set_gdbarch_long_bit (gdbarch, 32);
- set_gdbarch_long_long_bit (gdbarch, 64);
- set_gdbarch_ptr_bit (gdbarch, 32);
- set_gdbarch_float_bit (gdbarch, 32);
- set_gdbarch_float_format (gdbarch, floatformats_ieee_single);
if (elf_flags & E_FLAG_RX_64BIT_DOUBLES)
{
set_gdbarch_double_bit (gdbarch, 64);
@@ -1115,6 +1126,8 @@ rx_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
dwarf2_append_unwinders (gdbarch);
frame_unwind_append_unwinder (gdbarch, &rx_frame_unwind);
+ rx_register_g_packet_guesses (gdbarch);
+
/* Methods setting up a dummy call, and extracting the return value from
a call. */
set_gdbarch_push_dummy_call (gdbarch, rx_push_dummy_call);
@@ -1123,6 +1136,9 @@ rx_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
/* Virtual tables. */
set_gdbarch_vbit_in_delta (gdbarch, 1);
+ if (tdesc_data != NULL)
+ tdesc_use_registers (gdbarch, tdesc, tdesc_data);
+
return gdbarch;
}
@@ -1132,4 +1148,5 @@ void
_initialize_rx_tdep (void)
{
register_gdbarch_init (bfd_arch_rx, rx_gdbarch_init);
+ initialize_tdesc_rx ();
}