Remove use of deprecated_add_core_fns in cris_tdep.c

Message ID 20200210220001.39986-1-cbiesinger@google.com
State New, archived
Headers

Commit Message

Terekhov, Mikhail via Gdb-patches Feb. 10, 2020, 10 p.m. UTC
  The non-deprecated equivalent is implementing the gdbarch function
iterate_over_regset_sections, this patch does that.

Tested by generating a core file on cris under qemu and comparing
the output of "info registers".

gdb/ChangeLog:

2020-02-09  Christian Biesinger  <cbiesinger@google.com>

	* cris-tdep.c (cris_supply_gregset): Change signature to match
	what struct regset expects.
	(cris_regset): New struct.
	(fetch_core_registers): Remove.
	(cris_iterate_over_regset_sections): New function.
	(_initialize_cris_tdep): Don't call deprecated_add_core_fns.
	(cris_gdbarch_init): Call set_gdbarch_iterate_over_regset_sections.

Change-Id: Ieef895b5a2fdc797d1a913cd1c0c07563edfe8e7
---
 gdb/cris-tdep.c | 70 +++++++++++++++++++------------------------------
 1 file changed, 27 insertions(+), 43 deletions(-)
  

Comments

Tom Tromey Feb. 11, 2020, 3:59 p.m. UTC | #1
>>>>> "Christian" == Christian Biesinger via gdb-patches <gdb-patches@sourceware.org> writes:

Christian> The non-deprecated equivalent is implementing the gdbarch function
Christian> iterate_over_regset_sections, this patch does that.

Christian> Tested by generating a core file on cris under qemu and comparing
Christian> the output of "info registers".

Thank you for doing this.

Christian> 2020-02-09  Christian Biesinger  <cbiesinger@google.com>

Christian> 	* cris-tdep.c (cris_supply_gregset): Change signature to match
Christian> 	what struct regset expects.
Christian> 	(cris_regset): New struct.
Christian> 	(fetch_core_registers): Remove.
Christian> 	(cris_iterate_over_regset_sections): New function.
Christian> 	(_initialize_cris_tdep): Don't call deprecated_add_core_fns.
Christian> 	(cris_gdbarch_init): Call set_gdbarch_iterate_over_regset_sections.

Looks good to me.

Tom
  
Terekhov, Mikhail via Gdb-patches Feb. 11, 2020, 4:38 p.m. UTC | #2
On Tue, Feb 11, 2020 at 10:00 AM Tom Tromey <tom@tromey.com> wrote:
>
> >>>>> "Christian" == Christian Biesinger via gdb-patches <gdb-patches@sourceware.org> writes:
>
> Christian> The non-deprecated equivalent is implementing the gdbarch function
> Christian> iterate_over_regset_sections, this patch does that.
>
> Christian> Tested by generating a core file on cris under qemu and comparing
> Christian> the output of "info registers".
>
> Thank you for doing this.
>
> Christian> 2020-02-09  Christian Biesinger  <cbiesinger@google.com>
>
> Christian>      * cris-tdep.c (cris_supply_gregset): Change signature to match
> Christian>      what struct regset expects.
> Christian>      (cris_regset): New struct.
> Christian>      (fetch_core_registers): Remove.
> Christian>      (cris_iterate_over_regset_sections): New function.
> Christian>      (_initialize_cris_tdep): Don't call deprecated_add_core_fns.
> Christian>      (cris_gdbarch_init): Call set_gdbarch_iterate_over_regset_sections.
>
> Looks good to me.

