[1/8] Switch to current thread before finish_step_over
Commit Message
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(-)
Comments
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
@@ -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;
@@ -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)
{
@@ -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. */