This is the mail archive of the gdb-patches@sourceware.org mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH v2 1/5] Arm: Add read_description read funcs and use in GDB



> On 11 Jul 2019, at 14:57, Simon Marchi <simark@simark.ca> wrote:
> 
> Oh, also please test rebuilding each patch against latest master, you will find the
> places that need to be changed from "common" to "gdbsupport", such as here:
> 
>> 

I had rebased, but looks like I had something left over causing the build to work.
Fixed.



> On 11 Jul 2019, at 15:19, Simon Marchi <simark@simark.ca> wrote:
> 
> On 2019-07-11 9:46 a.m., Alan Hayward wrote:
>> Switch the Arm target to get target descriptions via arm_read_description
>> and aarch32_read_description, in the same style as other feature targets.
>> Add an enum to specify the different types - this will also be of use to
>> gdbserver in a later patch.
>> 
>> Under the hood return the same existing pre-feature target descriptions.
>> 
>> Note: This commit will break the AArch64 gdbserver build.
> 
> Sorry for spamming you, one last thing :)
> 
> I forgot to ask you, what is it in this patch that is breaking the AArch64
> gdbserver build in this?  And what would it take for you not to break it?
> 
> We generally try to keep things building at each commit, so I'd just like
> a bit more justifications about why it's worth breaking it here, rather than
> doing the necessary work to keep it building.
> 

Annoyingly (for me) it’s just a single ifdef needed in arch/arm.c. I originally
had patch 4 directly after patch 1, which would have then required adding the
Arm files that get generated at build time into the aarch64 build.
Anyway, fixed.

Updated patch below:


diff --git a/gdb/Makefile.in b/gdb/Makefile.in
index b6680b9afd..62a59c90ea 100644
--- a/gdb/Makefile.in
+++ b/gdb/Makefile.in
@@ -665,7 +665,9 @@ ALL_64_TARGET_OBS = \

 # All other target-dependent objects files (used with --enable-targets=all).
 ALL_TARGET_OBS = \
+       aarch32-tdep.o \
        arc-tdep.o \
+       arch/aarch32.o \
        arch/arm.o \
        arch/arm-get-next-pcs.o \
        arch/arm-linux.o \
@@ -1184,6 +1186,7 @@ SFILES = \
 # right, it is probably easiest just to list .h files here directly.

 HFILES_NO_SRCDIR = \
+       aarch32-tdep.h \
        aarch64-ravenscar-thread.h \
        aarch64-tdep.h \
        ada-lang.h \
@@ -1429,6 +1432,7 @@ HFILES_NO_SRCDIR = \
        xml-syscall.h \
        xml-tdesc.h \
        xtensa-tdep.h \
+       arch/aarch32.h \
        arch/aarch64.h \
        arch/aarch64-insn.h \
        arch/arm.h \
@@ -2133,6 +2137,7 @@ force_update:
 MAKEOVERRIDES =

 ALLDEPFILES = \
+       aarch32-tdep.c \
        aarch64-fbsd-nat.c \
        aarch64-fbsd-tdep.c \
        aarch64-linux-nat.c \
diff --git a/gdb/aarch32-tdep.c b/gdb/aarch32-tdep.c
new file mode 100644
index 0000000000..12d464171c
--- /dev/null
+++ b/gdb/aarch32-tdep.c
@@ -0,0 +1,33 @@
+/* Copyright (C) 2019 Free Software Foundation, Inc.
+
+   This file is part of GDB.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+#include "gdbsupport/common-defs.h"
+#include "gdbsupport/common-regcache.h"
+#include "arch/aarch32.h"
+
+static struct target_desc *tdesc_aarch32;
+
+/* See aarch32-tdep.h.  */
+
+const target_desc *
+aarch32_read_description ()
+{
+  if (tdesc_aarch32 == nullptr)
+    tdesc_aarch32 = aarch32_create_target_description ();
+
+  return tdesc_aarch32;
+}
diff --git a/gdb/aarch32-tdep.h b/gdb/aarch32-tdep.h
new file mode 100644
index 0000000000..7fcea0adb9
--- /dev/null
+++ b/gdb/aarch32-tdep.h
@@ -0,0 +1,25 @@
+/* Copyright (C) 2019 Free Software Foundation, Inc.
+
+   This file is part of GDB.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+#ifndef AARCH32_TDEP_H
+#define AARCH32_TDEP_H
+
+/* Get the AArch32 target description.  */
+
+const target_desc *aarch32_read_description ();
+
+#endif /* aarch32-tdep.h.  */
diff --git a/gdb/aarch64-linux-nat.c b/gdb/aarch64-linux-nat.c
index 7b60a9a0c3..2c1f4d9f98 100644
--- a/gdb/aarch64-linux-nat.c
+++ b/gdb/aarch64-linux-nat.c
@@ -30,6 +30,7 @@
 #include "aarch64-tdep.h"
 #include "aarch64-linux-tdep.h"
 #include "aarch32-linux-nat.h"
