Re: [PATCH v1] target/m68k: fix gdb for m68xxx

[KONRAD Frederic]

Le 4/20/20 à 5:46 PM, Laurent Vivier a écrit :
> Le 20/04/2020 à 16:01, address@hidden a écrit :
> > From: KONRAD Frederic <address@hidden>
> >
> > Currently "cf-core.xml" is sent to GDB when using any m68k flavor.  Thing is
> > it uses the "org.gnu.gdb.coldfire.core" feature name and gdb 8.3 then expects
> > a coldfire FPU instead of the default m68881 FPU.
>
>
> I checked in gdb sources and there is no cf definition.
>
> Moreover if I change only the cf to m68k in QEMU it seems to work in
> both cases:
>
> diff --git a/gdb-xml/cf-core.xml b/gdb-xml/cf-core.xml
> index b90af3042c..5b092d26de 100644
> --- a/gdb-xml/cf-core.xml
> +++ b/gdb-xml/cf-core.xml
> @@ -5,7 +5,7 @@
>        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.coldfire.core">
> +<feature name="org.gnu.gdb.m68k.core">
>     <reg name="d0" bitsize="32"/>
>     <reg name="d1" bitsize="32"/>
>     <reg name="d2" bitsize="32"/>

Doesn't that break gdb with coldfire?

> diff --git a/gdb-xml/m68k-fp.xml b/gdb-xml/m68k-fp.xml
> index 64290d1630..0ef74f7488 100644
> --- a/gdb-xml/m68k-fp.xml
> +++ b/gdb-xml/m68k-fp.xml
> @@ -5,7 +5,7 @@
>        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.coldfire.fp">
> +<feature name="org.gnu.gdb.m68k.fp">
>     <reg name="fp0" bitsize="96" type="float" group="float"/>
>     <reg name="fp1" bitsize="96" type="float" group="float"/>
>     <reg name="fp2" bitsize="96" type="float" group="float"/>
>
> As I have not checked the gdb sources for that, I'd like to have your
> opinion.

In the GDB 8.3 sources: m68k-tdep.c:1091:

      feature = tdesc_find_feature (info.target_desc,
                                    "org.gnu.gdb.m68k.core");
      if (feature == NULL)
        {
          feature = tdesc_find_feature (info.target_desc,
                                        "org.gnu.gdb.coldfire.core");
          if (feature != NULL)
            flavour = m68k_coldfire_flavour;
        }

Hence the change I suggested.  Little later it has also:

      feature = tdesc_find_feature (info.target_desc,
                                    "org.gnu.gdb.coldfire.fp");
      if (feature != NULL)
        {
          valid_p = 1;
          for (i = M68K_FP0_REGNUM; i <= M68K_FPI_REGNUM; i++)
            valid_p &= tdesc_numbered_register (feature, tdesc_data, i,
                                                m68k_register_names[i]);
          if (!valid_p)
            {
              tdesc_data_cleanup (tdesc_data);
              return NULL;
            }
        }
      else
        has_fp = 0;

Which is why I didn't made the change you suggested about the m68k-fp.xml but I
just tried with this additional change and it doesn't seem to hurt.

Cheers,
Fred

> Thanks,
> Laurent