* [PATCH 4/5] range stepping: gdbserver (x86 GNU/Linux)
2013-05-14 19:10 [PATCH 0/5 V3] target-assisted range stepping Pedro Alves
@ 2013-05-14 19:10 ` Pedro Alves
2013-05-14 19:47 ` Eli Zaretskii
` (3 more replies)
2013-05-14 19:10 ` [PATCH 2/5] Convert rs->support_vCont_t to a struct Pedro Alves
` (5 subsequent siblings)
6 siblings, 4 replies; 38+ messages in thread
From: Pedro Alves @ 2013-05-14 19:10 UTC (permalink / raw)
To: gdb-patches
This patch adds support for range stepping to GDBserver, teaching it
about vCont;r.
It'd be easy to enable it for all hardware single-step targets without
needing the linux_target_ops hook, however, at least PPC needs special
care, due to the fact that PPC atomic sequences can't be hardware
single-stepped through, a thing which GDBserver doesn't know about.
So this leaves the support limited to x86/x86_64, just like in Yao's
original series.
v3:
- lift the limitation that only a one r action was handled per vCont,
making the step range part of 'struct thread_resume'.
- Moved step range bits out of gdbthread.h/inferiors.c into linux-low.c.
- The stepping range of a thread that is quiesced because some other
thread is reporting an event is no longer left stale.
- Added NEWS entry for GDBserver.
- Many tweaks.
gdb/
2013-05-14 Pedro Alves <palves@redhat.com>
* NEWS: Mention GDBserver range stepping support.
gdb/gdbserver/
2013-05-14 Yao Qi <yao@codesourcery.com>
Pedro Alves <palves@redhat.com>
* linux-low.c (lwp_in_step_range): New function.
(linux_wait_1): If the thread was range stepping and stopped
outside the stepping range, report the stop to GDB. Otherwise,
continue stepping. Add range stepping debug output.
(linux_set_resume_request): Copy the step range from the resume
request to the lwp.
(linux_supports_range_stepping): New.
(linux_target_ops) <supports_range_stepping>: Set to
linux_supports_range_stepping.
* linux-low.h (struct linux_target_ops)
<supports_range_stepping>: New field.
(struct lwp_info) <step_range_start, step_range_end>: New fields.
* linux-x86-low.c (x86_supports_range_stepping): New.
(the_low_target) <supports_range_stepping>: Set to
x86_supports_range_stepping.
* server.c (handle_v_cont): Handle 'r' action.
(handle_v_requests): Append ";r" if the target supports range
stepping.
* target.h (struct thread_resume) <step_range_start,
step_range_end>: New fields.
(struct target_ops) <supports_range_stepping>:
New field.
(target_supports_range_stepping): New macro.
---
gdb/NEWS | 5 +++
gdb/gdbserver/linux-low.c | 63 ++++++++++++++++++++++++++++++++++-------
gdb/gdbserver/linux-low.h | 8 +++++
gdb/gdbserver/linux-x86-low.c | 7 +++++
gdb/gdbserver/server.c | 24 +++++++++++++++-
gdb/gdbserver/target.h | 18 ++++++++++++
6 files changed, 113 insertions(+), 12 deletions(-)
diff --git a/gdb/NEWS b/gdb/NEWS
index 3f2d757..a4a3c4a 100644
--- a/gdb/NEWS
+++ b/gdb/NEWS
@@ -81,6 +81,11 @@ vCont;r
stub to step through an address range itself, without GDB
involvemement at each single-step.
+* New features in the GDB remote stub, GDBserver
+
+ ** GDBserver now supports target-assisted range stepping. Currently
+ enabled on x86/x86_64 GNU/Linux targets.
+
*** Changes in GDB 7.6
* Target record has been renamed to record-full.
diff --git a/gdb/gdbserver/linux-low.c b/gdb/gdbserver/linux-low.c
index 72c51e0..5a9aad7 100644
--- a/gdb/gdbserver/linux-low.c
+++ b/gdb/gdbserver/linux-low.c
@@ -252,6 +252,16 @@ supports_fast_tracepoints (void)
return the_low_target.install_fast_tracepoint_jump_pad != NULL;
}
+/* True if LWP is stopped in its stepping range. */
+
+static int
+lwp_in_step_range (struct lwp_info *lwp)
+{
+ CORE_ADDR pc = lwp->stop_pc;
+
+ return (pc >= lwp->step_range_start && pc < lwp->step_range_end);
+}
+
struct pending_signals
{
int signal;
@@ -2313,6 +2323,7 @@ linux_wait_1 (ptid_t ptid,
int maybe_internal_trap;
int report_to_gdb;
int trace_event;
+ int in_step_range;
/* Translate generic target options into linux options. */
options = __WALL;
@@ -2322,6 +2333,7 @@ linux_wait_1 (ptid_t ptid,
retry:
bp_explains_trap = 0;
trace_event = 0;
+ in_step_range = 0;
ourstatus->kind = TARGET_WAITKIND_IGNORE;
/* If we were only supposed to resume one thread, only wait for
@@ -2615,18 +2627,24 @@ Check if we're already there.\n",
goto retry;
}
- /* If GDB wanted this thread to single step, we always want to
- report the SIGTRAP, and let GDB handle it. Watchpoints should
- always be reported. So should signals we can't explain. A
- SIGTRAP we can't explain could be a GDB breakpoint --- we may or
- not support Z0 breakpoints. If we do, we're be able to handle
- GDB breakpoints on top of internal breakpoints, by handling the
- internal breakpoint and still reporting the event to GDB. If we
- don't, we're out of luck, GDB won't see the breakpoint hit. */
+ /* Note that all addresses are always "out the step range" when
+ there's no range to begin with. */
+ in_step_range = lwp_in_step_range (event_child);
+
+ /* If GDB wanted this thread to single step, and the thread is out
+ of the step range, we always want to report the SIGTRAP, and let
+ GDB handle it. Watchpoints should always be reported. So should
+ signals we can't explain. A SIGTRAP we can't explain could be a
+ GDB breakpoint --- we may or not support Z0 breakpoints. If we
+ do, we're be able to handle GDB breakpoints on top of internal
+ breakpoints, by handling the internal breakpoint and still
+ reporting the event to GDB. If we don't, we're out of luck, GDB
+ won't see the breakpoint hit. */
report_to_gdb = (!maybe_internal_trap
- || current_inferior->last_resume_kind == resume_step
+ || (current_inferior->last_resume_kind == resume_step
+ && !in_step_range)
|| event_child->stopped_by_watchpoint
- || (!step_over_finished
+ || (!step_over_finished && !in_step_range
&& !bp_explains_trap && !trace_event)
|| (gdb_breakpoint_here (event_child->stop_pc)
&& gdb_condition_true_at_breakpoint (event_child->stop_pc)
@@ -2647,6 +2665,11 @@ Check if we're already there.\n",
fprintf (stderr, "Step-over finished.\n");
if (trace_event)
fprintf (stderr, "Tracepoint event.\n");
+ if (lwp_in_step_range (event_child))
+ fprintf (stderr, "Range stepping pc 0x%s [0x%s, 0x%s).\n",
+ paddress (event_child->stop_pc),
+ paddress (event_child->step_range_start),
+ paddress (event_child->step_range_end));
}
/* We're not reporting this breakpoint to GDB, so apply the
@@ -2678,7 +2701,12 @@ Check if we're already there.\n",
if (debug_threads)
{
if (current_inferior->last_resume_kind == resume_step)
- fprintf (stderr, "GDB wanted to single-step, reporting event.\n");
+ {
+ if (event_child->step_range_start == event_child->step_range_end)
+ fprintf (stderr, "GDB wanted to single-step, reporting event.\n");
+ else if (!lwp_in_step_range (event_child))
+ fprintf (stderr, "Out of step range, reporting event.\n");
+ }
if (event_child->stopped_by_watchpoint)
fprintf (stderr, "Stopped by watchpoint.\n");
if (gdb_breakpoint_here (event_child->stop_pc))
@@ -3377,6 +3405,9 @@ linux_set_resume_request (struct inferior_list_entry *entry, void *arg)
lwp->resume = &r->resume[ndx];
thread->last_resume_kind = lwp->resume->kind;
+ lwp->step_range_start = lwp->resume->step_range_start;
+ lwp->step_range_end = lwp->resume->step_range_end;
+
/* If we had a deferred signal to report, dequeue one now.
This can happen if LWP gets more than one signal while
trying to get out of a jump pad. */
@@ -5083,6 +5114,15 @@ linux_supports_agent (void)
return 1;
}
+static int
+linux_supports_range_stepping (void)
+{
+ if (*the_low_target.supports_range_stepping == NULL)
+ return 0;
+
+ return (*the_low_target.supports_range_stepping) ();
+}
+
/* Enumerate spufs IDs for process PID. */
static int
spu_enumerate_spu_ids (long pid, unsigned char *buf, CORE_ADDR offset, int len)
@@ -5939,6 +5979,7 @@ static struct target_ops linux_target_ops = {
NULL,
NULL,
#endif
+ linux_supports_range_stepping,
};
static void
diff --git a/gdb/gdbserver/linux-low.h b/gdb/gdbserver/linux-low.h
index 205d803..4dd3c9c 100644
--- a/gdb/gdbserver/linux-low.h
+++ b/gdb/gdbserver/linux-low.h
@@ -166,6 +166,8 @@ struct linux_target_ops
for use as a fast tracepoint. */
int (*get_min_fast_tracepoint_insn_len) (void);
+ /* Returns true if the low target supports range stepping. */
+ int (*supports_range_stepping) (void);
};
extern struct linux_target_ops the_low_target;
@@ -235,6 +237,12 @@ struct lwp_info
level on this process was a single-step. */
int stepping;
+ /* Range to single step within. This is a copy of the step range
+ passed along the last resume request. See 'struct
+ thread_resume'. */
+ CORE_ADDR step_range_start; /* Inclusive */
+ CORE_ADDR step_range_end; /* Exclusive */
+
/* If this flag is set, we need to set the event request flags the
next time we see this LWP stop. */
int must_set_ptrace_flags;
diff --git a/gdb/gdbserver/linux-x86-low.c b/gdb/gdbserver/linux-x86-low.c
index 31657d3..1d1df95 100644
--- a/gdb/gdbserver/linux-x86-low.c
+++ b/gdb/gdbserver/linux-x86-low.c
@@ -3175,6 +3175,12 @@ x86_emit_ops (void)
return &i386_emit_ops;
}
+static int
+x86_supports_range_stepping (void)
+{
+ return 1;
+}
+
/* This is initialized assuming an amd64 target.
x86_arch_setup will correct it for i386 or amd64 targets. */
@@ -3214,4 +3220,5 @@ struct linux_target_ops the_low_target =
x86_install_fast_tracepoint_jump_pad,
x86_emit_ops,
x86_get_min_fast_tracepoint_insn_len,
+ x86_supports_range_stepping,
};
diff --git a/gdb/gdbserver/server.c b/gdb/gdbserver/server.c
index 6bb36d8..1083aa9 100644
--- a/gdb/gdbserver/server.c
+++ b/gdb/gdbserver/server.c
@@ -2042,8 +2042,12 @@ handle_v_cont (char *own_buf)
{
p++;
+ memset (&resume_info[i], 0, sizeof resume_info[i]);
+
if (p[0] == 's' || p[0] == 'S')
resume_info[i].kind = resume_step;
+ else if (p[0] == 'r')
+ resume_info[i].kind = resume_step;
else if (p[0] == 'c' || p[0] == 'C')
resume_info[i].kind = resume_continue;
else if (p[0] == 't')
@@ -2063,9 +2067,22 @@ handle_v_cont (char *own_buf)
goto err;
resume_info[i].sig = gdb_signal_to_host (sig);
}
+ else if (p[0] == 'r')
+ {
+ char *p1;
+
+ p = p + 1;
+ p1 = strchr (p, ',');
+ decode_address (&resume_info[i].step_range_start, p, p1 - p);
+
+ p = p1 + 1;
+ p1 = strchr (p, ':');
+ decode_address (&resume_info[i].step_range_end, p, p1 - p);
+
+ p = p1;
+ }
else
{
- resume_info[i].sig = 0;
p = p + 1;
}
@@ -2311,6 +2328,11 @@ handle_v_requests (char *own_buf, int packet_len, int *new_packet_len)
if (strncmp (own_buf, "vCont?", 6) == 0)
{
strcpy (own_buf, "vCont;c;C;s;S;t");
+ if (target_supports_range_stepping ())
+ {
+ own_buf = own_buf + strlen (own_buf);
+ strcpy (own_buf, ";r");
+ }
return;
}
}
diff --git a/gdb/gdbserver/target.h b/gdb/gdbserver/target.h
index f257459..3a64975 100644
--- a/gdb/gdbserver/target.h
+++ b/gdb/gdbserver/target.h
@@ -57,6 +57,18 @@ struct thread_resume
linux; SuspendThread on win32). This is a host signal value (not
enum gdb_signal). */
int sig;
+
+ /* Range to single step within. Valid only iff KIND is resume_step.
+
+ Continuing stepping while the PC is in this range.
+
+ Note that STEP_RANGE_END is the address of the first instruction
+ beyond the step range, and NOT the address of the last
+ instruction within it. This has the property that START == END
+ means 'step only once', even if the instruction at START jumps to
+ START. */
+ CORE_ADDR step_range_start; /* Inclusive */
+ CORE_ADDR step_range_end; /* Exclusive */
};
/* Generally, what has the program done? */
@@ -414,6 +426,8 @@ struct target_ops
to break a cyclic dependency. */
void (*read_btrace) (struct btrace_target_info *, struct buffer *, int type);
+ /* Return true if target supports range stepping. */
+ int (*supports_range_stepping) (void);
};
extern struct target_ops *the_target;
@@ -549,6 +563,10 @@ int kill_inferior (int);
#define target_read_btrace(tinfo, buffer, type) \
(*the_target->read_btrace) (tinfo, buffer, type)
+#define target_supports_range_stepping() \
+ (the_target->supports_range_stepping ? \
+ (*the_target->supports_range_stepping) () : 0)
+
/* Start non-stop mode, returns 0 on success, -1 on failure. */
int start_non_stop (int nonstop);
^ permalink raw reply [flat|nested] 38+ messages in thread* Re: [PATCH 4/5] range stepping: gdbserver (x86 GNU/Linux)
2013-05-14 19:10 ` [PATCH 4/5] range stepping: gdbserver (x86 GNU/Linux) Pedro Alves
@ 2013-05-14 19:47 ` Eli Zaretskii
2013-05-14 20:14 ` Tom Tromey
` (2 subsequent siblings)
3 siblings, 0 replies; 38+ messages in thread
From: Eli Zaretskii @ 2013-05-14 19:47 UTC (permalink / raw)
To: Pedro Alves; +Cc: gdb-patches
> From: Pedro Alves <palves@redhat.com>
> Date: Tue, 14 May 2013 20:10:54 +0100
>
> This patch adds support for range stepping to GDBserver, teaching it
> about vCont;r.
OK for the NEWS part. Thanks.
^ permalink raw reply [flat|nested] 38+ messages in thread* Re: [PATCH 4/5] range stepping: gdbserver (x86 GNU/Linux)
2013-05-14 19:10 ` [PATCH 4/5] range stepping: gdbserver (x86 GNU/Linux) Pedro Alves
2013-05-14 19:47 ` Eli Zaretskii
@ 2013-05-14 20:14 ` Tom Tromey
2013-05-23 17:44 ` Pedro Alves
2013-05-15 12:14 ` Yao Qi
2013-05-23 0:56 ` Yao Qi
3 siblings, 1 reply; 38+ messages in thread
From: Tom Tromey @ 2013-05-14 20:14 UTC (permalink / raw)
To: Pedro Alves; +Cc: gdb-patches
Pedro> + /* Note that all addresses are always "out the step range" when
Pedro> + there's no range to begin with. */
I think "out of the step range".
Pedro> + else if (p[0] == 'r')
Pedro> + {
Pedro> + char *p1;
Pedro> +
Pedro> + p = p + 1;
Pedro> + p1 = strchr (p, ',');
Pedro> + decode_address (&resume_info[i].step_range_start, p, p1 - p);
Pedro> +
Pedro> + p = p1 + 1;
Pedro> + p1 = strchr (p, ':');
Pedro> + decode_address (&resume_info[i].step_range_end, p, p1 - p);
Pedro> +
Pedro> + p = p1;
Pedro> + }
I don't know much about gdbserver, but reading this made me wonder if it
needs to do any kind of error-checking on its input.
Like - what if the wrong format is sent, or if the range end is before
the range start?
Tom
^ permalink raw reply [flat|nested] 38+ messages in thread* Re: [PATCH 4/5] range stepping: gdbserver (x86 GNU/Linux)
2013-05-14 20:14 ` Tom Tromey
@ 2013-05-23 17:44 ` Pedro Alves
2013-05-24 11:33 ` Pedro Alves
0 siblings, 1 reply; 38+ messages in thread
From: Pedro Alves @ 2013-05-23 17:44 UTC (permalink / raw)
To: Tom Tromey; +Cc: gdb-patches
On 05/14/2013 09:14 PM, Tom Tromey wrote:
> Pedro> + /* Note that all addresses are always "out the step range" when
> Pedro> + there's no range to begin with. */
>
> I think "out of the step range".
Thanks. I fixed that one.
> Pedro> + else if (p[0] == 'r')
> Pedro> + {
> Pedro> + char *p1;
> Pedro> +
> Pedro> + p = p + 1;
> Pedro> + p1 = strchr (p, ',');
> Pedro> + decode_address (&resume_info[i].step_range_start, p, p1 - p);
> Pedro> +
> Pedro> + p = p1 + 1;
> Pedro> + p1 = strchr (p, ':');
> Pedro> + decode_address (&resume_info[i].step_range_end, p, p1 - p);
> Pedro> +
> Pedro> + p = p1;
> Pedro> + }
>
> I don't know much about gdbserver, but reading this made me wonder if it
> needs to do any kind of error-checking on its input.
Yeah, it doesn't tend to do that much validation. Perhaps we should.
> Like - what if the wrong format is sent,
Crash in many different places most likely. When looking at
validation, I'm more looking at making it easier for possible
future packet extensions not break older gdbserver.
In this case, I do believe this bit:
+ p1 = strchr (p, ':');
+ decode_address (&resume_info[i].step_range_end, p, p1 - p);
should not expect the ':' to be there. An action
without a ptid is valid. I means it applies to all and
is handled as the default action, further below:
if (p[0] == 0)
{
resume_info[i].thread = minus_one_ptid;
default_action = resume_info[i];
/* Note: we don't increment i here, we'll overwrite this entry
the next time through. */
}
else if (p[0] == ':')
I'll fix it in a follow up.
> or if the range end is before the range start?
Yeah, considering that MIPS has signed addresses, I'm
leaving this one open.
--
Pedro Alves
^ permalink raw reply [flat|nested] 38+ messages in thread* Re: [PATCH 4/5] range stepping: gdbserver (x86 GNU/Linux)
2013-05-23 17:44 ` Pedro Alves
@ 2013-05-24 11:33 ` Pedro Alves
0 siblings, 0 replies; 38+ messages in thread
From: Pedro Alves @ 2013-05-24 11:33 UTC (permalink / raw)
To: gdb-patches; +Cc: Tom Tromey
On 05/23/2013 06:44 PM, Pedro Alves wrote:
>> I don't know much about gdbserver, but reading this made me wonder if it
>> needs to do any kind of error-checking on its input.
>
> Yeah, it doesn't tend to do that much validation. Perhaps we should.
>
>> Like - what if the wrong format is sent,
>
> Crash in many different places most likely. When looking at
> validation, I'm more looking at making it easier for possible
> future packet extensions not break older gdbserver.
>
> In this case, I do believe this bit:
>
> + p1 = strchr (p, ':');
> + decode_address (&resume_info[i].step_range_end, p, p1 - p);
>
> should not expect the ':' to be there. An action
> without a ptid is valid. I means it applies to all and
> is handled as the default action, further below:
>
> if (p[0] == 0)
> {
> resume_info[i].thread = minus_one_ptid;
> default_action = resume_info[i];
>
> /* Note: we don't increment i here, we'll overwrite this entry
> the next time through. */
> }
> else if (p[0] == ':')
>
> I'll fix it in a follow up.
Fixed now:
http://sourceware.org/ml/gdb-patches/2013-05/msg00923.html
Thanks!
--
Pedro Alves
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [PATCH 4/5] range stepping: gdbserver (x86 GNU/Linux)
2013-05-14 19:10 ` [PATCH 4/5] range stepping: gdbserver (x86 GNU/Linux) Pedro Alves
2013-05-14 19:47 ` Eli Zaretskii
2013-05-14 20:14 ` Tom Tromey
@ 2013-05-15 12:14 ` Yao Qi
2013-05-20 18:01 ` Pedro Alves
2013-05-23 0:56 ` Yao Qi
3 siblings, 1 reply; 38+ messages in thread
From: Yao Qi @ 2013-05-15 12:14 UTC (permalink / raw)
To: Pedro Alves; +Cc: gdb-patches
On 05/15/2013 03:10 AM, Pedro Alves wrote:
> - Moved step range bits out of gdbthread.h/inferiors.c into linux-low.c.
Pedro,
What is your reason to do this move? because the range stepping is only
implemented on Linux? I chose to add in generic code because I thought
range stepping can be implemented for other targets.
--
Yao (é½å°§)
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [PATCH 4/5] range stepping: gdbserver (x86 GNU/Linux)
2013-05-15 12:14 ` Yao Qi
@ 2013-05-20 18:01 ` Pedro Alves
0 siblings, 0 replies; 38+ messages in thread
From: Pedro Alves @ 2013-05-20 18:01 UTC (permalink / raw)
To: Yao Qi; +Cc: gdb-patches
On 05/15/2013 01:13 PM, Yao Qi wrote:
> On 05/15/2013 03:10 AM, Pedro Alves wrote:
>> - Moved step range bits out of gdbthread.h/inferiors.c into linux-low.c.
>
> Pedro,
> What is your reason to do this move? because the range stepping is only implemented on Linux? I chose to add in generic code because I thought range stepping can be implemented for other targets.
Yes, kind of. At first, I saw that the v1/v2 implementation didn't
allow wildcard ptids, and I thought that it'd be best not to error
on it, as nothing in the protocol actually prohibits it. The way gdbserver
handles vCont requests currently is by letting the target match the ptid
to whatever thread/lwp. In the case of Linux, that's in
linux_set_resume_request. After putting the range in struct thread_resume,
and making linux_set_resume_request always copy it, as in:
thread->step_range_start = lwp->resume->step_range_start;
thread->step_range_end = lwp->resume->step_range_end;
I noticed that nothing outside of linux-low.c actually called
thread_clear_range_stepping, so I removed that. Then, I noticed that
thread_in_range_stepping_p was never used outside linux-low.c (and won't
unless gdbserver's backends are made to always do non-stop instead of pausing
all threads itself, along with all the step-over-breakpoint complication).
So in the end, seeing how all was always in the target, I just went with
leaving the fields Linux specific, rather than leave the fields dangling on
other targets, thinking it'd be easy enough to reconsider once other
targets implement range stepping. Meanwhile, other targets wouldn't
pay for extra fields they don't use.
--
Pedro Alves
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [PATCH 4/5] range stepping: gdbserver (x86 GNU/Linux)
2013-05-14 19:10 ` [PATCH 4/5] range stepping: gdbserver (x86 GNU/Linux) Pedro Alves
` (2 preceding siblings ...)
2013-05-15 12:14 ` Yao Qi
@ 2013-05-23 0:56 ` Yao Qi
2013-05-23 17:26 ` Pedro Alves
3 siblings, 1 reply; 38+ messages in thread
From: Yao Qi @ 2013-05-23 0:56 UTC (permalink / raw)
To: Pedro Alves; +Cc: gdb-patches
On 05/15/2013 03:10 AM, Pedro Alves wrote:
> @@ -2063,9 +2067,22 @@ handle_v_cont (char *own_buf)
> goto err;
> resume_info[i].sig = gdb_signal_to_host (sig);
> }
> + else if (p[0] == 'r')
> + {
> + char *p1;
> +
> + p = p + 1;
> + p1 = strchr (p, ',');
> + decode_address (&resume_info[i].step_range_start, p, p1 - p);
> +
> + p = p1 + 1;
> + p1 = strchr (p, ':');
> + decode_address (&resume_info[i].step_range_end, p, p1 - p);
> +
> + p = p1;
> + }
> else
> {
> - resume_info[i].sig = 0;
What is the purpose to remove this line?
> p = p + 1;
> }
The patch looks good to me.
> So in the end, seeing how all was always in the target, I just went with
> leaving the fields Linux specific, rather than leave the fields dangling on
> other targets, thinking it'd be easy enough to reconsider once other
> targets implement range stepping. Meanwhile, other targets wouldn't
> pay for extra fields they don't use.
OK, that is fine to me.
--
Yao (é½å°§)
^ permalink raw reply [flat|nested] 38+ messages in thread* Re: [PATCH 4/5] range stepping: gdbserver (x86 GNU/Linux)
2013-05-23 0:56 ` Yao Qi
@ 2013-05-23 17:26 ` Pedro Alves
0 siblings, 0 replies; 38+ messages in thread
From: Pedro Alves @ 2013-05-23 17:26 UTC (permalink / raw)
To: Yao Qi; +Cc: gdb-patches
On 05/23/2013 01:56 AM, Yao Qi wrote:
> On 05/15/2013 03:10 AM, Pedro Alves wrote:
>> @@ -2063,9 +2067,22 @@ handle_v_cont (char *own_buf)
>> goto err;
>> resume_info[i].sig = gdb_signal_to_host (sig);
>> }
>> + else if (p[0] == 'r')
>> + {
>> + char *p1;
>> +
>> + p = p + 1;
>> + p1 = strchr (p, ',');
>> + decode_address (&resume_info[i].step_range_start, p, p1 - p);
>> +
>> + p = p1 + 1;
>> + p1 = strchr (p, ':');
>> + decode_address (&resume_info[i].step_range_end, p, p1 - p);
>> +
>> + p = p1;
>> + }
>> else
>> {
>> - resume_info[i].sig = 0;
>
> What is the purpose to remove this line?
It's now unnecessary, given the new memset call added
further above:
@@ -2042,8 +2042,12 @@ handle_v_cont (char *own_buf)
{
p++;
+ memset (&resume_info[i], 0, sizeof resume_info[i]);
>>
>
> The patch looks good to me.
Thanks. Here's what I checked in. I adjusted the step_range_start/end
comment in the same spirit it was adjusted in the documentation and on
the gdb side. It also adds the missing word Tom pointed at.
----
range stepping: gdbserver (x86 GNU/Linux)
This patch adds support for range stepping to GDBserver, teaching it
about vCont;r.
It'd be easy to enable this for all hardware single-step targets
without needing the linux_target_ops hook, however, at least PPC needs
special care, due to the fact that PPC atomic sequences can't be
hardware single-stepped through, a thing which GDBserver doesn't know
about. So this leaves the support limited to x86/x86_64.
gdb/
2013-05-23 Pedro Alves <palves@redhat.com>
* NEWS: Mention GDBserver range stepping support.
gdb/gdbserver/
2013-05-23 Yao Qi <yao@codesourcery.com>
Pedro Alves <palves@redhat.com>
* linux-low.c (lwp_in_step_range): New function.
(linux_wait_1): If the thread was range stepping and stopped
outside the stepping range, report the stop to GDB. Otherwise,
continue stepping. Add range stepping debug output.
(linux_set_resume_request): Copy the step range from the resume
request to the lwp.
(linux_supports_range_stepping): New.
(linux_target_ops) <supports_range_stepping>: Set to
linux_supports_range_stepping.
* linux-low.h (struct linux_target_ops)
<supports_range_stepping>: New field.
(struct lwp_info) <step_range_start, step_range_end>: New fields.
* linux-x86-low.c (x86_supports_range_stepping): New.
(the_low_target) <supports_range_stepping>: Set to
x86_supports_range_stepping.
* server.c (handle_v_cont): Handle 'r' action.
(handle_v_requests): Append ";r" if the target supports range
stepping.
* target.h (struct thread_resume) <step_range_start,
step_range_end>: New fields.
(struct target_ops) <supports_range_stepping>:
New field.
(target_supports_range_stepping): New macro.
---
gdb/NEWS | 5 +++
gdb/gdbserver/linux-low.c | 63 ++++++++++++++++++++++++++++++++++-------
gdb/gdbserver/linux-low.h | 8 +++++
gdb/gdbserver/linux-x86-low.c | 7 +++++
gdb/gdbserver/server.c | 24 +++++++++++++++-
gdb/gdbserver/target.h | 15 ++++++++++
6 files changed, 110 insertions(+), 12 deletions(-)
diff --git a/gdb/NEWS b/gdb/NEWS
index 1a4140d..a23e8e3 100644
--- a/gdb/NEWS
+++ b/gdb/NEWS
@@ -95,6 +95,11 @@ vCont;r
stub to step through an address range itself, without GDB
involvemement at each single-step.
+* New features in the GDB remote stub, GDBserver
+
+ ** GDBserver now supports target-assisted range stepping. Currently
+ enabled on x86/x86_64 GNU/Linux targets.
+
*** Changes in GDB 7.6
* Target record has been renamed to record-full.
diff --git a/gdb/gdbserver/linux-low.c b/gdb/gdbserver/linux-low.c
index b01b37c..7e18942 100644
--- a/gdb/gdbserver/linux-low.c
+++ b/gdb/gdbserver/linux-low.c
@@ -276,6 +276,16 @@ supports_fast_tracepoints (void)
return the_low_target.install_fast_tracepoint_jump_pad != NULL;
}
+/* True if LWP is stopped in its stepping range. */
+
+static int
+lwp_in_step_range (struct lwp_info *lwp)
+{
+ CORE_ADDR pc = lwp->stop_pc;
+
+ return (pc >= lwp->step_range_start && pc < lwp->step_range_end);
+}
+
struct pending_signals
{
int signal;
@@ -2337,6 +2347,7 @@ linux_wait_1 (ptid_t ptid,
int maybe_internal_trap;
int report_to_gdb;
int trace_event;
+ int in_step_range;
/* Translate generic target options into linux options. */
options = __WALL;
@@ -2346,6 +2357,7 @@ linux_wait_1 (ptid_t ptid,
retry:
bp_explains_trap = 0;
trace_event = 0;
+ in_step_range = 0;
ourstatus->kind = TARGET_WAITKIND_IGNORE;
/* If we were only supposed to resume one thread, only wait for
@@ -2639,18 +2651,24 @@ Check if we're already there.\n",
goto retry;
}
- /* If GDB wanted this thread to single step, we always want to
- report the SIGTRAP, and let GDB handle it. Watchpoints should
- always be reported. So should signals we can't explain. A
- SIGTRAP we can't explain could be a GDB breakpoint --- we may or
- not support Z0 breakpoints. If we do, we're be able to handle
- GDB breakpoints on top of internal breakpoints, by handling the
- internal breakpoint and still reporting the event to GDB. If we
- don't, we're out of luck, GDB won't see the breakpoint hit. */
+ /* 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);
+
+ /* If GDB wanted this thread to single step, and the thread is out
+ of the step range, we always want to report the SIGTRAP, and let
+ GDB handle it. Watchpoints should always be reported. So should
+ signals we can't explain. A SIGTRAP we can't explain could be a
+ GDB breakpoint --- we may or not support Z0 breakpoints. If we
+ do, we're be able to handle GDB breakpoints on top of internal
+ breakpoints, by handling the internal breakpoint and still
+ reporting the event to GDB. If we don't, we're out of luck, GDB
+ won't see the breakpoint hit. */
report_to_gdb = (!maybe_internal_trap
- || current_inferior->last_resume_kind == resume_step
+ || (current_inferior->last_resume_kind == resume_step
+ && !in_step_range)
|| event_child->stopped_by_watchpoint
- || (!step_over_finished
+ || (!step_over_finished && !in_step_range
&& !bp_explains_trap && !trace_event)
|| (gdb_breakpoint_here (event_child->stop_pc)
&& gdb_condition_true_at_breakpoint (event_child->stop_pc)
@@ -2671,6 +2689,11 @@ Check if we're already there.\n",
fprintf (stderr, "Step-over finished.\n");
if (trace_event)
fprintf (stderr, "Tracepoint event.\n");
+ if (lwp_in_step_range (event_child))
+ fprintf (stderr, "Range stepping pc 0x%s [0x%s, 0x%s).\n",
+ paddress (event_child->stop_pc),
+ paddress (event_child->step_range_start),
+ paddress (event_child->step_range_end));
}
/* We're not reporting this breakpoint to GDB, so apply the
@@ -2702,7 +2725,12 @@ Check if we're already there.\n",
if (debug_threads)
{
if (current_inferior->last_resume_kind == resume_step)
- fprintf (stderr, "GDB wanted to single-step, reporting event.\n");
+ {
+ if (event_child->step_range_start == event_child->step_range_end)
+ fprintf (stderr, "GDB wanted to single-step, reporting event.\n");
+ else if (!lwp_in_step_range (event_child))
+ fprintf (stderr, "Out of step range, reporting event.\n");
+ }
if (event_child->stopped_by_watchpoint)
fprintf (stderr, "Stopped by watchpoint.\n");
if (gdb_breakpoint_here (event_child->stop_pc))
@@ -3401,6 +3429,9 @@ linux_set_resume_request (struct inferior_list_entry *entry, void *arg)
lwp->resume = &r->resume[ndx];
thread->last_resume_kind = lwp->resume->kind;
+ lwp->step_range_start = lwp->resume->step_range_start;
+ lwp->step_range_end = lwp->resume->step_range_end;
+
/* If we had a deferred signal to report, dequeue one now.
This can happen if LWP gets more than one signal while
trying to get out of a jump pad. */
@@ -5094,6 +5125,15 @@ linux_supports_agent (void)
return 1;
}
+static int
+linux_supports_range_stepping (void)
+{
+ if (*the_low_target.supports_range_stepping == NULL)
+ return 0;
+
+ return (*the_low_target.supports_range_stepping) ();
+}
+
/* Enumerate spufs IDs for process PID. */
static int
spu_enumerate_spu_ids (long pid, unsigned char *buf, CORE_ADDR offset, int len)
@@ -5952,6 +5992,7 @@ static struct target_ops linux_target_ops = {
NULL,
NULL,
#endif
+ linux_supports_range_stepping,
};
static void
diff --git a/gdb/gdbserver/linux-low.h b/gdb/gdbserver/linux-low.h
index 205d803..4dd3c9c 100644
--- a/gdb/gdbserver/linux-low.h
+++ b/gdb/gdbserver/linux-low.h
@@ -166,6 +166,8 @@ struct linux_target_ops
for use as a fast tracepoint. */
int (*get_min_fast_tracepoint_insn_len) (void);
+ /* Returns true if the low target supports range stepping. */
+ int (*supports_range_stepping) (void);
};
extern struct linux_target_ops the_low_target;
@@ -235,6 +237,12 @@ struct lwp_info
level on this process was a single-step. */
int stepping;
+ /* Range to single step within. This is a copy of the step range
+ passed along the last resume request. See 'struct
+ thread_resume'. */
+ CORE_ADDR step_range_start; /* Inclusive */
+ CORE_ADDR step_range_end; /* Exclusive */
+
/* If this flag is set, we need to set the event request flags the
next time we see this LWP stop. */
int must_set_ptrace_flags;
diff --git a/gdb/gdbserver/linux-x86-low.c b/gdb/gdbserver/linux-x86-low.c
index 31657d3..1d1df95 100644
--- a/gdb/gdbserver/linux-x86-low.c
+++ b/gdb/gdbserver/linux-x86-low.c
@@ -3175,6 +3175,12 @@ x86_emit_ops (void)
return &i386_emit_ops;
}
+static int
+x86_supports_range_stepping (void)
+{
+ return 1;
+}
+
/* This is initialized assuming an amd64 target.
x86_arch_setup will correct it for i386 or amd64 targets. */
@@ -3214,4 +3220,5 @@ struct linux_target_ops the_low_target =
x86_install_fast_tracepoint_jump_pad,
x86_emit_ops,
x86_get_min_fast_tracepoint_insn_len,
+ x86_supports_range_stepping,
};
diff --git a/gdb/gdbserver/server.c b/gdb/gdbserver/server.c
index 6bb36d8..1083aa9 100644
--- a/gdb/gdbserver/server.c
+++ b/gdb/gdbserver/server.c
@@ -2042,8 +2042,12 @@ handle_v_cont (char *own_buf)
{
p++;
+ memset (&resume_info[i], 0, sizeof resume_info[i]);
+
if (p[0] == 's' || p[0] == 'S')
resume_info[i].kind = resume_step;
+ else if (p[0] == 'r')
+ resume_info[i].kind = resume_step;
else if (p[0] == 'c' || p[0] == 'C')
resume_info[i].kind = resume_continue;
else if (p[0] == 't')
@@ -2063,9 +2067,22 @@ handle_v_cont (char *own_buf)
goto err;
resume_info[i].sig = gdb_signal_to_host (sig);
}
+ else if (p[0] == 'r')
+ {
+ char *p1;
+
+ p = p + 1;
+ p1 = strchr (p, ',');
+ decode_address (&resume_info[i].step_range_start, p, p1 - p);
+
+ p = p1 + 1;
+ p1 = strchr (p, ':');
+ decode_address (&resume_info[i].step_range_end, p, p1 - p);
+
+ p = p1;
+ }
else
{
- resume_info[i].sig = 0;
p = p + 1;
}
@@ -2311,6 +2328,11 @@ handle_v_requests (char *own_buf, int packet_len, int *new_packet_len)
if (strncmp (own_buf, "vCont?", 6) == 0)
{
strcpy (own_buf, "vCont;c;C;s;S;t");
+ if (target_supports_range_stepping ())
+ {
+ own_buf = own_buf + strlen (own_buf);
+ strcpy (own_buf, ";r");
+ }
return;
}
}
diff --git a/gdb/gdbserver/target.h b/gdb/gdbserver/target.h
index f257459..c57cb40 100644
--- a/gdb/gdbserver/target.h
+++ b/gdb/gdbserver/target.h
@@ -57,6 +57,15 @@ struct thread_resume
linux; SuspendThread on win32). This is a host signal value (not
enum gdb_signal). */
int sig;
+
+ /* Range to single step within. Valid only iff KIND is resume_step.
+
+ Single-step once, and then continuing stepping as long as the
+ thread stops in this range. (If the range is empty
+ [STEP_RANGE_START == STEP_RANGE_END], then this is a single-step
+ request.) */
+ CORE_ADDR step_range_start; /* Inclusive */
+ CORE_ADDR step_range_end; /* Exclusive */
};
/* Generally, what has the program done? */
@@ -414,6 +423,8 @@ struct target_ops
to break a cyclic dependency. */
void (*read_btrace) (struct btrace_target_info *, struct buffer *, int type);
+ /* Return true if target supports range stepping. */
+ int (*supports_range_stepping) (void);
};
extern struct target_ops *the_target;
@@ -549,6 +560,10 @@ int kill_inferior (int);
#define target_read_btrace(tinfo, buffer, type) \
(*the_target->read_btrace) (tinfo, buffer, type)
+#define target_supports_range_stepping() \
+ (the_target->supports_range_stepping ? \
+ (*the_target->supports_range_stepping) () : 0)
+
/* Start non-stop mode, returns 0 on success, -1 on failure. */
int start_non_stop (int nonstop);
^ permalink raw reply [flat|nested] 38+ messages in thread
* [PATCH 2/5] Convert rs->support_vCont_t to a struct
2013-05-14 19:10 [PATCH 0/5 V3] target-assisted range stepping Pedro Alves
2013-05-14 19:10 ` [PATCH 4/5] range stepping: gdbserver (x86 GNU/Linux) Pedro Alves
@ 2013-05-14 19:10 ` Pedro Alves
2013-05-14 19:40 ` Tom Tromey
2013-05-14 19:10 ` [PATCH 1/5] Factor out in-stepping-range checks Pedro Alves
` (4 subsequent siblings)
6 siblings, 1 reply; 38+ messages in thread
From: Pedro Alves @ 2013-05-14 19:10 UTC (permalink / raw)
To: gdb-patches
Convert the 'support_vCont_t' int field to a struct, in preparation
for adding more fields to it.
v3:
- s/support/supports/
- struct renamed.
- comments tweaked.
gdb/
2013-05-14 Yao Qi <yao@codesourcery.com>
Pedro Alves <palves@redhat.com>
* remote.c (struct vCont_action_support): New struct.
(struct remote_state) <support_vCont_t>: Remove field.
<vCont_actions_support>: New field.
(remote_vcont_probe, remote_stop_ns): Update.
---
gdb/remote.c | 21 ++++++++++++++++-----
1 file changed, 16 insertions(+), 5 deletions(-)
diff --git a/gdb/remote.c b/gdb/remote.c
index 7cae940..5d3950b 100644
--- a/gdb/remote.c
+++ b/gdb/remote.c
@@ -251,6 +251,17 @@ static struct cmd_list_element *remote_cmdlist;
static struct cmd_list_element *remote_set_cmdlist;
static struct cmd_list_element *remote_show_cmdlist;
+/* Stub vCont actions support.
+
+ Each field is a boolean flag indicating whether the stub reports
+ support for the corresponding action. */
+
+struct vCont_action_support
+{
+ /* vCont;t */
+ int t;
+};
+
/* Description of the remote protocol state for the currently
connected target. This is per-target state, and independent of the
selected architecture. */
@@ -308,8 +319,8 @@ struct remote_state
/* True if the stub reports support for non-stop mode. */
int non_stop_aware;
- /* True if the stub reports support for vCont;t. */
- int support_vCont_t;
+ /* The status of the stub support for the various vCont actions. */
+ struct vCont_action_support supports_vCont;
/* True if the stub reports support for conditional tracepoints. */
int cond_tracepoints;
@@ -4641,7 +4652,7 @@ remote_vcont_probe (struct remote_state *rs)
support_S = 0;
support_c = 0;
support_C = 0;
- rs->support_vCont_t = 0;
+ rs->supports_vCont.t = 0;
while (p && *p == ';')
{
p++;
@@ -4654,7 +4665,7 @@ remote_vcont_probe (struct remote_state *rs)
else if (*p == 'C' && (*(p + 1) == ';' || *(p + 1) == 0))
support_C = 1;
else if (*p == 't' && (*(p + 1) == ';' || *(p + 1) == 0))
- rs->support_vCont_t = 1;
+ rs->supports_vCont.t = 1;
p = strchr (p, ';');
}
@@ -5003,7 +5014,7 @@ remote_stop_ns (ptid_t ptid)
if (remote_protocol_packets[PACKET_vCont].support == PACKET_SUPPORT_UNKNOWN)
remote_vcont_probe (rs);
- if (!rs->support_vCont_t)
+ if (!rs->supports_vCont.t)
error (_("Remote server does not support stopping threads"));
if (ptid_equal (ptid, minus_one_ptid)
^ permalink raw reply [flat|nested] 38+ messages in thread* [PATCH 1/5] Factor out in-stepping-range checks.
2013-05-14 19:10 [PATCH 0/5 V3] target-assisted range stepping Pedro Alves
2013-05-14 19:10 ` [PATCH 4/5] range stepping: gdbserver (x86 GNU/Linux) Pedro Alves
2013-05-14 19:10 ` [PATCH 2/5] Convert rs->support_vCont_t to a struct Pedro Alves
@ 2013-05-14 19:10 ` Pedro Alves
2013-05-14 19:37 ` Tom Tromey
2013-05-14 19:10 ` [PATCH 3/5] range stepping: gdb Pedro Alves
` (3 subsequent siblings)
6 siblings, 1 reply; 38+ messages in thread
From: Pedro Alves @ 2013-05-14 19:10 UTC (permalink / raw)
To: gdb-patches
This adds a function for doing within-thread's-stepping-range checks, and converts
a couple spots to use it. Following patches will add more uses.
v3:
- macro -> function, and rename it.
gdb/
2013-05-09 Yao Qi <yao@codesourcery.com>
Pedro Alves <palves@redhat.com>
* gdbthread.h (pc_in_thread_step_range): New declaration.
* thread.c (pc_in_thread_step_range): New function.
* infrun.c (handle_inferior_event): Use it.
---
gdb/gdbthread.h | 4 ++++
gdb/infrun.c | 6 ++----
gdb/thread.c | 7 +++++++
3 files changed, 13 insertions(+), 4 deletions(-)
diff --git a/gdb/gdbthread.h b/gdb/gdbthread.h
index 0846322..a9f8a94 100644
--- a/gdb/gdbthread.h
+++ b/gdb/gdbthread.h
@@ -399,6 +399,10 @@ extern struct thread_info* inferior_thread (void);
extern void update_thread_list (void);
+/* Return true if PC is in the stepping range of THREAD. */
+
+int pc_in_thread_step_range (CORE_ADDR pc, struct thread_info *thread);
+
extern struct thread_info *thread_list;
#endif /* GDBTHREAD_H */
diff --git a/gdb/infrun.c b/gdb/infrun.c
index 54e92f2..57c427d 100644
--- a/gdb/infrun.c
+++ b/gdb/infrun.c
@@ -4337,8 +4337,7 @@ process_event_stop_test:
if (ecs->event_thread->control.step_range_end != 0
&& ecs->event_thread->suspend.stop_signal != GDB_SIGNAL_0
- && (ecs->event_thread->control.step_range_start <= stop_pc
- && stop_pc < ecs->event_thread->control.step_range_end)
+ && pc_in_thread_step_range (stop_pc, ecs->event_thread)
&& frame_id_eq (get_stack_frame_id (frame),
ecs->event_thread->control.step_stack_frame_id)
&& ecs->event_thread->control.step_resume_breakpoint == NULL)
@@ -4707,8 +4706,7 @@ process_event_stop_test:
through a function epilogue and therefore must detect when
the current-frame changes in the middle of a line. */
- if (stop_pc >= ecs->event_thread->control.step_range_start
- && stop_pc < ecs->event_thread->control.step_range_end
+ if (pc_in_thread_step_range (stop_pc, ecs->event_thread)
&& (execution_direction != EXEC_REVERSE
|| frame_id_eq (get_frame_id (frame),
ecs->event_thread->control.step_frame_id)))
diff --git a/gdb/thread.c b/gdb/thread.c
index 2a1d723..2eb506b 100644
--- a/gdb/thread.c
+++ b/gdb/thread.c
@@ -750,6 +750,13 @@ finish_thread_state_cleanup (void *arg)
finish_thread_state (*ptid_p);
}
+int
+pc_in_thread_step_range (CORE_ADDR pc, struct thread_info *thread)
+{
+ return (pc >= thread->control.step_range_start
+ && pc < thread->control.step_range_end);
+}
+
/* Prints the list of threads and their details on UIOUT.
This is a version of 'info_threads_command' suitable for
use from MI.
^ permalink raw reply [flat|nested] 38+ messages in thread* [PATCH 3/5] range stepping: gdb
2013-05-14 19:10 [PATCH 0/5 V3] target-assisted range stepping Pedro Alves
` (2 preceding siblings ...)
2013-05-14 19:10 ` [PATCH 1/5] Factor out in-stepping-range checks Pedro Alves
@ 2013-05-14 19:10 ` Pedro Alves
2013-05-14 19:46 ` Eli Zaretskii
2013-05-14 19:11 ` [PATCH 5/5] range stepping: tests Pedro Alves
` (2 subsequent siblings)
6 siblings, 1 reply; 38+ messages in thread
From: Pedro Alves @ 2013-05-14 19:10 UTC (permalink / raw)
To: gdb-patches
(text based on Yao's)
This patch teaches GDB to take advantage of target-assisted range
stepping. It adds a new 'r ADDR1,ADDR2' action to vCont (vCont;r),
meaning, "keep stepping as long as the thread is in the [ADDR1,ADDR2)
range".
Rationale:
When user issues the "step" command on the following line of source,
a = b + c + d * e - a;
GDB single-steps every single instruction until the program reaches a
new different line. E.g., on x86_64, that line compiles to:
0x08048434 <+65>: mov 0x1c(%esp),%eax
0x08048438 <+69>: mov 0x30(%esp),%edx
0x0804843c <+73>: add %eax,%edx
0x0804843e <+75>: mov 0x18(%esp),%eax
0x08048442 <+79>: imul 0x2c(%esp),%eax
0x08048447 <+84>: add %edx,%eax
0x08048449 <+86>: sub 0x34(%esp),%eax
0x0804844d <+90>: mov %eax,0x34(%esp)
0x08048451 <+94>: mov 0x1c(%esp),%eax
and the following is the RSP traffic between GDB and GDBserver:
--> vCont;s:p2e13.2e13;c
<-- T0505:68efffbf;04:30efffbf;08:3c840408;thread:p2e13.2e13;core:1;
--> vCont;s:p2e13.2e13;c
<-- T0505:68efffbf;04:30efffbf;08:3e840408;thread:p2e13.2e13;core:2;
--> vCont;s:p2e13.2e13;c
<-- T0505:68efffbf;04:30efffbf;08:42840408;thread:p2e13.2e13;core:2;
--> vCont;s:p2e13.2e13;c
<-- T0505:68efffbf;04:30efffbf;08:47840408;thread:p2e13.2e13;core:0;
--> vCont;s:p2e13.2e13;c
<-- T0505:68efffbf;04:30efffbf;08:49840408;thread:p2e13.2e13;core:0;
--> vCont;s:p2e13.2e13;c
<-- T0505:68efffbf;04:30efffbf;08:4d840408;thread:p2e13.2e13;core:0;
--> vCont;s:p2e13.2e13;c
<-- T0505:68efffbf;04:30efffbf;08:51840408;thread:p2e13.2e13;core:0;
IOW, a lot of roundtrips between GDB and GDBserver.
If we add a new command to the RSP, meaning "keep stepping and don't
report a stop until the program goes out of the [0x08048434,
0x08048451) address range", then the RSP traffic can be reduced down
to:
--> vCont;r8048434,8048451:p2db0.2db0;c
<-- T0505:68efffbf;04:30efffbf;08:51840408;thread:p2db0.2db0;core:1;
As number of packets is reduced dramatically, the performance of
stepping source lines is much improved.
In case something is wrong with range stepping on the stub side, the
debug info or even gdb, this adds a "set/show range-stepping" command
to be able to turn range stepping off.
v3:
- no longer use "is PC-in-stepping-range" or trap_expected checks deep
in remote.c's target_resume to decide whether to send vCont;r or
vCont;s. It is better to leave that choice to higher layers (IOW,
infrun.c/infcmd.c).
- "maint set/show range-stepping" renamed to "set/show range-stepping"
folded into this patch. The docs now explain what is range
stepping, and why a user might need this command.
- NEWS and docs for GDB bits folded into this patch, and
redone/expanded.
gdb/
2013-05-14 Yao Qi <yao@codesourcery.com>
Pedro Alves <palves@redhat.com>
* gdbthread.h (struct thread_control_state) <may_range_step>: New
field.
* infcmd.c (step_once, until_next_command): Enable range stepping.
* infrun.c (displaced_step_prepare): Disable range stepping.
(resume): Disable range stepping if stepping over a breakpoint or
we have software watchpoints. If range stepping is enabled,
assert the thread is in the stepping range.
(clear_proceed_status_thread): Clear may_range_step.
(handle_inferior_event): Disable range stepping as soon as we know
the thread that hit the event. Re-enable it whenever we're going
to step with a step range.
* remote.c (struct vCont_action_support) <r>: New field.
(use_range_stepping): New global.
(remote_vcont_probe): Handle 'r' action.
(append_resumption): Append an 'r' action if the thread may range
step.
(show_range_stepping): New function.
(set_range_stepping): New function.
(_initialize_remote): Call add_setshow_boolean_cmd to register the
'set range-stepping' and 'show range-stepping' commands.
* NEWS: Mention range stepping, the new vCont;r action, and the
new "set/show range-stepping" commands.
gdb/doc/
2013-05-14 Yao Qi <yao@codesourcery.com>
Pedro Alves <palves@redhat.com>
* gdb.texinfo (Packets): Document 'vCont;r'.
(Continuing and Stepping): Document target-assisted range
stepping, and the 'set range-stepping' and 'show range-stepping'
commands.
---
gdb/NEWS | 17 +++++++++
gdb/doc/gdb.texinfo | 49 ++++++++++++++++++++++++++
gdb/gdbthread.h | 7 ++++
gdb/infcmd.c | 8 ++++
gdb/infrun.c | 33 +++++++++++++++++
gdb/remote.c | 98 +++++++++++++++++++++++++++++++++++++++++++++++++++
6 files changed, 211 insertions(+), 1 deletion(-)
diff --git a/gdb/NEWS b/gdb/NEWS
index 7cd1646..3f2d757 100644
--- a/gdb/NEWS
+++ b/gdb/NEWS
@@ -35,6 +35,10 @@ set debug nios2
show debug nios2
Control display of debugging messages related to Nios II targets.
+set range-stepping
+show range-stepping
+ Control whether target-assisted range stepping is enabled.
+
* You can now use a literal value 'unlimited' for options that
interpret 0 or -1 as meaning "unlimited". E.g., "set
trace-buffer-size unlimited" is now an alias for "set
@@ -64,6 +68,19 @@ show debug nios2
** The -trace-save MI command can optionally save trace buffer in Common
Trace Format now.
+* GDB now supports target-assigned range stepping with remote targets.
+ This improves the performance of stepping source lines by reducing
+ the number of control packets from/to GDB. See "New remote packets"
+ below.
+
+* New remote packets
+
+vCont;r
+
+ The vCont packet supports a new 'r' action, that tells the remote
+ stub to step through an address range itself, without GDB
+ involvemement at each single-step.
+
*** Changes in GDB 7.6
* Target record has been renamed to record-full.
diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index 1869d74..5a90d8e 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -5219,6 +5219,38 @@ Execute one machine instruction, but if it is a function call,
proceed until the function returns.
An argument is a repeat count, as in @code{next}.
+
+@end table
+
+@anchor{range stepping}
+@cindex range stepping
+@cindex target-assisted range stepping
+By default, and if available, @value{GDBN} makes use of
+target-assisted @dfn{range stepping}. In other words, whenever you
+use a stepping command (e.g., @code{step}, @code{next}), @value{GDBN}
+tells the target to step the corresponding range of instruction
+addresses instead of issuing multiple single-steps. This speeds up
+line stepping, particularly for remote targets. Ideally, there should
+be no reason you would want to turn range stepping off. However, it's
+possible that a bug in the debug info, a bug in the remote stub (for
+remote targets), or even a bug in @value{GDBN} could make line
+stepping behave incorrectly when target-assisted range stepping is
+enabled. You can use the following command to turn off range stepping
+if necessary:
+
+@table @code
+@kindex set range-stepping
+@kindex show range-stepping
+@item set range-stepping
+@itemx show range-stepping
+Control whether range stepping is enabled.
+
+If @code{on}, and the target supports it, @value{GDBN} tells the
+target to step a range of addresses itself, instead of issuing
+multiple single-steps. If @code{off}, @value{GDBN} always issues
+single-steps, even if range stepping is supported by the target. The
+default is @code{on}.
+
@end table
@node Skipping Over Functions and Files
@@ -37373,6 +37405,23 @@ Step.
Step with signal @var{sig}. The signal @var{sig} should be two hex digits.
@item t
Stop.
+@item r @var{start},@var{end}
+Step repeatedly while the thread's PC is within the range
+[@var{start}, @var{end}). The remote stub reports a stop reply when
+either the thread goes out of the range or is stopped due to an
+unrelated reason, such as hitting a breakpoint. @xref{range
+stepping}.
+
+@var{end} is the address of the first instruction beyond the step
+range, and @strong{not} the address of the last instruction within it.
+(This has the property that @var{start} == @var{end} single-steps
+once, and only once, even if the instruction at @var{start} jumps to
+@var{end}.)
+
+A stop reply may be sent at any point even if the PC is still within
+the stepping range; for example, it is valid to implement this packet
+in a degenerate way as a single instruction step operation.
+
@end table
The optional argument @var{addr} normally associated with the
diff --git a/gdb/gdbthread.h b/gdb/gdbthread.h
index a9f8a94..b5c592b 100644
--- a/gdb/gdbthread.h
+++ b/gdb/gdbthread.h
@@ -65,6 +65,13 @@ struct thread_control_state
CORE_ADDR step_range_start; /* Inclusive */
CORE_ADDR step_range_end; /* Exclusive */
+ /* If GDB issues a target step request, and this is nonzero, the
+ target may continue single-stepping this thread without GDB
+ involvement as long as the PC is in the step range above. If
+ this is zero, the target should ignore the step range, and only
+ issue one single step. */
+ int may_range_step;
+
/* Stack frame address as of when stepping command was issued.
This is how we know when we step into a subroutine call, and how
to set the frame for the breakpoint used to step out. */
diff --git a/gdb/infcmd.c b/gdb/infcmd.c
index aeb24ff..30621e4 100644
--- a/gdb/infcmd.c
+++ b/gdb/infcmd.c
@@ -1046,9 +1046,14 @@ step_once (int skip_subroutines, int single_inst, int count, int thread)
&tp->control.step_range_start,
&tp->control.step_range_end);
+ tp->control.may_range_step = 1;
+
/* If we have no line info, switch to stepi mode. */
if (tp->control.step_range_end == 0 && step_stop_if_no_debug)
- tp->control.step_range_start = tp->control.step_range_end = 1;
+ {
+ tp->control.step_range_start = tp->control.step_range_end = 1;
+ tp->control.may_range_step = 0;
+ }
else if (tp->control.step_range_end == 0)
{
const char *name;
@@ -1337,6 +1342,7 @@ until_next_command (int from_tty)
tp->control.step_range_start = BLOCK_START (SYMBOL_BLOCK_VALUE (func));
tp->control.step_range_end = sal.end;
}
+ tp->control.may_range_step = 1;
tp->control.step_over_calls = STEP_OVER_ALL;
diff --git a/gdb/infrun.c b/gdb/infrun.c
index 57c427d..376a440 100644
--- a/gdb/infrun.c
+++ b/gdb/infrun.c
@@ -1311,6 +1311,7 @@ static int
displaced_step_prepare (ptid_t ptid)
{
struct cleanup *old_cleanups, *ignore_cleanups;
+ struct thread_info *tp = find_thread_ptid (ptid);
struct regcache *regcache = get_thread_regcache (ptid);
struct gdbarch *gdbarch = get_regcache_arch (regcache);
CORE_ADDR original, copy;
@@ -1323,6 +1324,12 @@ displaced_step_prepare (ptid_t ptid)
support displaced stepping. */
gdb_assert (gdbarch_displaced_step_copy_insn_p (gdbarch));
+ /* Disable range stepping while executing in the scratch pad. We
+ want a single-step even if executing the displaced instruction in
+ the scratch buffer lands within the stepping range (e.g., a
+ jump/branch). */
+ tp->control.may_range_step = 0;
+
/* We have to displaced step one thread at a time, as we only have
access to a single scratch space per inferior. */
@@ -1778,6 +1785,11 @@ how to step past a permanent breakpoint on this architecture. Try using\n\
a command like `return' or `jump' to continue execution."));
}
+ /* If we have a breakpoint to step over, make sure to do a single
+ step only. Same if we have software watchpoints. */
+ if (tp->control.trap_expected || bpstat_should_step ())
+ tp->control.may_range_step = 0;
+
/* If enabled, step over breakpoints by executing a copy of the
instruction at a different address.
@@ -1939,6 +1951,16 @@ a command like `return' or `jump' to continue execution."));
displaced_step_dump_bytes (gdb_stdlog, buf, sizeof (buf));
}
+ if (tp->control.may_range_step)
+ {
+ /* If we're resuming a thread with the PC out of the step
+ range, then we're doing some nested/finer run control
+ operation, like stepping the thread out of the dynamic
+ linker or the displaced stepping scratch pad. We
+ shouldn't have allowed a range step then. */
+ gdb_assert (pc_in_thread_step_range (pc, tp));
+ }
+
/* Install inferior's terminal modes. */
target_terminal_inferior ();
@@ -1980,6 +2002,7 @@ clear_proceed_status_thread (struct thread_info *tp)
tp->control.trap_expected = 0;
tp->control.step_range_start = 0;
tp->control.step_range_end = 0;
+ tp->control.may_range_step = 0;
tp->control.step_frame_id = null_frame_id;
tp->control.step_stack_frame_id = null_frame_id;
tp->control.step_over_calls = STEP_OVER_UNDEBUGGABLE;
@@ -3223,6 +3246,10 @@ handle_inferior_event (struct execution_control_state *ecs)
/* If it's a new thread, add it to the thread database. */
if (ecs->event_thread == NULL)
ecs->event_thread = add_thread (ecs->ptid);
+
+ /* Disable range stepping. If the next step request could use a
+ range, this will be end up re-enabled then. */
+ ecs->event_thread->control.may_range_step = 0;
}
/* Dependent on valid ECS->EVENT_THREAD. */
@@ -4717,6 +4744,11 @@ process_event_stop_test:
paddress (gdbarch, ecs->event_thread->control.step_range_start),
paddress (gdbarch, ecs->event_thread->control.step_range_end));
+ /* Tentatively re-enable range stepping; `resume' disables it if
+ necessary (e.g., if we're stepping over a breakpoint or we
+ have software watchpoints). */
+ ecs->event_thread->control.may_range_step = 1;
+
/* When stepping backward, stop at beginning of line range
(unless it's the function entry point, in which case
keep going back to the call point). */
@@ -5233,6 +5265,7 @@ process_event_stop_test:
ecs->event_thread->control.step_range_start = stop_pc_sal.pc;
ecs->event_thread->control.step_range_end = stop_pc_sal.end;
+ ecs->event_thread->control.may_range_step = 1;
set_step_info (frame, stop_pc_sal);
if (debug_infrun)
diff --git a/gdb/remote.c b/gdb/remote.c
index 5d3950b..cdb2f01 100644
--- a/gdb/remote.c
+++ b/gdb/remote.c
@@ -260,8 +260,15 @@ struct vCont_action_support
{
/* vCont;t */
int t;
+
+ /* vCont;r */
+ int r;
};
+/* Controls whether GDB is willing to use range stepping. */
+
+static int use_range_stepping = 1;
+
/* Description of the remote protocol state for the currently
connected target. This is per-target state, and independent of the
selected architecture. */
@@ -4653,6 +4660,7 @@ remote_vcont_probe (struct remote_state *rs)
support_c = 0;
support_C = 0;
rs->supports_vCont.t = 0;
+ rs->supports_vCont.r = 0;
while (p && *p == ';')
{
p++;
@@ -4666,6 +4674,8 @@ remote_vcont_probe (struct remote_state *rs)
support_C = 1;
else if (*p == 't' && (*(p + 1) == ';' || *(p + 1) == 0))
rs->supports_vCont.t = 1;
+ else if (*p == 'r' && (*(p + 1) == ';' || *(p + 1) == 0))
+ rs->supports_vCont.r = 1;
p = strchr (p, ';');
}
@@ -4697,6 +4707,42 @@ append_resumption (char *p, char *endp,
if (step && siggnal != GDB_SIGNAL_0)
p += xsnprintf (p, endp - p, ";S%02x", siggnal);
+ else if (step
+ /* GDB is willing to range step. */
+ && use_range_stepping
+ /* Target supports range stepping. */
+ && rs->supports_vCont.r
+ /* We don't currently support range stepping multiple
+ threads with a wildcard (though the protocol allows it,
+ so stubs shouldn't make an active effort to forbid
+ it). */
+ && !(remote_multi_process_p (rs) && ptid_is_pid (ptid)))
+ {
+ struct thread_info *tp;
+
+ if (ptid_equal (ptid, minus_one_ptid))
+ {
+ /* If we don't know about the target thread's tid, then
+ we're resuming magic_null_ptid (see caller). */
+ tp = find_thread_ptid (magic_null_ptid);
+ }
+ else
+ tp = find_thread_ptid (ptid);
+ gdb_assert (tp != NULL);
+
+ if (tp->control.may_range_step)
+ {
+ int addr_size = gdbarch_addr_bit (target_gdbarch ()) / 8;
+
+ p += xsnprintf (p, endp - p, ";r%s,%s",
+ phex_nz (tp->control.step_range_start,
+ addr_size),
+ phex_nz (tp->control.step_range_end,
+ addr_size));
+ }
+ else
+ p += xsnprintf (p, endp - p, ";s");
+ }
else if (step)
p += xsnprintf (p, endp - p, ";s");
else if (siggnal != GDB_SIGNAL_0)
@@ -11658,6 +11704,44 @@ remote_upload_trace_state_variables (struct uploaded_tsv **utsvp)
return 0;
}
+/* The "set/show range-stepping" show hook. */
+
+static void
+show_range_stepping (struct ui_file *file, int from_tty,
+ struct cmd_list_element *c,
+ const char *value)
+{
+ fprintf_filtered (file,
+ _("Debugger's willingness to use range stepping "
+ "is %s.\n"), value);
+}
+
+/* The "set/show range-stepping" set hook. */
+
+static void
+set_range_stepping (char *ignore_args, int from_tty,
+ struct cmd_list_element *c)
+{
+ /* Whene enabling, check whether range stepping is actually
+ supported by the target, and warn if not. */
+ if (use_range_stepping)
+ {
+ if (remote_desc != NULL)
+ {
+ struct remote_state *rs = get_remote_state ();
+
+ if (remote_protocol_packets[PACKET_vCont].support == PACKET_SUPPORT_UNKNOWN)
+ remote_vcont_probe (rs);
+
+ if (remote_protocol_packets[PACKET_vCont].support == PACKET_ENABLE
+ && rs->supports_vCont.r)
+ return;
+ }
+
+ warning (_("Range stepping is not supported by the current target"));
+ }
+}
+
void
_initialize_remote (void)
{
@@ -12055,6 +12139,20 @@ Set the remote pathname for \"run\""), _("\
Show the remote pathname for \"run\""), NULL, NULL, NULL,
&remote_set_cmdlist, &remote_show_cmdlist);
+ add_setshow_boolean_cmd ("range-stepping", class_run,
+ &use_range_stepping, _("\
+Enable or disable range stepping."), _("\
+Show whether target-assisted range stepping is enabled."), _("\
+If on, and the target supports it, when stepping a source line, GDB\n\
+tells the target to step the corresponding range of addresses itself instead\n\
+of issuing multiple single-steps. This speeds up source level\n\
+stepping. If off, GDB always issues single-steps, even if range\n\
+stepping is supported by the target. The default is on."),
+ set_range_stepping,
+ show_range_stepping,
+ &setlist,
+ &showlist);
+
/* Eventually initialize fileio. See fileio.c */
initialize_remote_fileio (remote_set_cmdlist, remote_show_cmdlist);
^ permalink raw reply [flat|nested] 38+ messages in thread* Re: [PATCH 3/5] range stepping: gdb
2013-05-14 19:10 ` [PATCH 3/5] range stepping: gdb Pedro Alves
@ 2013-05-14 19:46 ` Eli Zaretskii
2013-05-15 10:23 ` Pedro Alves
0 siblings, 1 reply; 38+ messages in thread
From: Eli Zaretskii @ 2013-05-14 19:46 UTC (permalink / raw)
To: Pedro Alves; +Cc: gdb-patches
> From: Pedro Alves <palves@redhat.com>
> Date: Tue, 14 May 2013 20:10:47 +0100
>
> When user issues the "step" command on the following line of source,
>
> a = b + c + d * e - a;
>
> GDB single-steps every single instruction until the program reaches a
> new different line.
I always thought that GDB sets a temporary breakpoint at the end, and
then lets the target run freely. Why not?
> +@var{end} is the address of the first instruction beyond the step
> +range, and @strong{not} the address of the last instruction within it.
> +(This has the property that @var{start} == @var{end} single-steps
> +once, and only once, even if the instruction at @var{start} jumps to
> +@var{end}.)
This sentence in parentheses got me completely confused. Before
reading it, I thought I understood what is this about; now I don't.
In particular, if START is equal to END, then how in the world could
the instruction at START jump to END? And if END is excluded from the
range, then why when START equals END do we step at all? Please
explain.
^ permalink raw reply [flat|nested] 38+ messages in thread* Re: [PATCH 3/5] range stepping: gdb
2013-05-14 19:46 ` Eli Zaretskii
@ 2013-05-15 10:23 ` Pedro Alves
2013-05-15 11:22 ` Eli Zaretskii
0 siblings, 1 reply; 38+ messages in thread
From: Pedro Alves @ 2013-05-15 10:23 UTC (permalink / raw)
To: Eli Zaretskii; +Cc: gdb-patches
On 05/14/2013 08:46 PM, Eli Zaretskii wrote:
>> From: Pedro Alves <palves@redhat.com>
>> Date: Tue, 14 May 2013 20:10:47 +0100
>>
>> When user issues the "step" command on the following line of source,
>>
>> a = b + c + d * e - a;
>>
>> GDB single-steps every single instruction until the program reaches a
>> new different line.
>
> I always thought that GDB sets a temporary breakpoint at the end, and
> then lets the target run freely. Why not?
Because we don't know whether there are instructions in the line that
jump/branch to a different place. We'd miss the breakpoint and lose
control.
>
>> +@var{end} is the address of the first instruction beyond the step
>> +range, and @strong{not} the address of the last instruction within it.
>> +(This has the property that @var{start} == @var{end} single-steps
>> +once, and only once, even if the instruction at @var{start} jumps to
>> +@var{end}.)
>
> This sentence in parentheses got me completely confused. Before
> reading it, I thought I understood what is this about; now I don't.
> In particular, if START is equal to END, then how in the world could
> the instruction at START jump to END?
Sorry, I had that typo in the gdbserver code as well, fixed it
there, but missed this one.
It should read, even if the instruction at @var{start} jumps to @var{start}.
vCont;r first steps, then checks. IOW:
vCont ;r ADDR1,ADDR1
is equivalent to (and could be thought to supersede):
vCont ;s
> And if END is excluded from the
> range, then why when START equals END do we step at all? Please
> explain.
It's just a design decision. I recall at least one target I saw I worked
with that supported range stepping, and it didn't even a distinction
between range vs no-range step commands. The way to do a single step
was to pass both addresses the same. I find it a better design than
requiring the target do one current-address check _before_ stepping,
and another _after_ single-stepping.
--
Pedro Alves
^ permalink raw reply [flat|nested] 38+ messages in thread* Re: [PATCH 3/5] range stepping: gdb
2013-05-15 10:23 ` Pedro Alves
@ 2013-05-15 11:22 ` Eli Zaretskii
2013-05-15 12:39 ` Pedro Alves
0 siblings, 1 reply; 38+ messages in thread
From: Eli Zaretskii @ 2013-05-15 11:22 UTC (permalink / raw)
To: Pedro Alves; +Cc: gdb-patches
> Date: Wed, 15 May 2013 11:23:24 +0100
> From: Pedro Alves <palves@redhat.com>
> CC: gdb-patches@sourceware.org
>
> >> +@var{end} is the address of the first instruction beyond the step
> >> +range, and @strong{not} the address of the last instruction within it.
> >> +(This has the property that @var{start} == @var{end} single-steps
> >> +once, and only once, even if the instruction at @var{start} jumps to
> >> +@var{end}.)
> >
> > This sentence in parentheses got me completely confused. Before
> > reading it, I thought I understood what is this about; now I don't.
> > In particular, if START is equal to END, then how in the world could
> > the instruction at START jump to END?
>
> Sorry, I had that typo in the gdbserver code as well, fixed it
> there, but missed this one.
>
> It should read, even if the instruction at @var{start} jumps to @var{start}.
>
> vCont;r first steps, then checks. IOW:
>
> vCont ;r ADDR1,ADDR1
>
> is equivalent to (and could be thought to supersede):
>
> vCont ;s
>
> > And if END is excluded from the
> > range, then why when START equals END do we step at all? Please
> > explain.
>
> It's just a design decision. I recall at least one target I saw I worked
> with that supported range stepping, and it didn't even a distinction
> between range vs no-range step commands. The way to do a single step
> was to pass both addresses the same. I find it a better design than
> requiring the target do one current-address check _before_ stepping,
> and another _after_ single-stepping.
Doesn't this mean that these two use cases are explicit exceptions
from the rule that END is excluded? If so, we should describe them as
exceptions, not use them as evidence for the rule (which they
evidently violate).
Or did I misunderstand again?
^ permalink raw reply [flat|nested] 38+ messages in thread* Re: [PATCH 3/5] range stepping: gdb
2013-05-15 11:22 ` Eli Zaretskii
@ 2013-05-15 12:39 ` Pedro Alves
2013-05-15 13:46 ` Eli Zaretskii
0 siblings, 1 reply; 38+ messages in thread
From: Pedro Alves @ 2013-05-15 12:39 UTC (permalink / raw)
To: Eli Zaretskii; +Cc: gdb-patches
On 05/15/2013 12:21 PM, Eli Zaretskii wrote:
>> Date: Wed, 15 May 2013 11:23:24 +0100
>> From: Pedro Alves <palves@redhat.com>
>> CC: gdb-patches@sourceware.org
>>
>>>> +@var{end} is the address of the first instruction beyond the step
>>>> +range, and @strong{not} the address of the last instruction within it.
>>>> +(This has the property that @var{start} == @var{end} single-steps
>>>> +once, and only once, even if the instruction at @var{start} jumps to
>>>> +@var{end}.)
>>>
>>> This sentence in parentheses got me completely confused. Before
>>> reading it, I thought I understood what is this about; now I don't.
>>> In particular, if START is equal to END, then how in the world could
>>> the instruction at START jump to END?
>>
>> Sorry, I had that typo in the gdbserver code as well, fixed it
>> there, but missed this one.
>>
>> It should read, even if the instruction at @var{start} jumps to @var{start}.
>>
>> vCont;r first steps, then checks. IOW:
>>
>> vCont ;r ADDR1,ADDR1
>>
>> is equivalent to (and could be thought to supersede):
>>
>> vCont ;s
>>
>>> And if END is excluded from the
>>> range, then why when START equals END do we step at all? Please
>>> explain.
>>
>> It's just a design decision. I recall at least one target I saw I worked
>> with that supported range stepping, and it didn't even a distinction
>> between range vs no-range step commands. The way to do a single step
>> was to pass both addresses the same. I find it a better design than
>> requiring the target do one current-address check _before_ stepping,
>> and another _after_ single-stepping.
>
> Doesn't this mean that these two use cases are explicit exceptions
> from the rule that END is excluded?
Nope. There's no exception.
With:
vCont ;r START,END
#1 - The stub single-steps the thread.
#2 - Once the thread stops, the stub checks whether the thread
stopped in the [START,END) range. If so, goto #1.
It not, goto #3.
#3 - The stub reports to gdb that the thread stopped stepping.
If it happens that START and END are the same, then #2 always
goes to #3.
When I said:
"(This has the property that @var{start} == @var{end} single-steps
once, and only once, even if the instruction at @var{start} jumps to
@var{start}.)"
I was trying to clarify the case of the instruction at START being:
jump START
Then,
vCont ;r START,START
always single-steps once, and only once, instead of
continuously single-stepping that instruction without
reporting to GDB.
> If so, we should describe them as
> exceptions, not use them as evidence for the rule (which they
> evidently violate).
>
> Or did I misunderstand again?
--
Pedro Alves
^ permalink raw reply [flat|nested] 38+ messages in thread* Re: [PATCH 3/5] range stepping: gdb
2013-05-15 12:39 ` Pedro Alves
@ 2013-05-15 13:46 ` Eli Zaretskii
2013-05-15 13:58 ` Pedro Alves
0 siblings, 1 reply; 38+ messages in thread
From: Eli Zaretskii @ 2013-05-15 13:46 UTC (permalink / raw)
To: Pedro Alves; +Cc: gdb-patches
> Date: Wed, 15 May 2013 13:39:05 +0100
> From: Pedro Alves <palves@redhat.com>
> CC: gdb-patches@sourceware.org
>
> > Doesn't this mean that these two use cases are explicit exceptions
> > from the rule that END is excluded?
>
> Nope. There's no exception.
>
> With:
>
> vCont ;r START,END
>
> #1 - The stub single-steps the thread.
> #2 - Once the thread stops, the stub checks whether the thread
> stopped in the [START,END) range. If so, goto #1.
> It not, goto #3.
> #3 - The stub reports to gdb that the thread stopped stepping.
>
> If it happens that START and END are the same, then #2 always
> goes to #3.
I'm simulating a naive reader, while you are replying to someone you
consider an experienced code developer ;-) So we are talking past
each other.
When you say "END is the address of the first instruction beyond the
step range", that means, simply put, that execution will always stop
before it executes the instruction at END. IOW, the instruction at
END will _not_ be executed. With that interpretation, a range
[START,START) is _empty_ and will never execute any instructions at
all.
It is OK to use a different interpretation, but then we should either
(a) describe the semantics differently to begin with, or (b) explain
that [START,START) is an exception. You seem to object to (b), which
then brings us back at (a), meaning that this text:
> +@var{end} is the address of the first instruction beyond the step
> +range, and @strong{not} the address of the last instruction within it.
needs to be reworded, so as not to say that END is _beyond_ the range.
If you want a specific response for the algorithm you show above, then
I would ask why does GDB single-step the stub at all, when START and
END are equal? The fact that we implemented this is a 'do-until' loop
rather than a 'while' loop, i.e. test at the end instead of at the
beginning, is an important implementation detail which must be present
explicitly in the description of what this feature does. If you hide
it behind the [a,b) notation, you get a problem you will need to
explain, as we see.
> When I said:
>
> "(This has the property that @var{start} == @var{end} single-steps
> once, and only once, even if the instruction at @var{start} jumps to
> @var{start}.)"
>
> I was trying to clarify the case of the instruction at START being:
>
> jump START
>
> Then,
>
> vCont ;r START,START
>
> always single-steps once, and only once, instead of
> continuously single-stepping that instruction without
> reporting to GDB.
The very need you felt to explain this is already a clear sign that
the original description is wrong.
^ permalink raw reply [flat|nested] 38+ messages in thread* Re: [PATCH 3/5] range stepping: gdb
2013-05-15 13:46 ` Eli Zaretskii
@ 2013-05-15 13:58 ` Pedro Alves
2013-05-15 18:20 ` Pedro Alves
0 siblings, 1 reply; 38+ messages in thread
From: Pedro Alves @ 2013-05-15 13:58 UTC (permalink / raw)
To: Eli Zaretskii; +Cc: gdb-patches
On 05/15/2013 02:46 PM, Eli Zaretskii wrote:
>> Date: Wed, 15 May 2013 13:39:05 +0100
>> From: Pedro Alves <palves@redhat.com>
>> CC: gdb-patches@sourceware.org
>>
>>> Doesn't this mean that these two use cases are explicit exceptions
>>> from the rule that END is excluded?
>>
>> Nope. There's no exception.
>>
>> With:
>>
>> vCont ;r START,END
>>
>> #1 - The stub single-steps the thread.
>> #2 - Once the thread stops, the stub checks whether the thread
>> stopped in the [START,END) range. If so, goto #1.
>> It not, goto #3.
>> #3 - The stub reports to gdb that the thread stopped stepping.
>>
>> If it happens that START and END are the same, then #2 always
>> goes to #3.
>
> I'm simulating a naive reader, while you are replying to someone you
> consider an experienced code developer ;-) So we are talking past
> each other.
:-)
> When you say "END is the address of the first instruction beyond the
> step range", that means, simply put, that execution will always stop
> before it executes the instruction at END. IOW, the instruction at
> END will _not_ be executed. With that interpretation, a range
> [START,START) is _empty_ and will never execute any instructions at
> all.
>
> It is OK to use a different interpretation, but then we should either
> (a) describe the semantics differently to begin with, or (b) explain
> that [START,START) is an exception. You seem to object to (b), which
> then brings us back at (a), meaning that this text:
>
>> +@var{end} is the address of the first instruction beyond the step
>> +range, and @strong{not} the address of the last instruction within it.
>
> needs to be reworded, so as not to say that END is _beyond_ the range.
I see what you mean now.
> If you want a specific response for the algorithm you show above, then
> I would ask why does GDB single-step the stub at all, when START and
> END are equal? The fact that we implemented this is a 'do-until' loop
> rather than a 'while' loop, i.e. test at the end instead of at the
> beginning, is an important implementation detail which must be present
> explicitly in the description of what this feature does.
I agree. This is the sort of detail I could see different stubs
ending up implementing differently, so I wanted to be sure it
was clearly specified. Well, clearly I failed. :-)
> The very need you felt to explain this is already a clear sign that
> the original description is wrong.
I'll try to come up with a better description.
Thanks!
--
Pedro Alves
^ permalink raw reply [flat|nested] 38+ messages in thread* Re: [PATCH 3/5] range stepping: gdb
2013-05-15 13:58 ` Pedro Alves
@ 2013-05-15 18:20 ` Pedro Alves
2013-05-16 6:08 ` Eli Zaretskii
0 siblings, 1 reply; 38+ messages in thread
From: Pedro Alves @ 2013-05-15 18:20 UTC (permalink / raw)
To: Eli Zaretskii; +Cc: gdb-patches
On 05/15/2013 02:58 PM, Pedro Alves wrote:
> On 05/15/2013 02:46 PM, Eli Zaretskii wrote:
>>> Date: Wed, 15 May 2013 13:39:05 +0100
>>> From: Pedro Alves <palves@redhat.com>
>>> CC: gdb-patches@sourceware.org
>>>
>>>> Doesn't this mean that these two use cases are explicit exceptions
>>>> from the rule that END is excluded?
>>>
>>> Nope. There's no exception.
>>>
>>> With:
>>>
>>> vCont ;r START,END
>>>
>>> #1 - The stub single-steps the thread.
>>> #2 - Once the thread stops, the stub checks whether the thread
>>> stopped in the [START,END) range. If so, goto #1.
>>> It not, goto #3.
>>> #3 - The stub reports to gdb that the thread stopped stepping.
>>>
>>> If it happens that START and END are the same, then #2 always
>>> goes to #3.
>>
>> I'm simulating a naive reader, while you are replying to someone you
>> consider an experienced code developer ;-) So we are talking past
>> each other.
>
> :-)
>
>> When you say "END is the address of the first instruction beyond the
>> step range", that means, simply put, that execution will always stop
>> before it executes the instruction at END. IOW, the instruction at
>> END will _not_ be executed. With that interpretation, a range
>> [START,START) is _empty_ and will never execute any instructions at
>> all.
>>
>> It is OK to use a different interpretation, but then we should either
>> (a) describe the semantics differently to begin with, or (b) explain
>> that [START,START) is an exception. You seem to object to (b), which
>> then brings us back at (a), meaning that this text:
>>
>>> +@var{end} is the address of the first instruction beyond the step
>>> +range, and @strong{not} the address of the last instruction within it.
>>
>> needs to be reworded, so as not to say that END is _beyond_ the range.
>
> I see what you mean now.
>
>> If you want a specific response for the algorithm you show above, then
>> I would ask why does GDB single-step the stub at all, when START and
>> END are equal? The fact that we implemented this is a 'do-until' loop
>> rather than a 'while' loop, i.e. test at the end instead of at the
>> beginning, is an important implementation detail which must be present
>> explicitly in the description of what this feature does.
>
> I agree. This is the sort of detail I could see different stubs
> ending up implementing differently, so I wanted to be sure it
> was clearly specified. Well, clearly I failed. :-)
>
>> The very need you felt to explain this is already a clear sign that
>> the original description is wrong.
>
> I'll try to come up with a better description.
What do you think of this? I'm okay with removing the
whole second paragraph ("If the range is empty...") if you
think the first paragraph is already clear enough.
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
@item r @var{start},@var{end}
Step once, and then keep stepping as long as the thread stops at
addresses between @var{start} (inclusive) and @var{end} (exclusive).
The remote stub reports a stop reply when either the thread goes out
of the range or is stopped due to an unrelated reason, such as hitting
a breakpoint. @xref{range stepping}.
If the range is empty (@var{start} == @var{end}), then the action
becomes equivalent to the @samp{s} action. In other words,
single-step once, and report the stop (even if the stepped instruction
jumps to @var{start}).
(A stop reply may be sent at any point even if the PC is still within
the stepping range; for example, it is valid to implement this packet
in a degenerate way as a single instruction step operation.)
@end table
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
--
Pedro Alves
^ permalink raw reply [flat|nested] 38+ messages in thread* Re: [PATCH 3/5] range stepping: gdb
2013-05-15 18:20 ` Pedro Alves
@ 2013-05-16 6:08 ` Eli Zaretskii
2013-05-20 18:43 ` Pedro Alves
0 siblings, 1 reply; 38+ messages in thread
From: Eli Zaretskii @ 2013-05-16 6:08 UTC (permalink / raw)
To: Pedro Alves; +Cc: gdb-patches
> Date: Wed, 15 May 2013 19:20:46 +0100
> From: Pedro Alves <palves@redhat.com>
> CC: gdb-patches@sourceware.org
>
> What do you think of this?
I think it's a really good and clear description.
> I'm okay with removing the whole second paragraph ("If the range is
> empty...") if you think the first paragraph is already clear enough.
The first paragraph is clear enough, but I find the second paragraph
reassuring me in that the interpretation of the first was correct. So
I think it is better to leave both.
Thanks.
^ permalink raw reply [flat|nested] 38+ messages in thread* Re: [PATCH 3/5] range stepping: gdb
2013-05-16 6:08 ` Eli Zaretskii
@ 2013-05-20 18:43 ` Pedro Alves
2013-05-20 19:05 ` Eli Zaretskii
2013-05-23 0:47 ` Yao Qi
0 siblings, 2 replies; 38+ messages in thread
From: Pedro Alves @ 2013-05-20 18:43 UTC (permalink / raw)
To: Eli Zaretskii; +Cc: gdb-patches
On 05/16/2013 07:08 AM, Eli Zaretskii wrote:
>> Date: Wed, 15 May 2013 19:20:46 +0100
>> From: Pedro Alves <palves@redhat.com>
>> CC: gdb-patches@sourceware.org
>>
>> What do you think of this?
>
> I think it's a really good and clear description.
>
>> I'm okay with removing the whole second paragraph ("If the range is
>> empty...") if you think the first paragraph is already clear enough.
>
> The first paragraph is clear enough, but I find the second paragraph
> reassuring me in that the interpretation of the first was correct. So
> I think it is better to leave both.
Sorry for taking long to respond, and letting the context potentially
slip from you. Here's the full updated patch with that bit merged in.
Nothing else in the docs changed. I've updated a comment in
gdbthread.h, giving it a similar clarification treatment.
---------
Subject: range stepping: gdb
(text based on Yao's)
This patch teaches GDB to take advantage of target-assisted range
stepping. It adds a new 'r ADDR1,ADDR2' action to vCont (vCont;r),
meaning, "step once, and keep stepping as long as the thread is in the
[ADDR1,ADDR2) range".
Rationale:
When user issues the "step" command on the following line of source,
a = b + c + d * e - a;
GDB single-steps every single instruction until the program reaches a
new different line. E.g., on x86_64, that line compiles to:
0x08048434 <+65>: mov 0x1c(%esp),%eax
0x08048438 <+69>: mov 0x30(%esp),%edx
0x0804843c <+73>: add %eax,%edx
0x0804843e <+75>: mov 0x18(%esp),%eax
0x08048442 <+79>: imul 0x2c(%esp),%eax
0x08048447 <+84>: add %edx,%eax
0x08048449 <+86>: sub 0x34(%esp),%eax
0x0804844d <+90>: mov %eax,0x34(%esp)
0x08048451 <+94>: mov 0x1c(%esp),%eax
and the following is the RSP traffic between GDB and GDBserver:
--> vCont;s:p2e13.2e13;c
<-- T0505:68efffbf;04:30efffbf;08:3c840408;thread:p2e13.2e13;core:1;
--> vCont;s:p2e13.2e13;c
<-- T0505:68efffbf;04:30efffbf;08:3e840408;thread:p2e13.2e13;core:2;
--> vCont;s:p2e13.2e13;c
<-- T0505:68efffbf;04:30efffbf;08:42840408;thread:p2e13.2e13;core:2;
--> vCont;s:p2e13.2e13;c
<-- T0505:68efffbf;04:30efffbf;08:47840408;thread:p2e13.2e13;core:0;
--> vCont;s:p2e13.2e13;c
<-- T0505:68efffbf;04:30efffbf;08:49840408;thread:p2e13.2e13;core:0;
--> vCont;s:p2e13.2e13;c
<-- T0505:68efffbf;04:30efffbf;08:4d840408;thread:p2e13.2e13;core:0;
--> vCont;s:p2e13.2e13;c
<-- T0505:68efffbf;04:30efffbf;08:51840408;thread:p2e13.2e13;core:0;
IOW, a lot of roundtrips between GDB and GDBserver.
If we add a new command to the RSP, meaning "keep stepping and don't
report a stop until the program goes out of the [0x08048434,
0x08048451) address range", then the RSP traffic can be reduced down
to:
--> vCont;r8048434,8048451:p2db0.2db0;c
<-- T0505:68efffbf;04:30efffbf;08:51840408;thread:p2db0.2db0;core:1;
As number of packets is reduced dramatically, the performance of
stepping source lines is much improved.
In case something is wrong with range stepping on the stub side, the
debug info or even gdb, this adds a "set/show range-stepping" command
to be able to turn range stepping off.
v3:
- no longer use "is PC-in-stepping-range" or trap_expected checks deep
in remote.c's target_resume to decide whether to send vCont;r or
vCont;s. It is better to leave that choice to higher layers (IOW,
infrun.c/infcmd.c).
- "maint set/show range-stepping" renamed to "set/show range-stepping"
folded into this patch. The docs now explain what is range
stepping, and why a user might need this command.
- NEWS and docs for GDB bits folded into this patch, and
redone/expanded.
gdb/
2013-05-20 Yao Qi <yao@codesourcery.com>
Pedro Alves <palves@redhat.com>
* gdbthread.h (struct thread_control_state) <may_range_step>: New
field.
* infcmd.c (step_once, until_next_command): Enable range stepping.
* infrun.c (displaced_step_prepare): Disable range stepping.
(resume): Disable range stepping if stepping over a breakpoint or
we have software watchpoints. If range stepping is enabled,
assert the thread is in the stepping range.
(clear_proceed_status_thread): Clear may_range_step.
(handle_inferior_event): Disable range stepping as soon as we know
the thread that hit the event. Re-enable it whenever we're going
to step with a step range.
* remote.c (struct vCont_action_support) <r>: New field.
(use_range_stepping): New global.
(remote_vcont_probe): Handle 'r' action.
(append_resumption): Append an 'r' action if the thread may range
step.
(show_range_stepping): New function.
(set_range_stepping): New function.
(_initialize_remote): Call add_setshow_boolean_cmd to register the
'set range-stepping' and 'show range-stepping' commands.
* NEWS: Mention range stepping, the new vCont;r action, and the
new "set/show range-stepping" commands.
gdb/doc/
2013-05-20 Yao Qi <yao@codesourcery.com>
Pedro Alves <palves@redhat.com>
* gdb.texinfo (Packets): Document 'vCont;r'.
(Continuing and Stepping): Document target-assisted range
stepping, and the 'set range-stepping' and 'show range-stepping'
commands.
---
gdb/NEWS | 17 +++++++++
gdb/doc/gdb.texinfo | 48 +++++++++++++++++++++++++
gdb/gdbthread.h | 7 ++++
gdb/infcmd.c | 8 ++++
gdb/infrun.c | 33 +++++++++++++++++
gdb/remote.c | 98 +++++++++++++++++++++++++++++++++++++++++++++++++++
6 files changed, 210 insertions(+), 1 deletion(-)
diff --git a/gdb/NEWS b/gdb/NEWS
index 9ea0b14..05ae0a1 100644
--- a/gdb/NEWS
+++ b/gdb/NEWS
@@ -41,6 +41,10 @@ set debug nios2
show debug nios2
Control display of debugging messages related to Nios II targets.
+set range-stepping
+show range-stepping
+ Control whether target-assisted range stepping is enabled.
+
* You can now use a literal value 'unlimited' for options that
interpret 0 or -1 as meaning "unlimited". E.g., "set
trace-buffer-size unlimited" is now an alias for "set
@@ -70,6 +74,19 @@ show debug nios2
** The -trace-save MI command can optionally save trace buffer in Common
Trace Format now.
+* GDB now supports target-assigned range stepping with remote targets.
+ This improves the performance of stepping source lines by reducing
+ the number of control packets from/to GDB. See "New remote packets"
+ below.
+
+* New remote packets
+
+vCont;r
+
+ The vCont packet supports a new 'r' action, that tells the remote
+ stub to step through an address range itself, without GDB
+ involvemement at each single-step.
+
*** Changes in GDB 7.6
* Target record has been renamed to record-full.
diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index 1907e59..4c41594 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -5219,6 +5219,38 @@ Execute one machine instruction, but if it is a function call,
proceed until the function returns.
An argument is a repeat count, as in @code{next}.
+
+@end table
+
+@anchor{range stepping}
+@cindex range stepping
+@cindex target-assisted range stepping
+By default, and if available, @value{GDBN} makes use of
+target-assisted @dfn{range stepping}. In other words, whenever you
+use a stepping command (e.g., @code{step}, @code{next}), @value{GDBN}
+tells the target to step the corresponding range of instruction
+addresses instead of issuing multiple single-steps. This speeds up
+line stepping, particularly for remote targets. Ideally, there should
+be no reason you would want to turn range stepping off. However, it's
+possible that a bug in the debug info, a bug in the remote stub (for
+remote targets), or even a bug in @value{GDBN} could make line
+stepping behave incorrectly when target-assisted range stepping is
+enabled. You can use the following command to turn off range stepping
+if necessary:
+
+@table @code
+@kindex set range-stepping
+@kindex show range-stepping
+@item set range-stepping
+@itemx show range-stepping
+Control whether range stepping is enabled.
+
+If @code{on}, and the target supports it, @value{GDBN} tells the
+target to step a range of addresses itself, instead of issuing
+multiple single-steps. If @code{off}, @value{GDBN} always issues
+single-steps, even if range stepping is supported by the target. The
+default is @code{on}.
+
@end table
@node Skipping Over Functions and Files
@@ -37401,6 +37433,22 @@ Step.
Step with signal @var{sig}. The signal @var{sig} should be two hex digits.
@item t
Stop.
+@item r @var{start},@var{end}
+Step once, and then keep stepping as long as the thread stops at
+addresses between @var{start} (inclusive) and @var{end} (exclusive).
+The remote stub reports a stop reply when either the thread goes out
+of the range or is stopped due to an unrelated reason, such as hitting
+a breakpoint. @xref{range stepping}.
+
+If the range is empty (@var{start} == @var{end}), then the action
+becomes equivalent to the @samp{s} action. In other words,
+single-step once, and report the stop (even if the stepped instruction
+jumps to @var{start}).
+
+(A stop reply may be sent at any point even if the PC is still within
+the stepping range; for example, it is valid to implement this packet
+in a degenerate way as a single instruction step operation.)
+
@end table
The optional argument @var{addr} normally associated with the
diff --git a/gdb/gdbthread.h b/gdb/gdbthread.h
index a9f8a94..e93b965 100644
--- a/gdb/gdbthread.h
+++ b/gdb/gdbthread.h
@@ -65,6 +65,13 @@ struct thread_control_state
CORE_ADDR step_range_start; /* Inclusive */
CORE_ADDR step_range_end; /* Exclusive */
+ /* If GDB issues a target step request, and this is nonzero, the
+ target should single-step this thread once, and then continue
+ single-stepping it without GDB involvement as long as thread
+ stops in step range above. If this is zero, the target should
+ ignore the step range, and only issue one single step. */
+ int may_range_step;
+
/* Stack frame address as of when stepping command was issued.
This is how we know when we step into a subroutine call, and how
to set the frame for the breakpoint used to step out. */
diff --git a/gdb/infcmd.c b/gdb/infcmd.c
index aeb24ff..30621e4 100644
--- a/gdb/infcmd.c
+++ b/gdb/infcmd.c
@@ -1046,9 +1046,14 @@ step_once (int skip_subroutines, int single_inst, int count, int thread)
&tp->control.step_range_start,
&tp->control.step_range_end);
+ tp->control.may_range_step = 1;
+
/* If we have no line info, switch to stepi mode. */
if (tp->control.step_range_end == 0 && step_stop_if_no_debug)
- tp->control.step_range_start = tp->control.step_range_end = 1;
+ {
+ tp->control.step_range_start = tp->control.step_range_end = 1;
+ tp->control.may_range_step = 0;
+ }
else if (tp->control.step_range_end == 0)
{
const char *name;
@@ -1337,6 +1342,7 @@ until_next_command (int from_tty)
tp->control.step_range_start = BLOCK_START (SYMBOL_BLOCK_VALUE (func));
tp->control.step_range_end = sal.end;
}
+ tp->control.may_range_step = 1;
tp->control.step_over_calls = STEP_OVER_ALL;
diff --git a/gdb/infrun.c b/gdb/infrun.c
index 57c427d..376a440 100644
--- a/gdb/infrun.c
+++ b/gdb/infrun.c
@@ -1311,6 +1311,7 @@ static int
displaced_step_prepare (ptid_t ptid)
{
struct cleanup *old_cleanups, *ignore_cleanups;
+ struct thread_info *tp = find_thread_ptid (ptid);
struct regcache *regcache = get_thread_regcache (ptid);
struct gdbarch *gdbarch = get_regcache_arch (regcache);
CORE_ADDR original, copy;
@@ -1323,6 +1324,12 @@ displaced_step_prepare (ptid_t ptid)
support displaced stepping. */
gdb_assert (gdbarch_displaced_step_copy_insn_p (gdbarch));
+ /* Disable range stepping while executing in the scratch pad. We
+ want a single-step even if executing the displaced instruction in
+ the scratch buffer lands within the stepping range (e.g., a
+ jump/branch). */
+ tp->control.may_range_step = 0;
+
/* We have to displaced step one thread at a time, as we only have
access to a single scratch space per inferior. */
@@ -1778,6 +1785,11 @@ how to step past a permanent breakpoint on this architecture. Try using\n\
a command like `return' or `jump' to continue execution."));
}
+ /* If we have a breakpoint to step over, make sure to do a single
+ step only. Same if we have software watchpoints. */
+ if (tp->control.trap_expected || bpstat_should_step ())
+ tp->control.may_range_step = 0;
+
/* If enabled, step over breakpoints by executing a copy of the
instruction at a different address.
@@ -1939,6 +1951,16 @@ a command like `return' or `jump' to continue execution."));
displaced_step_dump_bytes (gdb_stdlog, buf, sizeof (buf));
}
+ if (tp->control.may_range_step)
+ {
+ /* If we're resuming a thread with the PC out of the step
+ range, then we're doing some nested/finer run control
+ operation, like stepping the thread out of the dynamic
+ linker or the displaced stepping scratch pad. We
+ shouldn't have allowed a range step then. */
+ gdb_assert (pc_in_thread_step_range (pc, tp));
+ }
+
/* Install inferior's terminal modes. */
target_terminal_inferior ();
@@ -1980,6 +2002,7 @@ clear_proceed_status_thread (struct thread_info *tp)
tp->control.trap_expected = 0;
tp->control.step_range_start = 0;
tp->control.step_range_end = 0;
+ tp->control.may_range_step = 0;
tp->control.step_frame_id = null_frame_id;
tp->control.step_stack_frame_id = null_frame_id;
tp->control.step_over_calls = STEP_OVER_UNDEBUGGABLE;
@@ -3223,6 +3246,10 @@ handle_inferior_event (struct execution_control_state *ecs)
/* If it's a new thread, add it to the thread database. */
if (ecs->event_thread == NULL)
ecs->event_thread = add_thread (ecs->ptid);
+
+ /* Disable range stepping. If the next step request could use a
+ range, this will be end up re-enabled then. */
+ ecs->event_thread->control.may_range_step = 0;
}
/* Dependent on valid ECS->EVENT_THREAD. */
@@ -4717,6 +4744,11 @@ process_event_stop_test:
paddress (gdbarch, ecs->event_thread->control.step_range_start),
paddress (gdbarch, ecs->event_thread->control.step_range_end));
+ /* Tentatively re-enable range stepping; `resume' disables it if
+ necessary (e.g., if we're stepping over a breakpoint or we
+ have software watchpoints). */
+ ecs->event_thread->control.may_range_step = 1;
+
/* When stepping backward, stop at beginning of line range
(unless it's the function entry point, in which case
keep going back to the call point). */
@@ -5233,6 +5265,7 @@ process_event_stop_test:
ecs->event_thread->control.step_range_start = stop_pc_sal.pc;
ecs->event_thread->control.step_range_end = stop_pc_sal.end;
+ ecs->event_thread->control.may_range_step = 1;
set_step_info (frame, stop_pc_sal);
if (debug_infrun)
diff --git a/gdb/remote.c b/gdb/remote.c
index eb58b94..658fae2 100644
--- a/gdb/remote.c
+++ b/gdb/remote.c
@@ -260,8 +260,15 @@ struct vCont_action_support
{
/* vCont;t */
int t;
+
+ /* vCont;r */
+ int r;
};
+/* Controls whether GDB is willing to use range stepping. */
+
+static int use_range_stepping = 1;
+
/* Description of the remote protocol state for the currently
connected target. This is per-target state, and independent of the
selected architecture. */
@@ -4653,6 +4660,7 @@ remote_vcont_probe (struct remote_state *rs)
support_c = 0;
support_C = 0;
rs->supports_vCont.t = 0;
+ rs->supports_vCont.r = 0;
while (p && *p == ';')
{
p++;
@@ -4666,6 +4674,8 @@ remote_vcont_probe (struct remote_state *rs)
support_C = 1;
else if (*p == 't' && (*(p + 1) == ';' || *(p + 1) == 0))
rs->supports_vCont.t = 1;
+ else if (*p == 'r' && (*(p + 1) == ';' || *(p + 1) == 0))
+ rs->supports_vCont.r = 1;
p = strchr (p, ';');
}
@@ -4697,6 +4707,42 @@ append_resumption (char *p, char *endp,
if (step && siggnal != GDB_SIGNAL_0)
p += xsnprintf (p, endp - p, ";S%02x", siggnal);
+ else if (step
+ /* GDB is willing to range step. */
+ && use_range_stepping
+ /* Target supports range stepping. */
+ && rs->supports_vCont.r
+ /* We don't currently support range stepping multiple
+ threads with a wildcard (though the protocol allows it,
+ so stubs shouldn't make an active effort to forbid
+ it). */
+ && !(remote_multi_process_p (rs) && ptid_is_pid (ptid)))
+ {
+ struct thread_info *tp;
+
+ if (ptid_equal (ptid, minus_one_ptid))
+ {
+ /* If we don't know about the target thread's tid, then
+ we're resuming magic_null_ptid (see caller). */
+ tp = find_thread_ptid (magic_null_ptid);
+ }
+ else
+ tp = find_thread_ptid (ptid);
+ gdb_assert (tp != NULL);
+
+ if (tp->control.may_range_step)
+ {
+ int addr_size = gdbarch_addr_bit (target_gdbarch ()) / 8;
+
+ p += xsnprintf (p, endp - p, ";r%s,%s",
+ phex_nz (tp->control.step_range_start,
+ addr_size),
+ phex_nz (tp->control.step_range_end,
+ addr_size));
+ }
+ else
+ p += xsnprintf (p, endp - p, ";s");
+ }
else if (step)
p += xsnprintf (p, endp - p, ";s");
else if (siggnal != GDB_SIGNAL_0)
@@ -11659,6 +11705,44 @@ remote_upload_trace_state_variables (struct uploaded_tsv **utsvp)
return 0;
}
+/* The "set/show range-stepping" show hook. */
+
+static void
+show_range_stepping (struct ui_file *file, int from_tty,
+ struct cmd_list_element *c,
+ const char *value)
+{
+ fprintf_filtered (file,
+ _("Debugger's willingness to use range stepping "
+ "is %s.\n"), value);
+}
+
+/* The "set/show range-stepping" set hook. */
+
+static void
+set_range_stepping (char *ignore_args, int from_tty,
+ struct cmd_list_element *c)
+{
+ /* Whene enabling, check whether range stepping is actually
+ supported by the target, and warn if not. */
+ if (use_range_stepping)
+ {
+ if (remote_desc != NULL)
+ {
+ struct remote_state *rs = get_remote_state ();
+
+ if (remote_protocol_packets[PACKET_vCont].support == PACKET_SUPPORT_UNKNOWN)
+ remote_vcont_probe (rs);
+
+ if (remote_protocol_packets[PACKET_vCont].support == PACKET_ENABLE
+ && rs->supports_vCont.r)
+ return;
+ }
+
+ warning (_("Range stepping is not supported by the current target"));
+ }
+}
+
void
_initialize_remote (void)
{
@@ -12056,6 +12140,20 @@ Set the remote pathname for \"run\""), _("\
Show the remote pathname for \"run\""), NULL, NULL, NULL,
&remote_set_cmdlist, &remote_show_cmdlist);
+ add_setshow_boolean_cmd ("range-stepping", class_run,
+ &use_range_stepping, _("\
+Enable or disable range stepping."), _("\
+Show whether target-assisted range stepping is enabled."), _("\
+If on, and the target supports it, when stepping a source line, GDB\n\
+tells the target to step the corresponding range of addresses itself instead\n\
+of issuing multiple single-steps. This speeds up source level\n\
+stepping. If off, GDB always issues single-steps, even if range\n\
+stepping is supported by the target. The default is on."),
+ set_range_stepping,
+ show_range_stepping,
+ &setlist,
+ &showlist);
+
/* Eventually initialize fileio. See fileio.c */
initialize_remote_fileio (remote_set_cmdlist, remote_show_cmdlist);
^ permalink raw reply [flat|nested] 38+ messages in thread* Re: [PATCH 3/5] range stepping: gdb
2013-05-20 18:43 ` Pedro Alves
@ 2013-05-20 19:05 ` Eli Zaretskii
2013-05-23 0:47 ` Yao Qi
1 sibling, 0 replies; 38+ messages in thread
From: Eli Zaretskii @ 2013-05-20 19:05 UTC (permalink / raw)
To: Pedro Alves; +Cc: gdb-patches
> Date: Mon, 20 May 2013 19:43:13 +0100
> From: Pedro Alves <palves@redhat.com>
> CC: gdb-patches@sourceware.org
>
> On 05/16/2013 07:08 AM, Eli Zaretskii wrote:
> >> Date: Wed, 15 May 2013 19:20:46 +0100
> >> From: Pedro Alves <palves@redhat.com>
> >> CC: gdb-patches@sourceware.org
> >>
> >> What do you think of this?
> >
> > I think it's a really good and clear description.
> >
> >> I'm okay with removing the whole second paragraph ("If the range is
> >> empty...") if you think the first paragraph is already clear enough.
> >
> > The first paragraph is clear enough, but I find the second paragraph
> > reassuring me in that the interpretation of the first was correct. So
> > I think it is better to leave both.
>
> Sorry for taking long to respond, and letting the context potentially
> slip from you. Here's the full updated patch with that bit merged in.
> Nothing else in the docs changed. I've updated a comment in
> gdbthread.h, giving it a similar clarification treatment.
That's OK with me, thanks.
^ permalink raw reply [flat|nested] 38+ messages in thread* Re: [PATCH 3/5] range stepping: gdb
2013-05-20 18:43 ` Pedro Alves
2013-05-20 19:05 ` Eli Zaretskii
@ 2013-05-23 0:47 ` Yao Qi
2013-05-23 17:22 ` Pedro Alves
1 sibling, 1 reply; 38+ messages in thread
From: Yao Qi @ 2013-05-23 0:47 UTC (permalink / raw)
To: Pedro Alves; +Cc: Eli Zaretskii, gdb-patches
On 05/21/2013 02:43 AM, Pedro Alves wrote:
> diff --git a/gdb/gdbthread.h b/gdb/gdbthread.h
> index a9f8a94..e93b965 100644
> --- a/gdb/gdbthread.h
> +++ b/gdb/gdbthread.h
> @@ -65,6 +65,13 @@ struct thread_control_state
> CORE_ADDR step_range_start; /* Inclusive */
> CORE_ADDR step_range_end; /* Exclusive */
>
> + /* If GDB issues a target step request, and this is nonzero, the
> + target should single-step this thread once, and then continue
> + single-stepping it without GDB involvement as long as thread
^^^
Nit: How about "GDB core"? Since we didn't treat range stepping as
remote-specific, so other targets, such as native target, may implement
range stepping, and it is part of GDB.
This patch looks good to me.
--
Yao (é½å°§)
^ permalink raw reply [flat|nested] 38+ messages in thread* Re: [PATCH 3/5] range stepping: gdb
2013-05-23 0:47 ` Yao Qi
@ 2013-05-23 17:22 ` Pedro Alves
0 siblings, 0 replies; 38+ messages in thread
From: Pedro Alves @ 2013-05-23 17:22 UTC (permalink / raw)
To: Yao Qi; +Cc: gdb-patches
On 05/23/2013 01:47 AM, Yao Qi wrote:
> On 05/21/2013 02:43 AM, Pedro Alves wrote:
>> diff --git a/gdb/gdbthread.h b/gdb/gdbthread.h
>> index a9f8a94..e93b965 100644
>> --- a/gdb/gdbthread.h
>> +++ b/gdb/gdbthread.h
>> @@ -65,6 +65,13 @@ struct thread_control_state
>> CORE_ADDR step_range_start; /* Inclusive */
>> CORE_ADDR step_range_end; /* Exclusive */
>>
>> + /* If GDB issues a target step request, and this is nonzero, the
>> + target should single-step this thread once, and then continue
>> + single-stepping it without GDB involvement as long as thread
> ^^^
> Nit: How about "GDB core"? Since we didn't treat range stepping as remote-specific, so other targets, such as native target, may implement range stepping, and it is part of GDB.
Good idea. I did that.
>
> This patch looks good to me.
Thanks. Here's what I checked in. Nothing else changed.
----------
range stepping: gdb
This patch teaches GDB to take advantage of target-assisted range
stepping. It adds a new 'r ADDR1,ADDR2' action to vCont (vCont;r),
meaning, "step once, and keep stepping as long as the thread is in the
[ADDR1,ADDR2) range".
Rationale:
When user issues the "step" command on the following line of source,
a = b + c + d * e - a;
GDB single-steps every single instruction until the program reaches a
new different line. E.g., on x86_64, that line compiles to:
0x08048434 <+65>: mov 0x1c(%esp),%eax
0x08048438 <+69>: mov 0x30(%esp),%edx
0x0804843c <+73>: add %eax,%edx
0x0804843e <+75>: mov 0x18(%esp),%eax
0x08048442 <+79>: imul 0x2c(%esp),%eax
0x08048447 <+84>: add %edx,%eax
0x08048449 <+86>: sub 0x34(%esp),%eax
0x0804844d <+90>: mov %eax,0x34(%esp)
0x08048451 <+94>: mov 0x1c(%esp),%eax
and the following is the RSP traffic between GDB and GDBserver:
--> vCont;s:p2e13.2e13;c
<-- T0505:68efffbf;04:30efffbf;08:3c840408;thread:p2e13.2e13;core:1;
--> vCont;s:p2e13.2e13;c
<-- T0505:68efffbf;04:30efffbf;08:3e840408;thread:p2e13.2e13;core:2;
--> vCont;s:p2e13.2e13;c
<-- T0505:68efffbf;04:30efffbf;08:42840408;thread:p2e13.2e13;core:2;
--> vCont;s:p2e13.2e13;c
<-- T0505:68efffbf;04:30efffbf;08:47840408;thread:p2e13.2e13;core:0;
--> vCont;s:p2e13.2e13;c
<-- T0505:68efffbf;04:30efffbf;08:49840408;thread:p2e13.2e13;core:0;
--> vCont;s:p2e13.2e13;c
<-- T0505:68efffbf;04:30efffbf;08:4d840408;thread:p2e13.2e13;core:0;
--> vCont;s:p2e13.2e13;c
<-- T0505:68efffbf;04:30efffbf;08:51840408;thread:p2e13.2e13;core:0;
IOW, a lot of roundtrips between GDB and GDBserver.
If we add a new command to the RSP, meaning "keep stepping and don't
report a stop until the program goes out of the [0x08048434,
0x08048451) address range", then the RSP traffic can be reduced down
to:
--> vCont;r8048434,8048451:p2db0.2db0;c
<-- T0505:68efffbf;04:30efffbf;08:51840408;thread:p2db0.2db0;core:1;
As number of packets is reduced dramatically, the performance of
stepping source lines is much improved.
In case something is wrong with range stepping on the stub side, the
debug info or even gdb, this adds a "set/show range-stepping" command
to be able to turn range stepping off.
gdb/
2013-05-23 Yao Qi <yao@codesourcery.com>
Pedro Alves <palves@redhat.com>
* gdbthread.h (struct thread_control_state) <may_range_step>: New
field.
* infcmd.c (step_once, until_next_command): Enable range stepping.
* infrun.c (displaced_step_prepare): Disable range stepping.
(resume): Disable range stepping if stepping over a breakpoint or
we have software watchpoints. If range stepping is enabled,
assert the thread is in the stepping range.
(clear_proceed_status_thread): Clear may_range_step.
(handle_inferior_event): Disable range stepping as soon as we know
the thread that hit the event. Re-enable it whenever we're going
to step with a step range.
* remote.c (struct vCont_action_support) <r>: New field.
(use_range_stepping): New global.
(remote_vcont_probe): Handle 'r' action.
(append_resumption): Append an 'r' action if the thread may range
step.
(show_range_stepping): New function.
(set_range_stepping): New function.
(_initialize_remote): Call add_setshow_boolean_cmd to register the
'set range-stepping' and 'show range-stepping' commands.
* NEWS: Mention range stepping, the new vCont;r action, and the
new "set/show range-stepping" commands.
gdb/doc/
2013-05-23 Yao Qi <yao@codesourcery.com>
Pedro Alves <palves@redhat.com>
* gdb.texinfo (Packets): Document 'vCont;r'.
(Continuing and Stepping): Document target-assisted range
stepping, and the 'set range-stepping' and 'show range-stepping'
commands.
---
gdb/NEWS | 17 +++++++++
gdb/doc/gdb.texinfo | 48 +++++++++++++++++++++++++
gdb/gdbthread.h | 8 ++++
gdb/infcmd.c | 8 ++++
gdb/infrun.c | 33 +++++++++++++++++
gdb/remote.c | 98 +++++++++++++++++++++++++++++++++++++++++++++++++++
6 files changed, 211 insertions(+), 1 deletion(-)
diff --git a/gdb/NEWS b/gdb/NEWS
index c6a5e5d..1a4140d 100644
--- a/gdb/NEWS
+++ b/gdb/NEWS
@@ -41,6 +41,10 @@ set debug nios2
show debug nios2
Control display of debugging messages related to Nios II targets.
+set range-stepping
+show range-stepping
+ Control whether target-assisted range stepping is enabled.
+
* You can now use a literal value 'unlimited' for options that
interpret 0 or -1 as meaning "unlimited". E.g., "set
trace-buffer-size unlimited" is now an alias for "set
@@ -78,6 +82,19 @@ show debug nios2
** ElinOS
** Wind River Linux
+* GDB now supports target-assigned range stepping with remote targets.
+ This improves the performance of stepping source lines by reducing
+ the number of control packets from/to GDB. See "New remote packets"
+ below.
+
+* New remote packets
+
+vCont;r
+
+ The vCont packet supports a new 'r' action, that tells the remote
+ stub to step through an address range itself, without GDB
+ involvemement at each single-step.
+
*** Changes in GDB 7.6
* Target record has been renamed to record-full.
diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index b68d2f8..721e96a 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -5219,6 +5219,38 @@ Execute one machine instruction, but if it is a function call,
proceed until the function returns.
An argument is a repeat count, as in @code{next}.
+
+@end table
+
+@anchor{range stepping}
+@cindex range stepping
+@cindex target-assisted range stepping
+By default, and if available, @value{GDBN} makes use of
+target-assisted @dfn{range stepping}. In other words, whenever you
+use a stepping command (e.g., @code{step}, @code{next}), @value{GDBN}
+tells the target to step the corresponding range of instruction
+addresses instead of issuing multiple single-steps. This speeds up
+line stepping, particularly for remote targets. Ideally, there should
+be no reason you would want to turn range stepping off. However, it's
+possible that a bug in the debug info, a bug in the remote stub (for
+remote targets), or even a bug in @value{GDBN} could make line
+stepping behave incorrectly when target-assisted range stepping is
+enabled. You can use the following command to turn off range stepping
+if necessary:
+
+@table @code
+@kindex set range-stepping
+@kindex show range-stepping
+@item set range-stepping
+@itemx show range-stepping
+Control whether range stepping is enabled.
+
+If @code{on}, and the target supports it, @value{GDBN} tells the
+target to step a range of addresses itself, instead of issuing
+multiple single-steps. If @code{off}, @value{GDBN} always issues
+single-steps, even if range stepping is supported by the target. The
+default is @code{on}.
+
@end table
@node Skipping Over Functions and Files
@@ -37511,6 +37543,22 @@ Step.
Step with signal @var{sig}. The signal @var{sig} should be two hex digits.
@item t
Stop.
+@item r @var{start},@var{end}
+Step once, and then keep stepping as long as the thread stops at
+addresses between @var{start} (inclusive) and @var{end} (exclusive).
+The remote stub reports a stop reply when either the thread goes out
+of the range or is stopped due to an unrelated reason, such as hitting
+a breakpoint. @xref{range stepping}.
+
+If the range is empty (@var{start} == @var{end}), then the action
+becomes equivalent to the @samp{s} action. In other words,
+single-step once, and report the stop (even if the stepped instruction
+jumps to @var{start}).
+
+(A stop reply may be sent at any point even if the PC is still within
+the stepping range; for example, it is valid to implement this packet
+in a degenerate way as a single instruction step operation.)
+
@end table
The optional argument @var{addr} normally associated with the
diff --git a/gdb/gdbthread.h b/gdb/gdbthread.h
index a9f8a94..c3b85dc 100644
--- a/gdb/gdbthread.h
+++ b/gdb/gdbthread.h
@@ -65,6 +65,14 @@ struct thread_control_state
CORE_ADDR step_range_start; /* Inclusive */
CORE_ADDR step_range_end; /* Exclusive */
+ /* If GDB issues a target step request, and this is nonzero, the
+ target should single-step this thread once, and then continue
+ single-stepping it without GDB core involvement as long as the
+ thread stops in the step range above. If this is zero, the
+ target should ignore the step range, and only issue one single
+ step. */
+ int may_range_step;
+
/* Stack frame address as of when stepping command was issued.
This is how we know when we step into a subroutine call, and how
to set the frame for the breakpoint used to step out. */
diff --git a/gdb/infcmd.c b/gdb/infcmd.c
index aeb24ff..30621e4 100644
--- a/gdb/infcmd.c
+++ b/gdb/infcmd.c
@@ -1046,9 +1046,14 @@ step_once (int skip_subroutines, int single_inst, int count, int thread)
&tp->control.step_range_start,
&tp->control.step_range_end);
+ tp->control.may_range_step = 1;
+
/* If we have no line info, switch to stepi mode. */
if (tp->control.step_range_end == 0 && step_stop_if_no_debug)
- tp->control.step_range_start = tp->control.step_range_end = 1;
+ {
+ tp->control.step_range_start = tp->control.step_range_end = 1;
+ tp->control.may_range_step = 0;
+ }
else if (tp->control.step_range_end == 0)
{
const char *name;
@@ -1337,6 +1342,7 @@ until_next_command (int from_tty)
tp->control.step_range_start = BLOCK_START (SYMBOL_BLOCK_VALUE (func));
tp->control.step_range_end = sal.end;
}
+ tp->control.may_range_step = 1;
tp->control.step_over_calls = STEP_OVER_ALL;
diff --git a/gdb/infrun.c b/gdb/infrun.c
index 57c427d..376a440 100644
--- a/gdb/infrun.c
+++ b/gdb/infrun.c
@@ -1311,6 +1311,7 @@ static int
displaced_step_prepare (ptid_t ptid)
{
struct cleanup *old_cleanups, *ignore_cleanups;
+ struct thread_info *tp = find_thread_ptid (ptid);
struct regcache *regcache = get_thread_regcache (ptid);
struct gdbarch *gdbarch = get_regcache_arch (regcache);
CORE_ADDR original, copy;
@@ -1323,6 +1324,12 @@ displaced_step_prepare (ptid_t ptid)
support displaced stepping. */
gdb_assert (gdbarch_displaced_step_copy_insn_p (gdbarch));
+ /* Disable range stepping while executing in the scratch pad. We
+ want a single-step even if executing the displaced instruction in
+ the scratch buffer lands within the stepping range (e.g., a
+ jump/branch). */
+ tp->control.may_range_step = 0;
+
/* We have to displaced step one thread at a time, as we only have
access to a single scratch space per inferior. */
@@ -1778,6 +1785,11 @@ how to step past a permanent breakpoint on this architecture. Try using\n\
a command like `return' or `jump' to continue execution."));
}
+ /* If we have a breakpoint to step over, make sure to do a single
+ step only. Same if we have software watchpoints. */
+ if (tp->control.trap_expected || bpstat_should_step ())
+ tp->control.may_range_step = 0;
+
/* If enabled, step over breakpoints by executing a copy of the
instruction at a different address.
@@ -1939,6 +1951,16 @@ a command like `return' or `jump' to continue execution."));
displaced_step_dump_bytes (gdb_stdlog, buf, sizeof (buf));
}
+ if (tp->control.may_range_step)
+ {
+ /* If we're resuming a thread with the PC out of the step
+ range, then we're doing some nested/finer run control
+ operation, like stepping the thread out of the dynamic
+ linker or the displaced stepping scratch pad. We
+ shouldn't have allowed a range step then. */
+ gdb_assert (pc_in_thread_step_range (pc, tp));
+ }
+
/* Install inferior's terminal modes. */
target_terminal_inferior ();
@@ -1980,6 +2002,7 @@ clear_proceed_status_thread (struct thread_info *tp)
tp->control.trap_expected = 0;
tp->control.step_range_start = 0;
tp->control.step_range_end = 0;
+ tp->control.may_range_step = 0;
tp->control.step_frame_id = null_frame_id;
tp->control.step_stack_frame_id = null_frame_id;
tp->control.step_over_calls = STEP_OVER_UNDEBUGGABLE;
@@ -3223,6 +3246,10 @@ handle_inferior_event (struct execution_control_state *ecs)
/* If it's a new thread, add it to the thread database. */
if (ecs->event_thread == NULL)
ecs->event_thread = add_thread (ecs->ptid);
+
+ /* Disable range stepping. If the next step request could use a
+ range, this will be end up re-enabled then. */
+ ecs->event_thread->control.may_range_step = 0;
}
/* Dependent on valid ECS->EVENT_THREAD. */
@@ -4717,6 +4744,11 @@ process_event_stop_test:
paddress (gdbarch, ecs->event_thread->control.step_range_start),
paddress (gdbarch, ecs->event_thread->control.step_range_end));
+ /* Tentatively re-enable range stepping; `resume' disables it if
+ necessary (e.g., if we're stepping over a breakpoint or we
+ have software watchpoints). */
+ ecs->event_thread->control.may_range_step = 1;
+
/* When stepping backward, stop at beginning of line range
(unless it's the function entry point, in which case
keep going back to the call point). */
@@ -5233,6 +5265,7 @@ process_event_stop_test:
ecs->event_thread->control.step_range_start = stop_pc_sal.pc;
ecs->event_thread->control.step_range_end = stop_pc_sal.end;
+ ecs->event_thread->control.may_range_step = 1;
set_step_info (frame, stop_pc_sal);
if (debug_infrun)
diff --git a/gdb/remote.c b/gdb/remote.c
index eb58b94..658fae2 100644
--- a/gdb/remote.c
+++ b/gdb/remote.c
@@ -260,8 +260,15 @@ struct vCont_action_support
{
/* vCont;t */
int t;
+
+ /* vCont;r */
+ int r;
};
+/* Controls whether GDB is willing to use range stepping. */
+
+static int use_range_stepping = 1;
+
/* Description of the remote protocol state for the currently
connected target. This is per-target state, and independent of the
selected architecture. */
@@ -4653,6 +4660,7 @@ remote_vcont_probe (struct remote_state *rs)
support_c = 0;
support_C = 0;
rs->supports_vCont.t = 0;
+ rs->supports_vCont.r = 0;
while (p && *p == ';')
{
p++;
@@ -4666,6 +4674,8 @@ remote_vcont_probe (struct remote_state *rs)
support_C = 1;
else if (*p == 't' && (*(p + 1) == ';' || *(p + 1) == 0))
rs->supports_vCont.t = 1;
+ else if (*p == 'r' && (*(p + 1) == ';' || *(p + 1) == 0))
+ rs->supports_vCont.r = 1;
p = strchr (p, ';');
}
@@ -4697,6 +4707,42 @@ append_resumption (char *p, char *endp,
if (step && siggnal != GDB_SIGNAL_0)
p += xsnprintf (p, endp - p, ";S%02x", siggnal);
+ else if (step
+ /* GDB is willing to range step. */
+ && use_range_stepping
+ /* Target supports range stepping. */
+ && rs->supports_vCont.r
+ /* We don't currently support range stepping multiple
+ threads with a wildcard (though the protocol allows it,
+ so stubs shouldn't make an active effort to forbid
+ it). */
+ && !(remote_multi_process_p (rs) && ptid_is_pid (ptid)))
+ {
+ struct thread_info *tp;
+
+ if (ptid_equal (ptid, minus_one_ptid))
+ {
+ /* If we don't know about the target thread's tid, then
+ we're resuming magic_null_ptid (see caller). */
+ tp = find_thread_ptid (magic_null_ptid);
+ }
+ else
+ tp = find_thread_ptid (ptid);
+ gdb_assert (tp != NULL);
+
+ if (tp->control.may_range_step)
+ {
+ int addr_size = gdbarch_addr_bit (target_gdbarch ()) / 8;
+
+ p += xsnprintf (p, endp - p, ";r%s,%s",
+ phex_nz (tp->control.step_range_start,
+ addr_size),
+ phex_nz (tp->control.step_range_end,
+ addr_size));
+ }
+ else
+ p += xsnprintf (p, endp - p, ";s");
+ }
else if (step)
p += xsnprintf (p, endp - p, ";s");
else if (siggnal != GDB_SIGNAL_0)
@@ -11659,6 +11705,44 @@ remote_upload_trace_state_variables (struct uploaded_tsv **utsvp)
return 0;
}
+/* The "set/show range-stepping" show hook. */
+
+static void
+show_range_stepping (struct ui_file *file, int from_tty,
+ struct cmd_list_element *c,
+ const char *value)
+{
+ fprintf_filtered (file,
+ _("Debugger's willingness to use range stepping "
+ "is %s.\n"), value);
+}
+
+/* The "set/show range-stepping" set hook. */
+
+static void
+set_range_stepping (char *ignore_args, int from_tty,
+ struct cmd_list_element *c)
+{
+ /* Whene enabling, check whether range stepping is actually
+ supported by the target, and warn if not. */
+ if (use_range_stepping)
+ {
+ if (remote_desc != NULL)
+ {
+ struct remote_state *rs = get_remote_state ();
+
+ if (remote_protocol_packets[PACKET_vCont].support == PACKET_SUPPORT_UNKNOWN)
+ remote_vcont_probe (rs);
+
+ if (remote_protocol_packets[PACKET_vCont].support == PACKET_ENABLE
+ && rs->supports_vCont.r)
+ return;
+ }
+
+ warning (_("Range stepping is not supported by the current target"));
+ }
+}
+
void
_initialize_remote (void)
{
@@ -12056,6 +12140,20 @@ Set the remote pathname for \"run\""), _("\
Show the remote pathname for \"run\""), NULL, NULL, NULL,
&remote_set_cmdlist, &remote_show_cmdlist);
+ add_setshow_boolean_cmd ("range-stepping", class_run,
+ &use_range_stepping, _("\
+Enable or disable range stepping."), _("\
+Show whether target-assisted range stepping is enabled."), _("\
+If on, and the target supports it, when stepping a source line, GDB\n\
+tells the target to step the corresponding range of addresses itself instead\n\
+of issuing multiple single-steps. This speeds up source level\n\
+stepping. If off, GDB always issues single-steps, even if range\n\
+stepping is supported by the target. The default is on."),
+ set_range_stepping,
+ show_range_stepping,
+ &setlist,
+ &showlist);
+
/* Eventually initialize fileio. See fileio.c */
initialize_remote_fileio (remote_set_cmdlist, remote_show_cmdlist);
^ permalink raw reply [flat|nested] 38+ messages in thread
* [PATCH 5/5] range stepping: tests
2013-05-14 19:10 [PATCH 0/5 V3] target-assisted range stepping Pedro Alves
` (3 preceding siblings ...)
2013-05-14 19:10 ` [PATCH 3/5] range stepping: gdb Pedro Alves
@ 2013-05-14 19:11 ` Pedro Alves
2013-05-22 14:32 ` Yao Qi
2013-05-14 20:21 ` [PATCH 0/5 V3] target-assisted range stepping Tom Tromey
2013-05-23 1:02 ` Yao Qi
6 siblings, 1 reply; 38+ messages in thread
From: Pedro Alves @ 2013-05-14 19:11 UTC (permalink / raw)
To: gdb-patches
This adds tests to verify range stepping is used as expected, by
inspecting the rsp packets, looking for vCont;s and vCont;r packets.
- v3
- back to using rsp packets.
- use macros instead of writting many statements on the same line.
- added range stepping + software watchpoints test.
- tracepoint's test now uses a line that is a single range.
- all messages in gdb.sum are now unique.
- vCont counter function renamed to exec_cmd_expect_vCont_count,
adjusted to prevent expect buffer overflow, and adjusted issue a
single pass/fail.
- many other tweaks.
gdb/testsuite/
2013-05-14 Yao Qi <yao@codesourcery.com>
Pedro Alves <palves@redhat.com>
* gdb.base/range-stepping.c: New file.
* gdb.base/range-stepping.exp: New file.
* gdb.trace/range-stepping.c: New file.
* gdb.trace/range-stepping.exp: New file.
* lib/range-stepping-support.exp: New file.
---
gdb/testsuite/gdb.base/range-stepping.c | 103 +++++++++++
gdb/testsuite/gdb.base/range-stepping.exp | 237 ++++++++++++++++++++++++++
gdb/testsuite/gdb.trace/range-stepping.c | 56 ++++++
gdb/testsuite/gdb.trace/range-stepping.exp | 85 +++++++++
gdb/testsuite/lib/range-stepping-support.exp | 50 +++++
5 files changed, 531 insertions(+)
create mode 100644 gdb/testsuite/gdb.base/range-stepping.c
create mode 100644 gdb/testsuite/gdb.base/range-stepping.exp
create mode 100644 gdb/testsuite/gdb.trace/range-stepping.c
create mode 100644 gdb/testsuite/gdb.trace/range-stepping.exp
create mode 100644 gdb/testsuite/lib/range-stepping-support.exp
diff --git a/gdb/testsuite/gdb.base/range-stepping.c b/gdb/testsuite/gdb.base/range-stepping.c
new file mode 100644
index 0000000..eb469fd
--- /dev/null
+++ b/gdb/testsuite/gdb.base/range-stepping.c
@@ -0,0 +1,103 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+ Copyright 2013 Free Software Foundation, Inc.
+
+ This program is free software; you can redistribute it and/or modify
+ it under the terms of the GNU General Public License as published by
+ the Free Software Foundation; either version 3 of the License, or
+ (at your option) any later version.
+
+ This program is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ GNU General Public License for more details.
+
+ You should have received a copy of the GNU General Public License
+ along with this program. If not, see <http://www.gnu.org/licenses/>. */
+
+/* Note: 'volatile' is used to make sure the compiler doesn't fold /
+ optimize out the arithmetic that uses the variables. */
+
+static int
+func1 (int a, int b)
+{
+ volatile int r = a * b;
+
+ r += (a | b);
+ r += (a - b);
+
+ return r;
+}
+
+int
+main(void)
+{
+ volatile int a = 0;
+ volatile int b = 1;
+ volatile int c = 2;
+ volatile int d = 3;
+ volatile int e = 4;
+ volatile double d1 = 1.0;
+ volatile double d2 = 2.0;
+
+ /* A macro uses to build source lines that compile to a number of
+ instructions, with no branches. */
+#define MULTIPLE_INSTRUCTIONS \
+ do { a = b + c + d * e - a; } while (0)
+
+ /* A line of source code that will be compiled to a number of
+ instructions. */
+ MULTIPLE_INSTRUCTIONS; /* location 1 */
+
+ /* A line of source code that compiles to a function call (jump or
+ branch), surrounded by instructions before and after. IOW, this
+ will generate approximately the following pseudo-instructions:
+
+addr1:
+ insn1;
+ insn2;
+ ...
+ call func1;
+ ...
+ insn3;
+addr2:
+ insn4;
+*/
+ e = 10 + func1 (a + b, c * d); /* location 2 */
+
+ e = 10 + func1 (a + b, c * d);
+
+ /* Generate a range that includes a loop in it. */
+#define RANGE_WITH_LOOP \
+ do \
+ { \
+ for (a = 0, e = 0; a < 15; a++) \
+ e += a; \
+ } while (0)
+
+ RANGE_WITH_LOOP;
+
+ RANGE_WITH_LOOP;
+
+ /* Generate a range that includes a time-consuming loop. GDB breaks
+ the loop early by clearing variable 'c'. */
+#define RANGE_WITH_TIME_CONSUMING_LOOP \
+ do \
+ { \
+ for (c = 1, a = 0; a < 65535 && c; a++) \
+ for (b = 0; b < 65535 && c; b++) \
+ { \
+ d1 = d2 * a / b; \
+ d2 = d1 * a; \
+ } \
+ } while (0)
+
+ RANGE_WITH_TIME_CONSUMING_LOOP;
+
+ /* Some multi-instruction lines for software watchpoint tests. */
+ MULTIPLE_INSTRUCTIONS;
+ MULTIPLE_INSTRUCTIONS; /* soft-watch */
+ MULTIPLE_INSTRUCTIONS;
+
+ return 0;
+}
diff --git a/gdb/testsuite/gdb.base/range-stepping.exp b/gdb/testsuite/gdb.base/range-stepping.exp
new file mode 100644
index 0000000..def25ce
--- /dev/null
+++ b/gdb/testsuite/gdb.base/range-stepping.exp
@@ -0,0 +1,237 @@
+# Copyright 2013 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program. If not, see <http://www.gnu.org/licenses/>.
+
+load_lib "range-stepping-support.exp"
+
+standard_testfile
+set executable $testfile
+
+if { [prepare_for_testing $testfile.exp $testfile $srcfile {debug}] } {
+ return -1
+}
+
+if ![runto_main] {
+ fail "Can't run to main"
+ return -1
+}
+
+# Check whether range stepping is supported by the target.
+
+proc gdb_range_stepping_enabled { } {
+ global gdb_prompt
+
+ set command "set range-stepping on"
+ set message "probe range-stepping support"
+ gdb_test_multiple $command $message {
+ -re "Range stepping is not supported.*\r\n$gdb_prompt $" {
+ pass $message
+ return 0
+ }
+ -re "^$command\r\n$gdb_prompt $" {
+ pass $message
+ return 1
+ }
+ }
+
+ return 0
+}
+
+if ![gdb_range_stepping_enabled] {
+ unsupported "range stepping not supported by the target"
+ return -1
+}
+
+# Check that range stepping can step a range of multiple instructions.
+
+with_test_prefix "multi insns" {
+
+ gdb_breakpoint [gdb_get_line_number "location 1"]
+ gdb_continue_to_breakpoint "location 1"
+
+ set pc_before_stepping ""
+ set test "pc before stepping"
+ gdb_test_multiple "print/x \$pc" $test {
+ -re "\\\$$decimal = (\[^\r\n\]*)\r\n$gdb_prompt $" {
+ set pc_before_stepping $expect_out(1,string)
+ pass $test
+ }
+ }
+
+ # When "next" is executed, GDB should send one vCont;s and vCont;r
+ # and receive two stop replies:
+ #
+ # --> vCont;s (step over breakpoint)
+ # <-- T05
+ # --> vCont;rSTART,END (range step)
+ # <-- T05
+ exec_cmd_expect_vCont_count "next" 1 1
+
+ set pc_after_stepping ""
+ set msg "pc after stepping"
+ gdb_test_multiple "print/x \$pc" $msg {
+ -re "\\\$$decimal = (\[^\r\n\]*)\r\n$gdb_prompt $" {
+ set pc_after_stepping $expect_out(1,string)
+ pass $msg
+ }
+ }
+
+ # There should be at least two instructions between
+ # PC_BEFORE_STEPPING and PC_AFTER_STEPPING.
+ gdb_test "disassemble ${pc_before_stepping},${pc_after_stepping}" \
+ "${hex} <main\\+${decimal}>:.*${hex} <main\\+${decimal}>:.*" \
+ "stepped multiple insns"
+}
+
+# Check that range stepping can step over a function.
+
+with_test_prefix "step over func" {
+
+ set line_num [gdb_get_line_number "location 2"]
+ gdb_test "where" "main \\(\\) at .*${srcfile}:${line_num}.*"
+
+ # It's expected to get three stops and two 'vCont;r's. In the C
+ # code, the line of C source produces roughly the following
+ # instructions:
+ #
+ # addr1:
+ # insn1
+ # insn2
+ # ...
+ # call func1
+ # addr2:
+ # ...
+ # insn3
+ # addr3:
+ # insn4
+ #
+ # Something like this will happen:
+ # --> vCont;rADDR1,ADDR3 (range step from ADDR1 to ADDR3)
+ # <-- T05 (target single-stepped to func, which is out of the step range)
+ # --> $Z0,ADDR2 (place step-resume breakpoint at ADDR2)
+ # --> vCont;c (resume)
+ # <-- T05 (target stops at ADDR2)
+ # --> vCont;rADDR1,ADDR3 (continues range stepping)
+ # <-- T05
+ exec_cmd_expect_vCont_count "next" 0 2
+}
+
+# Check that breapoints interrupt range stepping correctly.
+
+with_test_prefix "breakpoint" {
+ gdb_breakpoint "func1"
+ # Something like this will happen:
+ # --> vCont;rADDR1,ADDR3
+ # <-- T05 (target single-steps to func1, which is out of the step range)
+ # --> $Z0,ADDR2 (step-resume breakpoint at ADDR2)
+ # --> vCont;c (resume)
+ # <-- T05 (target hits the breakpoint at func1)
+ exec_cmd_expect_vCont_count "next" 0 1
+
+ gdb_test "backtrace" "#0 .* func1 .*#1 .* main .*" \
+ "backtrace from func1"
+
+ # A cancelled range step should not confuse the following
+ # execution commands.
+ exec_cmd_expect_vCont_count "stepi" 1 0
+ gdb_test "finish" ".*"
+ gdb_test "next" ".*"
+ delete_breakpoints
+}
+
+# Check that range stepping works well even when there's a loop in the
+# step range.
+
+with_test_prefix "loop" {
+
+ # GDB should send one vCont;r and receive one stop reply:
+ # --> vCont;rSTART,END (range step)
+ # <-- T05
+ exec_cmd_expect_vCont_count "next" 0 1
+
+ # Confirm the loop completed.
+ gdb_test "print a" " = 15"
+ gdb_test "print e" " = 105"
+}
+
+# Check that range stepping works well even when the target's PC was
+# already within the loop's body.
+
+with_test_prefix "loop 2" {
+ # Stepi into the loop body. 15 should be large enough to make
+ # sure the program stops within the loop's body.
+ gdb_test "stepi 15" ".*"
+ # GDB should send one vCont;r and receive one stop reply:
+ # --> vCont;rSTART,END (range step)
+ # <-- T05
+ exec_cmd_expect_vCont_count "next" 0 1
+
+ # Confirm the loop completed.
+ gdb_test "print a" " = 15"
+ gdb_test "print e" " = 105"
+}
+
+# Check that range stepping works well even when it is interrupted by
+# ctrl-c.
+
+with_test_prefix "interrupt" {
+ gdb_test_no_output "set debug remote 1"
+
+ send_gdb "next\n"
+ sleep 1
+ send_gdb "\003"
+
+ # GDB should send one vCont;r and receive one stop reply for
+ # SIGINT:
+ # --> vCont;rSTART,END (range step)
+ # <-- T02 (SIGINT)
+
+ set vcont_r_counter 0
+
+ set test "send ctrl-c to GDB"
+ gdb_test_multiple "" $test {
+ -re "vCont;r\[^\r\n\]*\.\.\." {
+ incr vcont_r_counter
+ exp_continue
+ }
+ -re "Program received signal SIGINT.*$gdb_prompt $" {
+ pass $test
+ }
+ }
+ gdb_test_no_output "set debug remote 0"
+
+ # Check the number of 'vCont;r' packets.
+ if { $vcont_r_counter == 1 } {
+ pass "${test}: 1 vCont;r"
+ } else {
+ fail "${test}: 1 vCont;r"
+ }
+
+ # Break the loop earlier and continue range stepping.
+ gdb_test "set variable c = 0"
+ exec_cmd_expect_vCont_count "next" 0 1
+}
+
+# Check that range stepping doesn't break software watchpoints. With
+# those, GDB needs to be notified of all single-steps, to evaluate
+# whether the watched value changes at each step.
+with_test_prefix "software watchpoint" {
+ gdb_test "step" "soft-watch.*" "step into multiple instruction line"
+ # A software watchpoint at PC makes the thread stop before the
+ # whole line range is over (after one single-step, actually).
+ gdb_test "watch \$pc" ".*" "set watchpoint"
+ gdb_test "step" "soft-watch.*" "step still in same line"
+}
+
+return 0
diff --git a/gdb/testsuite/gdb.trace/range-stepping.c b/gdb/testsuite/gdb.trace/range-stepping.c
new file mode 100644
index 0000000..122c332
--- /dev/null
+++ b/gdb/testsuite/gdb.trace/range-stepping.c
@@ -0,0 +1,56 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+ Copyright 2013 Free Software Foundation, Inc.
+
+ This program is free software; you can redistribute it and/or modify
+ it under the terms of the GNU General Public License as published by
+ the Free Software Foundation; either version 3 of the License, or
+ (at your option) any later version.
+
+ This program is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ GNU General Public License for more details.
+
+ You should have received a copy of the GNU General Public License
+ along with this program. If not, see <http://www.gnu.org/licenses/>. */
+
+#ifdef SYMBOL_PREFIX
+#define SYMBOL(str) SYMBOL_PREFIX #str
+#else
+#define SYMBOL(str) #str
+#endif
+
+/* `set_point' further below is the label where we'll set tracepoints
+ at. The insn at the label must the large enough to fit a fast
+ tracepoint jump. */
+#if (defined __x86_64__ || defined __i386__)
+# define NOP " .byte 0xe9,0x00,0x00,0x00,0x00\n" /* jmp $+5 (5-byte nop) */
+#else
+# define NOP "" /* port me */
+#endif
+
+int
+main(void)
+{
+ /* Note: 'volatile' is used to make sure the compiler doesn't
+ optimize out these variables. We want to be sure instructions
+ are generated for accesses. */
+ volatile int i = 0;
+
+ /* Generate a single-range line with a label in the middle where we
+ can place either a trap tracepoint or a fast tracepoint. */
+#define LINE_WITH_FAST_TRACEPOINT \
+ do { \
+ i = 1; \
+ asm (" .global " SYMBOL(set_point) "\n" \
+ SYMBOL(set_point) ":\n" \
+ NOP \
+ ); \
+ i = 2; \
+ } while (0)
+
+ LINE_WITH_FAST_TRACEPOINT; /* location 1 */
+
+ return 0;
+}
diff --git a/gdb/testsuite/gdb.trace/range-stepping.exp b/gdb/testsuite/gdb.trace/range-stepping.exp
new file mode 100644
index 0000000..5cd81b6
--- /dev/null
+++ b/gdb/testsuite/gdb.trace/range-stepping.exp
@@ -0,0 +1,85 @@
+# Copyright 2013 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program. If not, see <http://www.gnu.org/licenses/>.
+
+load_lib "trace-support.exp"
+load_lib "range-stepping-support.exp"
+
+standard_testfile
+set executable $testfile
+
+if [prepare_for_testing $testfile.exp $executable $srcfile \
+ {debug nowarnings}] {
+ return -1
+}
+
+if ![runto_main] {
+ fail "Can't run to main to check for trace support"
+ return -1
+}
+
+if ![gdb_target_supports_trace] {
+ unsupported "target does not support trace"
+ return -1;
+}
+
+# Check that range stepping works well with tracepoints.
+
+proc range_stepping_with_tracepoint { type } {
+ with_test_prefix "${type}" {
+ gdb_breakpoint [gdb_get_line_number "location 1"]
+ gdb_continue_to_breakpoint "location 1"
+ delete_breakpoints
+
+ gdb_test "${type} *set_point" ".*"
+ gdb_test_no_output "tstart"
+
+ # Step a line with a tracepoint in the middle. The tracepoint
+ # itself shouldn't have any effect on range stepping. We
+ # should see one vCont;r and no vCont;s's.
+ exec_cmd_expect_vCont_count "step" 0 1
+ gdb_test_no_output "tstop"
+ gdb_test "tfind" "Found trace frame .*" "first tfind"
+ gdb_test "tfind" \
+ "Target failed to find requested trace frame.*" \
+ "second tfind"
+
+ delete_breakpoints
+ }
+}
+
+range_stepping_with_tracepoint "trace"
+
+set libipa [get_in_proc_agent]
+gdb_load_shlibs $libipa
+
+if { [gdb_compile "$srcdir/$subdir/$srcfile" $binfile \
+ executable [list debug nowarnings shlib=$libipa] ] != "" } {
+ untested "failed to compile ftrace tests"
+ return -1
+}
+
+clean_restart ${executable}
+
+if ![runto_main] {
+ fail "Can't run to main for ftrace tests"
+ return 0
+}
+
+gdb_reinitialize_dir $srcdir/$subdir
+if { [gdb_test "info sharedlibrary" ".*${libipa}.*" "IPA loaded"] != 0 } {
+ untested "Could not find IPA lib loaded"
+} else {
+ range_stepping_with_tracepoint "ftrace"
+}
diff --git a/gdb/testsuite/lib/range-stepping-support.exp b/gdb/testsuite/lib/range-stepping-support.exp
new file mode 100644
index 0000000..d849665
--- /dev/null
+++ b/gdb/testsuite/lib/range-stepping-support.exp
@@ -0,0 +1,50 @@
+# Copyright 2013 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program. If not, see <http://www.gnu.org/licenses/>.
+
+# Execute command CMD and check that GDB sends the expected number of
+# vCont;s and vCont;r packets.
+
+proc exec_cmd_expect_vCont_count { cmd exp_vCont_s exp_vCont_r } {
+ global gdb_prompt
+
+ gdb_test_no_output "set debug remote 1" ""
+
+ set test "${cmd}: vCont;s=${exp_vCont_s} vCont;r=${exp_vCont_r}"
+ set r_counter 0
+ set s_counter 0
+ gdb_test_multiple $cmd $test {
+ -re "vCont;s\[^\r\n\]*Packet received: T\[\[:xdigit:\]\]\[\[:xdigit:\]\]" {
+ incr s_counter
+ exp_continue
+ }
+ -re "vCont;r\[^\r\n\]*Packet received: T\[\[:xdigit:\]\]\[\[:xdigit:\]\]" {
+ incr r_counter
+ exp_continue
+ }
+ -re "\r\n" {
+ # Prevent overflowing the expect buffer.
+ exp_continue
+ }
+ -re "$gdb_prompt $" {
+ if { $r_counter == ${exp_vCont_r} && $s_counter == ${exp_vCont_s} } {
+ pass $test
+ } else {
+ fail $test
+ }
+ }
+ }
+
+ gdb_test_no_output "set debug remote 0" ""
+}
^ permalink raw reply [flat|nested] 38+ messages in thread* Re: [PATCH 5/5] range stepping: tests
2013-05-14 19:11 ` [PATCH 5/5] range stepping: tests Pedro Alves
@ 2013-05-22 14:32 ` Yao Qi
2013-05-23 17:34 ` Pedro Alves
2013-05-23 18:03 ` Pedro Alves
0 siblings, 2 replies; 38+ messages in thread
From: Yao Qi @ 2013-05-22 14:32 UTC (permalink / raw)
To: Pedro Alves; +Cc: gdb-patches
On 05/15/2013 03:11 AM, Pedro Alves wrote:
Pedro,
the tests look good to me overall. Some nits below,
> +
> + /* Generate a range that includes a loop in it. */
Nit: "a range" is not clear, and we should mention that they must
belong to a single line of source. How about this?
Generate a range of instructions compiled from a single line of
source that include a loop.
> +#define RANGE_WITH_LOOP \
> + do \
> + { \
> + for (a = 0, e = 0; a < 15; a++) \
> + e += a; \
> + } while (0)
> +
> + RANGE_WITH_LOOP;
> +
> + RANGE_WITH_LOOP;
> +
> + /* Generate a range that includes a time-consuming loop. GDB breaks
> + the loop early by clearing variable 'c'. */
Likewise.
> +
> +with_test_prefix "step over func" {
> +
> + set line_num [gdb_get_line_number "location 2"]
> + gdb_test "where" "main \\(\\) at .*${srcfile}:${line_num}.*"
> +
> + # It's expected to get three stops and two 'vCont;r's. In the C
> + # code, the line of C source produces roughly the following
> + # instructions:
> + #
> + # addr1:
> + # insn1
> + # insn2
> + # ...
> + # call func1
> + # addr2:
> + # ...
> + # insn3
> + # addr3:
> + # insn4
> + #
> + # Something like this will happen:
> + # --> vCont;rADDR1,ADDR3 (range step from ADDR1 to ADDR3)
> + # <-- T05 (target single-stepped to func, which is out of the step range)
> + # --> $Z0,ADDR2 (place step-resume breakpoint at ADDR2)
> + # --> vCont;c (resume)
> + # <-- T05 (target stops at ADDR2)
> + # --> vCont;rADDR1,ADDR3 (continues range stepping)
> + # <-- T05
> + exec_cmd_expect_vCont_count "next" 0 2
> +}
> +
> +# Check that breapoints interrupt range stepping correctly.
breakpoints
> +
> + /* Generate a single-range line with a label in the middle where we
^^^^^^^^^^^^^^^^^
How about "a single line"?
> +
> +# Check that range stepping works well with tracepoints.
> +
> +proc range_stepping_with_tracepoint { type } {
> + with_test_prefix "${type}" {
> + gdb_breakpoint [gdb_get_line_number "location 1"]
> + gdb_continue_to_breakpoint "location 1"
> + delete_breakpoints
> +
> + gdb_test "${type} *set_point" ".*"
> + gdb_test_no_output "tstart"
> +
> + # Step a line with a tracepoint in the middle. The tracepoint
> + # itself shouldn't have any effect on range stepping. We
> + # should see one vCont;r and no vCont;s's.
^^^^^^^^^
"vCont;s", singular form?
The patch below is to skip the rest of range stepping tests if the
first one fails, to avoid the issue of huge number of rsp packets in
gdb.log. It is on top of your series. It works well with my stub.
I've run out of time today, and I'll have a look at the rest of
this series tomorrow.
--
Yao (é½å°§)
gdb/testsuite:
2013-05-22 Yao Qi <yao@codesourcery.com>
* gdb.base/range-stepping.exp: Skip the rest of tests if test
fails.
---
gdb/testsuite/gdb.base/range-stepping.exp | 8 ++++++++
1 files changed, 8 insertions(+), 0 deletions(-)
diff --git a/gdb/testsuite/gdb.base/range-stepping.exp b/gdb/testsuite/gdb.base/range-stepping.exp
index def25ce..aa5d34f 100644
--- a/gdb/testsuite/gdb.base/range-stepping.exp
+++ b/gdb/testsuite/gdb.base/range-stepping.exp
@@ -85,6 +85,14 @@ with_test_prefix "multi insns" {
set pc_after_stepping $expect_out(1,string)
pass $msg
}
+ -re ".*" {
+ fail $msg
+ # It is the first test on range-stepping, and the simplest
+ # one. If it fails, probably the rest of the tests fail
+ # and huge number of rsp packets will blow up the gdb.log
+ # file. Skip the rest of the tests.
+ return
+ }
}
# There should be at least two instructions between
--
1.7.7.6
^ permalink raw reply [flat|nested] 38+ messages in thread* Re: [PATCH 5/5] range stepping: tests
2013-05-22 14:32 ` Yao Qi
@ 2013-05-23 17:34 ` Pedro Alves
2013-05-23 18:03 ` Pedro Alves
1 sibling, 0 replies; 38+ messages in thread
From: Pedro Alves @ 2013-05-23 17:34 UTC (permalink / raw)
To: Yao Qi; +Cc: gdb-patches
On 05/22/2013 03:33 PM, Yao Qi wrote:
> On 05/15/2013 03:11 AM, Pedro Alves wrote:
>
> Pedro,
> the tests look good to me overall. Some nits below,
>
>> +
>> + /* Generate a range that includes a loop in it. */
>
> Nit: "a range" is not clear, and we should mention that they must
Hmm. I see. It was clear to me, because I had been thinking of
inlining, where you have a single source line, but in the debug
info, the source line ends up described as separate lines.
E.g., with a source line like:
a = 1; foo (); b = 2;
if foo ends up inlined here, then you have 3 or more step
ranges as result of that single source line. One range for
the a = 1; bit, one or more lines/ranges for foo, with the
line numbers being of foo's source, and then a range/line
for b = 2;
But I do agree this is unnecessary complication to explain
here.
> belong to a single line of source. How about this?
>
> Generate a range of instructions compiled from a single line of
> source that include a loop.
Thanks. I went with something a little different, but that was
the right push.
>
>> +#define RANGE_WITH_LOOP \
I replaced RANGE with LINE in these too.
>> +# Check that breapoints interrupt range stepping correctly.
> breakpoints
Fixed.
This must be the word with the most typos in every debugger. ;-)
>> + # Step a line with a tracepoint in the middle. The tracepoint
>> + # itself shouldn't have any effect on range stepping. We
>> + # should see one vCont;r and no vCont;s's.
> ^^^^^^^^^
> "vCont;s", singular form?
I believe plural form is actually correct.
As in, "We should see one apple and no oranges".
Below's what I checked in.
Thanks,
Pedro Alves
------
range stepping: tests
This adds tests to verify range stepping is used as expected, by
inspecting the RSP traffic, looking for vCont;s and vCont;r packets.
gdb/testsuite/
2013-05-23 Yao Qi <yao@codesourcery.com>
Pedro Alves <palves@redhat.com>
* gdb.base/range-stepping.c: New file.
* gdb.base/range-stepping.exp: New file.
* gdb.trace/range-stepping.c: New file.
* gdb.trace/range-stepping.exp: New file.
* lib/range-stepping-support.exp: New file.
---
gdb/testsuite/gdb.base/range-stepping.c | 104 +++++++++++
gdb/testsuite/gdb.base/range-stepping.exp | 237 ++++++++++++++++++++++++++
gdb/testsuite/gdb.trace/range-stepping.c | 56 ++++++
gdb/testsuite/gdb.trace/range-stepping.exp | 85 +++++++++
gdb/testsuite/lib/range-stepping-support.exp | 50 +++++
5 files changed, 532 insertions(+)
create mode 100644 gdb/testsuite/gdb.base/range-stepping.c
create mode 100644 gdb/testsuite/gdb.base/range-stepping.exp
create mode 100644 gdb/testsuite/gdb.trace/range-stepping.c
create mode 100644 gdb/testsuite/gdb.trace/range-stepping.exp
create mode 100644 gdb/testsuite/lib/range-stepping-support.exp
diff --git a/gdb/testsuite/gdb.base/range-stepping.c b/gdb/testsuite/gdb.base/range-stepping.c
new file mode 100644
index 0000000..0d6c41a
--- /dev/null
+++ b/gdb/testsuite/gdb.base/range-stepping.c
@@ -0,0 +1,104 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+ Copyright 2013 Free Software Foundation, Inc.
+
+ This program is free software; you can redistribute it and/or modify
+ it under the terms of the GNU General Public License as published by
+ the Free Software Foundation; either version 3 of the License, or
+ (at your option) any later version.
+
+ This program is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ GNU General Public License for more details.
+
+ You should have received a copy of the GNU General Public License
+ along with this program. If not, see <http://www.gnu.org/licenses/>. */
+
+/* Note: 'volatile' is used to make sure the compiler doesn't fold /
+ optimize out the arithmetic that uses the variables. */
+
+static int
+func1 (int a, int b)
+{
+ volatile int r = a * b;
+
+ r += (a | b);
+ r += (a - b);
+
+ return r;
+}
+
+int
+main(void)
+{
+ volatile int a = 0;
+ volatile int b = 1;
+ volatile int c = 2;
+ volatile int d = 3;
+ volatile int e = 4;
+ volatile double d1 = 1.0;
+ volatile double d2 = 2.0;
+
+ /* A macro that expands to a single source line that compiles to a
+ number of instructions, with no branches. */
+#define LINE_WITH_MULTIPLE_INSTRUCTIONS \
+ do \
+ { \
+ a = b + c + d * e - a; \
+ } while (0)
+
+ LINE_WITH_MULTIPLE_INSTRUCTIONS; /* location 1 */
+
+ /* A line of source code that compiles to a function call (jump or
+ branch), surrounded by instructions before and after. IOW, this
+ will generate approximately the following pseudo-instructions:
+
+addr1:
+ insn1;
+ insn2;
+ ...
+ call func1;
+ ...
+ insn3;
+addr2:
+ insn4;
+*/
+ e = 10 + func1 (a + b, c * d); /* location 2 */
+
+ e = 10 + func1 (a + b, c * d);
+
+ /* Generate a single source line that includes a short loop. */
+#define LINE_WITH_LOOP \
+ do \
+ { \
+ for (a = 0, e = 0; a < 15; a++) \
+ e += a; \
+ } while (0)
+
+ LINE_WITH_LOOP;
+
+ LINE_WITH_LOOP;
+
+ /* Generate a single source line that includes a time-consuming
+ loop. GDB breaks the loop early by clearing variable 'c'. */
+#define LINE_WITH_TIME_CONSUMING_LOOP \
+ do \
+ { \
+ for (c = 1, a = 0; a < 65535 && c; a++) \
+ for (b = 0; b < 65535 && c; b++) \
+ { \
+ d1 = d2 * a / b; \
+ d2 = d1 * a; \
+ } \
+ } while (0)
+
+ LINE_WITH_TIME_CONSUMING_LOOP;
+
+ /* Some multi-instruction lines for software watchpoint tests. */
+ LINE_WITH_MULTIPLE_INSTRUCTIONS;
+ LINE_WITH_MULTIPLE_INSTRUCTIONS; /* soft-watch */
+ LINE_WITH_MULTIPLE_INSTRUCTIONS;
+
+ return 0;
+}
diff --git a/gdb/testsuite/gdb.base/range-stepping.exp b/gdb/testsuite/gdb.base/range-stepping.exp
new file mode 100644
index 0000000..48fc15b
--- /dev/null
+++ b/gdb/testsuite/gdb.base/range-stepping.exp
@@ -0,0 +1,237 @@
+# Copyright 2013 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program. If not, see <http://www.gnu.org/licenses/>.
+
+load_lib "range-stepping-support.exp"
+
+standard_testfile
+set executable $testfile
+
+if { [prepare_for_testing $testfile.exp $testfile $srcfile {debug}] } {
+ return -1
+}
+
+if ![runto_main] {
+ fail "Can't run to main"
+ return -1
+}
+
+# Check whether range stepping is supported by the target.
+
+proc gdb_range_stepping_enabled { } {
+ global gdb_prompt
+
+ set command "set range-stepping on"
+ set message "probe range-stepping support"
+ gdb_test_multiple $command $message {
+ -re "Range stepping is not supported.*\r\n$gdb_prompt $" {
+ pass $message
+ return 0
+ }
+ -re "^$command\r\n$gdb_prompt $" {
+ pass $message
+ return 1
+ }
+ }
+
+ return 0
+}
+
+if ![gdb_range_stepping_enabled] {
+ unsupported "range stepping not supported by the target"
+ return -1
+}
+
+# Check that range stepping can step a range of multiple instructions.
+
+with_test_prefix "multi insns" {
+
+ gdb_breakpoint [gdb_get_line_number "location 1"]
+ gdb_continue_to_breakpoint "location 1"
+
+ set pc_before_stepping ""
+ set test "pc before stepping"
+ gdb_test_multiple "print/x \$pc" $test {
+ -re "\\\$$decimal = (\[^\r\n\]*)\r\n$gdb_prompt $" {
+ set pc_before_stepping $expect_out(1,string)
+ pass $test
+ }
+ }
+
+ # When "next" is executed, GDB should send one vCont;s and vCont;r
+ # and receive two stop replies:
+ #
+ # --> vCont;s (step over breakpoint)
+ # <-- T05
+ # --> vCont;rSTART,END (range step)
+ # <-- T05
+ exec_cmd_expect_vCont_count "next" 1 1
+
+ set pc_after_stepping ""
+ set msg "pc after stepping"
+ gdb_test_multiple "print/x \$pc" $msg {
+ -re "\\\$$decimal = (\[^\r\n\]*)\r\n$gdb_prompt $" {
+ set pc_after_stepping $expect_out(1,string)
+ pass $msg
+ }
+ }
+
+ # There should be at least two instructions between
+ # PC_BEFORE_STEPPING and PC_AFTER_STEPPING.
+ gdb_test "disassemble ${pc_before_stepping},${pc_after_stepping}" \
+ "${hex} <main\\+${decimal}>:.*${hex} <main\\+${decimal}>:.*" \
+ "stepped multiple insns"
+}
+
+# Check that range stepping can step over a function.
+
+with_test_prefix "step over func" {
+
+ set line_num [gdb_get_line_number "location 2"]
+ gdb_test "where" "main \\(\\) at .*${srcfile}:${line_num}.*"
+
+ # It's expected to get three stops and two 'vCont;r's. In the C
+ # code, the line of C source produces roughly the following
+ # instructions:
+ #
+ # addr1:
+ # insn1
+ # insn2
+ # ...
+ # call func1
+ # addr2:
+ # ...
+ # insn3
+ # addr3:
+ # insn4
+ #
+ # Something like this will happen:
+ # --> vCont;rADDR1,ADDR3 (range step from ADDR1 to ADDR3)
+ # <-- T05 (target single-stepped to func, which is out of the step range)
+ # --> $Z0,ADDR2 (place step-resume breakpoint at ADDR2)
+ # --> vCont;c (resume)
+ # <-- T05 (target stops at ADDR2)
+ # --> vCont;rADDR1,ADDR3 (continues range stepping)
+ # <-- T05
+ exec_cmd_expect_vCont_count "next" 0 2
+}
+
+# Check that breakpoints interrupt range stepping correctly.
+
+with_test_prefix "breakpoint" {
+ gdb_breakpoint "func1"
+ # Something like this will happen:
+ # --> vCont;rADDR1,ADDR3
+ # <-- T05 (target single-steps to func1, which is out of the step range)
+ # --> $Z0,ADDR2 (step-resume breakpoint at ADDR2)
+ # --> vCont;c (resume)
+ # <-- T05 (target hits the breakpoint at func1)
+ exec_cmd_expect_vCont_count "next" 0 1
+
+ gdb_test "backtrace" "#0 .* func1 .*#1 .* main .*" \
+ "backtrace from func1"
+
+ # A cancelled range step should not confuse the following
+ # execution commands.
+ exec_cmd_expect_vCont_count "stepi" 1 0
+ gdb_test "finish" ".*"
+ gdb_test "next" ".*"
+ delete_breakpoints
+}
+
+# Check that range stepping works well even when there's a loop in the
+# step range.
+
+with_test_prefix "loop" {
+
+ # GDB should send one vCont;r and receive one stop reply:
+ # --> vCont;rSTART,END (range step)
+ # <-- T05
+ exec_cmd_expect_vCont_count "next" 0 1
+
+ # Confirm the loop completed.
+ gdb_test "print a" " = 15"
+ gdb_test "print e" " = 105"
+}
+
+# Check that range stepping works well even when the target's PC was
+# already within the loop's body.
+
+with_test_prefix "loop 2" {
+ # Stepi into the loop body. 15 should be large enough to make
+ # sure the program stops within the loop's body.
+ gdb_test "stepi 15" ".*"
+ # GDB should send one vCont;r and receive one stop reply:
+ # --> vCont;rSTART,END (range step)
+ # <-- T05
+ exec_cmd_expect_vCont_count "next" 0 1
+
+ # Confirm the loop completed.
+ gdb_test "print a" " = 15"
+ gdb_test "print e" " = 105"
+}
+
+# Check that range stepping works well even when it is interrupted by
+# ctrl-c.
+
+with_test_prefix "interrupt" {
+ gdb_test_no_output "set debug remote 1"
+
+ send_gdb "next\n"
+ sleep 1
+ send_gdb "\003"
+
+ # GDB should send one vCont;r and receive one stop reply for
+ # SIGINT:
+ # --> vCont;rSTART,END (range step)
+ # <-- T02 (SIGINT)
+
+ set vcont_r_counter 0
+
+ set test "send ctrl-c to GDB"
+ gdb_test_multiple "" $test {
+ -re "vCont;r\[^\r\n\]*\.\.\." {
+ incr vcont_r_counter
+ exp_continue
+ }
+ -re "Program received signal SIGINT.*$gdb_prompt $" {
+ pass $test
+ }
+ }
+ gdb_test_no_output "set debug remote 0"
+
+ # Check the number of 'vCont;r' packets.
+ if { $vcont_r_counter == 1 } {
+ pass "${test}: 1 vCont;r"
+ } else {
+ fail "${test}: 1 vCont;r"
+ }
+
+ # Break the loop earlier and continue range stepping.
+ gdb_test "set variable c = 0"
+ exec_cmd_expect_vCont_count "next" 0 1
+}
+
+# Check that range stepping doesn't break software watchpoints. With
+# those, GDB needs to be notified of all single-steps, to evaluate
+# whether the watched value changes at each step.
+with_test_prefix "software watchpoint" {
+ gdb_test "step" "soft-watch.*" "step into multiple instruction line"
+ # A software watchpoint at PC makes the thread stop before the
+ # whole line range is over (after one single-step, actually).
+ gdb_test "watch \$pc" ".*" "set watchpoint"
+ gdb_test "step" "soft-watch.*" "step still in same line"
+}
+
+return 0
diff --git a/gdb/testsuite/gdb.trace/range-stepping.c b/gdb/testsuite/gdb.trace/range-stepping.c
new file mode 100644
index 0000000..b0e93f9
--- /dev/null
+++ b/gdb/testsuite/gdb.trace/range-stepping.c
@@ -0,0 +1,56 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+ Copyright 2013 Free Software Foundation, Inc.
+
+ This program is free software; you can redistribute it and/or modify
+ it under the terms of the GNU General Public License as published by
+ the Free Software Foundation; either version 3 of the License, or
+ (at your option) any later version.
+
+ This program is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ GNU General Public License for more details.
+
+ You should have received a copy of the GNU General Public License
+ along with this program. If not, see <http://www.gnu.org/licenses/>. */
+
+#ifdef SYMBOL_PREFIX
+#define SYMBOL(str) SYMBOL_PREFIX #str
+#else
+#define SYMBOL(str) #str
+#endif
+
+/* `set_point' further below is the label where we'll set tracepoints
+ at. The insn at the label must the large enough to fit a fast
+ tracepoint jump. */
+#if (defined __x86_64__ || defined __i386__)
+# define NOP " .byte 0xe9,0x00,0x00,0x00,0x00\n" /* jmp $+5 (5-byte nop) */
+#else
+# define NOP "" /* port me */
+#endif
+
+int
+main(void)
+{
+ /* Note: 'volatile' is used to make sure the compiler doesn't
+ optimize out these variables. We want to be sure instructions
+ are generated for accesses. */
+ volatile int i = 0;
+
+ /* Generate a single line with a label in the middle where we can
+ place either a trap tracepoint or a fast tracepoint. */
+#define LINE_WITH_FAST_TRACEPOINT \
+ do { \
+ i = 1; \
+ asm (" .global " SYMBOL (set_point) "\n" \
+ SYMBOL (set_point) ":\n" \
+ NOP \
+ ); \
+ i = 2; \
+ } while (0)
+
+ LINE_WITH_FAST_TRACEPOINT; /* location 1 */
+
+ return 0;
+}
diff --git a/gdb/testsuite/gdb.trace/range-stepping.exp b/gdb/testsuite/gdb.trace/range-stepping.exp
new file mode 100644
index 0000000..5cd81b6
--- /dev/null
+++ b/gdb/testsuite/gdb.trace/range-stepping.exp
@@ -0,0 +1,85 @@
+# Copyright 2013 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program. If not, see <http://www.gnu.org/licenses/>.
+
+load_lib "trace-support.exp"
+load_lib "range-stepping-support.exp"
+
+standard_testfile
+set executable $testfile
+
+if [prepare_for_testing $testfile.exp $executable $srcfile \
+ {debug nowarnings}] {
+ return -1
+}
+
+if ![runto_main] {
+ fail "Can't run to main to check for trace support"
+ return -1
+}
+
+if ![gdb_target_supports_trace] {
+ unsupported "target does not support trace"
+ return -1;
+}
+
+# Check that range stepping works well with tracepoints.
+
+proc range_stepping_with_tracepoint { type } {
+ with_test_prefix "${type}" {
+ gdb_breakpoint [gdb_get_line_number "location 1"]
+ gdb_continue_to_breakpoint "location 1"
+ delete_breakpoints
+
+ gdb_test "${type} *set_point" ".*"
+ gdb_test_no_output "tstart"
+
+ # Step a line with a tracepoint in the middle. The tracepoint
+ # itself shouldn't have any effect on range stepping. We
+ # should see one vCont;r and no vCont;s's.
+ exec_cmd_expect_vCont_count "step" 0 1
+ gdb_test_no_output "tstop"
+ gdb_test "tfind" "Found trace frame .*" "first tfind"
+ gdb_test "tfind" \
+ "Target failed to find requested trace frame.*" \
+ "second tfind"
+
+ delete_breakpoints
+ }
+}
+
+range_stepping_with_tracepoint "trace"
+
+set libipa [get_in_proc_agent]
+gdb_load_shlibs $libipa
+
+if { [gdb_compile "$srcdir/$subdir/$srcfile" $binfile \
+ executable [list debug nowarnings shlib=$libipa] ] != "" } {
+ untested "failed to compile ftrace tests"
+ return -1
+}
+
+clean_restart ${executable}
+
+if ![runto_main] {
+ fail "Can't run to main for ftrace tests"
+ return 0
+}
+
+gdb_reinitialize_dir $srcdir/$subdir
+if { [gdb_test "info sharedlibrary" ".*${libipa}.*" "IPA loaded"] != 0 } {
+ untested "Could not find IPA lib loaded"
+} else {
+ range_stepping_with_tracepoint "ftrace"
+}
diff --git a/gdb/testsuite/lib/range-stepping-support.exp b/gdb/testsuite/lib/range-stepping-support.exp
new file mode 100644
index 0000000..d849665
--- /dev/null
+++ b/gdb/testsuite/lib/range-stepping-support.exp
@@ -0,0 +1,50 @@
+# Copyright 2013 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program. If not, see <http://www.gnu.org/licenses/>.
+
+# Execute command CMD and check that GDB sends the expected number of
+# vCont;s and vCont;r packets.
+
+proc exec_cmd_expect_vCont_count { cmd exp_vCont_s exp_vCont_r } {
+ global gdb_prompt
+
+ gdb_test_no_output "set debug remote 1" ""
+
+ set test "${cmd}: vCont;s=${exp_vCont_s} vCont;r=${exp_vCont_r}"
+ set r_counter 0
+ set s_counter 0
+ gdb_test_multiple $cmd $test {
+ -re "vCont;s\[^\r\n\]*Packet received: T\[\[:xdigit:\]\]\[\[:xdigit:\]\]" {
+ incr s_counter
+ exp_continue
+ }
+ -re "vCont;r\[^\r\n\]*Packet received: T\[\[:xdigit:\]\]\[\[:xdigit:\]\]" {
+ incr r_counter
+ exp_continue
+ }
+ -re "\r\n" {
+ # Prevent overflowing the expect buffer.
+ exp_continue
+ }
+ -re "$gdb_prompt $" {
+ if { $r_counter == ${exp_vCont_r} && $s_counter == ${exp_vCont_s} } {
+ pass $test
+ } else {
+ fail $test
+ }
+ }
+ }
+
+ gdb_test_no_output "set debug remote 0" ""
+}
^ permalink raw reply [flat|nested] 38+ messages in thread* Re: [PATCH 5/5] range stepping: tests
2013-05-22 14:32 ` Yao Qi
2013-05-23 17:34 ` Pedro Alves
@ 2013-05-23 18:03 ` Pedro Alves
2013-05-24 2:27 ` Yao Qi
1 sibling, 1 reply; 38+ messages in thread
From: Pedro Alves @ 2013-05-23 18:03 UTC (permalink / raw)
To: Yao Qi; +Cc: gdb-patches
On 05/22/2013 03:02 PM, Yao Qi wrote:> On 05/21/2013 02:28 AM, Pedro Alves wrote:
>> I see. Thanks, that's much more detailed info than just saying "it's unsafe".
>>
>> I'm guessing the huge number of RSP packets comes from that big loop in the
>> test:
>>
>> /* Generate a range that includes a loop, which is time consuming.
>> Variable C is used to terminate the loop earlier when GDB
>> wants. */
>> for (c = 1, a = 0; a < 65535 && c; a++) {for (b = 0; b < 65535 && c; b++) { d1 = d2 * a / b; d2 = d1 *
>>
>> We could skip most of the range stepping tests if e.g., the
>> test that steps the short line FAILs:
>>
>> /* A line of source will be generated to a number of
>> instructions by compiler. */
>> a = b + c + d * e - a; /* location 1 */
>>
>> WDYT?
>
> Pedro,
> That is a good idea. It works for my internal stub! I'll post a delta patch on top of yours.
...
> The patch below is to skip the rest of range stepping tests if the
> first one fails, to avoid the issue of huge number of rsp packets in
> gdb.log. It is on top of your series. It works well with my stub.
Thanks!
> 2013-05-22 Yao Qi <yao@codesourcery.com>
>
> * gdb.base/range-stepping.exp: Skip the rest of tests if test
> fails.
> ---
> gdb/testsuite/gdb.base/range-stepping.exp | 8 ++++++++
> 1 files changed, 8 insertions(+), 0 deletions(-)
>
> diff --git a/gdb/testsuite/gdb.base/range-stepping.exp b/gdb/testsuite/gdb.base/range-stepping.exp
> index def25ce..aa5d34f 100644
> --- a/gdb/testsuite/gdb.base/range-stepping.exp
> +++ b/gdb/testsuite/gdb.base/range-stepping.exp
> @@ -85,6 +85,14 @@ with_test_prefix "multi insns" {
> set pc_after_stepping $expect_out(1,string)
> pass $msg
> }
> + -re ".*" {
This should not leave the prompt in the buffer, as it may
confuse the next test.
But I'm not understanding how this is catching the issue. Why
would "print/x \$pc" fail if the stub degenerates to
implementing vCont;r as a single instruction step?
> + fail $msg
> + # It is the first test on range-stepping, and the simplest
> + # one. If it fails, probably the rest of the tests fail
> + # and huge number of rsp packets will blow up the gdb.log
> + # file. Skip the rest of the tests.
Suggest minor editing:
# This is the first range-stepping test, and the simplest
# one. If it fails, probably the rest of the tests would
# fail too, and the huge number of rsp packets in the test with
# the time-consuming loop would blow up the gdb.log file.
# Skip the rest of the tests.
> + return
> + }
> }
>
> # There should be at least two instructions between
--
Pedro Alves
^ permalink raw reply [flat|nested] 38+ messages in thread* Re: [PATCH 5/5] range stepping: tests
2013-05-23 18:03 ` Pedro Alves
@ 2013-05-24 2:27 ` Yao Qi
2013-05-24 9:45 ` Pedro Alves
0 siblings, 1 reply; 38+ messages in thread
From: Yao Qi @ 2013-05-24 2:27 UTC (permalink / raw)
To: Pedro Alves; +Cc: gdb-patches
On 05/24/2013 02:03 AM, Pedro Alves wrote:
> But I'm not understanding how this is catching the issue. Why
> would "print/x \$pc" fail if the stub degenerates to
> implementing vCont;r as a single instruction step?
I switch to letting proc exec_cmd_expect_vCont_count return 0 or 1 to
indicate the test pass or not, and skip the rest if it returns 1 in
the test. How about this below?
--
Yao (é½å°§)
gdb/testsuite:
2013-05-24 Yao Qi <yao@codesourcery.com>
Pedro Alves <palves@redhat.com>
* gdb.base/range-stepping.exp: Skip the rest of tests if the
test fails.
* lib/range-stepping-support.exp (exec_cmd_expect_vCont_count):
Return 0 if the test passes, otherwise return 1.
---
gdb/testsuite/gdb.base/range-stepping.exp | 10 +++++++++-
gdb/testsuite/lib/range-stepping-support.exp | 6 +++++-
2 files changed, 14 insertions(+), 2 deletions(-)
diff --git a/gdb/testsuite/gdb.base/range-stepping.exp b/gdb/testsuite/gdb.base/range-stepping.exp
index 48fc15b..d17596c 100644
--- a/gdb/testsuite/gdb.base/range-stepping.exp
+++ b/gdb/testsuite/gdb.base/range-stepping.exp
@@ -76,7 +76,15 @@ with_test_prefix "multi insns" {
# <-- T05
# --> vCont;rSTART,END (range step)
# <-- T05
- exec_cmd_expect_vCont_count "next" 1 1
+ set result [exec_cmd_expect_vCont_count "next" 1 1]
+ if { $result } {
+ # This is the first range-stepping test, and the simplest
+ # one. If it fails, probably the rest of the tests would
+ # fail too, and the huge number of rsp packets in the test
+ # with the time-consuming loop would blow up the gdb.log file.
+ # Skip the rest of the tests.
+ return
+ }
set pc_after_stepping ""
set msg "pc after stepping"
diff --git a/gdb/testsuite/lib/range-stepping-support.exp b/gdb/testsuite/lib/range-stepping-support.exp
index d849665..ab38b11 100644
--- a/gdb/testsuite/lib/range-stepping-support.exp
+++ b/gdb/testsuite/lib/range-stepping-support.exp
@@ -14,7 +14,8 @@
# along with this program. If not, see <http://www.gnu.org/licenses/>.
# Execute command CMD and check that GDB sends the expected number of
-# vCont;s and vCont;r packets.
+# vCont;s and vCont;r packets. Returns 0 if the test passes,
+# otherwise returns 1.
proc exec_cmd_expect_vCont_count { cmd exp_vCont_s exp_vCont_r } {
global gdb_prompt
@@ -24,6 +25,7 @@ proc exec_cmd_expect_vCont_count { cmd exp_vCont_s exp_vCont_r } {
set test "${cmd}: vCont;s=${exp_vCont_s} vCont;r=${exp_vCont_r}"
set r_counter 0
set s_counter 0
+ set ret 1
gdb_test_multiple $cmd $test {
-re "vCont;s\[^\r\n\]*Packet received: T\[\[:xdigit:\]\]\[\[:xdigit:\]\]" {
incr s_counter
@@ -40,6 +42,7 @@ proc exec_cmd_expect_vCont_count { cmd exp_vCont_s exp_vCont_r } {
-re "$gdb_prompt $" {
if { $r_counter == ${exp_vCont_r} && $s_counter == ${exp_vCont_s} } {
pass $test
+ set ret 0
} else {
fail $test
}
@@ -47,4 +50,5 @@ proc exec_cmd_expect_vCont_count { cmd exp_vCont_s exp_vCont_r } {
}
gdb_test_no_output "set debug remote 0" ""
+ return $ret
}
--
1.7.7.6
^ permalink raw reply [flat|nested] 38+ messages in thread* Re: [PATCH 5/5] range stepping: tests
2013-05-24 2:27 ` Yao Qi
@ 2013-05-24 9:45 ` Pedro Alves
2013-05-24 9:57 ` Yao Qi
0 siblings, 1 reply; 38+ messages in thread
From: Pedro Alves @ 2013-05-24 9:45 UTC (permalink / raw)
To: Yao Qi; +Cc: gdb-patches
On 05/24/2013 03:27 AM, Yao Qi wrote:
> 2013-05-24 Yao Qi <yao@codesourcery.com>
> Pedro Alves <palves@redhat.com>
>
> * gdb.base/range-stepping.exp: Skip the rest of tests if the
> test fails.
> * lib/range-stepping-support.exp (exec_cmd_expect_vCont_count):
> Return 0 if the test passes, otherwise return 1.
This version is okay, thanks.
--
Pedro Alves
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [PATCH 0/5 V3] target-assisted range stepping
2013-05-14 19:10 [PATCH 0/5 V3] target-assisted range stepping Pedro Alves
` (4 preceding siblings ...)
2013-05-14 19:11 ` [PATCH 5/5] range stepping: tests Pedro Alves
@ 2013-05-14 20:21 ` Tom Tromey
2013-05-23 17:44 ` Pedro Alves
2013-05-23 1:02 ` Yao Qi
6 siblings, 1 reply; 38+ messages in thread
From: Tom Tromey @ 2013-05-14 20:21 UTC (permalink / raw)
To: Pedro Alves; +Cc: gdb-patches
>>>>> "Pedro" == Pedro Alves <palves@redhat.com> writes:
Pedro> It took a while, but finally here's my take on the range stepping
Pedro> series. This is based on Yao's v1 and v2 series, so I'm calling it
Pedro> v3. The gist of the feature is the same, although the implementation
Pedro> in both GDB and GDBserver is a different.
I read through the series and commented where I could.
I don't know enough to comment intelligently on the main patch (#3).
The test suite patch seemed fine to me.
Tom
^ permalink raw reply [flat|nested] 38+ messages in thread* Re: [PATCH 0/5 V3] target-assisted range stepping
2013-05-14 20:21 ` [PATCH 0/5 V3] target-assisted range stepping Tom Tromey
@ 2013-05-23 17:44 ` Pedro Alves
0 siblings, 0 replies; 38+ messages in thread
From: Pedro Alves @ 2013-05-23 17:44 UTC (permalink / raw)
To: Tom Tromey; +Cc: gdb-patches
On 05/14/2013 09:21 PM, Tom Tromey wrote:
>>>>>> "Pedro" == Pedro Alves <palves@redhat.com> writes:
>
> Pedro> It took a while, but finally here's my take on the range stepping
> Pedro> series. This is based on Yao's v1 and v2 series, so I'm calling it
> Pedro> v3. The gist of the feature is the same, although the implementation
> Pedro> in both GDB and GDBserver is a different.
>
> I read through the series and commented where I could.
> I don't know enough to comment intelligently on the main patch (#3).
> The test suite patch seemed fine to me.
Thanks Tom.
--
Pedro Alves
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [PATCH 0/5 V3] target-assisted range stepping
2013-05-14 19:10 [PATCH 0/5 V3] target-assisted range stepping Pedro Alves
` (5 preceding siblings ...)
2013-05-14 20:21 ` [PATCH 0/5 V3] target-assisted range stepping Tom Tromey
@ 2013-05-23 1:02 ` Yao Qi
2013-05-23 17:46 ` Pedro Alves
6 siblings, 1 reply; 38+ messages in thread
From: Yao Qi @ 2013-05-23 1:02 UTC (permalink / raw)
To: Pedro Alves; +Cc: gdb-patches
On 05/15/2013 03:10 AM, Pedro Alves wrote:
> It took a while, but finally here's my take on the range stepping
> series. This is based on Yao's v1 and v2 series, so I'm calling it
> v3. The gist of the feature is the same, although the implementation
> in both GDB and GDBserver is a different.
Pedro,
I go through this series, and give some comments on patch 3/5, 4/5, and
5/5. With my addition on patch 5/5 (skip test earlier if it fails), it
works well on my stub. Thanks for refining the range stepping series.
--
Yao (é½å°§)
^ permalink raw reply [flat|nested] 38+ messages in thread* Re: [PATCH 0/5 V3] target-assisted range stepping
2013-05-23 1:02 ` Yao Qi
@ 2013-05-23 17:46 ` Pedro Alves
0 siblings, 0 replies; 38+ messages in thread
From: Pedro Alves @ 2013-05-23 17:46 UTC (permalink / raw)
To: Yao Qi; +Cc: gdb-patches
On 05/23/2013 02:02 AM, Yao Qi wrote:
> On 05/15/2013 03:10 AM, Pedro Alves wrote:
>> It took a while, but finally here's my take on the range stepping
>> series. This is based on Yao's v1 and v2 series, so I'm calling it
>> v3. The gist of the feature is the same, although the implementation
>> in both GDB and GDBserver is a different.
>
> Pedro,
> I go through this series, and give some comments on patch 3/5, 4/5, and 5/5. With my addition on patch 5/5 (skip test earlier if it fails), it works well on my stub. Thanks for refining the range stepping series.
Great! I'm now applied the series, after addressing your comments.
Tested on Fedora 17, native and gdbserver.
Thanks,
--
Pedro Alves
^ permalink raw reply [flat|nested] 38+ messages in thread