[1/3] Remove single-step breakpoint for GDBserver internal event
Commit Message
This patch removes single-step breakpoints if the event is only
GDBserver internal, IOW, isn't reported back to GDB.
gdb/gdbserver:
2016-09-25 Yao Qi <yao.qi@linaro.org>
* linux-low.c (linux_wait_1): If single-step breakpoints are
inserted, remove them.
---
gdb/gdbserver/linux-low.c | 32 +++++++++++++++++++++++---------
1 file changed, 23 insertions(+), 9 deletions(-)
Comments
On 09/26/2016 03:25 AM, Yao Qi wrote:
> This patch removes single-step breakpoints if the event is only
> GDBserver internal, IOW, isn't reported back to GDB.
Can you expand on rationale? Why does being internal matter?
The rest of the series looks fine to me.
Thanks,
Pedro Alves
On Wed, Oct 26, 2016 at 1:45 PM, Pedro Alves <palves@redhat.com> wrote:
> On 09/26/2016 03:25 AM, Yao Qi wrote:
>> This patch removes single-step breakpoints if the event is only
>> GDBserver internal, IOW, isn't reported back to GDB.
>
> Can you expand on rationale? Why does being internal matter?
We remove single-step breakpoints on the moment we report event
back to GDB. However, if we use single-step breakpoints and get
some events, we don't report them back to GDB and keep controlling
the inferior, we need to remove single-step breakpoints. For
example, in range stepping, until the program goes out of the range,
we keep doing single step (by software), in a loop of insert single-step
breakpoints, resume, remove them.
On 10/26/2016 09:14 PM, Yao Qi wrote:
> On Wed, Oct 26, 2016 at 1:45 PM, Pedro Alves <palves@redhat.com> wrote:
>> On 09/26/2016 03:25 AM, Yao Qi wrote:
>>> This patch removes single-step breakpoints if the event is only
>>> GDBserver internal, IOW, isn't reported back to GDB.
>>
>> Can you expand on rationale? Why does being internal matter?
>
> We remove single-step breakpoints on the moment we report event
> back to GDB. However, if we use single-step breakpoints and get
> some events, we don't report them back to GDB and keep controlling
> the inferior, we need to remove single-step breakpoints. For
> example, in range stepping, until the program goes out of the range,
> we keep doing single step (by software), in a loop of insert single-step
> breakpoints, resume, remove them.
>
Ah yes, sorry, I should have read the code in more detail.
I agree, totally makes sense to do this here.
Thanks,
Pedro Alves
On Thu, Oct 27, 2016 at 2:16 PM, Pedro Alves <palves@redhat.com> wrote:
>
> Ah yes, sorry, I should have read the code in more detail.
> I agree, totally makes sense to do this here.
>
So, the patch is OK to commit? :-)
On 10/27/2016 03:14 PM, Yao Qi wrote:
> On Thu, Oct 27, 2016 at 2:16 PM, Pedro Alves <palves@redhat.com> wrote:
>>
>> Ah yes, sorry, I should have read the code in more detail.
>> I agree, totally makes sense to do this here.
>>
>
> So, the patch is OK to commit? :-)
OK with me.
Thanks,
Pedro Alves
@@ -3670,17 +3670,31 @@ linux_wait_1 (ptid_t ptid,
(*the_low_target.set_pc) (regcache, event_child->stop_pc);
}
- /* We may have finished stepping over a breakpoint. If so,
- we've stopped and suspended all LWPs momentarily except the
- stepping one. This is where we resume them all again. We're
- going to keep waiting, so use proceed, which handles stepping
- over the next breakpoint. */
- if (debug_threads)
- debug_printf ("proceeding all threads.\n");
-
if (step_over_finished)
- unsuspend_all_lwps (event_child);
+ {
+ /* If we have finished stepping over a breakpoint, we've
+ stopped and suspended all LWPs momentarily except the
+ stepping one. This is where we resume them all again.
+ We're going to keep waiting, so use proceed, which
+ handles stepping over the next breakpoint. */
+ unsuspend_all_lwps (event_child);
+ }
+ else
+ {
+ /* Remove the single-step breakpoints if any. Note that
+ there isn't single-step breakpoint if we finished stepping
+ over. */
+ if (can_software_single_step ()
+ && has_single_step_breakpoints (current_thread))
+ {
+ stop_all_lwps (0, event_child);
+ delete_single_step_breakpoints (current_thread);
+ unstop_all_lwps (0, event_child);
+ }
+ }
+ if (debug_threads)
+ debug_printf ("proceeding all threads.\n");
proceed_all_lwps ();
return ignore_event (ourstatus);
}