+#include "aarch32-tdep.h"
 #include "arch/arm.h"
 #include "nat/aarch64-linux.h"
 #include "nat/aarch64-linux-hw-point.h"
@@ -631,8 +632,6 @@ aarch64_linux_nat_target::post_attach (int pid)
   linux_nat_target::post_attach (pid);
 }

-extern struct target_desc *tdesc_arm_with_neon;
-
 /* Implement the "read_description" target_ops method.  */

 const struct target_desc *
@@ -649,7 +648,7 @@ aarch64_linux_nat_target::read_description ()

   ret = ptrace (PTRACE_GETREGSET, tid, NT_ARM_VFP, &iovec);
   if (ret == 0)
-    return tdesc_arm_with_neon;
+    return aarch32_read_description ();

   CORE_ADDR hwcap = linux_get_hwcap (this);

diff --git a/gdb/arch/aarch32.c b/gdb/arch/aarch32.c
new file mode 100644
index 0000000000..14d6987d3f
--- /dev/null
+++ b/gdb/arch/aarch32.c
@@ -0,0 +1,29 @@
+/* Copyright (C) 2019 Free Software Foundation, Inc.
+
+   This file is part of GDB.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+#include "gdbsupport/common-defs.h"
+#include "aarch32.h"
+
+extern struct target_desc *tdesc_arm_with_neon;
+
+/* See aarch32.h.  */
+
+target_desc *
+aarch32_create_target_description ()
+{
+  return tdesc_arm_with_neon;
+}
diff --git a/gdb/arch/aarch32.h b/gdb/arch/aarch32.h
new file mode 100644
index 0000000000..d2c0047216
--- /dev/null
+++ b/gdb/arch/aarch32.h
@@ -0,0 +1,27 @@
+/* Copyright (C) 2019 Free Software Foundation, Inc.
+
+   This file is part of GDB.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+#ifndef ARCH_AARCH32_H
+#define ARCH_AARCH32_H
+
+#include "gdbsupport/tdesc.h"
+
+/* Create the AArch32 target description.  */
+
+target_desc *aarch32_create_target_description ();
+
+#endif /* aarch32.h.  */
diff --git a/gdb/arch/arm.c b/gdb/arch/arm.c
index 860ce02858..7a0f36e4c4 100644
--- a/gdb/arch/arm.c
+++ b/gdb/arch/arm.c
@@ -21,6 +21,17 @@
 #include "gdbsupport/common-regcache.h"
 #include "arm.h"

+extern struct target_desc *tdesc_arm_with_vfpv2;
+extern struct target_desc *tdesc_arm_with_vfpv3;
+extern struct target_desc *tdesc_arm_with_iwmmxt;
+
+/* Temporary ifdef.  Will be removed when target descriptions are switched.  */
+#ifndef GDBSERVER
+extern struct target_desc *tdesc_arm_with_m;
+extern struct target_desc *tdesc_arm_with_m_vfp_d16;
+extern struct target_desc *tdesc_arm_with_m_fpa_layout;
+#endif
+
 /* See arm.h.  */

 int
@@ -372,3 +383,52 @@ shifted_reg_val (struct regcache *regcache, unsigned long inst,

   return res & 0xffffffff;
 }
