[6/8] Make reinsert_breakpoint thread specific
Commit Message
This patch makes reinsert_breakpoint thread specific, which means we
insert and remove reinsert_breakpoint breakpoints for a specific
thread. This motivation of this change is that I'll use
reinsert_breakpoint for vCont;s on software single step target, so that
GDBserver may insert one reinsert_breakpoint for step-over, and the
other one for vCont;s. After the operation of one thread is finished,
GDBserver must remove reinsert_breakpoint for that thread only.
On the other hand, reinsert_breakpoint is used for step-over nowadays,
which is an operation to one thread in one go. In other words,
reinsert_breakpoint is globally thread specific, but in an implicit way.
It is natural to make it explicitly thread specific.
gdb/gdbserver:
2016-05-20 Yao Qi <yao.qi@linaro.org>
* mem-break.c (struct reinsert_breakpoint) <id>: New field.
(set_reinsert_breakpoint): New parameter id. Callers updated.
(delete_reinsert_breakpoints): Likewise.
(has_reinsert_breakpoints): Change parameter to thread. Callers
updated.
(clone_one_breakpoint): Likewise.
---
gdb/gdbserver/linux-low.c | 16 ++++++++--------
gdb/gdbserver/mem-break.c | 34 +++++++++++++++++++++++++---------
gdb/gdbserver/mem-break.h | 13 +++++++------
3 files changed, 40 insertions(+), 23 deletions(-)
Comments
On 05/20/2016 04:12 PM, Yao Qi wrote:
> This patch makes reinsert_breakpoint thread specific, which means we
> insert and remove reinsert_breakpoint breakpoints for a specific
> thread. This motivation of this change is that I'll use
> reinsert_breakpoint for vCont;s on software single step target, so that
> GDBserver may insert one reinsert_breakpoint for step-over, and the
> other one for vCont;s.
Here I don't understand what you mean by "the other one".
> After the operation of one thread is finished,
> GDBserver must remove reinsert_breakpoint for that thread only.
>
> On the other hand, reinsert_breakpoint is used for step-over nowadays,
> which is an operation to one thread in one go.
"in one go" ? WDYM?
> In other words,
> reinsert_breakpoint is globally thread specific, but in an implicit way.
Right.
> index 4e45c0e..5d699c3 100644
> --- a/gdb/gdbserver/mem-break.c
> +++ b/gdb/gdbserver/mem-break.c
> @@ -211,6 +211,9 @@ struct other_breakpoint
> struct reinsert_breakpoint
> {
> struct breakpoint base;
> +
> + /* Thread the reinsert breakpoint belongs to. */
> + ptid_t id;
It's much more usual to call this "ptid". From the ChangeLog,
I first though you were adding a "breakpoint number" kind of id.
> };
>
> /* Return the breakpoint size from its kind. */
> @@ -1476,25 +1479,32 @@ gdb_breakpoint_here (CORE_ADDR where)
> }
>
> void
> -set_reinsert_breakpoint (CORE_ADDR stop_at)
> +set_reinsert_breakpoint (CORE_ADDR stop_at, ptid_t id)
Ditto. (Likewise other places.)
Otherwise LGTM.
Thanks,
Pedro Alves
@@ -578,11 +578,11 @@ handle_extended_wait (struct lwp_info **orig_event_lwp, int wstat)
to access its memory without stopping all other threads
from other processes. */
current_thread = child_thr;
- delete_reinsert_breakpoints ();
+ delete_reinsert_breakpoints (ptid);
current_thread = saved_thread;
- gdb_assert (has_reinsert_breakpoints (parent_proc));
- gdb_assert (!has_reinsert_breakpoints (child_proc));
+ gdb_assert (has_reinsert_breakpoints (event_thr));
+ gdb_assert (!has_reinsert_breakpoints (child_thr));
}
/* Report the event. */
@@ -4097,7 +4097,7 @@ install_software_single_step_breakpoints (struct lwp_info *lwp)
next_pcs = (*the_low_target.get_next_pcs) (regcache);
for (i = 0; VEC_iterate (CORE_ADDR, next_pcs, i, pc); ++i)
- set_reinsert_breakpoint (pc);
+ set_reinsert_breakpoint (pc, current_ptid);
do_cleanups (old_chain);
}
@@ -4238,7 +4238,7 @@ linux_resume_one_lwp_throw (struct lwp_info *lwp,
else
{
gdb_assert (step == 0);
- gdb_assert (has_reinsert_breakpoints (proc));
+ gdb_assert (has_reinsert_breakpoints (thread));
}
}
@@ -4728,8 +4728,8 @@ finish_step_over (struct lwp_info *lwp)
threads but LWP stopped while doing that. */
if (!can_hardware_single_step ())
{
- gdb_assert (has_reinsert_breakpoints (current_process ()));
- delete_reinsert_breakpoints ();
+ gdb_assert (has_reinsert_breakpoints (get_lwp_thread (lwp)));
+ delete_reinsert_breakpoints (ptid_of_lwp (lwp));
}
step_over_bkpt = null_ptid;
@@ -5067,7 +5067,7 @@ proceed_one_lwp (struct inferior_list_entry *entry, void *except)
else
{
/* GDBserver must insert reinsert breakpoint for step-over. */
- gdb_assert (has_reinsert_breakpoints (get_thread_process (thread)));
+ gdb_assert (has_reinsert_breakpoints (thread));
}
}
@@ -211,6 +211,9 @@ struct other_breakpoint
struct reinsert_breakpoint
{
struct breakpoint base;
+
+ /* Thread the reinsert breakpoint belongs to. */
+ ptid_t id;
};
/* Return the breakpoint size from its kind. */
@@ -1476,25 +1479,32 @@ gdb_breakpoint_here (CORE_ADDR where)
}
void
-set_reinsert_breakpoint (CORE_ADDR stop_at)
+set_reinsert_breakpoint (CORE_ADDR stop_at, ptid_t id)
{
- struct breakpoint *bp;
+ struct reinsert_breakpoint *bp;
- bp = set_breakpoint_at_1 (reinsert_breakpoint, stop_at, NULL);
+ gdb_assert (ptid_get_pid (current_ptid) == ptid_get_pid (id));
+
+ bp = (struct reinsert_breakpoint *) set_breakpoint_at_1 (reinsert_breakpoint,
+ stop_at, NULL);
+ bp->id = id;
}
void
-delete_reinsert_breakpoints (void)
+delete_reinsert_breakpoints (ptid_t id)
{
struct process_info *proc = current_process ();
struct breakpoint *bp, **bp_link;
+ gdb_assert (ptid_get_pid (current_ptid) == ptid_get_pid (id));
+
bp = proc->breakpoints;
bp_link = &proc->breakpoints;
while (bp)
{
- if (bp->type == reinsert_breakpoint)
+ if (bp->type == reinsert_breakpoint
+ && ptid_equal (((struct reinsert_breakpoint *) bp)->id, id))
{
*bp_link = bp->next;
release_breakpoint (proc, bp);
@@ -1622,8 +1632,9 @@ reinsert_breakpoints_at (CORE_ADDR pc)
}
int
-has_reinsert_breakpoints (struct process_info *proc)
+has_reinsert_breakpoints (struct thread_info *thread)
{
+ struct process_info *proc = get_thread_process (thread);
struct breakpoint *bp, **bp_link;
bp = proc->breakpoints;
@@ -1631,7 +1642,9 @@ has_reinsert_breakpoints (struct process_info *proc)
while (bp)
{
- if (bp->type == reinsert_breakpoint)
+ if (bp->type == reinsert_breakpoint
+ && ptid_equal (((struct reinsert_breakpoint *) bp)->id,
+ ptid_of (thread)))
return 1;
else
{
@@ -2070,7 +2083,7 @@ clone_agent_expr (const struct agent_expr *src_ax)
/* Deep-copy the contents of one breakpoint to another. */
static struct breakpoint *
-clone_one_breakpoint (const struct breakpoint *src)
+clone_one_breakpoint (const struct breakpoint *src, ptid_t id)
{
struct breakpoint *dest;
struct raw_breakpoint *dest_raw;
@@ -2131,6 +2144,9 @@ clone_one_breakpoint (const struct breakpoint *src)
= XCNEW (struct reinsert_breakpoint);
dest = (struct breakpoint *) reinsert_dest;
+ /* Since reinsert breakpoint is thread specific, don't copy
+ thread id from SRC, use ID instead. */
+ reinsert_dest->id = id;
}
else
gdb_assert_not_reached ("unhandled breakpoint type");
@@ -2157,7 +2173,7 @@ clone_all_breakpoints (struct thread_info *child_thread,
for (bp = parent_proc->breakpoints; bp != NULL; bp = bp->next)
{
- new_bkpt = clone_one_breakpoint (bp);
+ new_bkpt = clone_one_breakpoint (bp, ptid_of (child_thread));
APPEND_TO_LIST (new_list, new_bkpt, bkpt_tail);
APPEND_TO_LIST (new_raw_list, new_bkpt->raw, raw_bkpt_tail);
}
@@ -152,22 +152,23 @@ struct breakpoint *set_breakpoint_at (CORE_ADDR where,
int delete_breakpoint (struct breakpoint *bkpt);
-/* Set a reinsert breakpoint at STOP_AT. */
+/* Set a reinsert breakpoint at STOP_AT for thread represented by
+ ID. */
-void set_reinsert_breakpoint (CORE_ADDR stop_at);
+void set_reinsert_breakpoint (CORE_ADDR stop_at, ptid_t id);
-/* Delete all reinsert breakpoints. */
+/* Delete all reinsert breakpoints of thread represented by ID. */
-void delete_reinsert_breakpoints (void);
+void delete_reinsert_breakpoints (ptid_t id);
/* Reinsert breakpoints at WHERE (and change their status to
inserted). */
void reinsert_breakpoints_at (CORE_ADDR where);
-/* Process PROC has reinsert breakpoints or not. */
+/* The THREAD has reinsert breakpoints or not. */
-int has_reinsert_breakpoints (struct process_info *proc);
+int has_reinsert_breakpoints (struct thread_info *thread);
/* Uninsert breakpoints at WHERE (and change their status to
uninserted). This still leaves the breakpoints in the table. */