[binutils-gdb] [PowerPC] Fix VSX registers in linux core files

[Pedro Franco de Carvalho <pedromfc at sourceware dot org>]

https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=2c3305f6b0432e37d26ce7761d0c4415ab5ca911

commit 2c3305f6b0432e37d26ce7761d0c4415ab5ca911
Author: Pedro Franco de Carvalho <pedromfc@linux.vnet.ibm.com>
Date:   Tue May 22 11:09:05 2018 -0300

    [PowerPC] Fix VSX registers in linux core files
    
    The functions used by the VSX regset to collect and supply registers
    from core files where incorrect. This patch changes the regset to use
    the standard regset collect/supply functions to fix this. The native
    target is also changed to use the same regset.
    
    gdb/ChangeLog:
    2018-05-22  Pedro Franco de Carvalho  <pedromfc@linux.vnet.ibm.com>
    
    	* ppc-linux-tdep.c (ppc_linux_vsxregset): New function.
    	(ppc32_linux_vsxregmap): New global.
    	(ppc32_linux_vsxregset): Initialize with ppc32_linux_vsxregmap,
    	regcache_supply_regset, and regcache_collect_regset.
    	* ppc-linux-tdep.h (ppc_linux_vsxregset): Declare.
    	* ppc-linux-nat.c (supply_vsxregset, fill_vsxregset): Remove.
    	(fetch_vsx_register, store_vsx_register): Remove.
    	(fetch_vsx_registers): Add regno parameter. Get regset using
    	ppc_linux_vsxregset. Use regset to supply registers.
    	(store_vsx_registers): Add regno parameter. Get regset using
    	ppc_linux_vsxregset. Use regset to collect registers.
    	(fetch_register): Call fetch_vsx_registers instead of
    	fetch_vsx_register.
    	(store_register): Call store_vsx_registers instead of
    	store_vsx_register.
    	(fetch_ppc_registers): Call fetch_vsx_registers with -1 for the
    	new regno parameter.
    	(store_ppc_registers): Call store_vsx_registers with -1 for the
    	new regno parameter.
    	* rs6000-tdep.c (ppc_vsx_support_p, ppc_supply_vsxreget)
    	(ppc_collect_vsxregset): Remove.
    
    gdb/testsuite/ChangeLog:
    2018-05-22  Pedro Franco de Carvalho  <pedromfc@linux.vnet.ibm.com>
    
    	* gdb.arch/powerpc-vsx-gcore.exp: New file.

Diff:
---
 gdb/ChangeLog                                |  24 ++++++
 gdb/ppc-linux-nat.c                          | 106 ++++-----------------------
 gdb/ppc-linux-tdep.c                         |  18 ++++-
 gdb/ppc-linux-tdep.h                         |   1 +
 gdb/rs6000-tdep.c                            |  74 -------------------
 gdb/testsuite/ChangeLog                      |   4 +
 gdb/testsuite/gdb.arch/powerpc-vsx-gcore.exp |  90 +++++++++++++++++++++++
 7 files changed, 149 insertions(+), 168 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index b234da8..9e74395 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,5 +1,29 @@
 2018-05-22  Pedro Franco de Carvalho  <pedromfc@linux.vnet.ibm.com>
 
+	* ppc-linux-tdep.c (ppc_linux_vsxregset): New function.
+	(ppc32_linux_vsxregmap): New global.
+	(ppc32_linux_vsxregset): Initialize with ppc32_linux_vsxregmap,
+	regcache_supply_regset, and regcache_collect_regset.
+	* ppc-linux-tdep.h (ppc_linux_vsxregset): Declare.
+	* ppc-linux-nat.c (supply_vsxregset, fill_vsxregset): Remove.
+	(fetch_vsx_register, store_vsx_register): Remove.
+	(fetch_vsx_registers): Add regno parameter. Get regset using
+	ppc_linux_vsxregset. Use regset to supply registers.
+	(store_vsx_registers): Add regno parameter. Get regset using
+	ppc_linux_vsxregset. Use regset to collect registers.
+	(fetch_register): Call fetch_vsx_registers instead of
+	fetch_vsx_register.
+	(store_register): Call store_vsx_registers instead of
+	store_vsx_register.
+	(fetch_ppc_registers): Call fetch_vsx_registers with -1 for the
+	new regno parameter.
+	(store_ppc_registers): Call store_vsx_registers with -1 for the
+	new regno parameter.
+	* rs6000-tdep.c (ppc_vsx_support_p, ppc_supply_vsxreget)
+	(ppc_collect_vsxregset): Remove.
+
+2018-05-22  Pedro Franco de Carvalho  <pedromfc@linux.vnet.ibm.com>
+
 	* ppc-tdep.h (struct ppc_reg_offsets): Remove vector register
 	offset fields.
 	* ppc-fbsd-tdep.c (ppc32_fbsd_reg_offsets): Remove initializers