+
+/* See arch/arm.h.  */
+
+target_desc *
+arm_create_target_description (arm_fp_type fp_type)
+{
+  switch (fp_type)
+    {
+    case ARM_FP_TYPE_NONE:
+      return nullptr;
+/* Temporary ifdef.  Will be removed when target descriptions are switched.  */
+#ifndef GDBSERVER
+    case ARM_FP_TYPE_VFPV2:
+      return tdesc_arm_with_vfpv2;
+
+    case ARM_FP_TYPE_VFPV3:
+      return tdesc_arm_with_vfpv3;
+
+    case ARM_FP_TYPE_IWMMXT:
+      return tdesc_arm_with_iwmmxt;
+#endif
+    default:
+      error (_("Invalid Arm FP type: %d"), fp_type);
+    }
+}
+
+/* See arch/arm.h.  */
+
+target_desc *
+arm_create_mprofile_target_description (arm_m_profile_type m_type)
+{
+  switch (m_type)
+    {
+/* Temporary ifdef.  Will be removed when target descriptions are switched.  */
+#ifndef GDBSERVER
+    case ARM_M_TYPE_M_PROFILE:
+      return tdesc_arm_with_m;
+
+    case ARM_M_TYPE_VFP_D16:
+      return tdesc_arm_with_m_fpa_layout;
+
+    case ARM_M_TYPE_WITH_FPA:
+      return tdesc_arm_with_m_vfp_d16;
+#endif
+    default:
+      error (_("Invalid Arm M type: %d"), m_type);
+    }
+}
+
diff --git a/gdb/arch/arm.h b/gdb/arch/arm.h
index dfbbd56d28..58511c7c6b 100644
--- a/gdb/arch/arm.h
+++ b/gdb/arch/arm.h
@@ -19,6 +19,8 @@
 #ifndef ARCH_ARM_H
 #define ARCH_ARM_H

+#include "gdbsupport/tdesc.h"
+
 /* Register numbers of various important registers.  */

 enum gdb_regnum {
@@ -66,6 +68,23 @@ enum arm_breakpoint_kinds
    ARM_BP_KIND_ARM = 4,
 };

+/* Supported Arm FP hardware types.  */
+enum arm_fp_type {
+   ARM_FP_TYPE_NONE = 0,
+   ARM_FP_TYPE_VFPV2,
+   ARM_FP_TYPE_VFPV3,
+   ARM_FP_TYPE_IWMMXT,
+   ARM_FP_TYPE_INVALID
+};
+
+/* Supported M-profile Arm types.  */
+enum arm_m_profile_type {
+   ARM_M_TYPE_M_PROFILE,
+   ARM_M_TYPE_VFP_D16,
+   ARM_M_TYPE_WITH_FPA,
+   ARM_M_TYPE_INVALID
+};
+
 /* Instruction condition field values.  */
 #define INST_EQ                0x0
 #define INST_NE                0x1
@@ -165,4 +184,12 @@ unsigned long shifted_reg_val (struct regcache *regcache,
                               unsigned long pc_val,
                               unsigned long status_reg);

+/* Create an Arm target description with the given FP hardware type.  */
+
+target_desc *arm_create_target_description (arm_fp_type fp_type);
+
+/* Create an Arm M-profile target description with the given hardware type.  */
+
+target_desc *arm_create_mprofile_target_description (arm_m_profile_type m_type);
+
 #endif /* ARCH_ARM_H */
diff --git a/gdb/arm-fbsd-tdep.c b/gdb/arm-fbsd-tdep.c
index dea3abbdd3..6e1af9cda3 100644
--- a/gdb/arm-fbsd-tdep.c
+++ b/gdb/arm-fbsd-tdep.c
@@ -20,6 +20,8 @@
 #include "defs.h"

 #include "elf/common.h"
+#include "target-descriptions.h"
+#include "aarch32-tdep.h"
 #include "arm-tdep.h"
 #include "arm-fbsd-tdep.h"
 #include "auxv.h"
@@ -178,20 +180,20 @@ arm_fbsd_read_description_auxv (struct target_ops *target)
   CORE_ADDR arm_hwcap = 0;

   if (target_auxv_search (target, AT_FREEBSD_HWCAP, &arm_hwcap) != 1)
-    return NULL;
+    return nullptr;

   if (arm_hwcap & HWCAP_VFP)
     {
       if (arm_hwcap & HWCAP_NEON)
-       return tdesc_arm_with_neon;
+       return aarch32_read_description ();
       else if ((arm_hwcap & (HWCAP_VFPv3 | HWCAP_VFPD32))
          == (HWCAP_VFPv3 | HWCAP_VFPD32))
-       return tdesc_arm_with_vfpv3;
+       return arm_read_description (ARM_FP_TYPE_VFPV3);
       else
-       return tdesc_arm_with_vfpv2;
+      return arm_read_description (ARM_FP_TYPE_VFPV2);
     }

-  return NULL;
+  return nullptr;
 }

 /* Implement the "core_read_description" gdbarch method.  */
