回复：[PATCH] gdb:csky using csky_gdbarch_tdep in csky_gdbarch_init

[李江帅 <jiangshuai_li@c-sky.com>]

Thanks Simon Marchi for your review. To find a mach is used in the version of gdb7.2,
the code is too old and is not suitable in the current GDB.

I have deleted it and changed this patch as below:

From 17f01b32d7e2794247f568c58f57f7e4fff2be98 Mon Sep 17 00:00:00 2001
From: Jiangshuai Li <jiangshuai_li@c-sky.com>
Date: Wed, 16 Mar 2022 15:02:45 +0800
Subject: [PATCH] gdb:csky save fpu and vdsp info to struct csky_gdbarch_tdep

First, add three variables fpu_abi, fpu_hardfp and vdsp_version
to csky_gdbarch_tdep. They will be initialized from info.abfd in
cskg_gdbarch_init.

Now, they are just used to find a candidate among the list of pre-declared
architectures.

Later, they will be used in gdbarch_return_value and gdbarch_push_dummy_call
for funtions described below:
fpu_abi: to check whether the bfd is using VAL_CSKY_FPU_ABI_HARD or
VAL_CSKY_FPU_ABI_SOFT
fpu_hardfp: to check whether the bfd is using VAL_CSKY_FPU_HARDFP_SINGLE
or VAL_CSKY_FPU_HARDFP_DOUBLE
vdsp_version: to check if a function is returned with CSKY_VRET_REGNUM
---
 gdb/csky-tdep.c | 42 +++++++++++++++++++++++++++++++++++++++---
 gdb/csky-tdep.h |  5 ++++-
 2 files changed, 43 insertions(+), 4 deletions(-)