diff --git a/gdb/ppc-linux-nat.c b/gdb/ppc-linux-nat.c
index e00831a..0f7dd4c 100644
--- a/gdb/ppc-linux-nat.c
+++ b/gdb/ppc-linux-nat.c
@@ -409,13 +409,11 @@ ppc_register_u_addr (struct gdbarch *gdbarch, int regno)
    registers set mechanism, as opposed to the interface for all the
    other registers, that stores/fetches each register individually.  */
 static void
-fetch_vsx_register (struct regcache *regcache, int tid, int regno)
+fetch_vsx_registers (struct regcache *regcache, int tid, int regno)
 {
   int ret;
   gdb_vsxregset_t regs;
-  struct gdbarch *gdbarch = regcache->arch ();
-  struct gdbarch_tdep *tdep = gdbarch_tdep (gdbarch);
-  int vsxregsize = register_size (gdbarch, tdep->ppc_vsr0_upper_regnum);
+  const struct regset *vsxregset = ppc_linux_vsxregset ();
 
   ret = ptrace (PTRACE_GETVSXREGS, tid, 0, &regs);
   if (ret < 0)
@@ -425,12 +423,11 @@ fetch_vsx_register (struct regcache *regcache, int tid, int regno)
 	  have_ptrace_getsetvsxregs = 0;
 	  return;
 	}
-      perror_with_name (_("Unable to fetch VSX register"));
+      perror_with_name (_("Unable to fetch VSX registers"));
     }
 
-  regcache_raw_supply (regcache, regno,
-		       regs + (regno - tdep->ppc_vsr0_upper_regnum)
-		       * vsxregsize);
+  vsxregset->supply_regset (vsxregset, regcache, regno, &regs,
+			    PPC_LINUX_SIZEOF_VSXREGSET);
 }
 
 /* The Linux kernel ptrace interface for AltiVec registers uses the
@@ -563,7 +560,7 @@ fetch_register (struct regcache *regcache, int tid, int regno)
     {
       if (have_ptrace_getsetvsxregs)
 	{
-	  fetch_vsx_register (regcache, tid, regno);
+	  fetch_vsx_registers (regcache, tid, regno);
 	  return;
 	}
     }
@@ -624,40 +621,6 @@ fetch_register (struct regcache *regcache, int tid, int regno)
                     gdbarch_byte_order (gdbarch));
 }
 
-static void
-supply_vsxregset (struct regcache *regcache, gdb_vsxregset_t *vsxregsetp)
-{
-  int i;
-  struct gdbarch *gdbarch = regcache->arch ();
-  struct gdbarch_tdep *tdep = gdbarch_tdep (gdbarch);
-  int vsxregsize = register_size (gdbarch, tdep->ppc_vsr0_upper_regnum);
-
-  for (i = 0; i < ppc_num_vshrs; i++)
-    {
-	regcache_raw_supply (regcache, tdep->ppc_vsr0_upper_regnum + i,
-			     *vsxregsetp + i * vsxregsize);
-    }
-}
-
-static void
-fetch_vsx_registers (struct regcache *regcache, int tid)
-{
-  int ret;
-  gdb_vsxregset_t regs;
-
-  ret = ptrace (PTRACE_GETVSXREGS, tid, 0, &regs);
-  if (ret < 0)
-    {
-      if (errno == EIO)
-	{
-	  have_ptrace_getsetvsxregs = 0;
-	  return;
-	}
-      perror_with_name (_("Unable to fetch VSX registers"));
-    }
-  supply_vsxregset (regcache, &regs);
-}
-
 /* This function actually issues the request to ptrace, telling
    it to get all general-purpose registers and put them into the
    specified regset.
@@ -799,7 +762,7 @@ fetch_ppc_registers (struct regcache *regcache, int tid)
       fetch_altivec_registers (regcache, tid, -1);
   if (have_ptrace_getsetvsxregs)
     if (tdep->ppc_vsr0_upper_regnum != -1)
-      fetch_vsx_registers (regcache, tid);
+      fetch_vsx_registers (regcache, tid, -1);
   if (tdep->ppc_ev0_upper_regnum >= 0)
     fetch_spe_register (regcache, tid, -1);
 }
@@ -818,15 +781,12 @@ ppc_linux_nat_target::fetch_registers (struct regcache *regcache, int regno)
     fetch_register (regcache, tid, regno);
 }
 
-/* Store one VSX register.  */
 static void