Thanks, pushing now with a style fix to a comment (missing . and two
spaces), and these two additional lines in the description:
 This also fixes this warning when loading cris core files:
   warning: Unexpected size of section `.reg/164' in core file.

Christian
  

Patch

diff --git a/gdb/cris-tdep.c b/gdb/cris-tdep.c
index cc45a7f8eb..121e1edce2 100644
--- a/gdb/cris-tdep.c
+++ b/gdb/cris-tdep.c
@@ -37,6 +37,7 @@ 
 #include "osabi.h"
 #include "arch-utils.h"
 #include "regcache.h"
+#include "regset.h"
 
 #include "objfiles.h"
 
@@ -3761,21 +3762,28 @@  typedef cris_elf_greg_t crisv32_elf_gregset_t[CRISV32_ELF_NGREG];
 /* Unpack a cris_elf_gregset_t into GDB's register cache.  */
 
 static void 
-cris_supply_gregset (struct regcache *regcache, cris_elf_gregset_t *gregsetp)
+cris_supply_gregset (const struct regset *regset, struct regcache *regcache,
+		     int regnum, const void *gregs, size_t len)
 {
   struct gdbarch *gdbarch = regcache->arch ();
   struct gdbarch_tdep *tdep = gdbarch_tdep (gdbarch);
   int i;
-  cris_elf_greg_t *regp = *gregsetp;
+  const cris_elf_greg_t *regp = static_cast<const cris_elf_greg_t *>(gregs);
+
+  if (len != sizeof (cris_elf_gregset_t)
+      && len != sizeof (crisv32_elf_gregset_t))
+    warning (_("wrong size gregset struct in core file"));
+  gdb_assert (len >= sizeof (crisv32_elf_gregset_t));
 
   /* The kernel dumps all 32 registers as unsigned longs, but supply_register
      knows about the actual size of each register so that's no problem.  */
   for (i = 0; i < NUM_GENREGS + NUM_SPECREGS; i++)
     {
-      regcache->raw_supply (i, (char *)&regp[i]);
+      if (regnum == -1 || regnum == i)
+	regcache->raw_supply (i, (char *)&regp[i]);
     }
 
-  if (tdep->cris_version == 32)
+  if (tdep->cris_version == 32 && (regnum == -1 || regnum == ERP_REGNUM))
     {
       /* Needed to set pseudo-register PC for CRISv32.  */
       /* FIXME: If ERP is in a delay slot at this point then the PC will
@@ -3788,47 +3796,24 @@  cris_supply_gregset (struct regcache *regcache, cris_elf_gregset_t *gregsetp)
     }
 }
 
-/*  Use a local version of this function to get the correct types for
-    regsets, until multi-arch core support is ready.  */
+static const struct regset cris_regset = {
+  nullptr,
+  cris_supply_gregset,
+  /* We don't need a collect function because we only use this for core files
+     (via iterate_over_regset_sections) */
+  nullptr,
+  REGSET_VARIABLE_SIZE
+};
 
-static void
-fetch_core_registers (struct regcache *regcache,
-		      gdb_byte *core_reg_sect, unsigned core_reg_size,
-                      int which, CORE_ADDR reg_addr)
+static void cris_iterate_over_regset_sections (struct gdbarch *gdbarch,
+					       iterate_over_regset_sections_cb *cb,
+					       void *cb_data,
+					       const struct regcache *regcache)
 {
-  cris_elf_gregset_t gregset;
-
-  switch (which)
-    {
-    case 0:
-      if (core_reg_size != sizeof (cris_elf_gregset_t)
-	  && core_reg_size != sizeof (crisv32_elf_gregset_t))
-        {
-          warning (_("wrong size gregset struct in core file"));
-        }
-      else
-        {
-          memcpy (&gregset, core_reg_sect, sizeof (gregset));
-          cris_supply_gregset (regcache, &gregset);
-        }
-
-    default:
-      /* We've covered all the kinds of registers we know about here,
-         so this must be something we wouldn't know what to do with
-         anyway.  Just ignore it.  */
-      break;
-    }
+  cb (".reg", sizeof (crisv32_elf_gregset_t), sizeof (crisv32_elf_gregset_t),
+      &cris_regset, NULL, cb_data);
 }
 
-static struct core_fns cris_elf_core_fns =
-{
-  bfd_target_elf_flavour,               /* core_flavour */
-  default_check_format,                 /* check_format */
-  default_core_sniffer,                 /* core_sniffer */
-  fetch_core_registers,                 /* core_read_registers */
-  NULL                                  /* next */
-};
-
 void _initialize_cris_tdep ();
 void
 _initialize_cris_tdep ()
@@ -3868,8 +3853,6 @@  Makes GDB use the NRP register instead of the ERP register in certain cases."),
 			   NULL, /* FIXME: i18n: Usage of Dwarf-2 CFI
 				    for CRIS is %d.  */
 			   &setlist, &showlist);
-
-  deprecated_add_core_fns (&cris_elf_core_fns);
 }
 
 /* Prints out all target specific values.  */
@@ -4059,6 +4042,7 @@  cris_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
 
   set_gdbarch_breakpoint_kind_from_pc (gdbarch, cris_breakpoint_kind_from_pc);
   set_gdbarch_sw_breakpoint_from_kind (gdbarch, cris_sw_breakpoint_from_kind);
+  set_gdbarch_iterate_over_regset_sections (gdbarch, cris_iterate_over_regset_sections);
   
   if (tdep->cris_dwarf2_cfi == 1)
     {