* [PATCH] Support for HWbreak/watchpoint accross fork/vfork on arm-native
@ 2014-01-30 22:12 Omair Javaid
2014-01-30 22:23 ` Omair Javaid
0 siblings, 1 reply; 10+ messages in thread
From: Omair Javaid @ 2014-01-30 22:12 UTC (permalink / raw)
To: gdb-patches; +Cc: patches, Omair Javaid
---
gdb/arm-linux-nat.c | 417 ++++++++++++++++++++++++++++++++++------------------
1 file changed, 273 insertions(+), 144 deletions(-)
diff --git a/gdb/arm-linux-nat.c b/gdb/arm-linux-nat.c
index 6f56634..9a756c1 100644
--- a/gdb/arm-linux-nat.c
+++ b/gdb/arm-linux-nat.c
@@ -787,70 +787,120 @@ struct arm_linux_hw_breakpoint
arm_hwbp_control_t control;
};
-/* Structure containing arrays of the break and watch points which are have
- active in each thread.
-
- The Linux ptrace interface to hardware break-/watch-points presents the
- values in a vector centred around 0 (which is used fo generic information).
- Positive indicies refer to breakpoint addresses/control registers, negative
- indices to watchpoint addresses/control registers.
-
- The Linux vector is indexed as follows:
- -((i << 1) + 2): Control register for watchpoint i.
- -((i << 1) + 1): Address register for watchpoint i.
- 0: Information register.
- ((i << 1) + 1): Address register for breakpoint i.
- ((i << 1) + 2): Control register for breakpoint i.
-
- This structure is used as a per-thread cache of the state stored by the
- kernel, so that we don't need to keep calling into the kernel to find a
- free breakpoint.
-
- We treat break-/watch-points with their enable bit clear as being deleted.
- */
-typedef struct arm_linux_thread_points
+/* Since we cannot dynamically allocate subfields of arm_linux_process_info,
+ assume a maximum number of supported break-/watchpoints. */
+#define MAX_BPTS 16
+#define MAX_WPTS 16
+
+/* Per-process arch-specific data we want to keep. */
+struct arm_linux_process_info
{
- /* Thread ID. */
- int tid;
- /* Breakpoints for thread. */
- struct arm_linux_hw_breakpoint *bpts;
- /* Watchpoint for threads. */
- struct arm_linux_hw_breakpoint *wpts;
-} *arm_linux_thread_points_p;
-DEF_VEC_P (arm_linux_thread_points_p);
-
-/* Vector of hardware breakpoints for each thread. */
-VEC(arm_linux_thread_points_p) *arm_threads = NULL;
-
-/* Find the list of hardware break-/watch-points for a thread with id TID.
- If no list exists for TID we return NULL if ALLOC_NEW is 0, otherwise we
- create a new list and return that. */
-static struct arm_linux_thread_points *
-arm_linux_find_breakpoints_by_tid (int tid, int alloc_new)
+ /* Linked list. */
+ struct arm_linux_process_info *next;
+ /* The process identifier. */
+ pid_t pid;
+ /* Hardware breakpoints for this process. */
+ struct arm_linux_hw_breakpoint bpts[MAX_BPTS];
+ /* Hardware watchpoints for this process. */
+ struct arm_linux_hw_breakpoint wpts[MAX_WPTS];
+};
+
+/* Per-thread arch-specific data we want to keep. */
+struct arch_lwp_info
{
- int i;
- struct arm_linux_thread_points *t;
+ /* Non-zero if our copy differs from what's recorded in the thread. */
+ char bpts_changed[MAX_BPTS];
+ char wpts_changed[MAX_WPTS];
+};
- for (i = 0; VEC_iterate (arm_linux_thread_points_p, arm_threads, i, t); ++i)
- {
- if (t->tid == tid)
- return t;
- }
+static struct arm_linux_process_info *arm_linux_process_list = NULL;
+
+/* Find process data for process PID. */
- t = NULL;
+static struct arm_linux_process_info *
+arm_linux_find_process_pid (pid_t pid)
+{
+ struct arm_linux_process_info *proc;
+
+ for (proc = arm_linux_process_list; proc; proc = proc->next)
+ if (proc->pid == pid)
+ return proc;
- if (alloc_new)
+ return NULL;
+}
+
+/* Add process data for process PID. Returns newly allocated info
+ object. */
+
+static struct arm_linux_process_info *
+arm_linux_add_process (pid_t pid)
+{
+ struct arm_linux_process_info *proc;
+
+ proc = xcalloc (1, sizeof (*proc));
+ proc->pid = pid;
+
+ proc->next = arm_linux_process_list;
+ arm_linux_process_list = proc;
+
+ return proc;
+}
+
+/* Get data specific info for process PID, creating it if necessary.
+ Never returns NULL. */
+
+static struct arm_linux_process_info *
+arm_linux_process_info_get (pid_t pid)
+{
+ struct arm_linux_process_info *proc;
+
+ proc = arm_linux_find_process_pid (pid);
+ if (proc == NULL)
+ proc = arm_linux_add_process (pid);
+
+ return proc;
+}
+
+/* Called whenever GDB is no longer debugging process PID. It deletes
+ data structures that keep track of debug register state. */
+
+static void
+arm_linux_forget_process (pid_t pid)
+{
+ struct arm_linux_process_info *proc, **proc_link;
+
+ proc = arm_linux_process_list;
+ proc_link = &arm_linux_process_list;
+
+ while (proc != NULL)
{
- t = xmalloc (sizeof (struct arm_linux_thread_points));
- t->tid = tid;
- t->bpts = xzalloc (arm_linux_get_hw_breakpoint_count ()
- * sizeof (struct arm_linux_hw_breakpoint));
- t->wpts = xzalloc (arm_linux_get_hw_watchpoint_count ()
- * sizeof (struct arm_linux_hw_breakpoint));
- VEC_safe_push (arm_linux_thread_points_p, arm_threads, t);
+ if (proc->pid == pid)
+ {
+ *proc_link = proc->next;
+
+ xfree (proc);
+ return;
+ }
+
+ proc_link = &proc->next;
+ proc = *proc_link;
}
+}
+
+/* Get hardware breakpoint state for process PID. */
- return t;
+static struct arm_linux_hw_breakpoint *
+arm_linux_get_hwbreak_state (pid_t pid)
+{
+ return arm_linux_process_info_get (pid)->bpts;
+}
+
+/* Get watchpoint state for process PID. */
+
+static struct arm_linux_hw_breakpoint *
+arm_linux_get_watch_state (pid_t pid)
+{
+ return arm_linux_process_info_get (pid)->wpts;
}
/* Initialize an ARM hardware break-/watch-point control register value.
@@ -950,45 +1000,76 @@ arm_linux_hw_breakpoint_equal (const struct arm_linux_hw_breakpoint *p1,
return p1->address == p2->address && p1->control == p2->control;
}
+/* Callback to mark a watch-/breakpoint to be updated in all threads of
+ the current process. */
+
+struct update_registers_data
+{
+ int watch;
+ int index;
+};
+
+static int
+update_registers_callback (struct lwp_info *lwp, void *arg)
+{
+ struct update_registers_data *data = (struct update_registers_data *) arg;
+
+ /* Force iterate_over_lwps to return matched lwp_info*. */
+ if (arg == NULL)
+ return 1;
+
+ if (lwp->arch_private == NULL)
+ lwp->arch_private = XCNEW (struct arch_lwp_info);
+
+ /* The actual update is done later just before resuming the lwp,
+ we just mark that the registers need updating. */
+ if (data->watch)
+ lwp->arch_private->wpts_changed[data->index] = 1;
+ else
+ lwp->arch_private->bpts_changed[data->index] = 1;
+
+ /* If the lwp isn't stopped, force it to momentarily pause, so
+ we can update its breakpoint registers. */
+ if (!lwp->stopped)
+ linux_stop_lwp (lwp);
+
+ return 0;
+}
+
/* Insert the hardware breakpoint (WATCHPOINT = 0) or watchpoint (WATCHPOINT
=1) BPT for thread TID. */
static void
arm_linux_insert_hw_breakpoint1 (const struct arm_linux_hw_breakpoint* bpt,
- int tid, int watchpoint)
+ int watchpoint)
{
- struct arm_linux_thread_points *t = arm_linux_find_breakpoints_by_tid (tid, 1);
+ int pid;
+ ptid_t pid_ptid;
gdb_byte count, i;
struct arm_linux_hw_breakpoint* bpts;
- int dir;
+ struct update_registers_data data;
- gdb_assert (t != NULL);
+ pid = ptid_get_pid (inferior_ptid);
+ pid_ptid = pid_to_ptid (pid);
if (watchpoint)
{
count = arm_linux_get_hw_watchpoint_count ();
- bpts = t->wpts;
- dir = -1;
+ bpts = arm_linux_get_watch_state(pid);
}
else
{
count = arm_linux_get_hw_breakpoint_count ();
- bpts = t->bpts;
- dir = 1;
+ bpts = arm_linux_get_hwbreak_state(pid);
}
for (i = 0; i < count; ++i)
if (!arm_hwbp_control_is_enabled (bpts[i].control))
{
- errno = 0;
- if (ptrace (PTRACE_SETHBPREGS, tid, dir * ((i << 1) + 1),
- &bpt->address) < 0)
- perror_with_name (_("Unexpected error setting breakpoint address"));
- if (ptrace (PTRACE_SETHBPREGS, tid, dir * ((i << 1) + 2),
- &bpt->control) < 0)
- perror_with_name (_("Unexpected error setting breakpoint"));
-
- memcpy (bpts + i, bpt, sizeof (struct arm_linux_hw_breakpoint));
- break;
+ data.watch = watchpoint;
+ data.index = i;
+ bpts[i] = *bpt;
+ iterate_over_lwps (pid_ptid, update_registers_callback, &data);
+ break;
}
gdb_assert (i != count);
@@ -998,37 +1079,36 @@ arm_linux_insert_hw_breakpoint1 (const struct arm_linux_hw_breakpoint* bpt,
(WATCHPOINT = 1) BPT for thread TID. */
static void
arm_linux_remove_hw_breakpoint1 (const struct arm_linux_hw_breakpoint *bpt,
- int tid, int watchpoint)
+ int watchpoint)
{
- struct arm_linux_thread_points *t = arm_linux_find_breakpoints_by_tid (tid, 0);
+ int pid;
gdb_byte count, i;
- struct arm_linux_hw_breakpoint *bpts;
- int dir;
+ ptid_t pid_ptid;
+ struct arm_linux_hw_breakpoint* bpts;
+ struct update_registers_data data;
- gdb_assert (t != NULL);
+ pid = ptid_get_pid (inferior_ptid);
+ pid_ptid = pid_to_ptid (pid);
if (watchpoint)
{
count = arm_linux_get_hw_watchpoint_count ();
- bpts = t->wpts;
- dir = -1;
+ bpts = arm_linux_get_watch_state(pid);
}
else
{
count = arm_linux_get_hw_breakpoint_count ();
- bpts = t->bpts;
- dir = 1;
+ bpts = arm_linux_get_hwbreak_state(pid);
}
for (i = 0; i < count; ++i)
if (arm_linux_hw_breakpoint_equal (bpt, bpts + i))
{
- errno = 0;
- bpts[i].control = arm_hwbp_control_disable (bpts[i].control);
- if (ptrace (PTRACE_SETHBPREGS, tid, dir * ((i << 1) + 2),
- &bpts[i].control) < 0)
- perror_with_name (_("Unexpected error clearing breakpoint"));
- break;
+ data.watch = watchpoint;
+ data.index = i;
+ bpts[i].control = arm_hwbp_control_disable (bpts[i].control);
+ iterate_over_lwps (pid_ptid, update_registers_callback, &data);
+ break;
}
gdb_assert (i != count);
@@ -1046,8 +1126,8 @@ arm_linux_insert_hw_breakpoint (struct gdbarch *gdbarch,
return -1;
arm_linux_hw_breakpoint_initialize (gdbarch, bp_tgt, &p);
- ALL_LWPS (lp)
- arm_linux_insert_hw_breakpoint1 (&p, ptid_get_lwp (lp->ptid), 0);
+
+ arm_linux_insert_hw_breakpoint1 (&p, 0);
return 0;
}
@@ -1064,8 +1144,8 @@ arm_linux_remove_hw_breakpoint (struct gdbarch *gdbarch,
return -1;
arm_linux_hw_breakpoint_initialize (gdbarch, bp_tgt, &p);
- ALL_LWPS (lp)
- arm_linux_remove_hw_breakpoint1 (&p, ptid_get_lwp (lp->ptid), 0);
+
+ arm_linux_remove_hw_breakpoint1 (&p, 0);
return 0;
}
@@ -1115,8 +1195,8 @@ arm_linux_insert_watchpoint (CORE_ADDR addr, int len, int rw,
return -1;
arm_linux_hw_watchpoint_initialize (addr, len, rw, &p);
- ALL_LWPS (lp)
- arm_linux_insert_hw_breakpoint1 (&p, ptid_get_lwp (lp->ptid), 1);
+
+ arm_linux_insert_hw_breakpoint1 (&p, 1);
return 0;
}
@@ -1133,8 +1213,8 @@ arm_linux_remove_watchpoint (CORE_ADDR addr, int len, int rw,
return -1;
arm_linux_hw_watchpoint_initialize (addr, len, rw, &p);
- ALL_LWPS (lp)
- arm_linux_remove_hw_breakpoint1 (&p, ptid_get_lwp (lp->ptid), 1);
+
+ arm_linux_remove_hw_breakpoint1 (&p, 1);
return 0;
}
@@ -1190,63 +1270,108 @@ arm_linux_watchpoint_addr_within_range (struct target_ops *target,
static void
arm_linux_new_thread (struct lwp_info *lp)
{
- int tid = ptid_get_lwp (lp->ptid);
- const struct arm_linux_hwbp_cap *info = arm_linux_get_hwbp_cap ();
+ int i;
+ struct arch_lwp_info *info = XCNEW (struct arch_lwp_info);
- if (info != NULL)
+ /* Mark that all the hardware breakpoint/watchpoint register pairs
+ for this thread need to be initialized. */
+
+ for (i = 0; i < MAX_BPTS; i++)
{
- int i;
- struct arm_linux_thread_points *p;
- struct arm_linux_hw_breakpoint *bpts;
-
- if (VEC_empty (arm_linux_thread_points_p, arm_threads))
- return;
-
- /* Get a list of breakpoints from any thread. */
- p = VEC_last (arm_linux_thread_points_p, arm_threads);
-
- /* Copy that thread's breakpoints and watchpoints to the new thread. */
- for (i = 0; i < info->bp_count; i++)
- if (arm_hwbp_control_is_enabled (p->bpts[i].control))
- arm_linux_insert_hw_breakpoint1 (p->bpts + i, tid, 0);
- for (i = 0; i < info->wp_count; i++)
- if (arm_hwbp_control_is_enabled (p->wpts[i].control))
- arm_linux_insert_hw_breakpoint1 (p->wpts + i, tid, 1);
+ info->bpts_changed[i] = 1;
+ info->wpts_changed[i] = 1;
}
+
+ lp->arch_private = info;
}
-/* Handle thread exit. Tidy up the memory that has been allocated for the
- thread. */
+/* Called when resuming a thread.
+ The hardware debug registers are updated when there is any change. */
+
static void
-arm_linux_thread_exit (struct thread_info *tp, int silent)
+arm_linux_prepare_to_resume (struct lwp_info *lwp)
{
- const struct arm_linux_hwbp_cap *info = arm_linux_get_hwbp_cap ();
+ int pid, i;
+ struct arm_linux_hw_breakpoint *bpts, *wpts;
+ struct arch_lwp_info *arm_lwp_info = lwp->arch_private;
+
+ /* NULL means this is the main thread still going through the shell,
+ or, no watchpoint has been set yet. In that case, there's
+ nothing to do. */
+ if (arm_lwp_info == NULL)
+ return;
- if (info != NULL)
- {
- int i;
- int tid = ptid_get_lwp (tp->ptid);
- struct arm_linux_thread_points *t = NULL, *p;
+ for (i = 0; i < arm_linux_get_hw_breakpoint_count (); i++)
+ if (arm_lwp_info->bpts_changed[i])
+ {
+ errno = 0;
+ pid = ptid_get_lwp (lwp->ptid);
+ bpts = arm_linux_get_hwbreak_state (ptid_get_pid (lwp->ptid));
- for (i = 0;
- VEC_iterate (arm_linux_thread_points_p, arm_threads, i, p); i++)
- {
- if (p->tid == tid)
- {
- t = p;
- break;
- }
- }
+ if (arm_hwbp_control_is_enabled (bpts[i].control))
+ if (ptrace (PTRACE_SETHBPREGS, pid,
+ (PTRACE_TYPE_ARG3) ((i << 1) + 1), &bpts[i].address) < 0)
+ perror_with_name ("Unexpected error setting breakpoint address");
- if (t == NULL)
- return;
+ if (bpts[i].control != 0)
+ if (ptrace (PTRACE_SETHBPREGS, pid,
+ (PTRACE_TYPE_ARG3) ((i << 1) + 2), &bpts[i].control) < 0)
+ perror_with_name ("Unexpected error setting breakpoint");
- VEC_unordered_remove (arm_linux_thread_points_p, arm_threads, i);
+ arm_lwp_info->bpts_changed[i] = 0;
+ }
- xfree (t->bpts);
- xfree (t->wpts);
- xfree (t);
- }
+ for (i = 0; i < arm_linux_get_hw_watchpoint_count (); i++)
+ if (arm_lwp_info->wpts_changed[i])
+ {
+ errno = 0;
+ pid = ptid_get_lwp (lwp->ptid);
+ wpts = arm_linux_get_watch_state (ptid_get_pid (lwp->ptid));
+
+ if (arm_hwbp_control_is_enabled (wpts[i].control))
+ if (ptrace (PTRACE_SETHBPREGS, pid,
+ (PTRACE_TYPE_ARG3) -((i << 1) + 1), &wpts[i].address) < 0)
+ perror_with_name ("Unexpected error setting watchpoint address");
+
+ if (wpts[i].control != 0)
+ if (ptrace (PTRACE_SETHBPREGS, pid,
+ (PTRACE_TYPE_ARG3) -((i << 1) + 2), &wpts[i].control) < 0)
+ perror_with_name ("Unexpected error setting watchpoint");
+
+ arm_lwp_info->wpts_changed[i] = 0;
+ }
+}
+
+/* linux_nat_new_fork hook. */
+
+static void
+arm_linux_new_fork (struct lwp_info *parent, pid_t child_pid)
+{
+ pid_t parent_pid;
+ struct arm_linux_hw_breakpoint *parent_point_state;
+ struct arm_linux_hw_breakpoint *child_point_state;
+
+ /* NULL means no watchpoint has ever been set in the parent. In
+ that case, there's nothing to do. */
+ if (parent->arch_private == NULL)
+ return;
+
+ /* GDB core assumes the child inherits the watchpoints/hw
+ breakpoints of the parent, and will remove them all from the
+ forked off process. Copy the debug registers mirrors into the
+ new process so that all breakpoints and watchpoints can be
+ removed together. */
+
+ parent_pid = ptid_get_pid (parent->ptid);
+ parent_point_state = arm_linux_get_hwbreak_state (parent_pid);
+ child_point_state = arm_linux_get_hwbreak_state (child_pid);
+ memcpy (child_point_state, parent_point_state,
+ sizeof (struct arm_linux_hw_breakpoint) * MAX_BPTS);
+
+ parent_point_state = arm_linux_get_watch_state (parent_pid);
+ child_point_state = arm_linux_get_watch_state (child_pid);
+ memcpy (child_point_state, parent_point_state,
+ sizeof (struct arm_linux_hw_breakpoint) * MAX_BPTS);
}
void _initialize_arm_linux_nat (void);
@@ -1279,7 +1404,11 @@ _initialize_arm_linux_nat (void)
/* Register the target. */
linux_nat_add_target (t);
- /* Handle thread creation and exit */
- observer_attach_thread_exit (arm_linux_thread_exit);
+ /* Handle thread creation and exit. */
linux_nat_set_new_thread (t, arm_linux_new_thread);
+ linux_nat_set_prepare_to_resume (t, arm_linux_prepare_to_resume);
+
+ /* Handle process creation and exit. */
+ linux_nat_set_new_fork (t, arm_linux_new_fork);
+ linux_nat_set_forget_process (t, arm_linux_forget_process);
}
--
1.8.3.2
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] Support for HWbreak/watchpoint accross fork/vfork on arm-native
2014-01-30 22:12 [PATCH] Support for HWbreak/watchpoint accross fork/vfork on arm-native Omair Javaid
@ 2014-01-30 22:23 ` Omair Javaid
2014-02-07 15:32 ` Pedro Alves
2014-02-08 0:49 ` Yao Qi
0 siblings, 2 replies; 10+ messages in thread
From: Omair Javaid @ 2014-01-30 22:23 UTC (permalink / raw)
To: gdb-patches
On 01/31/2014 03:09 AM, Omair Javaid wrote:
> ---
> gdb/arm-linux-nat.c | 417 ++++++++++++++++++++++++++++++++++------------------
> 1 file changed, 273 insertions(+), 144 deletions(-)
>
> diff --git a/gdb/arm-linux-nat.c b/gdb/arm-linux-nat.c
> index 6f56634..9a756c1 100644
> --- a/gdb/arm-linux-nat.c
> +++ b/gdb/arm-linux-nat.c
> @@ -787,70 +787,120 @@ struct arm_linux_hw_breakpoint
> arm_hwbp_control_t control;
> };
>
> -/* Structure containing arrays of the break and watch points which are have
> - active in each thread.
> -
> - The Linux ptrace interface to hardware break-/watch-points presents the
> - values in a vector centred around 0 (which is used fo generic information).
> - Positive indicies refer to breakpoint addresses/control registers, negative
> - indices to watchpoint addresses/control registers.
> -
> - The Linux vector is indexed as follows:
> - -((i << 1) + 2): Control register for watchpoint i.
> - -((i << 1) + 1): Address register for watchpoint i.
> - 0: Information register.
> - ((i << 1) + 1): Address register for breakpoint i.
> - ((i << 1) + 2): Control register for breakpoint i.
> -
> - This structure is used as a per-thread cache of the state stored by the
> - kernel, so that we don't need to keep calling into the kernel to find a
> - free breakpoint.
> -
> - We treat break-/watch-points with their enable bit clear as being deleted.
> - */
> -typedef struct arm_linux_thread_points
> +/* Since we cannot dynamically allocate subfields of arm_linux_process_info,
> + assume a maximum number of supported break-/watchpoints. */
> +#define MAX_BPTS 16
> +#define MAX_WPTS 16
> +
> +/* Per-process arch-specific data we want to keep. */
> +struct arm_linux_process_info
> {
> - /* Thread ID. */
> - int tid;
> - /* Breakpoints for thread. */
> - struct arm_linux_hw_breakpoint *bpts;
> - /* Watchpoint for threads. */
> - struct arm_linux_hw_breakpoint *wpts;
> -} *arm_linux_thread_points_p;
> -DEF_VEC_P (arm_linux_thread_points_p);
> -
> -/* Vector of hardware breakpoints for each thread. */
> -VEC(arm_linux_thread_points_p) *arm_threads = NULL;
> -
> -/* Find the list of hardware break-/watch-points for a thread with id TID.
> - If no list exists for TID we return NULL if ALLOC_NEW is 0, otherwise we
> - create a new list and return that. */
> -static struct arm_linux_thread_points *
> -arm_linux_find_breakpoints_by_tid (int tid, int alloc_new)
> + /* Linked list. */
> + struct arm_linux_process_info *next;
> + /* The process identifier. */
> + pid_t pid;
> + /* Hardware breakpoints for this process. */
> + struct arm_linux_hw_breakpoint bpts[MAX_BPTS];
> + /* Hardware watchpoints for this process. */
> + struct arm_linux_hw_breakpoint wpts[MAX_WPTS];
> +};
> +
> +/* Per-thread arch-specific data we want to keep. */
> +struct arch_lwp_info
> {
> - int i;
> - struct arm_linux_thread_points *t;
> + /* Non-zero if our copy differs from what's recorded in the thread. */
> + char bpts_changed[MAX_BPTS];
> + char wpts_changed[MAX_WPTS];
> +};
>
> - for (i = 0; VEC_iterate (arm_linux_thread_points_p, arm_threads, i, t); ++i)
> - {
> - if (t->tid == tid)
> - return t;
> - }
> +static struct arm_linux_process_info *arm_linux_process_list = NULL;
> +
> +/* Find process data for process PID. */
>
> - t = NULL;
> +static struct arm_linux_process_info *
> +arm_linux_find_process_pid (pid_t pid)
> +{
> + struct arm_linux_process_info *proc;
> +
> + for (proc = arm_linux_process_list; proc; proc = proc->next)
> + if (proc->pid == pid)
> + return proc;
>
> - if (alloc_new)
> + return NULL;
> +}
> +
> +/* Add process data for process PID. Returns newly allocated info
> + object. */
> +
> +static struct arm_linux_process_info *
> +arm_linux_add_process (pid_t pid)
> +{
> + struct arm_linux_process_info *proc;
> +
> + proc = xcalloc (1, sizeof (*proc));
> + proc->pid = pid;
> +
> + proc->next = arm_linux_process_list;
> + arm_linux_process_list = proc;
> +
> + return proc;
> +}
> +
> +/* Get data specific info for process PID, creating it if necessary.
> + Never returns NULL. */
> +
> +static struct arm_linux_process_info *
> +arm_linux_process_info_get (pid_t pid)
> +{
> + struct arm_linux_process_info *proc;
> +
> + proc = arm_linux_find_process_pid (pid);
> + if (proc == NULL)
> + proc = arm_linux_add_process (pid);
> +
> + return proc;
> +}
> +
> +/* Called whenever GDB is no longer debugging process PID. It deletes
> + data structures that keep track of debug register state. */
> +
> +static void
> +arm_linux_forget_process (pid_t pid)
> +{
> + struct arm_linux_process_info *proc, **proc_link;
> +
> + proc = arm_linux_process_list;
> + proc_link = &arm_linux_process_list;
> +
> + while (proc != NULL)
> {
> - t = xmalloc (sizeof (struct arm_linux_thread_points));
> - t->tid = tid;
> - t->bpts = xzalloc (arm_linux_get_hw_breakpoint_count ()
> - * sizeof (struct arm_linux_hw_breakpoint));
> - t->wpts = xzalloc (arm_linux_get_hw_watchpoint_count ()
> - * sizeof (struct arm_linux_hw_breakpoint));
> - VEC_safe_push (arm_linux_thread_points_p, arm_threads, t);
> + if (proc->pid == pid)
> + {
> + *proc_link = proc->next;
> +
> + xfree (proc);
> + return;
> + }
> +
> + proc_link = &proc->next;
> + proc = *proc_link;
> }
> +}
> +
> +/* Get hardware breakpoint state for process PID. */
>
> - return t;
> +static struct arm_linux_hw_breakpoint *
> +arm_linux_get_hwbreak_state (pid_t pid)
> +{
> + return arm_linux_process_info_get (pid)->bpts;
> +}
> +
> +/* Get watchpoint state for process PID. */
> +
> +static struct arm_linux_hw_breakpoint *
> +arm_linux_get_watch_state (pid_t pid)
> +{
> + return arm_linux_process_info_get (pid)->wpts;
> }
>
> /* Initialize an ARM hardware break-/watch-point control register value.
> @@ -950,45 +1000,76 @@ arm_linux_hw_breakpoint_equal (const struct arm_linux_hw_breakpoint *p1,
> return p1->address == p2->address && p1->control == p2->control;
> }
>
> +/* Callback to mark a watch-/breakpoint to be updated in all threads of
> + the current process. */
> +
> +struct update_registers_data
> +{
> + int watch;
> + int index;
> +};
> +
> +static int
> +update_registers_callback (struct lwp_info *lwp, void *arg)
> +{
> + struct update_registers_data *data = (struct update_registers_data *) arg;
> +
> + /* Force iterate_over_lwps to return matched lwp_info*. */
> + if (arg == NULL)
> + return 1;
> +
> + if (lwp->arch_private == NULL)
> + lwp->arch_private = XCNEW (struct arch_lwp_info);
> +
> + /* The actual update is done later just before resuming the lwp,
> + we just mark that the registers need updating. */
> + if (data->watch)
> + lwp->arch_private->wpts_changed[data->index] = 1;
> + else
> + lwp->arch_private->bpts_changed[data->index] = 1;
> +
> + /* If the lwp isn't stopped, force it to momentarily pause, so
> + we can update its breakpoint registers. */
> + if (!lwp->stopped)
> + linux_stop_lwp (lwp);
> +
> + return 0;
> +}
> +
> /* Insert the hardware breakpoint (WATCHPOINT = 0) or watchpoint (WATCHPOINT
> =1) BPT for thread TID. */
> static void
> arm_linux_insert_hw_breakpoint1 (const struct arm_linux_hw_breakpoint* bpt,
> - int tid, int watchpoint)
> + int watchpoint)
> {
> - struct arm_linux_thread_points *t = arm_linux_find_breakpoints_by_tid (tid, 1);
> + int pid;
> + ptid_t pid_ptid;
> gdb_byte count, i;
> struct arm_linux_hw_breakpoint* bpts;
> - int dir;
> + struct update_registers_data data;
>
> - gdb_assert (t != NULL);
> + pid = ptid_get_pid (inferior_ptid);
> + pid_ptid = pid_to_ptid (pid);
>
> if (watchpoint)
> {
> count = arm_linux_get_hw_watchpoint_count ();
> - bpts = t->wpts;
> - dir = -1;
> + bpts = arm_linux_get_watch_state(pid);
> }
> else
> {
> count = arm_linux_get_hw_breakpoint_count ();
> - bpts = t->bpts;
> - dir = 1;
> + bpts = arm_linux_get_hwbreak_state(pid);
> }
>
> for (i = 0; i < count; ++i)
> if (!arm_hwbp_control_is_enabled (bpts[i].control))
> {
> - errno = 0;
> - if (ptrace (PTRACE_SETHBPREGS, tid, dir * ((i << 1) + 1),
> - &bpt->address) < 0)
> - perror_with_name (_("Unexpected error setting breakpoint address"));
> - if (ptrace (PTRACE_SETHBPREGS, tid, dir * ((i << 1) + 2),
> - &bpt->control) < 0)
> - perror_with_name (_("Unexpected error setting breakpoint"));
> -
> - memcpy (bpts + i, bpt, sizeof (struct arm_linux_hw_breakpoint));
> - break;
> + data.watch = watchpoint;
> + data.index = i;
> + bpts[i] = *bpt;
> + iterate_over_lwps (pid_ptid, update_registers_callback, &data);
> + break;
> }
>
> gdb_assert (i != count);
> @@ -998,37 +1079,36 @@ arm_linux_insert_hw_breakpoint1 (const struct arm_linux_hw_breakpoint* bpt,
> (WATCHPOINT = 1) BPT for thread TID. */
> static void
> arm_linux_remove_hw_breakpoint1 (const struct arm_linux_hw_breakpoint *bpt,
> - int tid, int watchpoint)
> + int watchpoint)
> {
> - struct arm_linux_thread_points *t = arm_linux_find_breakpoints_by_tid (tid, 0);
> + int pid;
> gdb_byte count, i;
> - struct arm_linux_hw_breakpoint *bpts;
> - int dir;
> + ptid_t pid_ptid;
> + struct arm_linux_hw_breakpoint* bpts;
> + struct update_registers_data data;
>
> - gdb_assert (t != NULL);
> + pid = ptid_get_pid (inferior_ptid);
> + pid_ptid = pid_to_ptid (pid);
>
> if (watchpoint)
> {
> count = arm_linux_get_hw_watchpoint_count ();
> - bpts = t->wpts;
> - dir = -1;
> + bpts = arm_linux_get_watch_state(pid);
> }
> else
> {
> count = arm_linux_get_hw_breakpoint_count ();
> - bpts = t->bpts;
> - dir = 1;
> + bpts = arm_linux_get_hwbreak_state(pid);
> }
>
> for (i = 0; i < count; ++i)
> if (arm_linux_hw_breakpoint_equal (bpt, bpts + i))
> {
> - errno = 0;
> - bpts[i].control = arm_hwbp_control_disable (bpts[i].control);
> - if (ptrace (PTRACE_SETHBPREGS, tid, dir * ((i << 1) + 2),
> - &bpts[i].control) < 0)
> - perror_with_name (_("Unexpected error clearing breakpoint"));
> - break;
> + data.watch = watchpoint;
> + data.index = i;
> + bpts[i].control = arm_hwbp_control_disable (bpts[i].control);
> + iterate_over_lwps (pid_ptid, update_registers_callback, &data);
> + break;
> }
>
> gdb_assert (i != count);
> @@ -1046,8 +1126,8 @@ arm_linux_insert_hw_breakpoint (struct gdbarch *gdbarch,
> return -1;
>
> arm_linux_hw_breakpoint_initialize (gdbarch, bp_tgt, &p);
> - ALL_LWPS (lp)
> - arm_linux_insert_hw_breakpoint1 (&p, ptid_get_lwp (lp->ptid), 0);
> +
> + arm_linux_insert_hw_breakpoint1 (&p, 0);
>
> return 0;
> }
> @@ -1064,8 +1144,8 @@ arm_linux_remove_hw_breakpoint (struct gdbarch *gdbarch,
> return -1;
>
> arm_linux_hw_breakpoint_initialize (gdbarch, bp_tgt, &p);
> - ALL_LWPS (lp)
> - arm_linux_remove_hw_breakpoint1 (&p, ptid_get_lwp (lp->ptid), 0);
> +
> + arm_linux_remove_hw_breakpoint1 (&p, 0);
>
> return 0;
> }
> @@ -1115,8 +1195,8 @@ arm_linux_insert_watchpoint (CORE_ADDR addr, int len, int rw,
> return -1;
>
> arm_linux_hw_watchpoint_initialize (addr, len, rw, &p);
> - ALL_LWPS (lp)
> - arm_linux_insert_hw_breakpoint1 (&p, ptid_get_lwp (lp->ptid), 1);
> +
> + arm_linux_insert_hw_breakpoint1 (&p, 1);
>
> return 0;
> }
> @@ -1133,8 +1213,8 @@ arm_linux_remove_watchpoint (CORE_ADDR addr, int len, int rw,
> return -1;
>
> arm_linux_hw_watchpoint_initialize (addr, len, rw, &p);
> - ALL_LWPS (lp)
> - arm_linux_remove_hw_breakpoint1 (&p, ptid_get_lwp (lp->ptid), 1);
> +
> + arm_linux_remove_hw_breakpoint1 (&p, 1);
>
> return 0;
> }
> @@ -1190,63 +1270,108 @@ arm_linux_watchpoint_addr_within_range (struct target_ops *target,
> static void
> arm_linux_new_thread (struct lwp_info *lp)
> {
> - int tid = ptid_get_lwp (lp->ptid);
> - const struct arm_linux_hwbp_cap *info = arm_linux_get_hwbp_cap ();
> + int i;
> + struct arch_lwp_info *info = XCNEW (struct arch_lwp_info);
>
> - if (info != NULL)
> + /* Mark that all the hardware breakpoint/watchpoint register pairs
> + for this thread need to be initialized. */
> +
> + for (i = 0; i < MAX_BPTS; i++)
> {
> - int i;
> - struct arm_linux_thread_points *p;
> - struct arm_linux_hw_breakpoint *bpts;
> -
> - if (VEC_empty (arm_linux_thread_points_p, arm_threads))
> - return;
> -
> - /* Get a list of breakpoints from any thread. */
> - p = VEC_last (arm_linux_thread_points_p, arm_threads);
> -
> - /* Copy that thread's breakpoints and watchpoints to the new thread. */
> - for (i = 0; i < info->bp_count; i++)
> - if (arm_hwbp_control_is_enabled (p->bpts[i].control))
> - arm_linux_insert_hw_breakpoint1 (p->bpts + i, tid, 0);
> - for (i = 0; i < info->wp_count; i++)
> - if (arm_hwbp_control_is_enabled (p->wpts[i].control))
> - arm_linux_insert_hw_breakpoint1 (p->wpts + i, tid, 1);
> + info->bpts_changed[i] = 1;
> + info->wpts_changed[i] = 1;
> }
> +
> + lp->arch_private = info;
> }
>
> -/* Handle thread exit. Tidy up the memory that has been allocated for the
> - thread. */
> +/* Called when resuming a thread.
> + The hardware debug registers are updated when there is any change. */
> +
> static void
> -arm_linux_thread_exit (struct thread_info *tp, int silent)
> +arm_linux_prepare_to_resume (struct lwp_info *lwp)
> {
> - const struct arm_linux_hwbp_cap *info = arm_linux_get_hwbp_cap ();
> + int pid, i;
> + struct arm_linux_hw_breakpoint *bpts, *wpts;
> + struct arch_lwp_info *arm_lwp_info = lwp->arch_private;
> +
> + /* NULL means this is the main thread still going through the shell,
> + or, no watchpoint has been set yet. In that case, there's
> + nothing to do. */
> + if (arm_lwp_info == NULL)
> + return;
>
> - if (info != NULL)
> - {
> - int i;
> - int tid = ptid_get_lwp (tp->ptid);
> - struct arm_linux_thread_points *t = NULL, *p;
> + for (i = 0; i < arm_linux_get_hw_breakpoint_count (); i++)
> + if (arm_lwp_info->bpts_changed[i])
> + {
> + errno = 0;
> + pid = ptid_get_lwp (lwp->ptid);
> + bpts = arm_linux_get_hwbreak_state (ptid_get_pid (lwp->ptid));
>
> - for (i = 0;
> - VEC_iterate (arm_linux_thread_points_p, arm_threads, i, p); i++)
> - {
> - if (p->tid == tid)
> - {
> - t = p;
> - break;
> - }
> - }
> + if (arm_hwbp_control_is_enabled (bpts[i].control))
> + if (ptrace (PTRACE_SETHBPREGS, pid,
> + (PTRACE_TYPE_ARG3) ((i << 1) + 1), &bpts[i].address) < 0)
> + perror_with_name ("Unexpected error setting breakpoint address");
>
> - if (t == NULL)
> - return;
> + if (bpts[i].control != 0)
> + if (ptrace (PTRACE_SETHBPREGS, pid,
> + (PTRACE_TYPE_ARG3) ((i << 1) + 2), &bpts[i].control) < 0)
> + perror_with_name ("Unexpected error setting breakpoint");
>
> - VEC_unordered_remove (arm_linux_thread_points_p, arm_threads, i);
> + arm_lwp_info->bpts_changed[i] = 0;
> + }
>
> - xfree (t->bpts);
> - xfree (t->wpts);
> - xfree (t);
> - }
> + for (i = 0; i < arm_linux_get_hw_watchpoint_count (); i++)
> + if (arm_lwp_info->wpts_changed[i])
> + {
> + errno = 0;
> + pid = ptid_get_lwp (lwp->ptid);
> + wpts = arm_linux_get_watch_state (ptid_get_pid (lwp->ptid));
> +
> + if (arm_hwbp_control_is_enabled (wpts[i].control))
> + if (ptrace (PTRACE_SETHBPREGS, pid,
> + (PTRACE_TYPE_ARG3) -((i << 1) + 1), &wpts[i].address) < 0)
> + perror_with_name ("Unexpected error setting watchpoint address");
> +
> + if (wpts[i].control != 0)
> + if (ptrace (PTRACE_SETHBPREGS, pid,
> + (PTRACE_TYPE_ARG3) -((i << 1) + 2), &wpts[i].control) < 0)
> + perror_with_name ("Unexpected error setting watchpoint");
> +
> + arm_lwp_info->wpts_changed[i] = 0;
> + }
> +}
> +
> +/* linux_nat_new_fork hook. */
> +
> +static void
> +arm_linux_new_fork (struct lwp_info *parent, pid_t child_pid)
> +{
> + pid_t parent_pid;
> + struct arm_linux_hw_breakpoint *parent_point_state;
> + struct arm_linux_hw_breakpoint *child_point_state;
> +
> + /* NULL means no watchpoint has ever been set in the parent. In
> + that case, there's nothing to do. */
> + if (parent->arch_private == NULL)
> + return;
> +
> + /* GDB core assumes the child inherits the watchpoints/hw
> + breakpoints of the parent, and will remove them all from the
> + forked off process. Copy the debug registers mirrors into the
> + new process so that all breakpoints and watchpoints can be
> + removed together. */
> +
> + parent_pid = ptid_get_pid (parent->ptid);
> + parent_point_state = arm_linux_get_hwbreak_state (parent_pid);
> + child_point_state = arm_linux_get_hwbreak_state (child_pid);
> + memcpy (child_point_state, parent_point_state,
> + sizeof (struct arm_linux_hw_breakpoint) * MAX_BPTS);
> +
> + parent_point_state = arm_linux_get_watch_state (parent_pid);
> + child_point_state = arm_linux_get_watch_state (child_pid);
> + memcpy (child_point_state, parent_point_state,
> + sizeof (struct arm_linux_hw_breakpoint) * MAX_BPTS);
> }
>
> void _initialize_arm_linux_nat (void);
> @@ -1279,7 +1404,11 @@ _initialize_arm_linux_nat (void)
> /* Register the target. */
> linux_nat_add_target (t);
>
> - /* Handle thread creation and exit */
> - observer_attach_thread_exit (arm_linux_thread_exit);
> + /* Handle thread creation and exit. */
> linux_nat_set_new_thread (t, arm_linux_new_thread);
> + linux_nat_set_prepare_to_resume (t, arm_linux_prepare_to_resume);
> +
> + /* Handle process creation and exit. */
> + linux_nat_set_new_fork (t, arm_linux_new_fork);
> + linux_nat_set_forget_process (t, arm_linux_forget_process);
> }
>
Accidentally missed the patch description and changelog:
This patch updates arm native support for hardware breakpoints and watchpoints
to accommodate watchpoint/breakpoints across fork/vfork. Previously for arm
native a record of breakpoints at thread level was being kept which has been
changed to a process level record to come in sync with gdb-linux calls to
threads and process creation/destruction hooks.
gdb:
2014-01-31 Omair Javaid <omair.javaid@linaro.org>
* arm-linux-nat.c (struct arm_linux_thread_points): Removed.
(MAX_BPTS): Define.
(MAX_WPTS): Define.
(struct arm_linux_process_info): New.
(DEF_VEC_P (arm_linux_thread_points_p)): Removed.
(VEC(arm_linux_thread_points_p) *arm_threads): Removed.
(arm_linux_find_breakpoints_by_tid): Removed.
(struct arch_lwp_info): New.
(arm_linux_find_process_pid): New functions.
(arm_linux_add_process): New functions.
(arm_linux_process_info_get): New functions.
(arm_linux_forget_process): New function.
(arm_linux_get_hwbreak_state): New function.
(arm_linux_get_watch_state): New function.
(struct update_registers_data): New.
(update_registers_callback): New function.
(arm_linux_insert_hw_breakpoint1): Updated.
(arm_linux_remove_hw_breakpoint1): Updated.
(arm_linux_insert_hw_breakpoint): Updated.
(arm_linux_remove_hw_breakpoint): Updated.
(arm_linux_insert_watchpoint): Updated.
(arm_linux_remove_watchpoint): Updated.
(arm_linux_new_thread): Updated.
(arm_linux_prepare_to_resume): New function.
(arm_linux_new_fork): New function.
(_initialize_arm_linux_nat): Updated.
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] Support for HWbreak/watchpoint accross fork/vfork on arm-native
2014-01-30 22:23 ` Omair Javaid
@ 2014-02-07 15:32 ` Pedro Alves
2014-02-08 0:49 ` Yao Qi
1 sibling, 0 replies; 10+ messages in thread
From: Pedro Alves @ 2014-02-07 15:32 UTC (permalink / raw)
To: Omair Javaid; +Cc: gdb-patches
On 01/30/2014 10:23 PM, Omair Javaid wrote:
> Accidentally missed the patch description and changelog:
>
> This patch updates arm native support for hardware breakpoints and watchpoints
> to accommodate watchpoint/breakpoints across fork/vfork. Previously for arm
> native a record of breakpoints at thread level was being kept which has been
> changed to a process level record to come in sync with gdb-linux calls to
> threads and process creation/destruction hooks.
Thanks.
This also changes the hwbreak/watchpoint insertion mechanism
to the modern way of marking debug registers as needing update, but
only really updating on resume, which is necessary for supporting
watchpoints in non-stop mode (the current code tries to poke at
running threads with ptrace, which fails). Please update the
commit log to mention this.
The patch looks overall mostly good, but I have a few comments
below.
> ---
> gdb/arm-linux-nat.c | 417 ++++++++++++++++++++++++++++++++++------------------
> 1 file changed, 273 insertions(+), 144 deletions(-)
>
> diff --git a/gdb/arm-linux-nat.c b/gdb/arm-linux-nat.c
> index 6f56634..9a756c1 100644
> --- a/gdb/arm-linux-nat.c
> +++ b/gdb/arm-linux-nat.c
> @@ -787,70 +787,120 @@ struct arm_linux_hw_breakpoint
> arm_hwbp_control_t control;
> };
>
> -/* Structure containing arrays of the break and watch points which are have
> - active in each thread.
> -
> - The Linux ptrace interface to hardware break-/watch-points presents the
> - values in a vector centred around 0 (which is used fo generic information).
> - Positive indicies refer to breakpoint addresses/control registers, negative
> - indices to watchpoint addresses/control registers.
> -
> - The Linux vector is indexed as follows:
> - -((i << 1) + 2): Control register for watchpoint i.
> - -((i << 1) + 1): Address register for watchpoint i.
> - 0: Information register.
> - ((i << 1) + 1): Address register for breakpoint i.
> - ((i << 1) + 2): Control register for breakpoint i.
Why remove this ptrace comment? It looks quite useful to me.
> -
> - This structure is used as a per-thread cache of the state stored by the
> - kernel, so that we don't need to keep calling into the kernel to find a
> - free breakpoint.
> -
> - We treat break-/watch-points with their enable bit clear as being deleted.
> - */
> -typedef struct arm_linux_thread_points
> +/* Since we cannot dynamically allocate subfields of arm_linux_process_info,
> + assume a maximum number of supported break-/watchpoints. */
> +#define MAX_BPTS 16
> +#define MAX_WPTS 16
I saw no assertion making sure these maximums aren't
overflowed. see aarch64_linux_get_debug_reg_capacity.
> + /* Linked list. */
> + struct arm_linux_process_info *next;
> + /* The process identifier. */
> + pid_t pid;
> + /* Hardware breakpoints for this process. */
> + struct arm_linux_hw_breakpoint bpts[MAX_BPTS];
> + /* Hardware watchpoints for this process. */
> + struct arm_linux_hw_breakpoint wpts[MAX_WPTS];
It'd be a little cleaner if these two array were moved
to a separate structure, like e.g., in the aarch64 port.
> +/* Get hardware breakpoint state for process PID. */
>
> - return t;
> +static struct arm_linux_hw_breakpoint *
> +arm_linux_get_hwbreak_state (pid_t pid)
> +{
> + return arm_linux_process_info_get (pid)->bpts;
> +}
> +
> +/* Get watchpoint state for process PID. */
> +
> +static struct arm_linux_hw_breakpoint *
> +arm_linux_get_watch_state (pid_t pid)
> +{
> + return arm_linux_process_info_get (pid)->wpts;
> }
Then these two could be a single:
static struct arm_linux_hw_breakpoint *
arm_linux_get_point_state (pid_t pid)
{
return &arm_linux_process_info_get (pid)->state;
}
Users could then do:
arm_linux_get_point_state (pid)->wpts;
arm_linux_get_point_state (pid)->bpts;
> +static int
> +update_registers_callback (struct lwp_info *lwp, void *arg)
> +{
> + struct update_registers_data *data = (struct update_registers_data *) arg;
> +
> + /* Force iterate_over_lwps to return matched lwp_info*. */
> + if (arg == NULL)
> + return 1;
I don't understand this. It seems nothing passes a NULL arg.
> +
> + if (lwp->arch_private == NULL)
> + lwp->arch_private = XCNEW (struct arch_lwp_info);
> +
> + /* The actual update is done later just before resuming the lwp,
> + we just mark that the registers need updating. */
> + if (data->watch)
> + lwp->arch_private->wpts_changed[data->index] = 1;
> + else
> + lwp->arch_private->bpts_changed[data->index] = 1;
> +
> + /* If the lwp isn't stopped, force it to momentarily pause, so
> + we can update its breakpoint registers. */
> + if (!lwp->stopped)
> + linux_stop_lwp (lwp);
> +
> + return 0;
> +}
> if (watchpoint)
> {
> count = arm_linux_get_hw_watchpoint_count ();
> - bpts = t->wpts;
> - dir = -1;
> + bpts = arm_linux_get_watch_state(pid);
Space before parens.
> }
> else
> {
> count = arm_linux_get_hw_breakpoint_count ();
> - bpts = t->bpts;
> - dir = 1;
> + bpts = arm_linux_get_hwbreak_state(pid);
Ditto.
> }
> if (watchpoint)
> {
> count = arm_linux_get_hw_watchpoint_count ();
> - bpts = t->wpts;
> - dir = -1;
> + bpts = arm_linux_get_watch_state(pid);
Ditto.
> }
> else
> {
> count = arm_linux_get_hw_breakpoint_count ();
> - bpts = t->bpts;
> - dir = 1;
> + bpts = arm_linux_get_hwbreak_state(pid);
Ditto.
> }
>
> +arm_linux_prepare_to_resume (struct lwp_info *lwp)
> {
...
> + for (i = 0; i < arm_linux_get_hw_breakpoint_count (); i++)
> + if (arm_lwp_info->bpts_changed[i])
> + {
> + errno = 0;
> + pid = ptid_get_lwp (lwp->ptid);
> + bpts = arm_linux_get_hwbreak_state (ptid_get_pid (lwp->ptid));
This should probably be moved out of the loop. As is it's doing
a linear lookup per iteration.
> - }
> + if (arm_hwbp_control_is_enabled (bpts[i].control))
> + if (ptrace (PTRACE_SETHBPREGS, pid,
> + (PTRACE_TYPE_ARG3) ((i << 1) + 1), &bpts[i].address) < 0)
> + perror_with_name ("Unexpected error setting breakpoint address");
Missing _() for i18n. The old code had it.
>
> - if (t == NULL)
> - return;
> + if (bpts[i].control != 0)
> + if (ptrace (PTRACE_SETHBPREGS, pid,
> + (PTRACE_TYPE_ARG3) ((i << 1) + 2), &bpts[i].control) < 0)
> + perror_with_name ("Unexpected error setting breakpoint");
Ditto.
>
> - VEC_unordered_remove (arm_linux_thread_points_p, arm_threads, i);
> + arm_lwp_info->bpts_changed[i] = 0;
> + }
>
> - xfree (t->bpts);
> - xfree (t->wpts);
> - xfree (t);
> - }
> + for (i = 0; i < arm_linux_get_hw_watchpoint_count (); i++)
> + if (arm_lwp_info->wpts_changed[i])
> + {
> + errno = 0;
> + pid = ptid_get_lwp (lwp->ptid);
> + wpts = arm_linux_get_watch_state (ptid_get_pid (lwp->ptid));
Likewise as comment above.
> +
> + if (arm_hwbp_control_is_enabled (wpts[i].control))
> + if (ptrace (PTRACE_SETHBPREGS, pid,
> + (PTRACE_TYPE_ARG3) -((i << 1) + 1), &wpts[i].address) < 0)
> + perror_with_name ("Unexpected error setting watchpoint address");
Ditto, etc.
> +static void
> +arm_linux_new_fork (struct lwp_info *parent, pid_t child_pid)
> +{
> + parent_pid = ptid_get_pid (parent->ptid);
> + parent_point_state = arm_linux_get_hwbreak_state (parent_pid);
> + child_point_state = arm_linux_get_hwbreak_state (child_pid);
> + memcpy (child_point_state, parent_point_state,
> + sizeof (struct arm_linux_hw_breakpoint) * MAX_BPTS);
> +
> + parent_point_state = arm_linux_get_watch_state (parent_pid);
> + child_point_state = arm_linux_get_watch_state (child_pid);
> + memcpy (child_point_state, parent_point_state,
> + sizeof (struct arm_linux_hw_breakpoint) * MAX_BPTS);
The latter should be MAX_WPTS. But with the earlier proposed change,
these two would simply turn into a single:
parent_pid = ptid_get_pid (parent->ptid);
parent_point_state = arm_linux_get_point_state (parent_pid);
child_point_state = arm_linux_get_point_state (child_pid);
*child_point_state = *parent_point_state;
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] Support for HWbreak/watchpoint accross fork/vfork on arm-native
2014-01-30 22:23 ` Omair Javaid
2014-02-07 15:32 ` Pedro Alves
@ 2014-02-08 0:49 ` Yao Qi
2014-02-12 15:40 ` Omair Javaid
1 sibling, 1 reply; 10+ messages in thread
From: Yao Qi @ 2014-02-08 0:49 UTC (permalink / raw)
To: Omair Javaid; +Cc: gdb-patches
On 01/31/2014 06:23 AM, Omair Javaid wrote:
> This patch updates arm native support for hardware breakpoints and watchpoints
> to accommodate watchpoint/breakpoints across fork/vfork. Previously for arm
> native a record of breakpoints at thread level was being kept which has been
> changed to a process level record to come in sync with gdb-linux calls to
> threads and process creation/destruction hooks.
Is it possible to write a test case which fails before the patch applies
but passes after?
--
Yao (é½å°§)
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] Support for HWbreak/watchpoint accross fork/vfork on arm-native
2014-02-08 0:49 ` Yao Qi
@ 2014-02-12 15:40 ` Omair Javaid
2014-02-13 5:05 ` Yao Qi
2014-02-13 18:40 ` Pedro Alves
0 siblings, 2 replies; 10+ messages in thread
From: Omair Javaid @ 2014-02-12 15:40 UTC (permalink / raw)
To: Yao Qi; +Cc: gdb-patches
On Sat 08 Feb 2014 05:47:30 AM PKT, Yao Qi wrote:
> On 01/31/2014 06:23 AM, Omair Javaid wrote:
>> This patch updates arm native support for hardware breakpoints and watchpoints
>> to accommodate watchpoint/breakpoints across fork/vfork. Previously for arm
>> native a record of breakpoints at thread level was being kept which has been
>> changed to a process level record to come in sync with gdb-linux calls to
>> threads and process creation/destruction hooks.
>
> Is it possible to write a test case which fails before the patch applies
> but passes after?
>
This is an update on the previous patch after incorporating suggestions.
This patch updates arm native support for hwbreak-/watchpoints to enable
support for hwbreak-/watchpoints across fork/vfork. This involves changes to
hwbreak-/watchpoint insertion mechanism to the modern way, by marking debug
registers as needing update, but only really updating them on resume, which is
necessary for supporting watchpoints in non-stop mode. This also updates a
previously maintained per thread hwbreak-/watchpoint cache to a per process
cache which allows target specific code to come in sync with gdb-linux calls to
threads create/destroy and process fork/exit hooks.
This patch makes good improvement in arm-linux-gdb testsuite results.
Fixes 24 failures in watchpoint-fork.exp like the one copied below:
FAIL: gdb.threads/watchpoint-fork.exp: parent: singlethreaded: breakpoint after the first fork (GDB internal error)
Also fixes a few other watchpoint related failures in gdb.base and gdb.threads testsuite.
gdb:
2014-02-11 Omair Javaid <omair.javaid@linaro.org>
* arm-linux-nat.c (struct arm_linux_thread_points): Removed.
(MAX_BPTS): Define.
(MAX_WPTS): Define.
(struct arm_linux_process_info): New.
(DEF_VEC_P (arm_linux_thread_points_p)): Removed.
(VEC(arm_linux_thread_points_p) *arm_threads): Removed.
(arm_linux_find_breakpoints_by_tid): Removed.
(struct arch_lwp_info): New.
(arm_linux_find_process_pid): New functions.
(arm_linux_add_process): New functions.
(arm_linux_process_info_get): New functions.
(arm_linux_forget_process): New function.
(arm_linux_get_debug_reg_state): New function.
(struct update_registers_data): New.
(update_registers_callback): New function.
(arm_linux_insert_hw_breakpoint1): Updated.
(arm_linux_remove_hw_breakpoint1): Updated.
(arm_linux_insert_hw_breakpoint): Updated.
(arm_linux_remove_hw_breakpoint): Updated.
(arm_linux_insert_watchpoint): Updated.
(arm_linux_remove_watchpoint): Updated.
(arm_linux_new_thread): Updated.
(arm_linux_prepare_to_resume): New function.
(arm_linux_new_fork): New function.
(_initialize_arm_linux_nat): Updated.
---
gdb/arm-linux-nat.c | 397 +++++++++++++++++++++++++++++++++++-----------------
1 file changed, 272 insertions(+), 125 deletions(-)
diff --git a/gdb/arm-linux-nat.c b/gdb/arm-linux-nat.c
index 6f56634..4047725 100644
--- a/gdb/arm-linux-nat.c
+++ b/gdb/arm-linux-nat.c
@@ -787,8 +787,13 @@ struct arm_linux_hw_breakpoint
arm_hwbp_control_t control;
};
-/* Structure containing arrays of the break and watch points which are have
- active in each thread.
+/* Since we cannot dynamically allocate subfields of arm_linux_process_info,
+ assume a maximum number of supported break-/watchpoints. */
+#define MAX_BPTS 16
+#define MAX_WPTS 16
+
+/* Structure containing arrays of per process hardware break-/watchpoints
+ for caching address and control information.
The Linux ptrace interface to hardware break-/watch-points presents the
values in a vector centred around 0 (which is used fo generic information).
@@ -808,49 +813,114 @@ struct arm_linux_hw_breakpoint
We treat break-/watch-points with their enable bit clear as being deleted.
*/
-typedef struct arm_linux_thread_points
+struct arm_linux_debug_reg_state
{
- /* Thread ID. */
- int tid;
- /* Breakpoints for thread. */
- struct arm_linux_hw_breakpoint *bpts;
- /* Watchpoint for threads. */
- struct arm_linux_hw_breakpoint *wpts;
-} *arm_linux_thread_points_p;
-DEF_VEC_P (arm_linux_thread_points_p);
-
-/* Vector of hardware breakpoints for each thread. */
-VEC(arm_linux_thread_points_p) *arm_threads = NULL;
-
-/* Find the list of hardware break-/watch-points for a thread with id TID.
- If no list exists for TID we return NULL if ALLOC_NEW is 0, otherwise we
- create a new list and return that. */
-static struct arm_linux_thread_points *
-arm_linux_find_breakpoints_by_tid (int tid, int alloc_new)
+ /* Hardware breakpoints for this process. */
+ struct arm_linux_hw_breakpoint bpts[MAX_BPTS];
+ /* Hardware watchpoints for this process. */
+ struct arm_linux_hw_breakpoint wpts[MAX_WPTS];
+};
+
+/* Per-process arch-specific data we want to keep. */
+struct arm_linux_process_info
{
- int i;
- struct arm_linux_thread_points *t;
+ /* Linked list. */
+ struct arm_linux_process_info *next;
+ /* The process identifier. */
+ pid_t pid;
+ /* Hardware break-/watchpoints state information. */
+ struct arm_linux_debug_reg_state state;
- for (i = 0; VEC_iterate (arm_linux_thread_points_p, arm_threads, i, t); ++i)
- {
- if (t->tid == tid)
- return t;
- }
+};
+
+/* 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. */
+ char bpts_changed[MAX_BPTS];
+ char wpts_changed[MAX_WPTS];
+};
+
+static struct arm_linux_process_info *arm_linux_process_list = NULL;
+
+/* Find process data for process PID. */
+
+static struct arm_linux_process_info *
+arm_linux_find_process_pid (pid_t pid)
+{
+ struct arm_linux_process_info *proc;
+
+ for (proc = arm_linux_process_list; proc; proc = proc->next)
+ if (proc->pid == pid)
+ return proc;
+
+ return NULL;
+}
+
+/* Add process data for process PID. Returns newly allocated info
+ object. */
+
+static struct arm_linux_process_info *
+arm_linux_add_process (pid_t pid)
+{
+ struct arm_linux_process_info *proc;
- t = NULL;
+ proc = xcalloc (1, sizeof (*proc));
+ proc->pid = pid;
- if (alloc_new)
+ proc->next = arm_linux_process_list;
+ arm_linux_process_list = proc;
+
+ return proc;
+}
+
+/* Get data specific info for process PID, creating it if necessary.
+ Never returns NULL. */
+
+static struct arm_linux_process_info *
+arm_linux_process_info_get (pid_t pid)
+{
+ struct arm_linux_process_info *proc;
+
+ proc = arm_linux_find_process_pid (pid);
+ if (proc == NULL)
+ proc = arm_linux_add_process (pid);
+
+ return proc;
+}
+
+/* Called whenever GDB is no longer debugging process PID. It deletes
+ data structures that keep track of debug register state. */
+
+static void
+arm_linux_forget_process (pid_t pid)
+{
+ struct arm_linux_process_info *proc, **proc_link;
+
+ proc = arm_linux_process_list;
+ proc_link = &arm_linux_process_list;
+
+ while (proc != NULL)
{
- t = xmalloc (sizeof (struct arm_linux_thread_points));
- t->tid = tid;
- t->bpts = xzalloc (arm_linux_get_hw_breakpoint_count ()
- * sizeof (struct arm_linux_hw_breakpoint));
- t->wpts = xzalloc (arm_linux_get_hw_watchpoint_count ()
- * sizeof (struct arm_linux_hw_breakpoint));
- VEC_safe_push (arm_linux_thread_points_p, arm_threads, t);
+ if (proc->pid == pid)
+ {
+ *proc_link = proc->next;
+
+ xfree (proc);
+ return;
}
- return t;
+ proc_link = &proc->next;
+ proc = *proc_link;
+ }
+}
+
+/* Get hardware break-/watchpoint state for process PID. */
+
+static struct arm_linux_debug_reg_state *
+arm_linux_get_debug_reg_state (pid_t pid)
+{
+ return &arm_linux_process_info_get (pid)->state;
}
/* Initialize an ARM hardware break-/watch-point control register value.
@@ -950,45 +1020,82 @@ arm_linux_hw_breakpoint_equal (const struct arm_linux_hw_breakpoint *p1,
return p1->address == p2->address && p1->control == p2->control;
}
+/* Callback to mark a watch-/breakpoint to be updated in all threads of
+ the current process. */
+
+struct update_registers_data
+{
+ int watch;
+ int index;
+};
+
+static int
+update_registers_callback (struct lwp_info *lwp, void *arg)
+{
+ struct update_registers_data *data = (struct update_registers_data *) arg;
+
+ /* Force iterate_over_lwps to return matched lwp_info*. */
+ if (arg == NULL)
+ return 1;
+
+ if (lwp->arch_private == NULL)
+ lwp->arch_private = XCNEW (struct arch_lwp_info);
+
+ /* The actual update is done later just before resuming the lwp,
+ we just mark that the registers need updating. */
+ if (data->watch)
+ lwp->arch_private->wpts_changed[data->index] = 1;
+ else
+ lwp->arch_private->bpts_changed[data->index] = 1;
+
+ /* If the lwp isn't stopped, force it to momentarily pause, so
+ we can update its breakpoint registers. */
+ if (!lwp->stopped)
+ linux_stop_lwp (lwp);
+
+ return 0;
+}
+
/* Insert the hardware breakpoint (WATCHPOINT = 0) or watchpoint (WATCHPOINT
=1) BPT for thread TID. */
static void
arm_linux_insert_hw_breakpoint1 (const struct arm_linux_hw_breakpoint* bpt,
- int tid, int watchpoint)
+ int watchpoint)
{
- struct arm_linux_thread_points *t = arm_linux_find_breakpoints_by_tid (tid, 1);
+ int pid;
+ ptid_t pid_ptid;
gdb_byte count, i;
struct arm_linux_hw_breakpoint* bpts;
- int dir;
+ struct update_registers_data data;
- gdb_assert (t != NULL);
+ pid = ptid_get_pid (inferior_ptid);
+ pid_ptid = pid_to_ptid (pid);
if (watchpoint)
{
count = arm_linux_get_hw_watchpoint_count ();
- bpts = t->wpts;
- dir = -1;
+ bpts = arm_linux_get_debug_reg_state (pid)->wpts;
+ if (count > MAX_WPTS)
+ warning (_("arm-linux-gdb supports %d hardware watchpoints but target \
+ supports %d"), MAX_WPTS, count);
}
else
{
count = arm_linux_get_hw_breakpoint_count ();
- bpts = t->bpts;
- dir = 1;
+ bpts = arm_linux_get_debug_reg_state (pid)->bpts;
+ if (count > MAX_BPTS)
+ warning (_("arm-linux-gdb supports %d hardware breakpoints but target \
+ supports %d"), MAX_BPTS, count);
}
for (i = 0; i < count; ++i)
if (!arm_hwbp_control_is_enabled (bpts[i].control))
{
- errno = 0;
- if (ptrace (PTRACE_SETHBPREGS, tid, dir * ((i << 1) + 1),
- &bpt->address) < 0)
- perror_with_name (_("Unexpected error setting breakpoint address"));
- if (ptrace (PTRACE_SETHBPREGS, tid, dir * ((i << 1) + 2),
- &bpt->control) < 0)
- perror_with_name (_("Unexpected error setting breakpoint"));
-
- memcpy (bpts + i, bpt, sizeof (struct arm_linux_hw_breakpoint));
- break;
+ data.watch = watchpoint;
+ data.index = i;
+ bpts[i] = *bpt;
+ iterate_over_lwps (pid_ptid, update_registers_callback, &data);
+ break;
}
gdb_assert (i != count);
@@ -998,37 +1105,36 @@ arm_linux_insert_hw_breakpoint1 (const struct arm_linux_hw_breakpoint* bpt,
(WATCHPOINT = 1) BPT for thread TID. */
static void
arm_linux_remove_hw_breakpoint1 (const struct arm_linux_hw_breakpoint *bpt,
- int tid, int watchpoint)
+ int watchpoint)
{
- struct arm_linux_thread_points *t = arm_linux_find_breakpoints_by_tid (tid, 0);
+ int pid;
gdb_byte count, i;
- struct arm_linux_hw_breakpoint *bpts;
- int dir;
+ ptid_t pid_ptid;
+ struct arm_linux_hw_breakpoint* bpts;
+ struct update_registers_data data;
- gdb_assert (t != NULL);
+ pid = ptid_get_pid (inferior_ptid);
+ pid_ptid = pid_to_ptid (pid);
if (watchpoint)
{
count = arm_linux_get_hw_watchpoint_count ();
- bpts = t->wpts;
- dir = -1;
+ bpts = arm_linux_get_debug_reg_state (pid)->wpts;
}
else
{
count = arm_linux_get_hw_breakpoint_count ();
- bpts = t->bpts;
- dir = 1;
+ bpts = arm_linux_get_debug_reg_state (pid)->bpts;
}
for (i = 0; i < count; ++i)
if (arm_linux_hw_breakpoint_equal (bpt, bpts + i))
{
- errno = 0;
- bpts[i].control = arm_hwbp_control_disable (bpts[i].control);
- if (ptrace (PTRACE_SETHBPREGS, tid, dir * ((i << 1) + 2),
- &bpts[i].control) < 0)
- perror_with_name (_("Unexpected error clearing breakpoint"));
- break;
+ data.watch = watchpoint;
+ data.index = i;
+ bpts[i].control = arm_hwbp_control_disable (bpts[i].control);
+ iterate_over_lwps (pid_ptid, update_registers_callback, &data);
+ break;
}
gdb_assert (i != count);
@@ -1046,8 +1152,8 @@ arm_linux_insert_hw_breakpoint (struct gdbarch *gdbarch,
return -1;
arm_linux_hw_breakpoint_initialize (gdbarch, bp_tgt, &p);
- ALL_LWPS (lp)
- arm_linux_insert_hw_breakpoint1 (&p, ptid_get_lwp (lp->ptid), 0);
+
+ arm_linux_insert_hw_breakpoint1 (&p, 0);
return 0;
}
@@ -1064,8 +1170,8 @@ arm_linux_remove_hw_breakpoint (struct gdbarch *gdbarch,
return -1;
arm_linux_hw_breakpoint_initialize (gdbarch, bp_tgt, &p);
- ALL_LWPS (lp)
- arm_linux_remove_hw_breakpoint1 (&p, ptid_get_lwp (lp->ptid), 0);
+
+ arm_linux_remove_hw_breakpoint1 (&p, 0);
return 0;
}
@@ -1115,8 +1221,8 @@ arm_linux_insert_watchpoint (CORE_ADDR addr, int len, int rw,
return -1;
arm_linux_hw_watchpoint_initialize (addr, len, rw, &p);
- ALL_LWPS (lp)
- arm_linux_insert_hw_breakpoint1 (&p, ptid_get_lwp (lp->ptid), 1);
+
+ arm_linux_insert_hw_breakpoint1 (&p, 1);
return 0;
}
@@ -1133,8 +1239,8 @@ arm_linux_remove_watchpoint (CORE_ADDR addr, int len, int rw,
return -1;
arm_linux_hw_watchpoint_initialize (addr, len, rw, &p);
- ALL_LWPS (lp)
- arm_linux_remove_hw_breakpoint1 (&p, ptid_get_lwp (lp->ptid), 1);
+
+ arm_linux_remove_hw_breakpoint1 (&p, 1);
return 0;
}
@@ -1190,63 +1296,100 @@ arm_linux_watchpoint_addr_within_range (struct target_ops *target,
static void
arm_linux_new_thread (struct lwp_info *lp)
{
- int tid = ptid_get_lwp (lp->ptid);
- const struct arm_linux_hwbp_cap *info = arm_linux_get_hwbp_cap ();
+ int i;
+ struct arch_lwp_info *info = XCNEW (struct arch_lwp_info);
+
+ /* Mark that all the hardware breakpoint/watchpoint register pairs
+ for this thread need to be initialized. */
- if (info != NULL)
+ for (i = 0; i < MAX_BPTS; i++)
{
- int i;
- struct arm_linux_thread_points *p;
- struct arm_linux_hw_breakpoint *bpts;
-
- if (VEC_empty (arm_linux_thread_points_p, arm_threads))
- return;
-
- /* Get a list of breakpoints from any thread. */
- p = VEC_last (arm_linux_thread_points_p, arm_threads);
-
- /* Copy that thread's breakpoints and watchpoints to the new thread. */
- for (i = 0; i < info->bp_count; i++)
- if (arm_hwbp_control_is_enabled (p->bpts[i].control))
- arm_linux_insert_hw_breakpoint1 (p->bpts + i, tid, 0);
- for (i = 0; i < info->wp_count; i++)
- if (arm_hwbp_control_is_enabled (p->wpts[i].control))
- arm_linux_insert_hw_breakpoint1 (p->wpts + i, tid, 1);
+ info->bpts_changed[i] = 1;
+ info->wpts_changed[i] = 1;
}
+
+ lp->arch_private = info;
}
-/* Handle thread exit. Tidy up the memory that has been allocated for the
- thread. */
+/* Called when resuming a thread.
+ The hardware debug registers are updated when there is any change. */
+
static void
-arm_linux_thread_exit (struct thread_info *tp, int silent)
+arm_linux_prepare_to_resume (struct lwp_info *lwp)
{
- const struct arm_linux_hwbp_cap *info = arm_linux_get_hwbp_cap ();
+ int pid, i;
+ struct arm_linux_hw_breakpoint *bpts, *wpts;
+ struct arch_lwp_info *arm_lwp_info = lwp->arch_private;
+
+ pid = ptid_get_lwp (lwp->ptid);
+ bpts = arm_linux_get_debug_reg_state (ptid_get_pid (lwp->ptid))->bpts;
+ wpts = arm_linux_get_debug_reg_state (ptid_get_pid (lwp->ptid))->wpts;
+
+ /* NULL means this is the main thread still going through the shell,
+ or, no watchpoint has been set yet. In that case, there's
+ nothing to do. */
+ if (arm_lwp_info == NULL)
+ return;
- if (info != NULL)
- {
- int i;
- int tid = ptid_get_lwp (tp->ptid);
- struct arm_linux_thread_points *t = NULL, *p;
+ for (i = 0; i < arm_linux_get_hw_breakpoint_count (); i++)
+ if (arm_lwp_info->bpts_changed[i])
+ {
+ errno = 0;
+ if (arm_hwbp_control_is_enabled (bpts[i].control))
+ if (ptrace (PTRACE_SETHBPREGS, pid,
+ (PTRACE_TYPE_ARG3) ((i << 1) + 1), &bpts[i].address) < 0)
+ perror_with_name (_("Unexpected error setting breakpoint"));
+
+ if (bpts[i].control != 0)
+ if (ptrace (PTRACE_SETHBPREGS, pid,
+ (PTRACE_TYPE_ARG3) ((i << 1) + 2), &bpts[i].control) < 0)
+ perror_with_name (_("Unexpected error setting breakpoint"));
+
+ arm_lwp_info->bpts_changed[i] = 0;
+ }
- for (i = 0;
- VEC_iterate (arm_linux_thread_points_p, arm_threads, i, p); i++)
- {
- if (p->tid == tid)
- {
- t = p;
- break;
- }
- }
+ for (i = 0; i < arm_linux_get_hw_watchpoint_count (); i++)
+ if (arm_lwp_info->wpts_changed[i])
+ {
+ errno = 0;
+ if (arm_hwbp_control_is_enabled (wpts[i].control))
+ if (ptrace (PTRACE_SETHBPREGS, pid,
+ (PTRACE_TYPE_ARG3) -((i << 1) + 1), &wpts[i].address) < 0)
+ perror_with_name (_("Unexpected error setting watchpoint"));
+
+ if (wpts[i].control != 0)
+ if (ptrace (PTRACE_SETHBPREGS, pid,
+ (PTRACE_TYPE_ARG3) -((i << 1) + 2), &wpts[i].control) < 0)
+ perror_with_name (_("Unexpected error setting watchpoint"));
+
+ arm_lwp_info->wpts_changed[i] = 0;
+ }
+}
+
+/* linux_nat_new_fork hook. */
+
+static void
+arm_linux_new_fork (struct lwp_info *parent, pid_t child_pid)
+{
+ pid_t parent_pid;
+ struct arm_linux_debug_reg_state *parent_state;
+ struct arm_linux_debug_reg_state *child_state;
- if (t == NULL)
- return;
+ /* NULL means no watchpoint has ever been set in the parent. In
+ that case, there's nothing to do. */
+ if (parent->arch_private == NULL)
+ return;
- VEC_unordered_remove (arm_linux_thread_points_p, arm_threads, i);
+ /* GDB core assumes the child inherits the watchpoints/hw
+ breakpoints of the parent, and will remove them all from the
+ forked off process. Copy the debug registers mirrors into the
+ new process so that all breakpoints and watchpoints can be
+ removed together. */
- xfree (t->bpts);
- xfree (t->wpts);
- xfree (t);
- }
+ parent_pid = ptid_get_pid (parent->ptid);
+ parent_state = arm_linux_get_debug_reg_state (parent_pid);
+ child_state = arm_linux_get_debug_reg_state (child_pid);
+ *child_state = *parent_state;
}
void _initialize_arm_linux_nat (void);
@@ -1279,7 +1422,11 @@ _initialize_arm_linux_nat (void)
/* Register the target. */
linux_nat_add_target (t);
- /* Handle thread creation and exit */
- observer_attach_thread_exit (arm_linux_thread_exit);
+ /* Handle thread creation and exit. */
linux_nat_set_new_thread (t, arm_linux_new_thread);
+ linux_nat_set_prepare_to_resume (t, arm_linux_prepare_to_resume);
+
+ /* Handle process creation and exit. */
+ linux_nat_set_new_fork (t, arm_linux_new_fork);
+ linux_nat_set_forget_process (t, arm_linux_forget_process);
}
--
1.8.3.2
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] Support for HWbreak/watchpoint accross fork/vfork on arm-native
2014-02-12 15:40 ` Omair Javaid
@ 2014-02-13 5:05 ` Yao Qi
2014-02-13 18:40 ` Pedro Alves
1 sibling, 0 replies; 10+ messages in thread
From: Yao Qi @ 2014-02-13 5:05 UTC (permalink / raw)
To: Omair Javaid; +Cc: gdb-patches
On 02/12/2014 11:40 PM, Omair Javaid wrote:
> This patch makes good improvement in arm-linux-gdb testsuite results.
> Fixes 24 failures in watchpoint-fork.exp like the one copied below:
> FAIL: gdb.threads/watchpoint-fork.exp: parent: singlethreaded: breakpoint after the first fork (GDB internal error)
> Also fixes a few other watchpoint related failures in gdb.base and gdb.threads testsuite.
We have already had tests covered, so no need to write new ones.
I don't have any comments.
--
Yao (é½å°§)
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] Support for HWbreak/watchpoint accross fork/vfork on arm-native
2014-02-12 15:40 ` Omair Javaid
2014-02-13 5:05 ` Yao Qi
@ 2014-02-13 18:40 ` Pedro Alves
2014-02-14 0:06 ` Omair Javaid
1 sibling, 1 reply; 10+ messages in thread
From: Pedro Alves @ 2014-02-13 18:40 UTC (permalink / raw)
To: Omair Javaid; +Cc: Yao Qi, gdb-patches
> +static int
> +update_registers_callback (struct lwp_info *lwp, void *arg)
> +{
> + struct update_registers_data *data = (struct update_registers_data *) arg;
> +
> + /* Force iterate_over_lwps to return matched lwp_info*. */
> + if (arg == NULL)
> + return 1;
> +
You don't seem to have addressed my comment here? It was:
"I don't understand this. It seems nothing passes a NULL arg."
> if (watchpoint)
> {
> count = arm_linux_get_hw_watchpoint_count ();
> - bpts = t->wpts;
> - dir = -1;
> + bpts = arm_linux_get_debug_reg_state (pid)->wpts;
> + if (count > MAX_WPTS)
> + warning (_("arm-linux-gdb supports %d hardware watchpoints but target \
> + supports %d"), MAX_WPTS, count);
Like in the Aarch64 port, add a 'count = MAX_WPTS' as well, otherwise
GDB would access the arrays out of bounds when that happens.
> }
> else
> {
> count = arm_linux_get_hw_breakpoint_count ();
> - bpts = t->bpts;
> - dir = 1;
> + bpts = arm_linux_get_debug_reg_state (pid)->bpts;
> + if (count > MAX_BPTS)
> + warning (_("arm-linux-gdb supports %d hardware breakpoints but target \
> + supports %d"), MAX_BPTS, count);
Likewise.
> }
>
> for (i = 0; i < count; ++i)
> if (!arm_hwbp_control_is_enabled (bpts[i].control))
--
Pedro Alves
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] Support for HWbreak/watchpoint accross fork/vfork on arm-native
2014-02-13 18:40 ` Pedro Alves
@ 2014-02-14 0:06 ` Omair Javaid
2014-03-08 16:29 ` Omair Javaid
0 siblings, 1 reply; 10+ messages in thread
From: Omair Javaid @ 2014-02-14 0:06 UTC (permalink / raw)
To: Pedro Alves; +Cc: Yao Qi, gdb-patches
On 13/02/2014 23:39, Pedro Alves wrote:
>> +static int
>> +update_registers_callback (struct lwp_info *lwp, void *arg)
>> +{
>> + struct update_registers_data *data = (struct update_registers_data *) arg;
>> +
>> + /* Force iterate_over_lwps to return matched lwp_info*. */
>> + if (arg == NULL)
>> + return 1;
>> +
> You don't seem to have addressed my comment here? It was:
>
> "I don't understand this. It seems nothing passes a NULL arg."
I was using iterate_over_lwps to return matching lwp_info struct and
that happens only when if update_registers_callback returns 1.
Its not being used anywhere now and can be removed.
>
>
>> if (watchpoint)
>> {
>> count = arm_linux_get_hw_watchpoint_count ();
>> - bpts = t->wpts;
>> - dir = -1;
>> + bpts = arm_linux_get_debug_reg_state (pid)->wpts;
>> + if (count > MAX_WPTS)
>> + warning (_("arm-linux-gdb supports %d hardware watchpoints but target \
>> + supports %d"), MAX_WPTS, count);
> Like in the Aarch64 port, add a 'count = MAX_WPTS' as well, otherwise
> GDB would access the arrays out of bounds when that happens.
OK will update and repost.
>
>> }
>> else
>> {
>> count = arm_linux_get_hw_breakpoint_count ();
>> - bpts = t->bpts;
>> - dir = 1;
>> + bpts = arm_linux_get_debug_reg_state (pid)->bpts;
>> + if (count > MAX_BPTS)
>> + warning (_("arm-linux-gdb supports %d hardware breakpoints but target \
>> + supports %d"), MAX_BPTS, count);
> Likewise.
OK will update and repost.
>
>> }
>>
>> for (i = 0; i < count; ++i)
>> if (!arm_hwbp_control_is_enabled (bpts[i].control))
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] Support for HWbreak/watchpoint accross fork/vfork on arm-native
2014-02-14 0:06 ` Omair Javaid
@ 2014-03-08 16:29 ` Omair Javaid
2014-03-10 15:13 ` Pedro Alves
0 siblings, 1 reply; 10+ messages in thread
From: Omair Javaid @ 2014-03-08 16:29 UTC (permalink / raw)
To: Pedro Alves; +Cc: Yao Qi, gdb-patches
On 02/14/2014 05:06 AM, Omair Javaid wrote:
>
> On 13/02/2014 23:39, Pedro Alves wrote:
>>> +static int
>>> +update_registers_callback (struct lwp_info *lwp, void *arg)
>>> +{
>>> + struct update_registers_data *data = (struct update_registers_data *) arg;
>>> +
>>> + /* Force iterate_over_lwps to return matched lwp_info*. */
>>> + if (arg == NULL)
>>> + return 1;
>>> +
>> You don't seem to have addressed my comment here? It was:
>>
>> "I don't understand this. It seems nothing passes a NULL arg."
>
> I was using iterate_over_lwps to return matching lwp_info struct and
> that happens only when if update_registers_callback returns 1.
> Its not being used anywhere now and can be removed.
>
>>
>>
>>> if (watchpoint)
>>> {
>>> count = arm_linux_get_hw_watchpoint_count ();
>>> - bpts = t->wpts;
>>> - dir = -1;
>>> + bpts = arm_linux_get_debug_reg_state (pid)->wpts;
>>> + if (count > MAX_WPTS)
>>> + warning (_("arm-linux-gdb supports %d hardware watchpoints but target \
>>> + supports %d"), MAX_WPTS, count);
>> Like in the Aarch64 port, add a 'count = MAX_WPTS' as well, otherwise
>> GDB would access the arrays out of bounds when that happens.
> OK will update and repost.
>>
>>> }
>>> else
>>> {
>>> count = arm_linux_get_hw_breakpoint_count ();
>>> - bpts = t->bpts;
>>> - dir = 1;
>>> + bpts = arm_linux_get_debug_reg_state (pid)->bpts;
>>> + if (count > MAX_BPTS)
>>> + warning (_("arm-linux-gdb supports %d hardware breakpoints but target \
>>> + supports %d"), MAX_BPTS, count);
>> Likewise.
> OK will update and repost.
>>
>>> }
>>>
>>> for (i = 0; i < count; ++i)
>>> if (!arm_hwbp_control_is_enabled (bpts[i].control))
>
Sorry for the delay. Somehow this patch update seems to be sitting in my outgoing email for a while without being sent.
OK to commit?
gdb:
2014-02-25 Omair Javaid <omair.javaid@linaro.org>
* arm-linux-nat.c (arm_linux_get_hwbp_cap): Updated.
(MAX_BPTS): Define.
(MAX_WPTS): Define.
(struct arm_linux_thread_points): Removed.
(struct arm_linux_process_info): New.
(DEF_VEC_P (arm_linux_thread_points_p)): Removed.
(VEC(arm_linux_thread_points_p) *arm_threads): Removed.
(arm_linux_find_breakpoints_by_tid): Removed.
(struct arch_lwp_info): New.
(arm_linux_find_process_pid): New functions.
(arm_linux_add_process): New functions.
(arm_linux_process_info_get): New functions.
(arm_linux_forget_process): New function.
(arm_linux_get_debug_reg_state): New function.
(struct update_registers_data): New.
(update_registers_callback): New function.
(arm_linux_insert_hw_breakpoint1): Updated.
(arm_linux_remove_hw_breakpoint1): Updated.
(arm_linux_insert_hw_breakpoint): Updated.
(arm_linux_remove_hw_breakpoint): Updated.
(arm_linux_insert_watchpoint): Updated.
(arm_linux_remove_watchpoint): Updated.
(arm_linux_new_thread): Updated.
(arm_linux_prepare_to_resume): New function.
(arm_linux_new_fork): New function.
(_initialize_arm_linux_nat): Updated.
---
gdb/arm-linux-nat.c | 401 ++++++++++++++++++++++++++++++++++++----------------
1 file changed, 276 insertions(+), 125 deletions(-)
diff --git a/gdb/arm-linux-nat.c b/gdb/arm-linux-nat.c
index a92642d..2d2afb2 100644
--- a/gdb/arm-linux-nat.c
+++ b/gdb/arm-linux-nat.c
@@ -693,6 +693,11 @@ struct arm_linux_hwbp_cap
gdb_byte bp_count;
};
+/* Since we cannot dynamically allocate subfields of arm_linux_process_info,
+ assume a maximum number of supported break-/watchpoints. */
+#define MAX_BPTS 16
+#define MAX_WPTS 16
+
/* Get hold of the Hardware Breakpoint information for the target we are
attached to. Returns NULL if the kernel doesn't support Hardware
breakpoints at all, or a pointer to the information structure. */
@@ -721,6 +726,20 @@ arm_linux_get_hwbp_cap (void)
info.max_wp_length = (gdb_byte)((val >> 16) & 0xff);
info.wp_count = (gdb_byte)((val >> 8) & 0xff);
info.bp_count = (gdb_byte)(val & 0xff);
+
+ if (info.wp_count > MAX_WPTS)
+ {
+ warning (_("arm-linux-gdb supports %d hardware watchpoints but target \
+ supports %d"), MAX_WPTS, info.wp_count);
+ info.wp_count = MAX_WPTS;
+ }
+
+ if (info.bp_count > MAX_BPTS)
+ {
+ warning (_("arm-linux-gdb supports %d hardware breakpoints but target \
+ supports %d"), MAX_BPTS, info.bp_count);
+ info.bp_count = MAX_BPTS;
+ }
available = (info.arch != 0);
}
}
@@ -788,8 +807,8 @@ struct arm_linux_hw_breakpoint
arm_hwbp_control_t control;
};
-/* Structure containing arrays of the break and watch points which are have
- active in each thread.
+/* Structure containing arrays of per process hardware break-/watchpoints
+ for caching address and control information.
The Linux ptrace interface to hardware break-/watch-points presents the
values in a vector centred around 0 (which is used fo generic information).
@@ -809,49 +828,114 @@ struct arm_linux_hw_breakpoint
We treat break-/watch-points with their enable bit clear as being deleted.
*/
-typedef struct arm_linux_thread_points
+struct arm_linux_debug_reg_state
{
- /* Thread ID. */
- int tid;
- /* Breakpoints for thread. */
- struct arm_linux_hw_breakpoint *bpts;
- /* Watchpoint for threads. */
- struct arm_linux_hw_breakpoint *wpts;
-} *arm_linux_thread_points_p;
-DEF_VEC_P (arm_linux_thread_points_p);
-
-/* Vector of hardware breakpoints for each thread. */
-VEC(arm_linux_thread_points_p) *arm_threads = NULL;
-
-/* Find the list of hardware break-/watch-points for a thread with id TID.
- If no list exists for TID we return NULL if ALLOC_NEW is 0, otherwise we
- create a new list and return that. */
-static struct arm_linux_thread_points *
-arm_linux_find_breakpoints_by_tid (int tid, int alloc_new)
+ /* Hardware breakpoints for this process. */
+ struct arm_linux_hw_breakpoint bpts[MAX_BPTS];
+ /* Hardware watchpoints for this process. */
+ struct arm_linux_hw_breakpoint wpts[MAX_WPTS];
+};
+
+/* Per-process arch-specific data we want to keep. */
+struct arm_linux_process_info
{
- int i;
- struct arm_linux_thread_points *t;
+ /* Linked list. */
+ struct arm_linux_process_info *next;
+ /* The process identifier. */
+ pid_t pid;
+ /* Hardware break-/watchpoints state information. */
+ struct arm_linux_debug_reg_state state;
- for (i = 0; VEC_iterate (arm_linux_thread_points_p, arm_threads, i, t); ++i)
- {
- if (t->tid == tid)
- return t;
- }
+};
+
+/* 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. */
+ char bpts_changed[MAX_BPTS];
+ char wpts_changed[MAX_WPTS];
+};
+
+static struct arm_linux_process_info *arm_linux_process_list = NULL;
+
+/* Find process data for process PID. */
+
+static struct arm_linux_process_info *
+arm_linux_find_process_pid (pid_t pid)
+{
+ struct arm_linux_process_info *proc;
+
+ for (proc = arm_linux_process_list; proc; proc = proc->next)
+ if (proc->pid == pid)
+ return proc;
+
+ return NULL;
+}
+
+/* Add process data for process PID. Returns newly allocated info
+ object. */
+
+static struct arm_linux_process_info *
+arm_linux_add_process (pid_t pid)
+{
+ struct arm_linux_process_info *proc;
- t = NULL;
+ proc = xcalloc (1, sizeof (*proc));
+ proc->pid = pid;
- if (alloc_new)
+ proc->next = arm_linux_process_list;
+ arm_linux_process_list = proc;
+
+ return proc;
+}
+
+/* Get data specific info for process PID, creating it if necessary.
+ Never returns NULL. */
+
+static struct arm_linux_process_info *
+arm_linux_process_info_get (pid_t pid)
+{
+ struct arm_linux_process_info *proc;
+
+ proc = arm_linux_find_process_pid (pid);
+ if (proc == NULL)
+ proc = arm_linux_add_process (pid);
+
+ return proc;
+}
+
+/* Called whenever GDB is no longer debugging process PID. It deletes
+ data structures that keep track of debug register state. */
+
+static void
+arm_linux_forget_process (pid_t pid)
+{
+ struct arm_linux_process_info *proc, **proc_link;
+
+ proc = arm_linux_process_list;
+ proc_link = &arm_linux_process_list;
+
+ while (proc != NULL)
+ {
+ if (proc->pid == pid)
{
- t = xmalloc (sizeof (struct arm_linux_thread_points));
- t->tid = tid;
- t->bpts = xzalloc (arm_linux_get_hw_breakpoint_count ()
- * sizeof (struct arm_linux_hw_breakpoint));
- t->wpts = xzalloc (arm_linux_get_hw_watchpoint_count ()
- * sizeof (struct arm_linux_hw_breakpoint));
- VEC_safe_push (arm_linux_thread_points_p, arm_threads, t);
+ *proc_link = proc->next;
+
+ xfree (proc);
+ return;
}
- return t;
+ proc_link = &proc->next;
+ proc = *proc_link;
+ }
+}
+
+/* Get hardware break-/watchpoint state for process PID. */
+
+static struct arm_linux_debug_reg_state *
+arm_linux_get_debug_reg_state (pid_t pid)
+{
+ return &arm_linux_process_info_get (pid)->state;
}
/* Initialize an ARM hardware break-/watch-point control register value.
@@ -951,45 +1035,72 @@ arm_linux_hw_breakpoint_equal (const struct arm_linux_hw_breakpoint *p1,
return p1->address == p2->address && p1->control == p2->control;
}
+/* Callback to mark a watch-/breakpoint to be updated in all threads of
+ the current process. */
+
+struct update_registers_data
+{
+ int watch;
+ int index;
+};
+
+static int
+update_registers_callback (struct lwp_info *lwp, void *arg)
+{
+ struct update_registers_data *data = (struct update_registers_data *) arg;
+
+ if (lwp->arch_private == NULL)
+ lwp->arch_private = XCNEW (struct arch_lwp_info);
+
+ /* The actual update is done later just before resuming the lwp,
+ we just mark that the registers need updating. */
+ if (data->watch)
+ lwp->arch_private->wpts_changed[data->index] = 1;
+ else
+ lwp->arch_private->bpts_changed[data->index] = 1;
+
+ /* If the lwp isn't stopped, force it to momentarily pause, so
+ we can update its breakpoint registers. */
+ if (!lwp->stopped)
+ linux_stop_lwp (lwp);
+
+ return 0;
+}
+
/* Insert the hardware breakpoint (WATCHPOINT = 0) or watchpoint (WATCHPOINT
=1) BPT for thread TID. */
static void
arm_linux_insert_hw_breakpoint1 (const struct arm_linux_hw_breakpoint* bpt,
- int tid, int watchpoint)
+ int watchpoint)
{
- struct arm_linux_thread_points *t = arm_linux_find_breakpoints_by_tid (tid, 1);
+ int pid;
+ ptid_t pid_ptid;
gdb_byte count, i;
struct arm_linux_hw_breakpoint* bpts;
- int dir;
+ struct update_registers_data data;
- gdb_assert (t != NULL);
+ pid = ptid_get_pid (inferior_ptid);
+ pid_ptid = pid_to_ptid (pid);
if (watchpoint)
{
count = arm_linux_get_hw_watchpoint_count ();
- bpts = t->wpts;
- dir = -1;
+ bpts = arm_linux_get_debug_reg_state (pid)->wpts;
}
else
{
count = arm_linux_get_hw_breakpoint_count ();
- bpts = t->bpts;
- dir = 1;
+ bpts = arm_linux_get_debug_reg_state (pid)->bpts;
}
for (i = 0; i < count; ++i)
if (!arm_hwbp_control_is_enabled (bpts[i].control))
{
- errno = 0;
- if (ptrace (PTRACE_SETHBPREGS, tid, dir * ((i << 1) + 1),
- &bpt->address) < 0)
- perror_with_name (_("Unexpected error setting breakpoint address"));
- if (ptrace (PTRACE_SETHBPREGS, tid, dir * ((i << 1) + 2),
- &bpt->control) < 0)
- perror_with_name (_("Unexpected error setting breakpoint"));
-
- memcpy (bpts + i, bpt, sizeof (struct arm_linux_hw_breakpoint));
- break;
+ data.watch = watchpoint;
+ data.index = i;
+ bpts[i] = *bpt;
+ iterate_over_lwps (pid_ptid, update_registers_callback, &data);
+ break;
}
gdb_assert (i != count);
@@ -999,37 +1110,36 @@ arm_linux_insert_hw_breakpoint1 (const struct arm_linux_hw_breakpoint* bpt,
(WATCHPOINT = 1) BPT for thread TID. */
static void
arm_linux_remove_hw_breakpoint1 (const struct arm_linux_hw_breakpoint *bpt,
- int tid, int watchpoint)
+ int watchpoint)
{
- struct arm_linux_thread_points *t = arm_linux_find_breakpoints_by_tid (tid, 0);
+ int pid;
gdb_byte count, i;
- struct arm_linux_hw_breakpoint *bpts;
- int dir;
+ ptid_t pid_ptid;
+ struct arm_linux_hw_breakpoint* bpts;
+ struct update_registers_data data;
- gdb_assert (t != NULL);
+ pid = ptid_get_pid (inferior_ptid);
+ pid_ptid = pid_to_ptid (pid);
if (watchpoint)
{
count = arm_linux_get_hw_watchpoint_count ();
- bpts = t->wpts;
- dir = -1;
+ bpts = arm_linux_get_debug_reg_state (pid)->wpts;
}
else
{
count = arm_linux_get_hw_breakpoint_count ();
- bpts = t->bpts;
- dir = 1;
+ bpts = arm_linux_get_debug_reg_state (pid)->bpts;
}
for (i = 0; i < count; ++i)
if (arm_linux_hw_breakpoint_equal (bpt, bpts + i))
{
- errno = 0;
- bpts[i].control = arm_hwbp_control_disable (bpts[i].control);
- if (ptrace (PTRACE_SETHBPREGS, tid, dir * ((i << 1) + 2),
- &bpts[i].control) < 0)
- perror_with_name (_("Unexpected error clearing breakpoint"));
- break;
+ data.watch = watchpoint;
+ data.index = i;
+ bpts[i].control = arm_hwbp_control_disable (bpts[i].control);
+ iterate_over_lwps (pid_ptid, update_registers_callback, &data);
+ break;
}
gdb_assert (i != count);
@@ -1048,8 +1158,8 @@ arm_linux_insert_hw_breakpoint (struct target_ops *self,
return -1;
arm_linux_hw_breakpoint_initialize (gdbarch, bp_tgt, &p);
- ALL_LWPS (lp)
- arm_linux_insert_hw_breakpoint1 (&p, ptid_get_lwp (lp->ptid), 0);
+
+ arm_linux_insert_hw_breakpoint1 (&p, 0);
return 0;
}
@@ -1067,8 +1177,8 @@ arm_linux_remove_hw_breakpoint (struct target_ops *self,
return -1;
arm_linux_hw_breakpoint_initialize (gdbarch, bp_tgt, &p);
- ALL_LWPS (lp)
- arm_linux_remove_hw_breakpoint1 (&p, ptid_get_lwp (lp->ptid), 0);
+
+ arm_linux_remove_hw_breakpoint1 (&p, 0);
return 0;
}
@@ -1120,8 +1230,8 @@ arm_linux_insert_watchpoint (struct target_ops *self,
return -1;
arm_linux_hw_watchpoint_initialize (addr, len, rw, &p);
- ALL_LWPS (lp)
- arm_linux_insert_hw_breakpoint1 (&p, ptid_get_lwp (lp->ptid), 1);
+
+ arm_linux_insert_hw_breakpoint1 (&p, 1);
return 0;
}
@@ -1139,8 +1249,8 @@ arm_linux_remove_watchpoint (struct target_ops *self,
return -1;
arm_linux_hw_watchpoint_initialize (addr, len, rw, &p);
- ALL_LWPS (lp)
- arm_linux_remove_hw_breakpoint1 (&p, ptid_get_lwp (lp->ptid), 1);
+
+ arm_linux_remove_hw_breakpoint1 (&p, 1);
return 0;
}
@@ -1196,63 +1306,100 @@ arm_linux_watchpoint_addr_within_range (struct target_ops *target,
static void
arm_linux_new_thread (struct lwp_info *lp)
{
- int tid = ptid_get_lwp (lp->ptid);
- const struct arm_linux_hwbp_cap *info = arm_linux_get_hwbp_cap ();
+ int i;
+ struct arch_lwp_info *info = XCNEW (struct arch_lwp_info);
+
+ /* Mark that all the hardware breakpoint/watchpoint register pairs
+ for this thread need to be initialized. */
- if (info != NULL)
+ for (i = 0; i < MAX_BPTS; i++)
{
- int i;
- struct arm_linux_thread_points *p;
- struct arm_linux_hw_breakpoint *bpts;
-
- if (VEC_empty (arm_linux_thread_points_p, arm_threads))
- return;
-
- /* Get a list of breakpoints from any thread. */
- p = VEC_last (arm_linux_thread_points_p, arm_threads);
-
- /* Copy that thread's breakpoints and watchpoints to the new thread. */
- for (i = 0; i < info->bp_count; i++)
- if (arm_hwbp_control_is_enabled (p->bpts[i].control))
- arm_linux_insert_hw_breakpoint1 (p->bpts + i, tid, 0);
- for (i = 0; i < info->wp_count; i++)
- if (arm_hwbp_control_is_enabled (p->wpts[i].control))
- arm_linux_insert_hw_breakpoint1 (p->wpts + i, tid, 1);
+ info->bpts_changed[i] = 1;
+ info->wpts_changed[i] = 1;
}
+
+ lp->arch_private = info;
}
-/* Handle thread exit. Tidy up the memory that has been allocated for the
- thread. */
+/* Called when resuming a thread.
+ The hardware debug registers are updated when there is any change. */
+
static void
-arm_linux_thread_exit (struct thread_info *tp, int silent)
+arm_linux_prepare_to_resume (struct lwp_info *lwp)
{
- const struct arm_linux_hwbp_cap *info = arm_linux_get_hwbp_cap ();
+ int pid, i;
+ struct arm_linux_hw_breakpoint *bpts, *wpts;
+ struct arch_lwp_info *arm_lwp_info = lwp->arch_private;
+
+ pid = ptid_get_lwp (lwp->ptid);
+ bpts = arm_linux_get_debug_reg_state (ptid_get_pid (lwp->ptid))->bpts;
+ wpts = arm_linux_get_debug_reg_state (ptid_get_pid (lwp->ptid))->wpts;
+
+ /* NULL means this is the main thread still going through the shell,
+ or, no watchpoint has been set yet. In that case, there's
+ nothing to do. */
+ if (arm_lwp_info == NULL)
+ return;
- if (info != NULL)
- {
- int i;
- int tid = ptid_get_lwp (tp->ptid);
- struct arm_linux_thread_points *t = NULL, *p;
+ for (i = 0; i < arm_linux_get_hw_breakpoint_count (); i++)
+ if (arm_lwp_info->bpts_changed[i])
+ {
+ errno = 0;
+ if (arm_hwbp_control_is_enabled (bpts[i].control))
+ if (ptrace (PTRACE_SETHBPREGS, pid,
+ (PTRACE_TYPE_ARG3) ((i << 1) + 1), &bpts[i].address) < 0)
+ perror_with_name (_("Unexpected error setting breakpoint"));
+
+ if (bpts[i].control != 0)
+ if (ptrace (PTRACE_SETHBPREGS, pid,
+ (PTRACE_TYPE_ARG3) ((i << 1) + 2), &bpts[i].control) < 0)
+ perror_with_name (_("Unexpected error setting breakpoint"));
+
+ arm_lwp_info->bpts_changed[i] = 0;
+ }
- for (i = 0;
- VEC_iterate (arm_linux_thread_points_p, arm_threads, i, p); i++)
- {
- if (p->tid == tid)
- {
- t = p;
- break;
- }
- }
+ for (i = 0; i < arm_linux_get_hw_watchpoint_count (); i++)
+ if (arm_lwp_info->wpts_changed[i])
+ {
+ errno = 0;
+ if (arm_hwbp_control_is_enabled (wpts[i].control))
+ if (ptrace (PTRACE_SETHBPREGS, pid,
+ (PTRACE_TYPE_ARG3) -((i << 1) + 1), &wpts[i].address) < 0)
+ perror_with_name (_("Unexpected error setting watchpoint"));
+
+ if (wpts[i].control != 0)
+ if (ptrace (PTRACE_SETHBPREGS, pid,
+ (PTRACE_TYPE_ARG3) -((i << 1) + 2), &wpts[i].control) < 0)
+ perror_with_name (_("Unexpected error setting watchpoint"));
+
+ arm_lwp_info->wpts_changed[i] = 0;
+ }
+}
+
+/* linux_nat_new_fork hook. */
+
+static void
+arm_linux_new_fork (struct lwp_info *parent, pid_t child_pid)
+{
+ pid_t parent_pid;
+ struct arm_linux_debug_reg_state *parent_state;
+ struct arm_linux_debug_reg_state *child_state;
- if (t == NULL)
- return;
+ /* NULL means no watchpoint has ever been set in the parent. In
+ that case, there's nothing to do. */
+ if (parent->arch_private == NULL)
+ return;
- VEC_unordered_remove (arm_linux_thread_points_p, arm_threads, i);
+ /* GDB core assumes the child inherits the watchpoints/hw
+ breakpoints of the parent, and will remove them all from the
+ forked off process. Copy the debug registers mirrors into the
+ new process so that all breakpoints and watchpoints can be
+ removed together. */
- xfree (t->bpts);
- xfree (t->wpts);
- xfree (t);
- }
+ parent_pid = ptid_get_pid (parent->ptid);
+ parent_state = arm_linux_get_debug_reg_state (parent_pid);
+ child_state = arm_linux_get_debug_reg_state (child_pid);
+ *child_state = *parent_state;
}
void _initialize_arm_linux_nat (void);
@@ -1285,7 +1432,11 @@ _initialize_arm_linux_nat (void)
/* Register the target. */
linux_nat_add_target (t);
- /* Handle thread creation and exit */
- observer_attach_thread_exit (arm_linux_thread_exit);
+ /* Handle thread creation and exit. */
linux_nat_set_new_thread (t, arm_linux_new_thread);
+ linux_nat_set_prepare_to_resume (t, arm_linux_prepare_to_resume);
+
+ /* Handle process creation and exit. */
+ linux_nat_set_new_fork (t, arm_linux_new_fork);
+ linux_nat_set_forget_process (t, arm_linux_forget_process);
}
--
1.8.3.2
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] Support for HWbreak/watchpoint accross fork/vfork on arm-native
2014-03-08 16:29 ` Omair Javaid
@ 2014-03-10 15:13 ` Pedro Alves
0 siblings, 0 replies; 10+ messages in thread
From: Pedro Alves @ 2014-03-10 15:13 UTC (permalink / raw)
To: Omair Javaid; +Cc: Yao Qi, gdb-patches
On 03/08/2014 04:29 PM, Omair Javaid wrote:
>> >
> Sorry for the delay. Somehow this patch update seems to be sitting in my outgoing email for a while without being sent.
>
> OK to commit?
Yes, thanks.
(I noticed a typo in the commit log / subject line: "accross")
--
Pedro Alves
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2014-03-10 15:13 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-01-30 22:12 [PATCH] Support for HWbreak/watchpoint accross fork/vfork on arm-native Omair Javaid
2014-01-30 22:23 ` Omair Javaid
2014-02-07 15:32 ` Pedro Alves
2014-02-08 0:49 ` Yao Qi
2014-02-12 15:40 ` Omair Javaid
2014-02-13 5:05 ` Yao Qi
2014-02-13 18:40 ` Pedro Alves
2014-02-14 0:06 ` Omair Javaid
2014-03-08 16:29 ` Omair Javaid
2014-03-10 15:13 ` Pedro Alves
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).