-store_vsx_register (const struct regcache *regcache, int tid, int regno)
+store_vsx_registers (const struct regcache *regcache, int tid, int regno)
 {
   int ret;
   gdb_vsxregset_t regs;
-  struct gdbarch *gdbarch = regcache->arch ();
-  struct gdbarch_tdep *tdep = gdbarch_tdep (gdbarch);
-  int vsxregsize = register_size (gdbarch, tdep->ppc_vsr0_upper_regnum);
+  const struct regset *vsxregset = ppc_linux_vsxregset ();
 
   ret = ptrace (PTRACE_GETVSXREGS, tid, 0, &regs);
   if (ret < 0)
@@ -836,15 +796,15 @@ store_vsx_register (const struct regcache *regcache, int tid, int regno)
 	  have_ptrace_getsetvsxregs = 0;
 	  return;
 	}
-      perror_with_name (_("Unable to fetch VSX register"));
+      perror_with_name (_("Unable to fetch VSX registers"));
     }
 
-  regcache_raw_collect (regcache, regno, regs +
-			(regno - tdep->ppc_vsr0_upper_regnum) * vsxregsize);
+  vsxregset->collect_regset (vsxregset, regcache, regno, &regs,
+			     PPC_LINUX_SIZEOF_VSXREGSET);
 
   ret = ptrace (PTRACE_SETVSXREGS, tid, 0, &regs);
   if (ret < 0)
-    perror_with_name (_("Unable to store VSX register"));
+    perror_with_name (_("Unable to store VSX registers"));
 }
 
 static void
@@ -980,7 +940,7 @@ store_register (const struct regcache *regcache, int tid, int regno)
     }
   if (vsx_register_p (gdbarch, regno))
     {
-      store_vsx_register (regcache, tid, regno);
+      store_vsx_registers (regcache, tid, regno);
       return;
     }
   else if (spe_register_p (gdbarch, regno))
@@ -1038,42 +998,6 @@ store_register (const struct regcache *regcache, int tid, int regno)
     }
 }
 
-static void
-fill_vsxregset (const struct regcache *regcache, gdb_vsxregset_t *vsxregsetp)
-{
-  int i;
-  struct gdbarch *gdbarch = regcache->arch ();
-  struct gdbarch_tdep *tdep = gdbarch_tdep (gdbarch);
-  int vsxregsize = register_size (gdbarch, tdep->ppc_vsr0_upper_regnum);
-
-  for (i = 0; i < ppc_num_vshrs; i++)
-    regcache_raw_collect (regcache, tdep->ppc_vsr0_upper_regnum + i,
-			  *vsxregsetp + i * vsxregsize);
-}
-
-static void
-store_vsx_registers (const struct regcache *regcache, int tid)
-{
-  int ret;
-  gdb_vsxregset_t regs;
-
-  ret = ptrace (PTRACE_GETVSXREGS, tid, 0, &regs);
-  if (ret < 0)
-    {
-      if (errno == EIO)
-	{
-	  have_ptrace_getsetvsxregs = 0;
-	  return;
-	}
-      perror_with_name (_("Couldn't get VSX registers"));
-    }
-
-  fill_vsxregset (regcache, &regs);
-
-  if (ptrace (PTRACE_SETVSXREGS, tid, 0, &regs) < 0)
-    perror_with_name (_("Couldn't write VSX registers"));
-}
-
 /* This function actually issues the request to ptrace, telling
    it to store all general-purpose registers present in the specified
    regset.
@@ -1235,7 +1159,7 @@ store_ppc_registers (const struct regcache *regcache, int tid)
       store_altivec_registers (regcache, tid, -1);
   if (have_ptrace_getsetvsxregs)
     if (tdep->ppc_vsr0_upper_regnum != -1)
-      store_vsx_registers (regcache, tid);
+      store_vsx_registers (regcache, tid, -1);
   if (tdep->ppc_ev0_upper_regnum >= 0)
     store_spe_register (regcache, tid, -1);
 }
diff --git a/gdb/ppc-linux-tdep.c b/gdb/ppc-linux-tdep.c
index 7362c4b..293353a 100644
--- a/gdb/ppc-linux-tdep.c
+++ b/gdb/ppc-linux-tdep.c
@@ -553,10 +553,16 @@ static const struct regset ppc32_be_linux_vrregset = {
   ppc_linux_collect_vrregset
 };
 
+static const struct regcache_map_entry ppc32_linux_vsxregmap[] =
+  {
+      { 32, PPC_VSR0_UPPER_REGNUM, 8 },
+      { 0 }
+  };
+
 static const struct regset ppc32_linux_vsxregset = {
-  &ppc32_linux_reg_offsets,
-  ppc_supply_vsxregset,
-  ppc_collect_vsxregset
+  ppc32_linux_vsxregmap,
+  regcache_supply_regset,
+  regcache_collect_regset
 };
 
 const struct regset *
@@ -580,6 +586,12 @@ ppc_linux_vrregset (struct gdbarch *gdbarch)
     return &ppc32_le_linux_vrregset;
 }
 
+const struct regset *
+ppc_linux_vsxregset (void)
+{
+  return &ppc32_linux_vsxregset;
+}
+
 /* Iterate over supported core file register note sections. */
 
 static void