diff --git a/gdb/arm-linux-nat.c b/gdb/arm-linux-nat.c
index fe8a113a27..6a374bbc74 100644
--- a/gdb/arm-linux-nat.c
+++ b/gdb/arm-linux-nat.c
@@ -27,6 +27,7 @@
 #include "observable.h"
 #include "gdbthread.h"

+#include "aarch32-tdep.h"
 #include "arm-tdep.h"
 #include "arm-linux-tdep.h"
 #include "aarch32-linux-nat.h"
@@ -551,7 +552,7 @@ arm_linux_nat_target::read_description ()
     }

   if (arm_hwcap & HWCAP_IWMMXT)
-    return tdesc_arm_with_iwmmxt;
+    return arm_read_description (ARM_FP_TYPE_IWMMXT);

   if (arm_hwcap & HWCAP_VFP)
     {
@@ -566,11 +567,11 @@ arm_linux_nat_target::read_description ()
       /* NEON implies VFPv3-D32 or no-VFP unit.  Say that we only support
         Neon with VFPv3-D32.  */
       if (arm_hwcap & HWCAP_NEON)
-       return tdesc_arm_with_neon;
+       return aarch32_read_description ();
       else if ((arm_hwcap & (HWCAP_VFPv3 | HWCAP_VFPv3D16)) == HWCAP_VFPv3)
-       return tdesc_arm_with_vfpv3;
-      else
-       return tdesc_arm_with_vfpv2;
+       return arm_read_description (ARM_FP_TYPE_VFPV3);
+
+      return arm_read_description (ARM_FP_TYPE_VFPV2);
     }

   return this->beneath ()->read_description ();
diff --git a/gdb/arm-linux-tdep.c b/gdb/arm-linux-tdep.c
index d846749e0b..aec20877d9 100644
--- a/gdb/arm-linux-tdep.c
+++ b/gdb/arm-linux-tdep.c
@@ -33,6 +33,7 @@
 #include "auxv.h"
 #include "xml-syscall.h"

+#include "aarch32-tdep.h"
 #include "arch/arm.h"
 #include "arch/arm-get-next-pcs.h"
 #include "arch/arm-linux.h"
@@ -738,14 +739,14 @@ arm_linux_core_read_description (struct gdbarch *gdbarch,
       /* NEON implies VFPv3-D32 or no-VFP unit.  Say that we only support
          Neon with VFPv3-D32.  */
       if (arm_hwcap & HWCAP_NEON)
-       return tdesc_arm_with_neon;
+       return aarch32_read_description ();
       else if ((arm_hwcap & (HWCAP_VFPv3 | HWCAP_VFPv3D16)) == HWCAP_VFPv3)
-       return tdesc_arm_with_vfpv3;
-      else
-       return tdesc_arm_with_vfpv2;
+       return arm_read_description (ARM_FP_TYPE_VFPV3);
+
+      return arm_read_description (ARM_FP_TYPE_VFPV2);
     }

-  return NULL;
+  return nullptr;
 }


diff --git a/gdb/arm-tdep.c b/gdb/arm-tdep.c
index c1ee39714f..1b19b72ca1 100644
--- a/gdb/arm-tdep.c
+++ b/gdb/arm-tdep.c
@@ -240,6 +240,10 @@ static const char **valid_disassembly_styles;
 /* Disassembly style to use. Default to "std" register names.  */
 static const char *disassembly_style;

+/* All possible arm target descriptors.  */
+static struct target_desc *tdesc_arm_list[ARM_FP_TYPE_INVALID];
+static struct target_desc *tdesc_arm_mprofile_list[ARM_M_TYPE_INVALID];
+
 /* This is used to keep the bfd arch_info in sync with the disassembly
    style.  */
 static void set_disassembly_style_sfunc (const char *, int,
@@ -8739,7 +8743,6 @@ arm_register_reggroup_p (struct gdbarch *gdbarch, int regnum,
     return default_register_reggroup_p (gdbarch, regnum, group);
 }

-^L
 /* For backward-compatibility we allow two 'g' packet lengths with
    the remote protocol depending on whether FPA registers are
    supplied.  M-profile targets do not have FPA registers, but some
@@ -8753,21 +8756,26 @@ arm_register_g_packet_guesses (struct gdbarch *gdbarch)
 {
   if (gdbarch_tdep (gdbarch)->is_m)
     {
+      const target_desc *tdesc;
+
       /* If we know from the executable this is an M-profile target,
         cater for remote targets whose register set layout is the
         same as the FPA layout.  */
+      tdesc = arm_read_mprofile_description (ARM_M_TYPE_WITH_FPA);
       register_remote_g_packet_guess (gdbarch,
                                      ARM_CORE_REGS_SIZE + ARM_FP_REGS_SIZE,
-                                     tdesc_arm_with_m_fpa_layout);
+                                     tdesc);

       /* The regular M-profile layout.  */