diff --git a/gdb/csky-tdep.c b/gdb/csky-tdep.c
index cba0065fa53..04f558cf14b 100644
--- a/gdb/csky-tdep.c
+++ b/gdb/csky-tdep.c
@@ -2157,16 +2157,52 @@ static struct gdbarch *
 csky_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
 {
   struct gdbarch *gdbarch;
+  /* Analyze info.abfd.  */
+  unsigned int fpu_abi = 0;
+  unsigned int vdsp_version = 0;
+  unsigned int fpu_hardfp = 0;
+
+  /* When the type of bfd file is srec(or any files are not elf),
+     the E_FLAGS will be not credible.  */
+  if (info.abfd != NULL && bfd_get_flavour (info.abfd) == bfd_target_elf_flavour)
+    {
+      /* Get FPU, VDSP build options.  */
+      fpu_abi = bfd_elf_get_obj_attr_int (info.abfd,
+                                          OBJ_ATTR_PROC,
+                                          Tag_CSKY_FPU_ABI);
+      vdsp_version = bfd_elf_get_obj_attr_int (info.abfd,
+                                               OBJ_ATTR_PROC,
+                                               Tag_CSKY_VDSP_VERSION);
+      fpu_hardfp = bfd_elf_get_obj_attr_int (info.abfd,
+                                             OBJ_ATTR_PROC,
+                                             Tag_CSKY_FPU_HARDFP);
+    }
   /* Find a candidate among the list of pre-declared architectures.  */
-  arches = gdbarch_list_lookup_by_info (arches, &info);
-  if (arches != NULL)
-    return arches->gdbarch;
+  for (arches = gdbarch_list_lookup_by_info (arches, &info);
+       arches != NULL;
+       arches = gdbarch_list_lookup_by_info (arches->next, &info))
+    {
+      csky_gdbarch_tdep *tdep
+        = (csky_gdbarch_tdep *) gdbarch_tdep (arches->gdbarch);
+      if (fpu_abi != tdep->fpu_abi)
+        continue;
+      if (vdsp_version != tdep->vdsp_version)
+        continue;
+      if (fpu_hardfp != tdep->fpu_hardfp)
+        continue;
+
+      /* Found a match.  */
+      return arches->gdbarch;
+    }
   /* None found, create a new architecture from the information
      provided.  */
   csky_gdbarch_tdep *tdep = new csky_gdbarch_tdep;
   gdbarch = gdbarch_alloc (&info, tdep);
+  tdep->fpu_abi = fpu_abi;
+  tdep->vdsp_version = vdsp_version;
+  tdep->fpu_hardfp = fpu_hardfp;
   /* Target data types.  */
   set_gdbarch_ptr_bit (gdbarch, 32);
diff --git a/gdb/csky-tdep.h b/gdb/csky-tdep.h
index 7898e0d325b..d0e5fc09270 100644
--- a/gdb/csky-tdep.h
+++ b/gdb/csky-tdep.h
@@ -33,7 +33,10 @@ enum lr_type_t
 /* Target-dependent structure in gdbarch.  */
 struct csky_gdbarch_tdep : gdbarch_tdep
 {
-  /* This is Unused.  */
+  /* Save FPU, VDSP ABI.  */
+  unsigned int fpu_abi;
+  unsigned int fpu_hardfp;
+  unsigned int vdsp_version;
 };
 /* Instruction sizes.  */
-- 2.17.1


------------------------------------------------------------------
发件人：Simon Marchi <simark@simark.ca>
发送时间：2022年3月22日(星期二) 22:19
收件人：李江帅 <jiangshuai_li@c-sky.com>; gdb-patches <gdb-patches@sourceware.org>
主　题：Re: [PATCH] gdb:csky using csky_gdbarch_tdep in csky_gdbarch_init

On 2022-03-22 07:52, Jiangshuai Li wrote:
> add three points:
> 1. add vars to csky_gdbarch_tdep
> 2. save mach, fpu_abi, vdsp_verion and fpu_hardfp from info.abfd
> 3. find a pre-declared gdbarch via point 2
>
> fpu_abi and vdsp_verion will be used by gdbarch_return_value
> and gdbarch_push_dummy_call later.
>
> gdb/
>  *csky-tdep.h: add vars to csky_gdbarch_tdep
>  *csky-tdep.c: save msgs from info.abfd and use them
>  to find a pre-declared gdbarch

Don't need a ChangeLog entry for GDB.

The subject of the patch:

  csky using csky_gdbarch_tdep in csky_gdbarch_init

is not clear.  What behavior change do you want to introduce here?

> ---
>  gdb/csky-tdep.c | 87 +++++++++++++++++++++++++++++++++++++++++++++++--
>  gdb/csky-tdep.h | 17 +++++++++-
>  2 files changed, 100 insertions(+), 4 deletions(-)
>
> diff --git a/gdb/csky-tdep.c b/gdb/csky-tdep.c
> index cba0065fa53..44c2b56fba4 100644
> --- a/gdb/csky-tdep.c
> +++ b/gdb/csky-tdep.c
> @@ -2157,16 +2157,97 @@ static struct gdbarch *
>  csky_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
>  {
>    struct gdbarch *gdbarch;
> +  /* Analysis info.abfd.  */

Analysis -> Analyze ?

> +  unsigned int mach;
> +  int mach_find = 0;
> +  unsigned long abfd_e_flags;
> +  unsigned int fpu_abi = 0;
> +  unsigned int vdsp_version = 0;
> +  unsigned int fpu_hardfp = 0;
> +  static bfd_arch_info_type bfd_csky_arch_t;

What's the purpose of this variable being static?

> +
> +  /* When the type of bfd file is srec(or any files are not elf),
> +     the E_FLAGS will be not credible.  */
> +  if (info.abfd && bfd_get_flavour (info.abfd) == bfd_target_elf_flavour)

info.abfd != nullptr

> +    {
> +      /*  Get e_flags form abfd.  */
> +      abfd_e_flags = elf_elfheader (info.abfd)->e_flags;
> +      if (IS_CSKY_V2 (abfd_e_flags))
> +        {
> +          switch (abfd_e_flags & CSKY_ARCH_MASK)

Please correct the indentation to fit the coding style (tabs + spaces).

> +            {
> +            case CSKY_ARCH_801:
> +            case CSKY_ARCH_802:
> +            case CSKY_ARCH_803:
> +            case CSKY_ARCH_807:
> +            case CSKY_ARCH_810:
> +            case CSKY_ARCH_860:
> +              mach = abfd_e_flags & CSKY_ARCH_MASK;
> +              mach_find = 1;

It's not clear why you need to do this here.  Selecting the right
machine type based on flags is normally the kind of things done in BFD.
So by the time it reaches GDB, the correct bfd_arch_info should have
already been selected by BFD.  For example, if the ELF file is a
CSKY_ARCH_803, then info.abfd->arch_info should already point to
bfd_mach_ck803.

> +              break;
> +            default:
> +              mach = CSKY_ARCH_810;
> +              break;
> +            }
> +        }
> +      else
> +        {
> +          mach = CSKY_ARCH_810;
> +        }
> +
> +      /* Get FPU, VDSP build options.  */
> +      fpu_abi = bfd_elf_get_obj_attr_int (info.abfd,
> +                                          OBJ_ATTR_PROC,
> +                                          Tag_CSKY_FPU_ABI);
> +      vdsp_version = bfd_elf_get_obj_attr_int (info.abfd,
> +                                               OBJ_ATTR_PROC,
> +                                               Tag_CSKY_VDSP_VERSION);
> +      fpu_hardfp = bfd_elf_get_obj_attr_int (info.abfd,
> +                                             OBJ_ATTR_PROC,
> +                                             Tag_CSKY_FPU_HARDFP);
> +    }
> +  else
> +    {
> +      mach = CSKY_ARCH_810;
> +    }
> +
> +  /* Set info.bfd_arch_info.  */
> +  memcpy (&bfd_csky_arch_t, info.bfd_arch_info, sizeof (bfd_csky_arch_t));
> +  if (mach_find)
> +    bfd_csky_arch_t.mach = abfd_e_flags;
> +  else
> +    bfd_csky_arch_t.mach = mach | CSKY_ABI_V2;
> +  info.bfd_arch_info = &bfd_csky_arch_t;
>
>    /* Find a candidate among the list of pre-declared architectures.  */
> -  arches = gdbarch_list_lookup_by_info (arches, &info);
> -  if (arches != NULL)
> -    return arches->gdbarch;
> +  for (arches = gdbarch_list_lookup_by_info (arches, &info);
> +       arches != NULL;
> +       arches = gdbarch_list_lookup_by_info (arches->next, &info))
> +    {
> +      csky_gdbarch_tdep *tdep
> +        = (csky_gdbarch_tdep *) gdbarch_tdep (arches->gdbarch);
> +      if (mach != tdep->mach)
> +        continue;

As explained above, this filtering on mach should not be necessary: two
ELF files with different machine values (in their e_flags) should lead
to two abfds with different arch infos.  And gdbarch_list_lookup_by_info
will automatically filters out those with different arch infos.

It's not clear what problem you are trying to solve, maybe if try to
explain that first it will be easier to understand what you are trying
to achieve.

Simon