* [PATCH 1/8] Switch to current thread before finish_step_over
2016-05-20 15:12 [PATCH 0/8] Use reinsert breakpoint for vCont;s Yao Qi
@ 2016-05-20 15:13 ` Yao Qi
2016-05-24 15:59 ` Pedro Alves
2016-05-20 15:13 ` [PATCH 8/8] [GDBserver] Support vCont s and S actions with software single step Yao Qi
` (7 subsequent siblings)
8 siblings, 1 reply; 20+ messages in thread
From: Yao Qi @ 2016-05-20 15:13 UTC (permalink / raw)
To: gdb-patches
This patch adds some sanity check that reinsert breakpoints must be
there when doing step-over on software single step target. The check
triggers an assert when running forking-threads-plus-breakpoint.exp
on arm-linux target,
gdb/gdbserver/linux-low.c:4714: A problem internal to GDBserver has been detected.^M
int finish_step_over(lwp_info*): Assertion `has_reinsert_breakpoints ()' failed.
the error happens when GDBserver has already resumed a thread of
process A for step-over (and wait for it hitting reinsert breakpoint),
but receives detach request for process B from GDB, which is shown in
the backtrace below,
(gdb) bt
#2 0x000228aa in finish_step_over (lwp=0x12bbd98) at /home/yao/SourceCode/gnu/gdb/git/gdb/gdbserver/linux-low.c:4703
#3 0x00025a50 in finish_step_over (lwp=0x12bbd98) at /home/yao/SourceCode/gnu/gdb/git/gdb/gdbserver/linux-low.c:4749
#4 complete_ongoing_step_over () at /home/yao/SourceCode/gnu/gdb/git/gdb/gdbserver/linux-low.c:4760
#5 linux_detach (pid=25228) at /home/yao/SourceCode/gnu/gdb/git/gdb/gdbserver/linux-low.c:1503
#6 0x00012bae in process_serial_event () at /home/yao/SourceCode/gnu/gdb/git/gdb/gdbserver/server.c:3974
#7 handle_serial_event (err=<optimized out>, client_data=<optimized out>) at /home/yao/SourceCode/gnu/gdb/git/gdb/gdbserver/server.c:4347
#8 0x00016d68 in handle_file_event (event_file_desc=<optimized out>) at /home/yao/SourceCode/gnu/gdb/git/gdb/gdbserver/event-loop.c:429
#9 0x000173ea in process_event () at /home/yao/SourceCode/gnu/gdb/git/gdb/gdbserver/event-loop.c:184
#10 start_event_loop () at /home/yao/SourceCode/gnu/gdb/git/gdb/gdbserver/event-loop.c:547
#11 0x0000aa2c in captured_main (argv=<optimized out>, argc=<optimized out>) at /home/yao/SourceCode/gnu/gdb/git/gdb/gdbserver/server.c:3719
#12 main (argc=<optimized out>, argv=<optimized out>) at /home/yao/SourceCode/gnu/gdb/git/gdb/gdbserver/server.c:3804
the sanity check tries to find the reinsert breakpoint from process B,
but nothing is found. It is wrong, we need to search in process A,
since we started step-over of a thread of process A.
(gdb) p lwp->thread->entry.id
$3 = {pid = 25120, lwp = 25131, tid = 0}
(gdb) p current_thread->entry.id
$4 = {pid = 25228, lwp = 25228, tid = 0}
This patch switched current_thread to the step we are doing step-over
in complete_ongoing_step_over.
gdb/gdbserver:
2016-05-19 Yao Qi <yao.qi@linaro.org>
* linux-low.c (linux_resume_one_lwp_throw): Call
has_reinsert_breakpoints.
(finish_step_over): Likewise.
(proceed_one_lwp): Likewise.
* mem-break.c (has_reinsert_breakpoints): New function.
* mem-break.h (has_reinsert_breakpoints): Declare.
---
gdb/gdbserver/linux-low.c | 30 +++++++++++++++++++++++++-----
gdb/gdbserver/mem-break.c | 22 ++++++++++++++++++++++
gdb/gdbserver/mem-break.h | 4 ++++
3 files changed, 51 insertions(+), 5 deletions(-)
diff --git a/gdb/gdbserver/linux-low.c b/gdb/gdbserver/linux-low.c
index 81134b0..8e8f710 100644
--- a/gdb/gdbserver/linux-low.c
+++ b/gdb/gdbserver/linux-low.c
@@ -4218,6 +4218,11 @@ linux_resume_one_lwp_throw (struct lwp_info *lwp,
step = 1;
}
+ else
+ {
+ gdb_assert (step == 0);
+ gdb_assert (has_reinsert_breakpoints (proc));
+ }
}
if (fast_tp_collecting == 1)
@@ -4705,7 +4710,10 @@ finish_step_over (struct lwp_info *lwp)
because we were stepping over a breakpoint, and we hold all
threads but LWP stopped while doing that. */
if (!can_hardware_single_step ())
- delete_reinsert_breakpoints ();
+ {
+ gdb_assert (has_reinsert_breakpoints (current_process ()));
+ delete_reinsert_breakpoints ();
+ }
step_over_bkpt = null_ptid;
return 1;
@@ -4741,7 +4749,13 @@ complete_ongoing_step_over (void)
lwp = find_lwp_pid (step_over_bkpt);
if (lwp != NULL)
- finish_step_over (lwp);
+ {
+ struct thread_info *saved_thread = current_thread;
+
+ current_thread = get_lwp_thread (lwp);
+ finish_step_over (lwp);
+ current_thread = saved_thread;
+ }
step_over_bkpt = null_ptid;
unsuspend_all_lwps (lwp);
}
@@ -5017,6 +5031,7 @@ proceed_one_lwp (struct inferior_list_entry *entry, void *except)
send_sigstop (lwp);
}
+ step = 0;
if (thread->last_resume_kind == resume_step)
{
if (debug_threads)
@@ -5029,10 +5044,15 @@ proceed_one_lwp (struct inferior_list_entry *entry, void *except)
if (debug_threads)
debug_printf (" stepping LWP %ld, reinsert set\n",
lwpid_of (thread));
- step = 1;
+
+ if (can_hardware_single_step ())
+ step = 1;
+ else
+ {
+ /* GDBserver must insert reinsert breakpoint for step-over. */
+ gdb_assert (has_reinsert_breakpoints (get_thread_process (thread)));
+ }
}
- else
- step = 0;
linux_resume_one_lwp (lwp, step, 0, NULL);
return 0;
diff --git a/gdb/gdbserver/mem-break.c b/gdb/gdbserver/mem-break.c
index 363d7ca..3313459 100644
--- a/gdb/gdbserver/mem-break.c
+++ b/gdb/gdbserver/mem-break.c
@@ -1553,6 +1553,28 @@ reinsert_breakpoints_at (CORE_ADDR pc)
}
}
+int
+has_reinsert_breakpoints (struct process_info *proc)
+{
+ struct breakpoint *bp, **bp_link;
+
+ bp = proc->breakpoints;
+ bp_link = &proc->breakpoints;
+
+ while (bp)
+ {
+ if (bp->type == reinsert_breakpoint)
+ return 1;
+ else
+ {
+ bp_link = &bp->next;
+ bp = *bp_link;
+ }
+ }
+
+ return 0;
+}
+
void
reinsert_all_breakpoints (void)
{
diff --git a/gdb/gdbserver/mem-break.h b/gdb/gdbserver/mem-break.h
index 4d9a76c..b84dc1e 100644
--- a/gdb/gdbserver/mem-break.h
+++ b/gdb/gdbserver/mem-break.h
@@ -163,6 +163,10 @@ void delete_reinsert_breakpoints (void);
void reinsert_breakpoints_at (CORE_ADDR where);
+/* Process PROC has reinsert breakpoints or not. */
+
+int has_reinsert_breakpoints (struct process_info *proc);
+
/* Uninsert breakpoints at WHERE (and change their status to
uninserted). This still leaves the breakpoints in the table. */
--
1.9.1
^ permalink raw reply [flat|nested] 20+ messages in thread* Re: [PATCH 1/8] Switch to current thread before finish_step_over
2016-05-20 15:13 ` [PATCH 1/8] Switch to current thread before finish_step_over Yao Qi
@ 2016-05-24 15:59 ` Pedro Alves
0 siblings, 0 replies; 20+ messages in thread
From: Pedro Alves @ 2016-05-24 15:59 UTC (permalink / raw)
To: Yao Qi, gdb-patches
On 05/20/2016 04:12 PM, Yao Qi wrote:
> This patch switched current_thread to the step we are doing step-over
s/to the step/to the thread/
> @@ -4741,7 +4749,13 @@ complete_ongoing_step_over (void)
>
> lwp = find_lwp_pid (step_over_bkpt);
> if (lwp != NULL)
> - finish_step_over (lwp);
> + {
> + struct thread_info *saved_thread = current_thread;
> +
> + current_thread = get_lwp_thread (lwp);
> + finish_step_over (lwp);
> + current_thread = saved_thread;
> + }
So the pattern should be that if a function takes a thread/lwp
parameter, then it should work with that, instead of relying on
the global state. So I think that it should be finish_step_over
itself that switches the current thread, because it is an implementation
detail of finish_step_over that is calls into functions that rely
on the global current thread. Putting the save/restore there
should make it more obvious why it's needed.
Thanks,
Pedro Alves
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH 8/8] [GDBserver] Support vCont s and S actions with software single step
2016-05-20 15:12 [PATCH 0/8] Use reinsert breakpoint for vCont;s Yao Qi
2016-05-20 15:13 ` [PATCH 1/8] Switch to current thread before finish_step_over Yao Qi
@ 2016-05-20 15:13 ` Yao Qi
2016-05-24 17:41 ` Pedro Alves
2016-05-20 15:13 ` [PATCH 6/8] Make reinsert_breakpoint thread specific Yao Qi
` (6 subsequent siblings)
8 siblings, 1 reply; 20+ messages in thread
From: Yao Qi @ 2016-05-20 15:13 UTC (permalink / raw)
To: gdb-patches
GDBserver with software single step should be able to claim supporting
vCont s and S actions, so that GDB knows the remote target can do
single step. It doesn't matter to GDB that the single step in the
remote target is done via hardware or software.
gdb/gdbserver:
2016-05-20 Yao Qi <yao.qi@linaro.org>
* server.c (handle_v_requests): Support s and S actions
if target_supports_software_single_step return true.
---
gdb/gdbserver/server.c | 13 ++++++++-----
1 file changed, 8 insertions(+), 5 deletions(-)
diff --git a/gdb/gdbserver/server.c b/gdb/gdbserver/server.c
index 18517bc..6d6cb09 100644
--- a/gdb/gdbserver/server.c
+++ b/gdb/gdbserver/server.c
@@ -2958,12 +2958,15 @@ handle_v_requests (char *own_buf, int packet_len, int *new_packet_len)
{
strcpy (own_buf, "vCont;c;C;t");
- if (target_supports_hardware_single_step () || !vCont_supported)
+ if (target_supports_hardware_single_step ()
+ || target_supports_software_single_step ()
+ || !vCont_supported)
{
- /* If target supports hardware single step, add actions s
- and S to the list of supported actions. On the other
- hand, if GDB doesn't request the supported vCont actions
- in qSupported packet, add s and S to the list too. */
+ /* If target supports single step either by hardware or by
+ software, add actions s and S to the list of supported
+ actions. On the other hand, if GDB doesn't request the
+ supported vCont actions in qSupported packet, add s and
+ S to the list too. */
own_buf = own_buf + strlen (own_buf);
strcpy (own_buf, ";s;S");
}
--
1.9.1
^ permalink raw reply [flat|nested] 20+ messages in thread* Re: [PATCH 8/8] [GDBserver] Support vCont s and S actions with software single step
2016-05-20 15:13 ` [PATCH 8/8] [GDBserver] Support vCont s and S actions with software single step Yao Qi
@ 2016-05-24 17:41 ` Pedro Alves
0 siblings, 0 replies; 20+ messages in thread
From: Pedro Alves @ 2016-05-24 17:41 UTC (permalink / raw)
To: Yao Qi, gdb-patches
On 05/20/2016 04:12 PM, Yao Qi wrote:
> GDBserver with software single step should be able to claim supporting
> vCont s and S actions, so that GDB knows the remote target can do
> single step. It doesn't matter to GDB that the single step in the
> remote target is done via hardware or software.
>
> gdb/gdbserver:
>
> 2016-05-20 Yao Qi <yao.qi@linaro.org>
>
> * server.c (handle_v_requests): Support s and S actions
> if target_supports_software_single_step return true.
LGTM, once the rest of the series is in.
Thanks,
Pedro Alves
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH 6/8] Make reinsert_breakpoint thread specific
2016-05-20 15:12 [PATCH 0/8] Use reinsert breakpoint for vCont;s Yao Qi
2016-05-20 15:13 ` [PATCH 1/8] Switch to current thread before finish_step_over Yao Qi
2016-05-20 15:13 ` [PATCH 8/8] [GDBserver] Support vCont s and S actions with software single step Yao Qi
@ 2016-05-20 15:13 ` Yao Qi
2016-05-24 16:39 ` Pedro Alves
2016-05-20 15:13 ` [PATCH 5/8] Refactor clone_all_breakpoints Yao Qi
` (5 subsequent siblings)
8 siblings, 1 reply; 20+ messages in thread
From: Yao Qi @ 2016-05-20 15:13 UTC (permalink / raw)
To: gdb-patches
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(-)
diff --git a/gdb/gdbserver/linux-low.c b/gdb/gdbserver/linux-low.c
index 539d787..d5f7097 100644
--- a/gdb/gdbserver/linux-low.c
+++ b/gdb/gdbserver/linux-low.c
@@ -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));
}
}
diff --git a/gdb/gdbserver/mem-break.c b/gdb/gdbserver/mem-break.c
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;
};
/* 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);
}
diff --git a/gdb/gdbserver/mem-break.h b/gdb/gdbserver/mem-break.h
index 757399a..38d5911 100644
--- a/gdb/gdbserver/mem-break.h
+++ b/gdb/gdbserver/mem-break.h
@@ -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. */
--
1.9.1
^ permalink raw reply [flat|nested] 20+ messages in thread* Re: [PATCH 6/8] Make reinsert_breakpoint thread specific
2016-05-20 15:13 ` [PATCH 6/8] Make reinsert_breakpoint thread specific Yao Qi
@ 2016-05-24 16:39 ` Pedro Alves
0 siblings, 0 replies; 20+ messages in thread
From: Pedro Alves @ 2016-05-24 16:39 UTC (permalink / raw)
To: Yao Qi, gdb-patches
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
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH 5/8] Refactor clone_all_breakpoints
2016-05-20 15:12 [PATCH 0/8] Use reinsert breakpoint for vCont;s Yao Qi
` (2 preceding siblings ...)
2016-05-20 15:13 ` [PATCH 6/8] Make reinsert_breakpoint thread specific Yao Qi
@ 2016-05-20 15:13 ` Yao Qi
2016-05-24 16:27 ` Pedro Alves
2016-05-20 15:13 ` [PATCH 2/8] Delete reinsert breakpoints from forked child Yao Qi
` (4 subsequent siblings)
8 siblings, 1 reply; 20+ messages in thread
From: Yao Qi @ 2016-05-20 15:13 UTC (permalink / raw)
To: gdb-patches
This patch is to change the interface of clone_all_breakpoints, from
lists of breakpoints and raw_breakpoints to child thread and parent
process. I choose child thread to pass because we need the ptid of
the child thread in the following patch.
gdb/gdbserver:
2016-05-20 Yao Qi <yao.qi@linaro.org>
* mem-break.c (clone_all_breakpoints): Remove all parameters.
Add new parameters child_thread and parent_proc. Callers
updated.
* mem-break.h (clone_all_breakpoints): Update declaration.
---
gdb/gdbserver/linux-low.c | 4 +---
gdb/gdbserver/mem-break.c | 14 +++++++-------
gdb/gdbserver/mem-break.h | 8 ++++----
3 files changed, 12 insertions(+), 14 deletions(-)
diff --git a/gdb/gdbserver/linux-low.c b/gdb/gdbserver/linux-low.c
index a63cc7a..539d787 100644
--- a/gdb/gdbserver/linux-low.c
+++ b/gdb/gdbserver/linux-low.c
@@ -542,9 +542,7 @@ handle_extended_wait (struct lwp_info **orig_event_lwp, int wstat)
parent_proc = get_thread_process (event_thr);
child_proc->attached = parent_proc->attached;
- clone_all_breakpoints (&child_proc->breakpoints,
- &child_proc->raw_breakpoints,
- parent_proc->breakpoints);
+ clone_all_breakpoints (child_thr, parent_proc);
tdesc = XNEW (struct target_desc);
copy_target_description (tdesc, parent_proc->tdesc);
diff --git a/gdb/gdbserver/mem-break.c b/gdb/gdbserver/mem-break.c
index 1fe1d20..4e45c0e 100644
--- a/gdb/gdbserver/mem-break.c
+++ b/gdb/gdbserver/mem-break.c
@@ -2141,21 +2141,21 @@ clone_one_breakpoint (const struct breakpoint *src)
return dest;
}
-/* Create a new breakpoint list NEW_LIST that is a copy of the
- list starting at SRC_LIST. Create the corresponding new
- raw_breakpoint list NEW_RAW_LIST as well. */
+/* See mem-break.h. */
void
-clone_all_breakpoints (struct breakpoint **new_list,
- struct raw_breakpoint **new_raw_list,
- const struct breakpoint *src_list)
+clone_all_breakpoints (struct thread_info *child_thread,
+ struct process_info *parent_proc)
{
const struct breakpoint *bp;
struct breakpoint *new_bkpt;
struct breakpoint *bkpt_tail = NULL;
struct raw_breakpoint *raw_bkpt_tail = NULL;
+ struct process_info *child_proc = get_thread_process (child_thread);
+ struct breakpoint **new_list = &child_proc->breakpoints;
+ struct raw_breakpoint **new_raw_list = &child_proc->raw_breakpoints;
- for (bp = src_list; bp != NULL; bp = bp->next)
+ for (bp = parent_proc->breakpoints; bp != NULL; bp = bp->next)
{
new_bkpt = clone_one_breakpoint (bp);
APPEND_TO_LIST (new_list, new_bkpt, bkpt_tail);
diff --git a/gdb/gdbserver/mem-break.h b/gdb/gdbserver/mem-break.h
index 280e4f8..757399a 100644
--- a/gdb/gdbserver/mem-break.h
+++ b/gdb/gdbserver/mem-break.h
@@ -258,10 +258,10 @@ int insert_memory_breakpoint (struct raw_breakpoint *bp);
int remove_memory_breakpoint (struct raw_breakpoint *bp);
-/* Create a new breakpoint list NEW_BKPT_LIST that is a copy of SRC. */
+/* Create a new breakpoint list in CHILD_THREAD's process that is a
+ copy of breakpoint list in PARENT_PROC. */
-void clone_all_breakpoints (struct breakpoint **new_bkpt_list,
- struct raw_breakpoint **new_raw_bkpt_list,
- const struct breakpoint *src);
+void clone_all_breakpoints (struct thread_info *child_thread,
+ struct process_info *parent_proc);
#endif /* MEM_BREAK_H */
--
1.9.1
^ permalink raw reply [flat|nested] 20+ messages in thread* [PATCH 2/8] Delete reinsert breakpoints from forked child
2016-05-20 15:12 [PATCH 0/8] Use reinsert breakpoint for vCont;s Yao Qi
` (3 preceding siblings ...)
2016-05-20 15:13 ` [PATCH 5/8] Refactor clone_all_breakpoints Yao Qi
@ 2016-05-20 15:13 ` Yao Qi
2016-05-24 16:06 ` Pedro Alves
2016-05-20 15:13 ` [PATCH 4/8] Create sub classes of 'struct breakpoint' Yao Qi
` (3 subsequent siblings)
8 siblings, 1 reply; 20+ messages in thread
From: Yao Qi @ 2016-05-20 15:13 UTC (permalink / raw)
To: gdb-patches
When a thread is stepping over a syscall instruction with software
single step, GDBserver inserts reinsert breakpoints at the next pcs.
If the syscall call is fork, the forked child has reinsert breakpoint
in its space, and GDBserver clones parent's breakpoint list to child's.
When GDBserver resumes the child, it hits the reinsert breakpoint. Since
it is a GDBserver internal breakpoint, GDBserver will do step-over again,
and remove this reinsert breakpoint on step-over is finished, so the
reinsert breakpoint left in the child process doesn't make any trouble.
However, GDBserver still need to remove the reinsert breakpoints from
the child, in order to avoid the unnecessary breakpoint hit and
step-over.
gdb/gdbserver:
2016-05-20 Yao Qi <yao.qi@linaro.org>
* linux-low.c (handle_extended_wait): If the parent is doing
step-over, remove the reinsert breakpoints from the child.
---
gdb/gdbserver/linux-low.c | 19 +++++++++++++++++++
1 file changed, 19 insertions(+)
diff --git a/gdb/gdbserver/linux-low.c b/gdb/gdbserver/linux-low.c
index 8e8f710..a63cc7a 100644
--- a/gdb/gdbserver/linux-low.c
+++ b/gdb/gdbserver/linux-low.c
@@ -568,6 +568,25 @@ handle_extended_wait (struct lwp_info **orig_event_lwp, int wstat)
event_lwp->status_pending_p = 1;
event_lwp->status_pending = wstat;
+ /* If the parent thread is doing step-over with reinsert
+ breakpoints, the reinsert breakpoints are still in child's
+ process space and cloned to its breakpoint list from the
+ parent's. Remove them. */
+ if (event_lwp->bp_reinsert != 0 && can_software_single_step ())
+ {
+ struct thread_info *saved_thread = current_thread;
+
+ /* The child process is forked and stopped, so it is safe
+ to access its memory without stopping all other threads
+ from other processes. */
+ current_thread = child_thr;
+ delete_reinsert_breakpoints ();
+ current_thread = saved_thread;
+
+ gdb_assert (has_reinsert_breakpoints (parent_proc));
+ gdb_assert (!has_reinsert_breakpoints (child_proc));
+ }
+
/* Report the event. */
return 0;
}
--
1.9.1
^ permalink raw reply [flat|nested] 20+ messages in thread* [PATCH 4/8] Create sub classes of 'struct breakpoint'
2016-05-20 15:12 [PATCH 0/8] Use reinsert breakpoint for vCont;s Yao Qi
` (4 preceding siblings ...)
2016-05-20 15:13 ` [PATCH 2/8] Delete reinsert breakpoints from forked child Yao Qi
@ 2016-05-20 15:13 ` Yao Qi
2016-05-24 16:23 ` Pedro Alves
2016-05-20 15:13 ` [PATCH 7/8] Use reinsert_breakpoint for vCont;s Yao Qi
` (2 subsequent siblings)
8 siblings, 1 reply; 20+ messages in thread
From: Yao Qi @ 2016-05-20 15:13 UTC (permalink / raw)
To: gdb-patches
Nowadays, there are three types of breakpoint in GDBserver,
- gdb breakpoints,
- reinsert breakpoints, used for software single step,
- other breakpoints, used for tracepoint,
but we only have one 'struct breakpoint' for all of them. Some fields
are only useful to one type of breakpoint. For example, cond_list
and command_list are only used by gdb breakpoints, while handler is
only used by other breakpoints.
This patch changes 'struct breakpoint' to a base class, which has fields
needed by all breakpoint types, also add three sub-classes to
'struct breakpoint' to these three types of breakpoints.
gdb/gdbserver:
2016-05-20 Yao Qi <yao.qi@linaro.org>
* mem-break.c (struct breakpoint) <cond_list>: Remove.
<command_list, handler>: Remove.
(struct gdb_breakpoint): New.
(struct other_breakpoint): New.
(struct reinsert_breakpoint): New.
(is_gdb_breakpoint): New function.
(any_persistent_commands): Update command_list if
is_gdb_breakpoint returns true.
(set_breakpoint): Create breakpoints according to their types.
(find_gdb_breakpoint): Return 'struct gdb_breakpoint *'.
(set_gdb_breakpoint_1): Likewise.
(set_gdb_breakpoint): Likewise.
(clear_breakpoint_conditions): Change parameter type to
'struct gdb_breakpoint *'.
(clear_breakpoint_commands): Likewise.
(clear_breakpoint_conditions_and_commands): Likewise.
(add_condition_to_breakpoint): Likewise.
(add_breakpoint_condition): Likewise.
(add_commands_to_breakpoint): Likewise.
(check_breakpoints): Check other_breakpoint.
(clone_one_breakpoint): Clone breakpopint according to its type.
* mem-break.h (struct gdb_breakpoint): Declare.
(set_gdb_breakpoint): Update declaration.
(clear_breakpoint_conditions_and_commands): Likewise.
(add_breakpoint_condition): Likewise.
(add_breakpoint_commands): Likewise.
* server.c (process_point_options): Change parameter type to
'struct gdb_breakpoint *'.
---
gdb/gdbserver/mem-break.c | 209 +++++++++++++++++++++++++++++++++-------------
gdb/gdbserver/mem-break.h | 11 +--
gdb/gdbserver/server.c | 4 +-
3 files changed, 157 insertions(+), 67 deletions(-)
diff --git a/gdb/gdbserver/mem-break.c b/gdb/gdbserver/mem-break.c
index 3e3be8b..1fe1d20 100644
--- a/gdb/gdbserver/mem-break.c
+++ b/gdb/gdbserver/mem-break.c
@@ -173,6 +173,17 @@ struct breakpoint
/* The breakpoint's type. */
enum bkpt_type type;
+ /* Link to this breakpoint's raw breakpoint. This is always
+ non-NULL. */
+ struct raw_breakpoint *raw;
+};
+
+/* Breakpoint requested by GDB. */
+
+struct gdb_breakpoint
+{
+ struct breakpoint base;
+
/* Pointer to the condition list that should be evaluated on
the target or NULL if the breakpoint is unconditional or
if GDB doesn't want us to evaluate the conditionals on the
@@ -181,10 +192,13 @@ struct breakpoint
/* Point to the list of commands to run when this is hit. */
struct point_command_list *command_list;
+};
- /* Link to this breakpoint's raw breakpoint. This is always
- non-NULL. */
- struct raw_breakpoint *raw;
+/* Breakpoint used by GDBserver. */
+
+struct other_breakpoint
+{
+ struct breakpoint base;
/* Function to call when we hit this breakpoint. If it returns 1,
the breakpoint shall be deleted; 0 or if this callback is NULL,
@@ -192,6 +206,13 @@ struct breakpoint
int (*handler) (CORE_ADDR);
};
+/* Reinsert breakpoint. */
+
+struct reinsert_breakpoint
+{
+ struct breakpoint base;
+};
+
/* Return the breakpoint size from its kind. */
static int
@@ -266,6 +287,18 @@ Z_packet_to_raw_bkpt_type (char z_type)
}
}
+/* Return true if breakpoint TYPE is a GDB breakpoint. */
+
+static int
+is_gdb_breakpoint (enum bkpt_type type)
+{
+ return (type == gdb_breakpoint_Z0
+ || type == gdb_breakpoint_Z1
+ || type == gdb_breakpoint_Z2
+ || type == gdb_breakpoint_Z3
+ || type == gdb_breakpoint_Z4);
+}
+
int
any_persistent_commands (void)
{
@@ -275,9 +308,14 @@ any_persistent_commands (void)
for (bp = proc->breakpoints; bp != NULL; bp = bp->next)
{
- for (cl = bp->command_list; cl != NULL; cl = cl->next)
- if (cl->persistence)
- return 1;
+ if (is_gdb_breakpoint (bp->type))
+ {
+ struct gdb_breakpoint *gdb_bp = (struct gdb_breakpoint *) bp;
+
+ for (cl = gdb_bp->command_list; cl != NULL; cl = cl->next)
+ if (cl->persistence)
+ return 1;
+ }
}
return 0;
@@ -773,11 +811,32 @@ set_breakpoint (enum bkpt_type type, enum raw_bkpt_type raw_type,
return NULL;
}
- bp = XCNEW (struct breakpoint);
- bp->type = type;
+ if (is_gdb_breakpoint (type))
+ {
+ struct gdb_breakpoint *gdb_bp = XCNEW (struct gdb_breakpoint);
+
+ bp = (struct breakpoint *) gdb_bp;
+ gdb_assert (handler == NULL);
+ }
+ else if (type == other_breakpoint)
+ {
+ struct other_breakpoint *other_bp = XCNEW (struct other_breakpoint);
+
+ other_bp->handler = handler;
+ bp = (struct breakpoint *) other_bp;
+ }
+ else if (type == reinsert_breakpoint)
+ {
+ struct reinsert_breakpoint *reinsert_bp
+ = XCNEW (struct reinsert_breakpoint);
+ bp = (struct breakpoint *) reinsert_bp;
+ }
+ else
+ gdb_assert_not_reached ("unhandled breakpoint type");
+
+ bp->type = type;
bp->raw = raw;
- bp->handler = handler;
bp->next = proc->breakpoints;
proc->breakpoints = bp;
@@ -924,7 +983,7 @@ delete_breakpoint (struct breakpoint *todel)
address ADDR and return a pointer to its structure. If KIND is -1,
the breakpoint's kind is ignored. */
-static struct breakpoint *
+static struct gdb_breakpoint *
find_gdb_breakpoint (char z_type, CORE_ADDR addr, int kind)
{
struct process_info *proc = current_process ();
@@ -934,7 +993,7 @@ find_gdb_breakpoint (char z_type, CORE_ADDR addr, int kind)
for (bp = proc->breakpoints; bp != NULL; bp = bp->next)
if (bp->type == type && bp->raw->pc == addr
&& (kind == -1 || bp->raw->kind == kind))
- return bp;
+ return (gdb_breakpoint *) bp;
return NULL;
}
@@ -952,10 +1011,10 @@ z_type_supported (char z_type)
failure returns NULL and sets *ERR to either -1 for error, or 1 if
Z_TYPE breakpoints are not supported on this target. */
-static struct breakpoint *
+static struct gdb_breakpoint *
set_gdb_breakpoint_1 (char z_type, CORE_ADDR addr, int kind, int *err)
{
- struct breakpoint *bp;
+ struct gdb_breakpoint *bp;
enum bkpt_type type;
enum raw_bkpt_type raw_type;
@@ -981,12 +1040,12 @@ set_gdb_breakpoint_1 (char z_type, CORE_ADDR addr, int kind, int *err)
if (bp != NULL)
{
- if (bp->raw->kind != kind)
+ if (bp->base.raw->kind != kind)
{
/* A different kind than previously seen. The previous
breakpoint must be gone then. */
- bp->raw->inserted = -1;
- delete_breakpoint (bp);
+ bp->base.raw->inserted = -1;
+ delete_breakpoint ((struct breakpoint *) bp);
bp = NULL;
}
else if (z_type == Z_PACKET_SW_BP)
@@ -1022,7 +1081,8 @@ set_gdb_breakpoint_1 (char z_type, CORE_ADDR addr, int kind, int *err)
raw_type = Z_packet_to_raw_bkpt_type (z_type);
type = Z_packet_to_bkpt_type (z_type);
- return set_breakpoint (type, raw_type, addr, kind, NULL, err);
+ return (struct gdb_breakpoint *) set_breakpoint (type, raw_type, addr,
+ kind, NULL, err);
}
static int
@@ -1047,10 +1107,10 @@ check_gdb_bp_preconditions (char z_type, int *err)
/* See mem-break.h. This is a wrapper for set_gdb_breakpoint_1 that
knows to prepare to access memory for Z0 breakpoints. */
-struct breakpoint *
+struct gdb_breakpoint *
set_gdb_breakpoint (char z_type, CORE_ADDR addr, int kind, int *err)
{
- struct breakpoint *bp;
+ struct gdb_breakpoint *bp;
if (!check_gdb_bp_preconditions (z_type, err))
return NULL;
@@ -1082,7 +1142,7 @@ set_gdb_breakpoint (char z_type, CORE_ADDR addr, int kind, int *err)
static int
delete_gdb_breakpoint_1 (char z_type, CORE_ADDR addr, int kind)
{
- struct breakpoint *bp;
+ struct gdb_breakpoint *bp;
int err;
bp = find_gdb_breakpoint (z_type, addr, kind);
@@ -1092,7 +1152,7 @@ delete_gdb_breakpoint_1 (char z_type, CORE_ADDR addr, int kind)
/* Before deleting the breakpoint, make sure to free its condition
and command lists. */
clear_breakpoint_conditions_and_commands (bp);
- err = delete_breakpoint (bp);
+ err = delete_breakpoint ((struct breakpoint *) bp);
if (err != 0)
return -1;
@@ -1132,7 +1192,7 @@ delete_gdb_breakpoint (char z_type, CORE_ADDR addr, int kind)
/* Clear all conditions associated with a breakpoint. */
static void
-clear_breakpoint_conditions (struct breakpoint *bp)
+clear_breakpoint_conditions (struct gdb_breakpoint *bp)
{
struct point_cond_list *cond;
@@ -1157,7 +1217,7 @@ clear_breakpoint_conditions (struct breakpoint *bp)
/* Clear all commands associated with a breakpoint. */
static void
-clear_breakpoint_commands (struct breakpoint *bp)
+clear_breakpoint_commands (struct gdb_breakpoint *bp)
{
struct point_command_list *cmd;
@@ -1180,7 +1240,7 @@ clear_breakpoint_commands (struct breakpoint *bp)
}
void
-clear_breakpoint_conditions_and_commands (struct breakpoint *bp)
+clear_breakpoint_conditions_and_commands (struct gdb_breakpoint *bp)
{
clear_breakpoint_conditions (bp);
clear_breakpoint_commands (bp);
@@ -1189,7 +1249,7 @@ clear_breakpoint_conditions_and_commands (struct breakpoint *bp)
/* Add condition CONDITION to GDBserver's breakpoint BP. */
static void
-add_condition_to_breakpoint (struct breakpoint *bp,
+add_condition_to_breakpoint (struct gdb_breakpoint *bp,
struct agent_expr *condition)
{
struct point_cond_list *new_cond;
@@ -1206,7 +1266,7 @@ add_condition_to_breakpoint (struct breakpoint *bp,
/* Add a target-side condition CONDITION to a breakpoint. */
int
-add_breakpoint_condition (struct breakpoint *bp, char **condition)
+add_breakpoint_condition (struct gdb_breakpoint *bp, char **condition)
{
char *actparm = *condition;
struct agent_expr *cond;
@@ -1240,7 +1300,7 @@ static int
gdb_condition_true_at_breakpoint_z_type (char z_type, CORE_ADDR addr)
{
/* Fetch registers for the current inferior. */
- struct breakpoint *bp = find_gdb_breakpoint (z_type, addr, -1);
+ struct gdb_breakpoint *bp = find_gdb_breakpoint (z_type, addr, -1);
ULONGEST value = 0;
struct point_cond_list *cl;
int err = 0;
@@ -1287,7 +1347,7 @@ gdb_condition_true_at_breakpoint (CORE_ADDR where)
/* Add commands COMMANDS to GDBserver's breakpoint BP. */
static void
-add_commands_to_breakpoint (struct breakpoint *bp,
+add_commands_to_breakpoint (struct gdb_breakpoint *bp,
struct agent_expr *commands, int persist)
{
struct point_command_list *new_cmd;
@@ -1305,7 +1365,7 @@ add_commands_to_breakpoint (struct breakpoint *bp,
/* Add a target-side command COMMAND to the breakpoint at ADDR. */
int
-add_breakpoint_commands (struct breakpoint *bp, char **command,
+add_breakpoint_commands (struct gdb_breakpoint *bp, char **command,
int persist)
{
char *actparm = *command;
@@ -1339,7 +1399,7 @@ add_breakpoint_commands (struct breakpoint *bp, char **command,
static int
gdb_no_commands_at_breakpoint_z_type (char z_type, CORE_ADDR addr)
{
- struct breakpoint *bp = find_gdb_breakpoint (z_type, addr, -1);
+ struct gdb_breakpoint *bp = find_gdb_breakpoint (z_type, addr, -1);
if (bp == NULL)
return 1;
@@ -1369,7 +1429,7 @@ static int
run_breakpoint_commands_z_type (char z_type, CORE_ADDR addr)
{
/* Fetch registers for the current inferior. */
- struct breakpoint *bp = find_gdb_breakpoint (z_type, addr, -1);
+ struct gdb_breakpoint *bp = find_gdb_breakpoint (z_type, addr, -1);
ULONGEST value = 0;
struct point_command_list *cl;
int err = 0;
@@ -1619,14 +1679,20 @@ check_breakpoints (CORE_ADDR stop_pc)
return;
}
- if (bp->handler != NULL && (*bp->handler) (stop_pc))
+ if (bp->type == other_breakpoint)
{
- *bp_link = bp->next;
+ struct other_breakpoint *other_bp
+ = (struct other_breakpoint *) bp;
+
+ if (other_bp->handler != NULL && (*other_bp->handler) (stop_pc))
+ {
+ *bp_link = bp->next;
- release_breakpoint (proc, bp);
+ release_breakpoint (proc, bp);
- bp = *bp_link;
- continue;
+ bp = *bp_link;
+ continue;
+ }
}
}
@@ -2008,12 +2074,6 @@ clone_one_breakpoint (const struct breakpoint *src)
{
struct breakpoint *dest;
struct raw_breakpoint *dest_raw;
- struct point_cond_list *current_cond;
- struct point_cond_list *new_cond;
- struct point_cond_list *cond_tail = NULL;
- struct point_command_list *current_cmd;
- struct point_command_list *new_cmd;
- struct point_command_list *cmd_tail = NULL;
/* Clone the raw breakpoint. */
dest_raw = XCNEW (struct raw_breakpoint);
@@ -2025,29 +2085,58 @@ clone_one_breakpoint (const struct breakpoint *src)
dest_raw->inserted = src->raw->inserted;
/* Clone the high-level breakpoint. */
- dest = XCNEW (struct breakpoint);
- dest->type = src->type;
- dest->raw = dest_raw;
- dest->handler = src->handler;
-
- /* Clone the condition list. */
- for (current_cond = src->cond_list; current_cond != NULL;
- current_cond = current_cond->next)
+ if (is_gdb_breakpoint (src->type))
{
- new_cond = XCNEW (struct point_cond_list);
- new_cond->cond = clone_agent_expr (current_cond->cond);
- APPEND_TO_LIST (&dest->cond_list, new_cond, cond_tail);
+ struct gdb_breakpoint *gdb_dest = XCNEW (struct gdb_breakpoint);
+ struct point_cond_list *current_cond;
+ struct point_cond_list *new_cond;
+ struct point_cond_list *cond_tail = NULL;
+ struct point_command_list *current_cmd;
+ struct point_command_list *new_cmd;
+ struct point_command_list *cmd_tail = NULL;
+
+ /* Clone the condition list. */
+ for (current_cond = ((struct gdb_breakpoint *) src)->cond_list;
+ current_cond != NULL;
+ current_cond = current_cond->next)
+ {
+ new_cond = XCNEW (struct point_cond_list);
+ new_cond->cond = clone_agent_expr (current_cond->cond);
+ APPEND_TO_LIST (&gdb_dest->cond_list, new_cond, cond_tail);
+ }
+
+ /* Clone the command list. */
+ for (current_cmd = ((struct gdb_breakpoint *) src)->command_list;
+ current_cmd != NULL;
+ current_cmd = current_cmd->next)
+ {
+ new_cmd = XCNEW (struct point_command_list);
+ new_cmd->cmd = clone_agent_expr (current_cmd->cmd);
+ new_cmd->persistence = current_cmd->persistence;
+ APPEND_TO_LIST (&gdb_dest->command_list, new_cmd, cmd_tail);
+ }
+
+ dest = (struct breakpoint *) gdb_dest;
}
+ else if (src->type == other_breakpoint)
+ {
+ struct other_breakpoint *other_dest = XCNEW (struct other_breakpoint);
- /* Clone the command list. */
- for (current_cmd = src->command_list; current_cmd != NULL;
- current_cmd = current_cmd->next)
+ other_dest->handler = ((struct other_breakpoint *) src)->handler;
+ dest = (struct breakpoint *) other_dest;
+ }
+ else if (src->type == reinsert_breakpoint)
{
- new_cmd = XCNEW (struct point_command_list);
- new_cmd->cmd = clone_agent_expr (current_cmd->cmd);
- new_cmd->persistence = current_cmd->persistence;
- APPEND_TO_LIST (&dest->command_list, new_cmd, cmd_tail);
+ struct reinsert_breakpoint *reinsert_dest
+ = XCNEW (struct reinsert_breakpoint);
+
+ dest = (struct breakpoint *) reinsert_dest;
}
+ else
+ gdb_assert_not_reached ("unhandled breakpoint type");
+
+ dest->type = src->type;
+ dest->raw = dest_raw;
return dest;
}
diff --git a/gdb/gdbserver/mem-break.h b/gdb/gdbserver/mem-break.h
index f6c9631..280e4f8 100644
--- a/gdb/gdbserver/mem-break.h
+++ b/gdb/gdbserver/mem-break.h
@@ -25,6 +25,7 @@
/* Breakpoints are opaque. */
struct breakpoint;
+struct gdb_breakpoint;
struct fast_tracepoint_jump;
struct raw_breakpoint;
struct process_info;
@@ -70,8 +71,8 @@ enum target_hw_bp_type raw_bkpt_type_to_target_hw_bp_type
failure returns NULL and sets *ERR to either -1 for error, or 1 if
Z_TYPE breakpoints are not supported on this target. */
-struct breakpoint *set_gdb_breakpoint (char z_type, CORE_ADDR addr, int kind,
- int *err);
+struct gdb_breakpoint *set_gdb_breakpoint (char z_type, CORE_ADDR addr,
+ int kind, int *err);
/* Delete a GDB breakpoint of type Z_TYPE and kind KIND previously
inserted at ADDR with set_gdb_breakpoint_at. Returns 0 on success,
@@ -107,20 +108,20 @@ int reinsert_breakpoint_inserted_here (CORE_ADDR addr);
/* Clear all breakpoint conditions and commands associated with a
breakpoint. */
-void clear_breakpoint_conditions_and_commands (struct breakpoint *bp);
+void clear_breakpoint_conditions_and_commands (struct gdb_breakpoint *bp);
/* Set target-side condition CONDITION to the breakpoint at ADDR.
Returns false on failure. On success, advances CONDITION pointer
past the condition and returns true. */
-int add_breakpoint_condition (struct breakpoint *bp, char **condition);
+int add_breakpoint_condition (struct gdb_breakpoint *bp, char **condition);
/* Set target-side commands COMMANDS to the breakpoint at ADDR.
Returns false on failure. On success, advances COMMANDS past the
commands and returns true. If PERSIST, the commands should run
even while GDB is disconnected. */
-int add_breakpoint_commands (struct breakpoint *bp, char **commands,
+int add_breakpoint_commands (struct gdb_breakpoint *bp, char **commands,
int persist);
int any_persistent_commands (void);
diff --git a/gdb/gdbserver/server.c b/gdb/gdbserver/server.c
index 9c50929..18517bc 100644
--- a/gdb/gdbserver/server.c
+++ b/gdb/gdbserver/server.c
@@ -3825,7 +3825,7 @@ main (int argc, char *argv[])
after the last processed option. */
static void
-process_point_options (struct breakpoint *bp, char **packet)
+process_point_options (struct gdb_breakpoint *bp, char **packet)
{
char *dataptr = *packet;
int persist;
@@ -4197,7 +4197,7 @@ process_serial_event (void)
if (insert)
{
- struct breakpoint *bp;
+ struct gdb_breakpoint *bp;
bp = set_gdb_breakpoint (type, addr, kind, &res);
if (bp != NULL)
--
1.9.1
^ permalink raw reply [flat|nested] 20+ messages in thread* Re: [PATCH 4/8] Create sub classes of 'struct breakpoint'
2016-05-20 15:13 ` [PATCH 4/8] Create sub classes of 'struct breakpoint' Yao Qi
@ 2016-05-24 16:23 ` Pedro Alves
0 siblings, 0 replies; 20+ messages in thread
From: Pedro Alves @ 2016-05-24 16:23 UTC (permalink / raw)
To: Yao Qi, gdb-patches
On 05/20/2016 04:12 PM, Yao Qi wrote:
> Nowadays, there are three types of breakpoint in GDBserver,
>
> - gdb breakpoints,
> - reinsert breakpoints, used for software single step,
> - other breakpoints, used for tracepoint,
We should probably rename the latter two, so we didn't
have to explain what they are. (Doesn't have to be
in this patch, but it's true this patch exposes the naming
to even more places...)
Anyway, LGTM.
The clone_one_breakpoint changes are calling for src->vtable->clone(); :-)
Thanks,
Pedro Alves
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH 7/8] Use reinsert_breakpoint for vCont;s
2016-05-20 15:12 [PATCH 0/8] Use reinsert breakpoint for vCont;s Yao Qi
` (5 preceding siblings ...)
2016-05-20 15:13 ` [PATCH 4/8] Create sub classes of 'struct breakpoint' Yao Qi
@ 2016-05-20 15:13 ` Yao Qi
2016-05-24 17:39 ` Pedro Alves
2016-05-20 15:13 ` [PATCH 3/8] Pass breakpoint type in set_breakpoint_at Yao Qi
2016-05-24 15:08 ` [PATCH 0/8] Use reinsert breakpoint for vCont;s Antoine Tremblay
8 siblings, 1 reply; 20+ messages in thread
From: Yao Qi @ 2016-05-20 15:13 UTC (permalink / raw)
To: gdb-patches
This patch is to teach GDBserver using software single step to handle
vCont;s. Simply speaking, if the thread's resume request is resume_step,
install reinsert breakpoint at the next pcs when GDBserver is about to
resume threads. These reinsert breakpoints of a thread are removed,
when GDBserver gets an event from that thread. Note that GDBserver may
or may not report this event back to GDB.
gdb/gdbserver:
2016-05-20 Yao Qi <yao.qi@linaro.org>
* linux-low.c (resume_stopped_resumed_lwps): If resume request
is resume_step, either set step to 1 or assert that reinsert
breakpoints are installed.
(linux_wait_1): Stop all threads, remove reinsert breakpoints,
and unstop them.
(linux_resume_one_thread): If resume request is resume_step,
either set step to 1 or assert that reinsert breakpoints are
installed.
(linux_resume): Install reinsert breakpoints if the thread is
requested to resume_step.
(proceed_one_lwp): If resume request is resume_step,
either set step to 1 or assert that reinsert breakpoints are
installed.
(proceed_all_lwps): Install reinsert breakpoints if the thread is
requested to resume_step.
---
gdb/gdbserver/linux-low.c | 125 ++++++++++++++++++++++++++++++++++++++++++++--
1 file changed, 121 insertions(+), 4 deletions(-)
diff --git a/gdb/gdbserver/linux-low.c b/gdb/gdbserver/linux-low.c
index d5f7097..22cf51a 100644
--- a/gdb/gdbserver/linux-low.c
+++ b/gdb/gdbserver/linux-low.c
@@ -2526,7 +2526,18 @@ resume_stopped_resumed_lwps (struct inferior_list_entry *entry)
&& !lp->status_pending_p
&& thread->last_status.kind == TARGET_WAITKIND_IGNORE)
{
- int step = thread->last_resume_kind == resume_step;
+ int step = 0;
+
+ if (thread->last_resume_kind == resume_step)
+ {
+ if (can_hardware_single_step ())
+ step = 1;
+ else
+ {
+ /* GDBserver must insert reinsert breakpoint for vCont;s. */
+ gdb_assert (has_reinsert_breakpoints (thread));
+ }
+ }
if (debug_threads)
debug_printf ("RSRL: resuming stopped-resumed LWP %s at %s: step=%d\n",
@@ -3477,6 +3488,18 @@ linux_wait_1 (ptid_t ptid,
return ignore_event (ourstatus);
}
+ /* Remove reinsert breakpoints for resume_step. */
+ if (can_software_single_step ()
+ && current_thread->last_resume_kind == resume_step
+ && has_reinsert_breakpoints (current_thread))
+ {
+ stop_all_lwps (1, event_child);
+
+ delete_reinsert_breakpoints (current_ptid);
+
+ unstop_all_lwps (1, event_child);
+ }
+
/* Note that all addresses are always "out of the step range" when
there's no range to begin with. */
in_step_range = lwp_in_step_range (event_child);
@@ -4796,7 +4819,6 @@ linux_resume_one_thread (struct inferior_list_entry *entry, void *arg)
{
struct thread_info *thread = (struct thread_info *) entry;
struct lwp_info *lwp = get_thread_lwp (thread);
- int step;
int leave_all_stopped = * (int *) arg;
int leave_pending;
@@ -4865,10 +4887,22 @@ linux_resume_one_thread (struct inferior_list_entry *entry, void *arg)
if (!leave_pending)
{
+ int step = 0;
+
if (debug_threads)
debug_printf ("resuming LWP %ld\n", lwpid_of (thread));
- step = (lwp->resume->kind == resume_step);
+ if (lwp->resume->kind == resume_step)
+ {
+ if (can_hardware_single_step ())
+ step = 1;
+ else
+ {
+ /* GDBserver must insert reinsert breakpoint for vCont;s. */
+ gdb_assert (has_reinsert_breakpoints (thread));
+ }
+ }
+
linux_resume_one_lwp (lwp, step, lwp->resume->sig, NULL);
}
else
@@ -4909,6 +4943,7 @@ linux_resume (struct thread_resume *resume_info, size_t n)
struct thread_info *need_step_over = NULL;
int any_pending;
int leave_all_stopped;
+ int resume_step_is_handled = 0;
if (debug_threads)
{
@@ -4952,12 +4987,57 @@ linux_resume (struct thread_resume *resume_info, size_t n)
debug_printf ("Resuming, no pending status or step over needed\n");
}
+ /* Before we resume the threads, if resume_step is requested by GDB,
+ stop all threads and install reinsert breakpoints. */
+ if (!leave_all_stopped && can_software_single_step ())
+ {
+ struct inferior_list_entry *inf, *tmp;
+
+ if (debug_threads)
+ debug_printf ("Handle resume_step.\n");
+
+ ALL_INFERIORS (&all_threads, inf, tmp)
+ {
+ struct thread_info *thread = (struct thread_info *) inf;
+ struct lwp_info *lwp = get_thread_lwp (thread);
+
+ if (lwp->resume != NULL && lwp->resume->kind == resume_step)
+ {
+ struct thread_info *saved_thread;
+
+ if (!resume_step_is_handled)
+ {
+ stop_all_lwps (0, NULL);
+
+ if (debug_threads)
+ debug_printf ("Done stopping all threads.\n");
+
+ resume_step_is_handled = 1;
+ }
+
+ saved_thread = current_thread;
+ current_thread = thread;
+ install_software_single_step_breakpoints (lwp);
+ current_thread = saved_thread;
+
+ if (debug_threads)
+ debug_printf ("Insert breakpoint for resume_step LWP %ld\n",
+ lwpid_of (thread));
+ }
+ }
+
+ if (debug_threads)
+ debug_printf ("Handle resume_step. Done\n");
+ }
+
/* Even if we're leaving threads stopped, queue all signals we'd
otherwise deliver. */
find_inferior (&all_threads, linux_resume_one_thread, &leave_all_stopped);
if (need_step_over)
start_step_over (get_thread_lwp (need_step_over));
+ else if (resume_step_is_handled)
+ unstop_all_lwps (0, NULL);
if (debug_threads)
{
@@ -5054,7 +5134,14 @@ proceed_one_lwp (struct inferior_list_entry *entry, void *except)
if (debug_threads)
debug_printf (" stepping LWP %ld, client wants it stepping\n",
lwpid_of (thread));
- step = 1;
+
+ if (can_hardware_single_step ())
+ step = 1;
+ else
+ {
+ /* GDBserver must insert reinsert breakpoint for vCont;s. */
+ gdb_assert (has_reinsert_breakpoints (thread));
+ }
}
else if (lwp->bp_reinsert != 0)
{
@@ -5124,6 +5211,36 @@ proceed_all_lwps (void)
if (debug_threads)
debug_printf ("Proceeding, no step-over needed\n");
+ /* Re-install the reinsert breakpoints on software single step target
+ if the client wants it step. */
+ if (can_software_single_step ())
+ {
+ struct inferior_list_entry *inf, *tmp;
+
+ ALL_INFERIORS (&all_threads, inf, tmp)
+ {
+ struct thread_info *thread = (struct thread_info *) inf;
+
+ if (thread->last_resume_kind == resume_step)
+ {
+ struct lwp_info *lwp = get_thread_lwp (thread);
+ struct thread_info *saved_thread;
+
+ saved_thread = current_thread;
+ current_thread = thread;
+
+ if (!has_reinsert_breakpoints (thread))
+ install_software_single_step_breakpoints (lwp);
+
+ current_thread = saved_thread;
+
+ if (debug_threads)
+ debug_printf ("Insert breakpoint for resume_step LWP %ld\n",
+ lwpid_of (thread));
+ }
+ }
+ }
+
find_inferior (&all_threads, proceed_one_lwp, NULL);
}
--
1.9.1
^ permalink raw reply [flat|nested] 20+ messages in thread* Re: [PATCH 7/8] Use reinsert_breakpoint for vCont;s
2016-05-20 15:13 ` [PATCH 7/8] Use reinsert_breakpoint for vCont;s Yao Qi
@ 2016-05-24 17:39 ` Pedro Alves
2016-05-26 16:52 ` Yao Qi
0 siblings, 1 reply; 20+ messages in thread
From: Pedro Alves @ 2016-05-24 17:39 UTC (permalink / raw)
To: Yao Qi, gdb-patches
On 05/20/2016 04:12 PM, Yao Qi wrote:
> This patch is to teach GDBserver using software single step to handle
> vCont;s. Simply speaking, if the thread's resume request is resume_step,
> install reinsert breakpoint at the next pcs when GDBserver is about to
> resume threads. These reinsert breakpoints of a thread are removed,
> when GDBserver gets an event from that thread. Note that GDBserver may
> or may not report this event back to GDB.
>
> gdb/gdbserver:
>
> 2016-05-20 Yao Qi <yao.qi@linaro.org>
>
> * linux-low.c (resume_stopped_resumed_lwps): If resume request
> is resume_step, either set step to 1 or assert that reinsert
> breakpoints are installed.
> (linux_wait_1): Stop all threads, remove reinsert breakpoints,
> and unstop them.
> (linux_resume_one_thread): If resume request is resume_step,
> either set step to 1 or assert that reinsert breakpoints are
> installed.
> (linux_resume): Install reinsert breakpoints if the thread is
> requested to resume_step.
> (proceed_one_lwp): If resume request is resume_step,
> either set step to 1 or assert that reinsert breakpoints are
> installed.
> (proceed_all_lwps): Install reinsert breakpoints if the thread is
> requested to resume_step.
Spaces vs tabs, I think.
> ---
> gdb/gdbserver/linux-low.c | 125 ++++++++++++++++++++++++++++++++++++++++++++--
> 1 file changed, 121 insertions(+), 4 deletions(-)
>
> diff --git a/gdb/gdbserver/linux-low.c b/gdb/gdbserver/linux-low.c
> index d5f7097..22cf51a 100644
> --- a/gdb/gdbserver/linux-low.c
> +++ b/gdb/gdbserver/linux-low.c
> @@ -2526,7 +2526,18 @@ resume_stopped_resumed_lwps (struct inferior_list_entry *entry)
> && !lp->status_pending_p
> && thread->last_status.kind == TARGET_WAITKIND_IGNORE)
> {
> - int step = thread->last_resume_kind == resume_step;
> + int step = 0;
> +
> + if (thread->last_resume_kind == resume_step)
> + {
...
> + if (can_hardware_single_step ())
> + step = 1;
> + else
> + {
> + /* GDBserver must insert reinsert breakpoint for vCont;s. */
> + gdb_assert (has_reinsert_breakpoints (thread));
> + }
This pattern seems to appear in many places in the series.
Maybe we could add a function to wrap it? Something like:
static int
maybe_hw_step (struct thread_info *thread)
{
if (can_hardware_single_step ())
return 1;
else
{
gdb_assert (has_reinsert_breakpoints (thread));
return 0;
}
}
?
> + }
>
> if (debug_threads)
> debug_printf ("RSRL: resuming stopped-resumed LWP %s at %s: step=%d\n",
> @@ -3477,6 +3488,18 @@ linux_wait_1 (ptid_t ptid,
> return ignore_event (ourstatus);
> }
>
> + /* Remove reinsert breakpoints for resume_step. */
> + if (can_software_single_step ()
> + && current_thread->last_resume_kind == resume_step
What if a vCont;t came in after the step had started and
before it finished? Won't we have lost the resume_step
linux_set_resume_request then?
> + && has_reinsert_breakpoints (current_thread))
> + {
> + stop_all_lwps (1, event_child);
> +
> + delete_reinsert_breakpoints (current_ptid);
> +
> + unstop_all_lwps (1, event_child);
> + }
> +
> /* Note that all addresses are always "out of the step range" when
> there's no range to begin with. */
> in_step_range = lwp_in_step_range (event_child);
> @@ -4796,7 +4819,6 @@ linux_resume_one_thread (struct inferior_list_entry *entry, void *arg)
> {
> struct thread_info *thread = (struct thread_info *) entry;
> struct lwp_info *lwp = get_thread_lwp (thread);
> - int step;
> int leave_all_stopped = * (int *) arg;
> int leave_pending;
>
> @@ -4865,10 +4887,22 @@ linux_resume_one_thread (struct inferior_list_entry *entry, void *arg)
>
> if (!leave_pending)
> {
> + int step = 0;
> +
> if (debug_threads)
> debug_printf ("resuming LWP %ld\n", lwpid_of (thread));
>
> - step = (lwp->resume->kind == resume_step);
> + if (lwp->resume->kind == resume_step)
> + {
> + if (can_hardware_single_step ())
> + step = 1;
> + else
> + {
> + /* GDBserver must insert reinsert breakpoint for vCont;s. */
> + gdb_assert (has_reinsert_breakpoints (thread));
> + }
> + }
> +
> linux_resume_one_lwp (lwp, step, lwp->resume->sig, NULL);
> }
> else
> @@ -4909,6 +4943,7 @@ linux_resume (struct thread_resume *resume_info, size_t n)
> struct thread_info *need_step_over = NULL;
> int any_pending;
> int leave_all_stopped;
> + int resume_step_is_handled = 0;
>
> if (debug_threads)
> {
> @@ -4952,12 +4987,57 @@ linux_resume (struct thread_resume *resume_info, size_t n)
> debug_printf ("Resuming, no pending status or step over needed\n");
> }
>
> + /* Before we resume the threads, if resume_step is requested by GDB,
> + stop all threads and install reinsert breakpoints. */
> + if (!leave_all_stopped && can_software_single_step ())
> + {
> + struct inferior_list_entry *inf, *tmp;
> +
> + if (debug_threads)
> + debug_printf ("Handle resume_step.\n");
> +
> + ALL_INFERIORS (&all_threads, inf, tmp)
> + {
> + struct thread_info *thread = (struct thread_info *) inf;
> + struct lwp_info *lwp = get_thread_lwp (thread);
> +
> + if (lwp->resume != NULL && lwp->resume->kind == resume_step)
> + {
> + struct thread_info *saved_thread;
> +
> + if (!resume_step_is_handled)
> + {
> + stop_all_lwps (0, NULL);
> +
> + if (debug_threads)
> + debug_printf ("Done stopping all threads.\n");
> +
> + resume_step_is_handled = 1;
> + }
> +
> + saved_thread = current_thread;
> + current_thread = thread;
> + install_software_single_step_breakpoints (lwp);
> + current_thread = saved_thread;
> +
This current thread save/restore should move to
install_software_single_step_breakpoints, as per the other patch.
> + if (debug_threads)
> + debug_printf ("Insert breakpoint for resume_step LWP %ld\n",
> + lwpid_of (thread));
> + }
> + }
> +
> + if (debug_threads)
> + debug_printf ("Handle resume_step. Done\n");
> + }
> +
> /* Even if we're leaving threads stopped, queue all signals we'd
> otherwise deliver. */
> find_inferior (&all_threads, linux_resume_one_thread, &leave_all_stopped);
>
> if (need_step_over)
> start_step_over (get_thread_lwp (need_step_over));
> + else if (resume_step_is_handled)
> + unstop_all_lwps (0, NULL);
>
> if (debug_threads)
> {
> @@ -5054,7 +5134,14 @@ proceed_one_lwp (struct inferior_list_entry *entry, void *except)
> if (debug_threads)
> debug_printf (" stepping LWP %ld, client wants it stepping\n",
> lwpid_of (thread));
> - step = 1;
> +
> + if (can_hardware_single_step ())
> + step = 1;
> + else
> + {
> + /* GDBserver must insert reinsert breakpoint for vCont;s. */
> + gdb_assert (has_reinsert_breakpoints (thread));
> + }
> }
> else if (lwp->bp_reinsert != 0)
> {
> @@ -5124,6 +5211,36 @@ proceed_all_lwps (void)
> if (debug_threads)
> debug_printf ("Proceeding, no step-over needed\n");
>
> + /* Re-install the reinsert breakpoints on software single step target
> + if the client wants it step. */
> + if (can_software_single_step ())
> + {
> + struct inferior_list_entry *inf, *tmp;
> +
> + ALL_INFERIORS (&all_threads, inf, tmp)
> + {
> + struct thread_info *thread = (struct thread_info *) inf;
> +
> + if (thread->last_resume_kind == resume_step)
> + {
> + struct lwp_info *lwp = get_thread_lwp (thread);
> + struct thread_info *saved_thread;
> +
> + saved_thread = current_thread;
> + current_thread = thread;
> +
> + if (!has_reinsert_breakpoints (thread))
> + install_software_single_step_breakpoints (lwp);
> +
> + current_thread = saved_thread;
Ditto.
> +
> + if (debug_threads)
> + debug_printf ("Insert breakpoint for resume_step LWP %ld\n",
> + lwpid_of (thread));
> + }
> + }
> + }
> +
> find_inferior (&all_threads, proceed_one_lwp, NULL);
> }
>
>
Thanks,
Pedro Alves
^ permalink raw reply [flat|nested] 20+ messages in thread* Re: [PATCH 7/8] Use reinsert_breakpoint for vCont;s
2016-05-24 17:39 ` Pedro Alves
@ 2016-05-26 16:52 ` Yao Qi
2016-05-26 17:00 ` Pedro Alves
0 siblings, 1 reply; 20+ messages in thread
From: Yao Qi @ 2016-05-26 16:52 UTC (permalink / raw)
To: Pedro Alves; +Cc: Yao Qi, gdb-patches
Pedro Alves <palves@redhat.com> writes:
>> + /* Remove reinsert breakpoints for resume_step. */
>> + if (can_software_single_step ()
>> + && current_thread->last_resume_kind == resume_step
>
> What if a vCont;t came in after the step had started and
> before it finished? Won't we have lost the resume_step
> linux_set_resume_request then?
Do you mean a case like this? GDB sends vCont;s, and then GDBserver
started step-over, in the middle of step-over, GDB sends vCont;t, so
thread->last_resume_kind is set from resume_step to resume_stop, and the
check here fails, so reinsert breakpoints are not deleted.
--
Yao (齐尧)
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 7/8] Use reinsert_breakpoint for vCont;s
2016-05-26 16:52 ` Yao Qi
@ 2016-05-26 17:00 ` Pedro Alves
0 siblings, 0 replies; 20+ messages in thread
From: Pedro Alves @ 2016-05-26 17:00 UTC (permalink / raw)
To: Yao Qi; +Cc: gdb-patches
On 05/26/2016 05:52 PM, Yao Qi wrote:
> Pedro Alves <palves@redhat.com> writes:
>
>>> + /* Remove reinsert breakpoints for resume_step. */
>>> + if (can_software_single_step ()
>>> + && current_thread->last_resume_kind == resume_step
>>
>> What if a vCont;t came in after the step had started and
>> before it finished? Won't we have lost the resume_step
>> linux_set_resume_request then?
>
> Do you mean a case like this? GDB sends vCont;s, and then GDBserver
> started step-over, in the middle of step-over, GDB sends vCont;t, so
> thread->last_resume_kind is set from resume_step to resume_stop, and the
> check here fails, so reinsert breakpoints are not deleted.
Exactly.
Thanks,
Pedro Alves
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH 3/8] Pass breakpoint type in set_breakpoint_at
2016-05-20 15:12 [PATCH 0/8] Use reinsert breakpoint for vCont;s Yao Qi
` (6 preceding siblings ...)
2016-05-20 15:13 ` [PATCH 7/8] Use reinsert_breakpoint for vCont;s Yao Qi
@ 2016-05-20 15:13 ` Yao Qi
2016-05-24 16:10 ` Pedro Alves
2016-05-24 15:08 ` [PATCH 0/8] Use reinsert breakpoint for vCont;s Antoine Tremblay
8 siblings, 1 reply; 20+ messages in thread
From: Yao Qi @ 2016-05-20 15:13 UTC (permalink / raw)
To: gdb-patches
Nowadays, set_breakpoint_at creates breakpoint of type
other_breakpoint, but we also use set_breakpoint_at
in set_reinsert_breakpoint to create breakpoint, so that
we have to overwrite the breakpoint type like this,
bp = set_breakpoint_at (stop_at, NULL);
bp->type = reinsert_breakpoint;
which looks not very good. This patch changes set_breakpoint_at
to receive breakpoint type. Since set_breakpoint_at is
used in many places, I rename it to set_breakpoint_at_1, and wrap
it with set_breakpoint_at, and pass other_breakpoint. In this way,
we can call set_breakpoint_at_1 with reinsert_breakpoint in
set_reinsert_breakpoint too, and code looks cleaner.
gdb/gdbserver:
2016-05-20 Yao Qi <yao.qi@linaro.org>
* mem-break.c (set_breakpoint_at): Rename it to ...
(set_breakpoint_at_1): ... it.
(set_breakpoint_at): Call set_breakpoint_at_1.
(set_reinsert_breakpoint): Call set_breakpoint_at_1.
* mem-break.h (set_breakpoint_at): Update comments.
---
gdb/gdbserver/mem-break.c | 20 ++++++++++++++------
gdb/gdbserver/mem-break.h | 3 ++-
2 files changed, 16 insertions(+), 7 deletions(-)
diff --git a/gdb/gdbserver/mem-break.c b/gdb/gdbserver/mem-break.c
index 3313459..3e3be8b 100644
--- a/gdb/gdbserver/mem-break.c
+++ b/gdb/gdbserver/mem-break.c
@@ -785,20 +785,29 @@ set_breakpoint (enum bkpt_type type, enum raw_bkpt_type raw_type,
return bp;
}
-/* See mem-break.h */
+/* Set breakpoint of TYPE on address WHERE with handler HANDLER. */
-struct breakpoint *
-set_breakpoint_at (CORE_ADDR where, int (*handler) (CORE_ADDR))
+static struct breakpoint *
+set_breakpoint_at_1 (enum bkpt_type type, CORE_ADDR where,
+ int (*handler) (CORE_ADDR))
{
int err_ignored;
CORE_ADDR placed_address = where;
int breakpoint_kind = target_breakpoint_kind_from_pc (&placed_address);
- return set_breakpoint (other_breakpoint, raw_bkpt_type_sw,
+ return set_breakpoint (type, raw_bkpt_type_sw,
placed_address, breakpoint_kind, handler,
&err_ignored);
}
+/* See mem-break.h */
+
+struct breakpoint *
+set_breakpoint_at (CORE_ADDR where, int (*handler) (CORE_ADDR))
+{
+ return set_breakpoint_at_1 (other_breakpoint, where, handler);
+}
+
static int
delete_raw_breakpoint (struct process_info *proc, struct raw_breakpoint *todel)
@@ -1411,8 +1420,7 @@ set_reinsert_breakpoint (CORE_ADDR stop_at)
{
struct breakpoint *bp;
- bp = set_breakpoint_at (stop_at, NULL);
- bp->type = reinsert_breakpoint;
+ bp = set_breakpoint_at_1 (reinsert_breakpoint, stop_at, NULL);
}
void
diff --git a/gdb/gdbserver/mem-break.h b/gdb/gdbserver/mem-break.h
index b84dc1e..f6c9631 100644
--- a/gdb/gdbserver/mem-break.h
+++ b/gdb/gdbserver/mem-break.h
@@ -141,7 +141,8 @@ int gdb_breakpoint_here (CORE_ADDR where);
/* Create a new breakpoint at WHERE, and call HANDLER when
it is hit. HANDLER should return 1 if the breakpoint
- should be deleted, 0 otherwise. */
+ should be deleted, 0 otherwise. The type of the created
+ breakpoint is other_breakpoint. */
struct breakpoint *set_breakpoint_at (CORE_ADDR where,
int (*handler) (CORE_ADDR));
--
1.9.1
^ permalink raw reply [flat|nested] 20+ messages in thread* Re: [PATCH 3/8] Pass breakpoint type in set_breakpoint_at
2016-05-20 15:13 ` [PATCH 3/8] Pass breakpoint type in set_breakpoint_at Yao Qi
@ 2016-05-24 16:10 ` Pedro Alves
0 siblings, 0 replies; 20+ messages in thread
From: Pedro Alves @ 2016-05-24 16:10 UTC (permalink / raw)
To: Yao Qi, gdb-patches
On 05/20/2016 04:12 PM, Yao Qi wrote:
>
> -/* See mem-break.h */
> +/* Set breakpoint of TYPE on address WHERE with handler HANDLER. */
>
> -struct breakpoint *
> -set_breakpoint_at (CORE_ADDR where, int (*handler) (CORE_ADDR))
> +static struct breakpoint *
> +set_breakpoint_at_1 (enum bkpt_type type, CORE_ADDR where,
> + int (*handler) (CORE_ADDR))
Maybe avoid the bland "_1", like:
set_breakpoint_type_at (enum bkpt_type type, CORE_ADDR where,
int (*handler) (CORE_ADDR))
But LGTM.
Thanks,
Pedro Alves
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 0/8] Use reinsert breakpoint for vCont;s
2016-05-20 15:12 [PATCH 0/8] Use reinsert breakpoint for vCont;s Yao Qi
` (7 preceding siblings ...)
2016-05-20 15:13 ` [PATCH 3/8] Pass breakpoint type in set_breakpoint_at Yao Qi
@ 2016-05-24 15:08 ` Antoine Tremblay
8 siblings, 0 replies; 20+ messages in thread
From: Antoine Tremblay @ 2016-05-24 15:08 UTC (permalink / raw)
To: Yao Qi; +Cc: gdb-patches
Yao Qi writes:
> Nowadays, reinsert breakpoint is used in GDBserver to step over a
> breakpoint. I want to use it to handle vCont;s too. The idea is that
> when GDBserver receives resume_step request from GDB, use the software
> single step logic, insert reinsert_breakpoint on the next pcs, and
> resume the thread. This means we have multiple reinsert_breakpoints
> for different threads for either step-over or vCont;s. Patch #6 makes
> reinsert breakpoints thread specific, patch #7 does the major thing,
> and patch #8 claims supporting vCont;s with software single step.
>
> Patch #1 and #2 fixes the existing bugs. Patch #3, #4, and #5 refactor
> the code.
>
> The whole series are tested on arm-linux, aarch64-linux and
> x86_64-linux.
>
> *** BLURB HERE ***
>
> Yao Qi (8):
> Switch to current thread before finish_step_over
> Delete reinsert breakpoints from forked child
> Pass breakpoint type in set_breakpoint_at
> Create sub classes of 'struct breakpoint'
> Refactor clone_all_breakpoints
> Make reinsert_breakpoint thread specific
> Use reinsert_breakpoint for vCont;s
> [GDBserver] Support vCont s and S actions with software single step
>
> gdb/gdbserver/linux-low.c | 180 ++++++++++++++++++++++++++--
> gdb/gdbserver/mem-break.c | 293 +++++++++++++++++++++++++++++++++-------------
> gdb/gdbserver/mem-break.h | 35 +++---
> gdb/gdbserver/server.c | 17 +--
> 4 files changed, 412 insertions(+), 113 deletions(-)
Series LGTM. Thanks for working on this!
Antoine
^ permalink raw reply [flat|nested] 20+ messages in thread