This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [PATCH 1/4] Cleanups to FreeBSD/mips native register operations.
- From: Luis Machado <lgustavo at codesourcery dot com>
- To: John Baldwin <jhb at FreeBSD dot org>, <gdb-patches at sourceware dot org>
- Date: Thu, 13 Apr 2017 10:48:48 -0500
- Subject: Re: [PATCH 1/4] Cleanups to FreeBSD/mips native register operations.
- Authentication-results: sourceware.org; auth=none
- References: <20170412183727.22483-1-jhb@FreeBSD.org> <20170412183727.22483-2-jhb@FreeBSD.org>
- Reply-to: Luis Machado <lgustavo at codesourcery dot com>
On 04/12/2017 01:37 PM, John Baldwin wrote:
Compare against the "raw" PC register number instead of the cooked
register number when determining if a register was handled by
PT_GETREGS. Previously the register fetch/store operations only tried
PT_GETREGS to fetch any individual register. The result was that
fetching or storing an individual register not covered by PT_GETREGS
(such as floating point registers) did not work.
While here, remove an early exit to simplify the code flow from the
PT_GETREGS / PT_SETREGS case, and add a getfpregs_supplies similar to
getregs_supplies to describe the registers supplied by PT_GETFPREGS
and PT_SETFPREGS.
gdb/ChangeLog:
* mips-fbsd-nat.c (getregs_supplies): Fix upper bound comparison.
(getpfpregs_supplies): New function.
(mips_fbsd_fetch_inferior_registers): Remove early exit and use
getfpregs_supplies.
(mips_fbsd_store_inferior_registers): Likewise.
---
gdb/ChangeLog | 8 ++++++++
gdb/mips-fbsd-nat.c | 26 ++++++++++++++------------
2 files changed, 22 insertions(+), 12 deletions(-)
Only a few nits.
diff --git a/gdb/mips-fbsd-nat.c b/gdb/mips-fbsd-nat.c
index 078df52db6..e2ed63e829 100644
--- a/gdb/mips-fbsd-nat.c
+++ b/gdb/mips-fbsd-nat.c
@@ -37,7 +37,16 @@ static bool
getregs_supplies (struct gdbarch *gdbarch, int regnum)
{
return (regnum >= MIPS_ZERO_REGNUM
- && regnum <= gdbarch_pc_regnum (gdbarch));
+ && regnum <= mips_regnum (gdbarch)->pc);
+}
Can the BSD backend override the pc value in gdbarch_pc_regnum (...) so
it fits what is expected? Or is this a case where the cooked pc register
number is still useful and we need to handle things differently for the
raw pc register number?
+
+/* Determine if PT_GETFPREGS fetches this register. */
Pedantically, "... fetches REGNUM".
@@ -47,9 +56,9 @@ static void
mips_fbsd_fetch_inferior_registers (struct target_ops *ops,
struct regcache *regcache, int regnum)
{
+ struct gdbarch *gdbarch = get_regcache_arch (regcache);
pid_t pid = get_ptrace_pid (regcache_get_ptid (regcache));
- struct gdbarch *gdbarch = get_regcache_arch (regcache);
if (regnum == -1 || getregs_supplies (gdbarch, regnum))
With C++ we can leave the declaration closer to its use. Same in the
other case below.
@@ -58,12 +67,9 @@ mips_fbsd_fetch_inferior_registers (struct target_ops *ops,
perror_with_name (_("Couldn't get registers"));
mips_fbsd_supply_gregs (regcache, regnum, ®s, sizeof (register_t));
- if (regnum != -1)
- return;
}
- if (regnum == -1
- || regnum >= gdbarch_fp0_regnum (get_regcache_arch (regcache)))
+ if (regnum == -1 || getfpregs_supplies (gdbarch, regnum))
Does MIPS on fsbd handle vector registers? I ask this because regnum >=
"fp0 regnum" may mean anything other than general purpose registers.
If there are vector (or higher-numbered registers), the new conditional
block means something different compared to the old one.
If not, then the change looks sane.
@@ -82,9 +88,9 @@ static void
mips_fbsd_store_inferior_registers (struct target_ops *ops,
struct regcache *regcache, int regnum)
{
+ struct gdbarch *gdbarch = get_regcache_arch (regcache);
pid_t pid = get_ptrace_pid (regcache_get_ptid (regcache));
- struct gdbarch *gdbarch = get_regcache_arch (regcache);
if (regnum == -1 || getregs_supplies (gdbarch, regnum))
{
struct reg regs;
Same as above about declaring something closer to where it is used.
@@ -97,13 +103,9 @@ mips_fbsd_store_inferior_registers (struct target_ops *ops,
if (ptrace (PT_SETREGS, pid, (PTRACE_TYPE_ARG3) ®s, 0) == -1)
perror_with_name (_("Couldn't write registers"));
-
- if (regnum != -1)
- return;
}
- if (regnum == -1
- || regnum >= gdbarch_fp0_regnum (get_regcache_arch (regcache)))
+ if (regnum == -1 || getfpregs_supplies (gdbarch, regnum))
Same thing as above, about higher-numbered registers.
Otherwise i have no further comments.