This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [PATCH 5/5] MIPS GDBserver watchpoint
- From: Yao Qi <yao at codesourcery dot com>
- To: "Maciej W. Rozycki" <macro at codesourcery dot com>
- Cc: <gdb-patches at sourceware dot org>
- Date: Thu, 25 Jul 2013 08:16:20 +0800
- Subject: Re: [PATCH 5/5] MIPS GDBserver watchpoint
- References: <1369881867-11372-1-git-send-email-yao at codesourcery dot com> <1372475427-24862-1-git-send-email-yao at codesourcery dot com> <1372475427-24862-6-git-send-email-yao at codesourcery dot com> <alpine dot DEB dot 1 dot 10 dot 1307232257170 dot 32382 at tp dot orcam dot me dot uk>
On 07/24/2013 08:34 AM, Maciej W. Rozycki wrote:
>> +static enum target_hw_bp_type
>> >+rsp_bp_type_to_target_hw_bp_type (char type)
>> >+{
>> >+ switch (type)
>> >+ {
>> >+ case '2':
>> >+ return hw_write;
>> >+ case '3':
>> >+ return hw_read;
>> >+ case '4':
>> >+ return hw_access;
>> >+ }
>> >+
>> >+ return -1;
> This looks unclean to me, you're returning a value that's outside the
> valid range of the enum type used. By the structure of the callers this
> code is expected to be dead though, so please use gdb_assert_not_reached.
OK, gdb_assert_not_reached is added.
>
> Please resend with the adjustment requested, the change looks otherwise
> OK to me.
Here is the updated patch, with adding more words in ChangeLog to
address Pedro's comment.
--
Yao (éå)
gdb/gdbserver:
2013-07-25 Jie Zhang <jie@codesourcery.com>
Daniel Jacobowitz <dan@codesourcery.com>
Yao Qi <yao@codesourcery.com>
* Makefile.in (SFILES): Add common/mips-linux-watch.c.
(mips-linux-watch.o): New rule.
* configure.srv <mips*-*-linux*>: Add mips-linux-watch.o to
srv_tgtobj.
* linux-mips-low.c: Include mips-linux-watch.h.
(struct arch_process_info, struct arch_lwp_info): New.
(update_watch_registers_callback): New.
(mips_linux_new_process, mips_linux_new_thread) New.
(mips_linux_prepare_to_resume, mips_insert_point): New.
(mips_remove_point, mips_stopped_by_watchpoint): New.
(rsp_bp_type_to_target_hw_bp_type): New.
(mips_stopped_data_address): New.
(the_low_target): Add watchpoint support functions.
gdb:
2013-07-25 Yao Qi <yao@codesourcery.com>
* NEWS: Mention that GDBserver now supports hardware
watchpoints on the MIPS GNU/Linux target.
---
gdb/NEWS | 3 +
gdb/gdbserver/Makefile.in | 7 +-
gdb/gdbserver/configure.srv | 1 +
gdb/gdbserver/linux-mips-low.c | 366 ++++++++++++++++++++++++++++++++++++++++
4 files changed, 376 insertions(+), 1 deletions(-)
diff --git a/gdb/NEWS b/gdb/NEWS
index a4238d0..31a8bc6 100644
--- a/gdb/NEWS
+++ b/gdb/NEWS
@@ -128,6 +128,9 @@ qXfer:libraries-svr4:read's annex
'qXfer:traceframe-info:read'. It has the id of the collected
trace state variables.
+ ** GDBserver now supports hardware watchpoints on the MIPS GNU/Linux
+ target.
+
*** Changes in GDB 7.6
* Target record has been renamed to record-full.
diff --git a/gdb/gdbserver/Makefile.in b/gdb/gdbserver/Makefile.in
index 2cbf208..fe16cb4 100644
--- a/gdb/gdbserver/Makefile.in
+++ b/gdb/gdbserver/Makefile.in
@@ -157,7 +157,7 @@ SFILES= $(srcdir)/gdbreplay.c $(srcdir)/inferiors.c $(srcdir)/dll.c \
$(srcdir)/common/common-utils.c $(srcdir)/common/xml-utils.c \
$(srcdir)/common/linux-osdata.c $(srcdir)/common/ptid.c \
$(srcdir)/common/buffer.c $(srcdir)/common/linux-btrace.c \
- $(srcdir)/common/filestuff.c
+ $(srcdir)/common/filestuff.c $(srcdir)/common/mips-linux-watch.c
DEPFILES = @GDBSERVER_DEPFILES@
@@ -457,6 +457,8 @@ lynx_low_h = $(srcdir)/lynx-low.h $(srcdir)/server.h
nto_low_h = $(srcdir)/nto-low.h
+mips_linux_watch_h = $(srcdir)/../common/mips-linux-watch.h
+
UST_CFLAGS = $(ustinc) -DCONFIG_UST_GDB_INTEGRATION
# Note, we only build the IPA if -fvisibility=hidden is supported in
@@ -552,6 +554,9 @@ agent.o: ../common/agent.c
linux-btrace.o: ../common/linux-btrace.c $(linux_btrace_h) $(server_h)
$(CC) -c $(CPPFLAGS) $(INTERNAL_CFLAGS) $<
+mips-linux-watch.o: ../common/mips-linux-watch.c $(mips_linux_watch_h) $(server_h)
+ $(CC) -c $(CPPFLAGS) $(INTERNAL_CFLAGS) $<
+
# We build vasprintf with -DHAVE_CONFIG_H because we want that unit to
# include our config.h file. Otherwise, some system headers do not get
# included, and the compiler emits a warning about implicitly defined
diff --git a/gdb/gdbserver/configure.srv b/gdb/gdbserver/configure.srv
index 879d0de..b9dfd6c 100644
--- a/gdb/gdbserver/configure.srv
+++ b/gdb/gdbserver/configure.srv
@@ -184,6 +184,7 @@ case "${target}" in
srv_regobj="${srv_regobj} mips64-dsp-linux.o"
srv_tgtobj="linux-low.o linux-osdata.o linux-mips-low.o linux-procfs.o"
srv_tgtobj="${srv_tgtobj} linux-ptrace.o"
+ srv_tgtobj="${srv_tgtobj} mips-linux-watch.o"
srv_xmlfiles="mips-linux.xml"
srv_xmlfiles="${srv_xmlfiles} mips-dsp-linux.xml"
srv_xmlfiles="${srv_xmlfiles} mips-cpu.xml"
diff --git a/gdb/gdbserver/linux-mips-low.c b/gdb/gdbserver/linux-mips-low.c
index 1010528..dd6ebaf 100644
--- a/gdb/gdbserver/linux-mips-low.c
+++ b/gdb/gdbserver/linux-mips-low.c
@@ -22,6 +22,7 @@
#include <sys/ptrace.h>
#include <endian.h>
+#include "mips-linux-watch.h"
#include "gdb_proc_service.h"
/* Defined in auto-generated file mips-linux.c. */
@@ -159,6 +160,39 @@ get_usrregs_info (void)
return regs_info->usrregs;
}
+/* Per-process arch-specific data we want to keep. */
+
+struct arch_process_info
+{
+ /* -1 if the kernel and/or CPU do not support watch registers.
+ 1 if watch_readback is valid and we can read style, num_valid
+ and the masks.
+ 0 if we need to read the watch_readback. */
+
+ int watch_readback_valid;
+
+ /* Cached watch register read values. */
+
+ struct pt_watch_regs watch_readback;
+
+ /* Current watchpoint requests for this process. */
+
+ struct mips_watchpoint *current_watches;
+
+ /* The current set of watch register values for writing the
+ registers. */
+
+ struct pt_watch_regs watch_mirror;
+};
+
+/* Per-thread arch-specific data we want to keep. */
+
+struct arch_lwp_info
+{
+ /* Non-zero if our copy differs from what's recorded in the thread. */
+ int watch_registers_changed;
+};
+
/* From mips-linux-nat.c. */
/* Pseudo registers can not be read. ptrace does not provide a way to
@@ -257,6 +291,328 @@ mips_breakpoint_at (CORE_ADDR where)
return 0;
}
+/* Mark the watch registers of lwp, represented by ENTRY, as changed,
+ if the lwp's process id is *PID_P. */
+
+static int
+update_watch_registers_callback (struct inferior_list_entry *entry,
+ void *pid_p)
+{
+ struct lwp_info *lwp = (struct lwp_info *) entry;
+ int pid = *(int *) pid_p;
+
+ /* Only update the threads of this process. */
+ if (pid_of (lwp) == pid)
+ {
+ /* The actual update is done later just before resuming the lwp,
+ we just mark that the registers need updating. */
+ lwp->arch_private->watch_registers_changed = 1;
+
+ /* If the lwp isn't stopped, force it to momentarily pause, so
+ we can update its watch registers. */
+ if (!lwp->stopped)
+ linux_stop_lwp (lwp);
+ }
+
+ return 0;
+}
+
+/* This is the implementation of linux_target_ops method
+ new_process. */
+
+static struct arch_process_info *
+mips_linux_new_process (void)
+{
+ struct arch_process_info *info = xcalloc (1, sizeof (*info));
+
+ return info;
+}
+
+/* This is the implementation of linux_target_ops method new_thread.
+ Mark the watch registers as changed, so the threads' copies will
+ be updated. */
+
+static struct arch_lwp_info *
+mips_linux_new_thread (void)
+{
+ struct arch_lwp_info *info = xcalloc (1, sizeof (*info));
+
+ info->watch_registers_changed = 1;
+
+ return info;
+}
+
+/* This is the implementation of linux_target_ops method
+ prepare_to_resume. If the watch regs have changed, update the
+ thread's copies. */
+
+static void
+mips_linux_prepare_to_resume (struct lwp_info *lwp)
+{
+ ptid_t ptid = ptid_of (lwp);
+ struct process_info *proc = find_process_pid (ptid_get_pid (ptid));
+ struct arch_process_info *private = proc->private->arch_private;
+
+ if (lwp->arch_private->watch_registers_changed)
+ {
+ /* Only update the watch registers if we have set or unset a
+ watchpoint already. */
+ if (mips_linux_watch_get_num_valid (&private->watch_mirror) > 0)
+ {
+ /* Write the mirrored watch register values. */
+ int tid = ptid_get_lwp (ptid);
+
+ if (-1 == ptrace (PTRACE_SET_WATCH_REGS, tid,
+ &private->watch_mirror))
+ perror_with_name ("Couldn't write watch register");
+ }
+
+ lwp->arch_private->watch_registers_changed = 0;
+ }
+}
+
+/* Translate breakpoint type TYPE in rsp to 'enum target_hw_bp_type'. */
+
+static enum target_hw_bp_type
+rsp_bp_type_to_target_hw_bp_type (char type)
+{
+ switch (type)
+ {
+ case '2':
+ return hw_write;
+ case '3':
+ return hw_read;
+ case '4':
+ return hw_access;
+ }
+
+ gdb_assert_not_reached ("unhandled RSP breakpoint type");
+ return -1;
+}
+
+/* This is the implementation of linux_target_ops method
+ insert_point. */
+
+static int
+mips_insert_point (char type, CORE_ADDR addr, int len)
+{
+ struct process_info *proc = current_process ();
+ struct arch_process_info *private = proc->private->arch_private;
+ struct pt_watch_regs regs;
+ struct mips_watchpoint *new_watch;
+ struct mips_watchpoint **pw;
+ int pid;
+ long lwpid;
+ enum target_hw_bp_type watch_type;
+ uint32_t irw;
+
+ /* Breakpoint/watchpoint types:
+ '0' - software-breakpoint (not supported)
+ '1' - hardware-breakpoint (not supported)
+ '2' - write watchpoint (supported)
+ '3' - read watchpoint (supported)
+ '4' - access watchpoint (supported). */
+
+ if (type < '2' || type > '4')
+ {
+ /* Unsupported. */
+ return 1;
+ }
+
+ lwpid = lwpid_of (get_thread_lwp (current_inferior));
+ if (!mips_linux_read_watch_registers (lwpid,
+ &private->watch_readback,
+ &private->watch_readback_valid,
+ 0))
+ return -1;
+
+ if (len <= 0)
+ return -1;
+
+ regs = private->watch_readback;
+ /* Add the current watches. */
+ mips_linux_watch_populate_regs (private->current_watches, ®s);
+
+ /* Now try to add the new watch. */
+ watch_type = rsp_bp_type_to_target_hw_bp_type (type);
+ irw = mips_linux_watch_type_to_irw (watch_type);
+ if (!mips_linux_watch_try_one_watch (®s, addr, len, irw))
+ return -1;
+
+ /* It fit. Stick it on the end of the list. */
+ new_watch = xmalloc (sizeof (struct mips_watchpoint));
+ new_watch->addr = addr;
+ new_watch->len = len;
+ new_watch->type = watch_type;
+ new_watch->next = NULL;
+
+ pw = &private->current_watches;
+ while (*pw != NULL)
+ pw = &(*pw)->next;
+ *pw = new_watch;
+
+ private->watch_mirror = regs;
+
+ /* Only update the threads of this process. */
+ pid = pid_of (proc);
+ find_inferior (&all_lwps, update_watch_registers_callback, &pid);
+
+ return 0;
+}
+
+/* This is the implementation of linux_target_ops method
+ remove_point. */
+
+static int
+mips_remove_point (char type, CORE_ADDR addr, int len)
+{
+ struct process_info *proc = current_process ();
+ struct arch_process_info *private = proc->private->arch_private;
+
+ int deleted_one;
+ int pid;
+ enum target_hw_bp_type watch_type;
+
+ struct mips_watchpoint **pw;
+ struct mips_watchpoint *w;
+
+ /* Breakpoint/watchpoint types:
+ '0' - software-breakpoint (not supported)
+ '1' - hardware-breakpoint (not supported)
+ '2' - write watchpoint (supported)
+ '3' - read watchpoint (supported)
+ '4' - access watchpoint (supported). */
+
+ if (type < '2' || type > '4')
+ {
+ /* Unsupported. */
+ return 1;
+ }
+
+ /* Search for a known watch that matches. Then unlink and free it. */
+ watch_type = rsp_bp_type_to_target_hw_bp_type (type);
+ deleted_one = 0;
+ pw = &private->current_watches;
+ while ((w = *pw))
+ {
+ if (w->addr == addr && w->len == len && w->type == watch_type)
+ {
+ *pw = w->next;
+ free (w);
+ deleted_one = 1;
+ break;
+ }
+ pw = &(w->next);
+ }
+
+ if (!deleted_one)
+ return -1; /* We don't know about it, fail doing nothing. */
+
+ /* At this point watch_readback is known to be valid because we
+ could not have added the watch without reading it. */
+ gdb_assert (private->watch_readback_valid == 1);
+
+ private->watch_mirror = private->watch_readback;
+ mips_linux_watch_populate_regs (private->current_watches,
+ &private->watch_mirror);
+
+ /* Only update the threads of this process. */
+ pid = pid_of (proc);
+ find_inferior (&all_lwps, update_watch_registers_callback, &pid);
+ return 0;
+}
+
+/* This is the implementation of linux_target_ops method
+ stopped_by_watchpoint. The watchhi R and W bits indicate
+ the watch register triggered. */
+
+static int
+mips_stopped_by_watchpoint (void)
+{
+ struct process_info *proc = current_process ();
+ struct arch_process_info *private = proc->private->arch_private;
+ int n;
+ int num_valid;
+ long lwpid = lwpid_of (get_thread_lwp (current_inferior));
+
+ if (!mips_linux_read_watch_registers (lwpid,
+ &private->watch_readback,
+ &private->watch_readback_valid,
+ 1))
+ return 0;
+
+ num_valid = mips_linux_watch_get_num_valid (&private->watch_readback);
+
+ for (n = 0; n < MAX_DEBUG_REGISTER && n < num_valid; n++)
+ if (mips_linux_watch_get_watchhi (&private->watch_readback, n)
+ & (R_MASK | W_MASK))
+ return 1;
+
+ return 0;
+}
+
+/* This is the implementation of linux_target_ops method
+ stopped_data_address. */
+
+static CORE_ADDR
+mips_stopped_data_address (void)
+{
+ struct process_info *proc = current_process ();
+ struct arch_process_info *private = proc->private->arch_private;
+ int n;
+ int num_valid;
+ long lwpid = lwpid_of (get_thread_lwp (current_inferior));
+
+ /* On MIPS we don't know the low order 3 bits of the data address.
+ GDB does not support remote targets that can't report the
+ watchpoint address. So, make our best guess; return the starting
+ address of a watchpoint request which overlaps the one that
+ triggered. */
+
+ if (!mips_linux_read_watch_registers (lwpid,
+ &private->watch_readback,
+ &private->watch_readback_valid,
+ 0))
+ return 0;
+
+ num_valid = mips_linux_watch_get_num_valid (&private->watch_readback);
+
+ for (n = 0; n < MAX_DEBUG_REGISTER && n < num_valid; n++)
+ if (mips_linux_watch_get_watchhi (&private->watch_readback, n)
+ & (R_MASK | W_MASK))
+ {
+ CORE_ADDR t_low, t_hi;
+ int t_irw;
+ struct mips_watchpoint *watch;
+
+ t_low = mips_linux_watch_get_watchlo (&private->watch_readback, n);
+ t_irw = t_low & IRW_MASK;
+ t_hi = (mips_linux_watch_get_watchhi (&private->watch_readback, n)
+ | IRW_MASK);
+ t_low &= ~(CORE_ADDR)t_hi;
+
+ for (watch = private->current_watches;
+ watch != NULL;
+ watch = watch->next)
+ {
+ CORE_ADDR addr = watch->addr;
+ CORE_ADDR last_byte = addr + watch->len - 1;
+
+ if ((t_irw & mips_linux_watch_type_to_irw (watch->type)) == 0)
+ {
+ /* Different type. */
+ continue;
+ }
+ /* Check for overlap of even a single byte. */
+ if (last_byte >= t_low && addr <= t_low + t_hi)
+ return addr;
+ }
+ }
+
+ /* Shouldn't happen. */
+ return 0;
+}
+
/* Fetch the thread-local storage pointer for libthread_db. */
ps_err_e
@@ -506,6 +862,16 @@ struct linux_target_ops the_low_target = {
mips_reinsert_addr,
0,
mips_breakpoint_at,
+ mips_insert_point,
+ mips_remove_point,
+ mips_stopped_by_watchpoint,
+ mips_stopped_data_address,
+ NULL,
+ NULL,
+ NULL, /* siginfo_fixup */
+ mips_linux_new_process,
+ mips_linux_new_thread,
+ mips_linux_prepare_to_resume
};
void
--
1.7.7.6