This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [pushed] gdbserver: redo stepping over breakpoint that was on top of a permanent breakpoint
- From: Pedro Alves <palves at redhat dot com>
- To: Yao Qi <qiyaoltc at gmail dot com>
- Cc: gdb-patches at sourceware dot org
- Date: Thu, 26 Feb 2015 19:01:34 +0000
- Subject: Re: [pushed] gdbserver: redo stepping over breakpoint that was on top of a permanent breakpoint
- Authentication-results: sourceware.org; auth=none
- References: <1424723261-15719-1-git-send-email-palves at redhat dot com> <86lhjnxzrs dot fsf at gmail dot com>
On 02/24/2015 12:25 PM, Yao Qi wrote:
> Pedro Alves <palves@redhat.com> writes:
>
>> + unsigned int increment_pc;
>> +
>> + if (the_low_target.breakpoint_len > the_low_target.decr_pc_after_break)
>> + increment_pc = the_low_target.breakpoint_len;
>> + else
>> + increment_pc = the_low_target.decr_pc_after_break;
>
> AFAICS, the_low_target.breakpoint_len is greater or equal to
> the_low_target.decr_pc_after_break. Why don't we use .breakpoint_len
> instead? The code is correct, however, the comparison here leads me to
> think under what situation .breakpoint_len is less than .decr_pc_after_break.
Good point. The history is that I started out by always incrementing
by decr_pc_after_break. Later, I recalled that that's not always
the same as breakpoint length, and thinking that maybe archs that
set decr_pc_after_break may not set breakpoint_len, wrote
"increment = max (decr_pc_after_break, breakpoint_len);". That didn't
work (we don't define max anywhere), so I expanded that into the "if",
and never thought about it again. :-) It's hard to tell whether
there are such ports, but likely not. If there are, bp-permanent.exp
should catch them failing, I think. :-) So I went ahead and pushed
the patch below.
Thanks.
---
[PATCH] gdbserver/Linux: Simplify stepping past program breakpoint a
little
.decr_pc_after_break is never higher than .breakpoint_len, so use
.breakpoint_len directly. Based on idea from Yao here:
https://sourceware.org/ml/gdb-patches/2015-02/msg00689.html
gdb/gdbserver/ChangeLog:
2015-02-26 Pedro Alves <palves@redhat.com>
* linux-low.c (linux_wait_1): When incrementing the PC past a
program breakpoint always use the_low_target.breakpoint_len as
increment, rather than the maximum between that and
the_low_target.decr_pc_after_break.
---
gdb/gdbserver/ChangeLog | 7 +++++++
gdb/gdbserver/linux-low.c | 7 +------
2 files changed, 8 insertions(+), 6 deletions(-)
diff --git a/gdb/gdbserver/ChangeLog b/gdb/gdbserver/ChangeLog
index a130aab..6bb8950 100644
--- a/gdb/gdbserver/ChangeLog
+++ b/gdb/gdbserver/ChangeLog
@@ -1,3 +1,10 @@
+2015-02-26 Pedro Alves <palves@redhat.com>
+
+ * linux-low.c (linux_wait_1): When incrementing the PC past a
+ program breakpoint always use the_low_target.breakpoint_len as
+ increment, rather than the maximum between that and
+ the_low_target.decr_pc_after_break.
+
2015-02-23 Pedro Alves <palves@redhat.com>
* linux-low.c (check_stopped_by_breakpoint): Don't check if the
diff --git a/gdb/gdbserver/linux-low.c b/gdb/gdbserver/linux-low.c
index 1c66985..c0d3b0d 100644
--- a/gdb/gdbserver/linux-low.c
+++ b/gdb/gdbserver/linux-low.c
@@ -2555,12 +2555,7 @@ linux_wait_1 (ptid_t ptid,
if (!ptid_equal (step_over_bkpt, null_ptid)
&& event_child->stop_reason == LWP_STOPPED_BY_SW_BREAKPOINT)
{
- unsigned int increment_pc;
-
- if (the_low_target.breakpoint_len > the_low_target.decr_pc_after_break)
- increment_pc = the_low_target.breakpoint_len;
- else
- increment_pc = the_low_target.decr_pc_after_break;
+ unsigned int increment_pc = the_low_target.breakpoint_len;
if (debug_threads)
{
--
1.9.3