diff --git a/gdb/ppc-linux-tdep.h b/gdb/ppc-linux-tdep.h
index a8715bd..51f4b50 100644
--- a/gdb/ppc-linux-tdep.h
+++ b/gdb/ppc-linux-tdep.h
@@ -30,6 +30,7 @@ const struct regset *ppc_linux_fpregset (void);
 
 /* Get the vector regset that matches the target byte order.  */
 const struct regset *ppc_linux_vrregset (struct gdbarch *gdbarch);
+const struct regset *ppc_linux_vsxregset (void);
 
 /* Extra register number constants.  The Linux kernel stores a
    "trap" code and the original value of r3 into special "registers";
diff --git a/gdb/rs6000-tdep.c b/gdb/rs6000-tdep.c
index 4ef6f9e..0a56c78 100644
--- a/gdb/rs6000-tdep.c
+++ b/gdb/rs6000-tdep.c
@@ -224,16 +224,6 @@ ppc_floating_point_unit_p (struct gdbarch *gdbarch)
 }
 
 /* Return non-zero if the architecture described by GDBARCH has
-   VSX registers (vsr0 --- vsr63).  */
-static int
-ppc_vsx_support_p (struct gdbarch *gdbarch)
-{
-  struct gdbarch_tdep *tdep = gdbarch_tdep (gdbarch);
-
-  return tdep->ppc_vsr0_regnum >= 0;
-}
-
-/* Return non-zero if the architecture described by GDBARCH has
    Altivec registers (vr0 --- vr31, vrsave and vscr).  */
 int
 ppc_altivec_support_p (struct gdbarch *gdbarch)
@@ -573,37 +563,6 @@ ppc_supply_fpregset (const struct regset *regset, struct regcache *regcache,
 		  regnum == tdep->ppc_fpscr_regnum ? offsets->fpscr_size : 8);
 }
 
