This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [PATCH] Improve the fetch/store of general-purpose and floating-point PowerPC registers
- From: Sérgio Durigan Júnior <sergiodj at linux dot vnet dot ibm dot com>
- To: Joel Brobecker <brobecker at adacore dot com>
- Cc: luisgpm at linux dot vnet dot ibm dot com, gdb-patches at sourceware dot org, Mark Kettenis <mark dot kettenis at xs4all dot nl>
- Date: Tue, 05 May 2009 15:34:24 -0300
- Subject: Re: [PATCH] Improve the fetch/store of general-purpose and floating-point PowerPC registers
- References: <1223404355.7030.20.camel@miki> <1224184035.27672.64.camel@miki> <1231289020.2026.3.camel@miki> <200901070955.n079twBj014790@brahms.sibelius.xs4all.nl> <1231344540.2026.7.camel@miki> <1231436749.2026.19.camel@miki> <1231503988.12849.6.camel@gargoyle> <1232558849.6328.23.camel@miki> <20090428200725.GC31821@adacore.com> <1240953172.2392.1.camel@miki> <20090429040506.GI31821@adacore.com>
Hi Joel,
On Tue, 2009-04-28 at 21:05 -0700, Joel Brobecker wrote:
> Actually, no. The comments are for the functions themselves. We're
> trying to make sure that every new function gets in with some
> documentation of what it does. It doesn't have to be very long,
> but sometimes writing what the return value is about is very useful.
> For instance, I remember that some of your functions will return
> zero if the operation failed, I think. That's an interesting piece
> of information to put in the documentation. When the function is
> obvious, or when it implements a routine that's part of the gdbarch
> vector, then what we've been doing, lately, is just say "Implements
> the "bla_bla_bla" gdbarch method." or somesuch (we try not to repeat
> the documentation to avoid maintenance issues).
Here goes the reviewed version of the patch, addressing your
observations. Is it OK to check in now?
Thanks,
--
Sérgio Durigan Júnior
Linux on Power Toolchain - Software Engineer
Linux Technology Center - LTC
IBM Brazil
2009-05-05 Sergio Durigan Junior <sergiodj@linux.vnet.ibm.com>
* ppc-linux-nat.c (have_ptrace_getsetregs): New variable.
(have_ptrace_getsetfpregs): Likewise.
fetch_all_gp_regs): New function.
(fetch_gp_regs): New function.
(fetch_all_fp_regs): Likewise.
(fetch_fp_regs): New function.
(fetch_ppc_registers): Using the new methods to fetch general-
purpose and floating-pointer registers.
(store_all_gp_regs): New function.
(store_gp_regs): Likewise.
(store_all_fp_regs): New function.
(store_fp_regs): Likewise.
(store_ppc_registers): Using the new methods to store general-
purpose and floating-pointer registers.
diff --git a/gdb/ppc-linux-nat.c b/gdb/ppc-linux-nat.c
index 8710d51..297365b 100644
--- a/gdb/ppc-linux-nat.c
+++ b/gdb/ppc-linux-nat.c
@@ -108,6 +108,21 @@
#define PTRACE_GETSIGINFO 0x4202
#endif
+/* Similarly for the general-purpose (gp0 -- gp31)
+ and floating-point registers (fp0 -- fp31). */
+#ifndef PTRACE_GETREGS
+#define PTRACE_GETREGS 12
+#endif
+#ifndef PTRACE_SETREGS
+#define PTRACE_SETREGS 13
+#endif
+#ifndef PTRACE_GETFPREGS
+#define PTRACE_GETFPREGS 14
+#endif
+#ifndef PTRACE_SETFPREGS
+#define PTRACE_SETFPREGS 15
+#endif
+
/* This oddity is because the Linux kernel defines elf_vrregset_t as
an array of 33 16 bytes long elements. I.e. it leaves out vrsave.
However the PTRACE_GETVRREGS and PTRACE_SETVRREGS requests return
@@ -218,6 +233,18 @@ int have_ptrace_getvrregs = 1;
error. */
int have_ptrace_getsetevrregs = 1;
+/* Non-zero if our kernel may support the PTRACE_GETREGS and
+ PTRACE_SETREGS requests, for reading and writing the
+ general-purpose registers. Zero if we've tried one of
+ them and gotten an error. */
+int have_ptrace_getsetregs = 1;
+
+/* Non-zero if our kernel may support the PTRACE_GETFPREGS and
+ PTRACE_SETFPREGS requests, for reading and writing the
+ floating-pointers registers. Zero if we've tried one of
+ them and gotten an error. */
+int have_ptrace_getsetfpregs = 1;
+
/* *INDENT-OFF* */
/* registers layout, as presented by the ptrace interface:
PT_R0, PT_R1, PT_R2, PT_R3, PT_R4, PT_R5, PT_R6, PT_R7,
@@ -601,6 +628,112 @@ fetch_altivec_registers (struct regcache *regcache, int tid)
supply_vrregset (regcache, ®s);
}
+/* This function actually issues the request to ptrace, telling
+ it to get all general-purpose registers and put them into the
+ specified regset.
+
+ If the ptrace request does not exist, this function returns 0
+ and properly sets the have_ptrace_* flag. If the request fails,
+ this function calls perror_with_name. Otherwise, if the request
+ succeeds, then the regcache gets filled and 1 is returned. */
+static int
+fetch_all_gp_regs (struct regcache *regcache, int tid)
+{
+ struct gdbarch *gdbarch = get_regcache_arch (regcache);
+ struct gdbarch_tdep *tdep = gdbarch_tdep (gdbarch);
+ gdb_gregset_t gregset;
+
+ if (ptrace (PTRACE_GETREGS, tid, 0, (void *) &gregset) < 0)
+ {
+ if (errno == EIO)
+ {
+ have_ptrace_getsetregs = 0;
+ return 0;
+ }
+ perror_with_name (_("Couldn't get general-purpose registers."));
+ }
+
+ supply_gregset (regcache, (const gdb_gregset_t *) &gregset);
+
+ return 1;
+}
+
+/* This is a wrapper for the fetch_all_gp_regs function. It is
+ responsible for verifying if this target has the ptrace request
+ that can be used to fetch all general-purpose registers at one
+ shot. If it doesn't, then we should fetch them using the
+ old-fashioned way, which is to iterate over the registers and
+ request them one by one. */
+static void
+fetch_gp_regs (struct regcache *regcache, int tid)
+{
+ struct gdbarch *gdbarch = get_regcache_arch (regcache);
+ struct gdbarch_tdep *tdep = gdbarch_tdep (gdbarch);
+ int i;
+
+ if (have_ptrace_getsetregs)
+ if (fetch_all_gp_regs (regcache, tid))
+ return;
+
+ /* If we've hit this point, it doesn't really matter which
+ architecture we are using. We just need to read the
+ registers in the "old-fashioned way". */
+ for (i = 0; i < ppc_num_gprs; i++)
+ fetch_register (regcache, tid, tdep->ppc_gp0_regnum + i);
+}
+
+/* This function actually issues the request to ptrace, telling
+ it to get all floating-point registers and put them into the
+ specified regset.
+
+ If the ptrace request does not exist, this function returns 0
+ and properly sets the have_ptrace_* flag. If the request fails,
+ this function calls perror_with_name. Otherwise, if the request
+ succeeds, then the regcache gets filled and 1 is returned. */
+static int
+fetch_all_fp_regs (struct regcache *regcache, int tid)
+{
+ gdb_fpregset_t fpregs;
+
+ if (ptrace (PTRACE_GETFPREGS, tid, 0, (void *) &fpregs) < 0)
+ {
+ if (errno == EIO)
+ {
+ have_ptrace_getsetfpregs = 0;
+ return 0;
+ }
+ perror_with_name (_("Couldn't get floating-point registers."));
+ }
+
+ supply_fpregset (regcache, (const gdb_fpregset_t *) &fpregs);
+
+ return 1;
+}
+
+/* This is a wrapper for the fetch_all_fp_regs function. It is
+ responsible for verifying if this target has the ptrace request
+ that can be used to fetch all floating-point registers at one
+ shot. If it doesn't, then we should fetch them using the
+ old-fashioned way, which is to iterate over the registers and
+ request them one by one. */
+static void
+fetch_fp_regs (struct regcache *regcache, int tid)
+{
+ struct gdbarch *gdbarch = get_regcache_arch (regcache);
+ struct gdbarch_tdep *tdep = gdbarch_tdep (gdbarch);
+ int i;
+
+ if (have_ptrace_getsetfpregs)
+ if (fetch_all_fp_regs (regcache, tid))
+ return;
+
+ /* If we've hit this point, it doesn't really matter which
+ architecture we are using. We just need to read the
+ registers in the "old-fashioned way". */
+ for (i = 0; i < ppc_num_fprs; i++)
+ fetch_register (regcache, tid, tdep->ppc_fp0_regnum + i);
+}
+
static void
fetch_ppc_registers (struct regcache *regcache, int tid)
{
@@ -608,11 +741,9 @@ fetch_ppc_registers (struct regcache *regcache, int tid)
struct gdbarch *gdbarch = get_regcache_arch (regcache);
struct gdbarch_tdep *tdep = gdbarch_tdep (gdbarch);
- for (i = 0; i < ppc_num_gprs; i++)
- fetch_register (regcache, tid, tdep->ppc_gp0_regnum + i);
+ fetch_gp_regs (regcache, tid);
if (tdep->ppc_fp0_regnum >= 0)
- for (i = 0; i < ppc_num_fprs; i++)
- fetch_register (regcache, tid, tdep->ppc_fp0_regnum + i);
+ fetch_fp_regs (regcache, tid);
fetch_register (regcache, tid, gdbarch_pc_regnum (gdbarch));
if (tdep->ppc_ps_regnum != -1)
fetch_register (regcache, tid, tdep->ppc_ps_regnum);
@@ -970,18 +1101,142 @@ store_altivec_registers (const struct regcache *regcache, int tid)
perror_with_name (_("Couldn't write AltiVec registers"));
}
+/* This function actually issues the request to ptrace, telling
+ it to store all general-purpose registers present in the specified
+ regset.
+
+ If the ptrace request does not exist, this function returns 0
+ and properly sets the have_ptrace_* flag. If the request fails,
+ this function calls perror_with_name. Otherwise, if the request
+ succeeds, then the regcache is stored and 1 is returned. */
+static int
+store_all_gp_regs (const struct regcache *regcache, int tid, int regno)
+{
+ struct gdbarch *gdbarch = get_regcache_arch (regcache);
+ struct gdbarch_tdep *tdep = gdbarch_tdep (gdbarch);
+ gdb_gregset_t gregset;
+
+ if (ptrace (PTRACE_GETREGS, tid, 0, (void *) &gregset) < 0)
+ {
+ if (errno == EIO)
+ {
+ have_ptrace_getsetregs = 0;
+ return 0;
+ }
+ perror_with_name (_("Couldn't get general-purpose registers."));
+ }
+
+ fill_gregset (regcache, &gregset, regno);
+
+ if (ptrace (PTRACE_SETREGS, tid, 0, (void *) &gregset) < 0)
+ {
+ if (errno == EIO)
+ {
+ have_ptrace_getsetregs = 0;
+ return 0;
+ }
+ perror_with_name (_("Couldn't set general-purpose registers."));
+ }
+
+ return 1;
+}
+
+/* This is a wrapper for the store_all_gp_regs function. It is
+ responsible for verifying if this target has the ptrace request
+ that can be used to store all general-purpose registers at one
+ shot. If it doesn't, then we should store them using the
+ old-fashioned way, which is to iterate over the registers and
+ store them one by one. */
static void
-store_ppc_registers (const struct regcache *regcache, int tid)
+store_gp_regs (const struct regcache *regcache, int tid, int regno)
{
- int i;
struct gdbarch *gdbarch = get_regcache_arch (regcache);
struct gdbarch_tdep *tdep = gdbarch_tdep (gdbarch);
-
+ int i;
+
+ if (have_ptrace_getsetregs)
+ if (store_all_gp_regs (regcache, tid, regno))
+ return;
+
+ /* If we hit this point, it doesn't really matter which
+ architecture we are using. We just need to store the
+ registers in the "old-fashioned way". */
for (i = 0; i < ppc_num_gprs; i++)
store_register (regcache, tid, tdep->ppc_gp0_regnum + i);
+}
+
+/* This function actually issues the request to ptrace, telling
+ it to store all floating-point registers present in the specified
+ regset.
+
+ If the ptrace request does not exist, this function returns 0
+ and properly sets the have_ptrace_* flag. If the request fails,
+ this function calls perror_with_name. Otherwise, if the request
+ succeeds, then the regcache is stored and 1 is returned. */
+static int
+store_all_fp_regs (const struct regcache *regcache, int tid, int regno)
+{
+ gdb_fpregset_t fpregs;
+
+ if (ptrace (PTRACE_GETFPREGS, tid, 0, (void *) &fpregs) < 0)
+ {
+ if (errno == EIO)
+ {
+ have_ptrace_getsetfpregs = 0;
+ return 0;
+ }
+ perror_with_name (_("Couldn't get floating-point registers."));
+ }
+
+ fill_fpregset (regcache, &fpregs, regno);
+
+ if (ptrace (PTRACE_SETFPREGS, tid, 0, (void *) &fpregs) < 0)
+ {
+ if (errno == EIO)
+ {
+ have_ptrace_getsetfpregs = 0;
+ return 0;
+ }
+ perror_with_name (_("Couldn't set floating-point registers."));
+ }
+
+ return 1;
+}
+
+/* This is a wrapper for the store_all_fp_regs function. It is
+ responsible for verifying if this target has the ptrace request
+ that can be used to store all floating-point registers at one
+ shot. If it doesn't, then we should store them using the
+ old-fashioned way, which is to iterate over the registers and
+ store them one by one. */
+static void
+store_fp_regs (const struct regcache *regcache, int tid, int regno)
+{
+ struct gdbarch *gdbarch = get_regcache_arch (regcache);
+ struct gdbarch_tdep *tdep = gdbarch_tdep (gdbarch);
+ int i;
+
+ if (have_ptrace_getsetfpregs)
+ if (store_all_fp_regs (regcache, tid, regno))
+ return;
+
+ /* If we hit this point, it doesn't really matter which
+ architecture we are using. We just need to store the
+ registers in the "old-fashioned way". */
+ for (i = 0; i < ppc_num_fprs; i++)
+ store_register (regcache, tid, tdep->ppc_fp0_regnum + i);
+}
+
+static void
+store_ppc_registers (const struct regcache *regcache, int tid)
+{
+ int i;
+ struct gdbarch *gdbarch = get_regcache_arch (regcache);
+ struct gdbarch_tdep *tdep = gdbarch_tdep (gdbarch);
+
+ store_gp_regs (regcache, tid, -1);
if (tdep->ppc_fp0_regnum >= 0)
- for (i = 0; i < ppc_num_fprs; i++)
- store_register (regcache, tid, tdep->ppc_fp0_regnum + i);
+ store_fp_regs (regcache, tid, -1);
store_register (regcache, tid, gdbarch_pc_regnum (gdbarch));
if (tdep->ppc_ps_regnum != -1)
store_register (regcache, tid, tdep->ppc_ps_regnum);