+      tdesc = arm_read_mprofile_description (ARM_M_TYPE_M_PROFILE);
       register_remote_g_packet_guess (gdbarch, ARM_CORE_REGS_SIZE,
-                                     tdesc_arm_with_m);
+                                     tdesc);

       /* M-profile plus M4F VFP.  */
+      tdesc = arm_read_mprofile_description (ARM_M_TYPE_VFP_D16);
       register_remote_g_packet_guess (gdbarch,
                                      ARM_CORE_REGS_SIZE + ARM_VFP2_REGS_SIZE,
-                                     tdesc_arm_with_m_vfp_d16);
+                                     tdesc);
     }

   /* Otherwise we don't have a useful guess.  */
@@ -13310,3 +13318,35 @@ arm_process_record (struct gdbarch *gdbarch, struct regcache *regcache,

   return ret;
 }
+
+/* See arm-tdep.h.  */
+
+const target_desc *
+arm_read_description (arm_fp_type fp_type)
+{
+  struct target_desc *tdesc = tdesc_arm_list[fp_type];
+
+  if (tdesc == nullptr)
+    {
+      tdesc = arm_create_target_description (fp_type);
+      tdesc_arm_list[fp_type] = tdesc;
+    }
+
+  return tdesc;
+}
+
+/* See arm-tdep.h.  */
+
+const target_desc *
+arm_read_mprofile_description (arm_m_profile_type m_type)
+{
+  struct target_desc *tdesc = tdesc_arm_mprofile_list[m_type];
+
+  if (tdesc == nullptr)
+    {
+      tdesc = arm_create_mprofile_target_description (m_type);
+      tdesc_arm_mprofile_list[m_type] = tdesc;
+    }
+
+  return tdesc;
+}
diff --git a/gdb/arm-tdep.h b/gdb/arm-tdep.h
index 36d2d381cf..6d1a91ca35 100644
--- a/gdb/arm-tdep.h
+++ b/gdb/arm-tdep.h
@@ -281,11 +281,11 @@ extern void
                                       void *cb_data,
                                       const struct regcache *regcache);

-/* Target descriptions.  */
-extern struct target_desc *tdesc_arm_with_m;
-extern struct target_desc *tdesc_arm_with_iwmmxt;
-extern struct target_desc *tdesc_arm_with_vfpv2;
-extern struct target_desc *tdesc_arm_with_vfpv3;
-extern struct target_desc *tdesc_arm_with_neon;
+/* Get the correct Arm target description with given FP hardware type.  */
+const target_desc *arm_read_description (arm_fp_type fp_type);
+
+/* Get the correct Arm M-Profile target description with given hardware
+   type.  */
+const target_desc *arm_read_mprofile_description (arm_m_profile_type m_type);

 #endif /* arm-tdep.h */
diff --git a/gdb/configure.tgt b/gdb/configure.tgt
index 27f122ad04..7c0215e89a 100644
--- a/gdb/configure.tgt
+++ b/gdb/configure.tgt
@@ -48,8 +48,9 @@ amd64_tobjs="amd64-tdep.o arch/amd64.o ${x86_tobjs}"

 case "${targ}" in
 aarch64*-*-*)
-       cpu_obs="aarch64-tdep.o arch/aarch64-insn.o arch/aarch64.o \
-                ravenscar-thread.o aarch64-ravenscar-thread.o";;
+       cpu_obs="aarch32-tdep.o aarch64-tdep.o arch/aarch32.o \
+                arch/aarch64-insn.o arch/aarch64.o  ravenscar-thread.o \
+                aarch64-ravenscar-thread.o";;

 alpha*-*-*)
        # Target: Alpha
@@ -62,7 +63,8 @@ arc*-*-*)
        ;;

 arm*-*-*)
-       cpu_obs="arch/arm.o arch/arm-get-next-pcs.o arm-tdep.o";;
+       cpu_obs="aarch32-tdep.o arch/aarch32.o arch/arm.o \
+                arch/arm-get-next-pcs.o arm-tdep.o";;

 hppa*-*-*)
        # Target: HP PA-RISC


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]