-/* Supply register REGNUM in the VSX register set REGSET
-   from the buffer specified by VSXREGS and LEN to register cache
-   REGCACHE.  If REGNUM is -1, do this for all registers in REGSET.  */
-
-void
-ppc_supply_vsxregset (const struct regset *regset, struct regcache *regcache,
-		     int regnum, const void *vsxregs, size_t len)
-{
-  struct gdbarch *gdbarch = regcache->arch ();
-  struct gdbarch_tdep *tdep;
-
-  if (!ppc_vsx_support_p (gdbarch))
-    return;
-
-  tdep = gdbarch_tdep (gdbarch);
-
-  if (regnum == -1)
-    {
-      int i;
-
-      for (i = tdep->ppc_vsr0_upper_regnum;
-	   i < tdep->ppc_vsr0_upper_regnum + 32;
-	   i++)
-	ppc_supply_reg (regcache, i, (const gdb_byte *) vsxregs, 0, 8);
-
-      return;
-    }
-  else
-    ppc_supply_reg (regcache, regnum, (const gdb_byte *) vsxregs, 0, 8);
-}
-
 /* Collect register REGNUM in the general-purpose register set
    REGSET from register cache REGCACHE into the buffer specified by
    GREGS and LEN.  If REGNUM is -1, do this for all registers in
@@ -695,39 +654,6 @@ ppc_collect_fpregset (const struct regset *regset,
 		   regnum == tdep->ppc_fpscr_regnum ? offsets->fpscr_size : 8);
 }
 
-/* Collect register REGNUM in the VSX register set
-   REGSET from register cache REGCACHE into the buffer specified by
-   VSXREGS and LEN.  If REGNUM is -1, do this for all registers in
-   REGSET.  */
-
-void
-ppc_collect_vsxregset (const struct regset *regset,
-		      const struct regcache *regcache,
-		      int regnum, void *vsxregs, size_t len)
-{
-  struct gdbarch *gdbarch = regcache->arch ();
-  struct gdbarch_tdep *tdep;
-
-  if (!ppc_vsx_support_p (gdbarch))
-    return;
-
-  tdep = gdbarch_tdep (gdbarch);
-
-  if (regnum == -1)
-    {
-      int i;
-
-      for (i = tdep->ppc_vsr0_upper_regnum;
-	   i < tdep->ppc_vsr0_upper_regnum + 32;
-	   i++)
-	ppc_collect_reg (regcache, i, (gdb_byte *) vsxregs, 0, 8);
-
-      return;
-    }
-  else
-    ppc_collect_reg (regcache, regnum, (gdb_byte *) vsxregs, 0, 8);
-}
-
 static int
 insn_changes_sp_or_jumps (unsigned long insn)
 {
diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog
index cdd4ba9..82ab864 100644
--- a/gdb/testsuite/ChangeLog
+++ b/gdb/testsuite/ChangeLog
@@ -1,3 +1,7 @@
+2018-05-22  Pedro Franco de Carvalho  <pedromfc@linux.vnet.ibm.com>
+
+	* gdb.arch/powerpc-vsx-gcore.exp: New file.
+
 2018-05-18  Tom Tromey  <tom@tromey.com>
 
 	* gdb.base/ptype-offsets.exp: Update.
diff --git a/gdb/testsuite/gdb.arch/powerpc-vsx-gcore.exp b/gdb/testsuite/gdb.arch/powerpc-vsx-gcore.exp
new file mode 100644
index 0000000..e9bdfcd
--- /dev/null
+++ b/gdb/testsuite/gdb.arch/powerpc-vsx-gcore.exp
@@ -0,0 +1,90 @@
+# Copyright (C) 2018 Free Software Foundation, Inc.
+#
+# 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/>.
+
+# This file is part of the gdb testsuite.
+
+# This test checks that generating and loading a core file preserves
+# the correct VSX register state.
+
+if {![istarget "powerpc*-*-linux*"] || [skip_vsx_tests]} then {
+    verbose "Skipping PowerPC test for corefiles with VSX registers."
+    return
+}
+
+standard_testfile .c
+
+set gen_src [standard_output_file $srcfile]
+
+gdb_produce_source $gen_src {
+	int main() {
+	    return 0;
+	}
+}
+
+if {[build_executable "compile" $binfile $gen_src] == -1} {
+    return -1
+}
+
+clean_restart $binfile
+
+if ![runto_main] then {
+    fail "could not run to main"
+    return -1
+}
+
+# Check if VSX register access through gdb is supported
+proc check_vsx_access {} {
+    global gdb_prompt
+
+    set test "vsx register access"
+    gdb_test_multiple "info reg vs0" "$test" {
+	-re "Invalid register.*\r\n$gdb_prompt $" {
+	    unsupported "$test"
+	    return 0
+	}
+	-re "\r\nvs0.*\r\n$gdb_prompt $" {
+	    pass "$test"
+	    return 1
+	}
+    }
+    return 0
+}
+
+if { ![check_vsx_access] } {
+    return -1
+}
+
+for {set i 0} {$i < 64} {incr i 1} {
+    gdb_test_no_output "set \$vs$i.uint128 = $i"
+}
+
+set core_filename [standard_output_file "$testfile.core"]
+set core_generated [gdb_gcore_cmd "$core_filename" "generate core file"]
+
+if { !$core_generated } {
+    return -1
+}
+
+clean_restart
+
+set core_loaded [gdb_core_cmd "$core_filename" "load core file"]
+
+if { $core_loaded != 1 } {
+    return -1
+}
+
+for {set i 0} {$i < 64} {incr i 1} {
+    gdb_test "print \$vs$i.uint128" ".* = $i" "print vs$i"
+}