[PATCH 0/7] Range stepping

[PATCH 0/7] Range stepping

[Yao Qi]

Hi,
When user types "next" command on the following line of source, 

   a = b + c + d * e - a;

GDB will single step every instruction belongs to this source line
and stop when program goes to the next line, the following is the
assembly of this source line

   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

the following will happen between GDB and GDBserver in remote
debugging,

--> 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;

There are some round-trip between GDB and GDBserver.  Isn't it boring
and inefficient? so GDB has to say something
different to GDBserver, that is, "keep stepping and report a stop
to me until program goes out of the range of [0x08048434, 0x08048451)"
then, the RSP traffic looks like this,

--> vCont;r8048434,8048451:p2db0.2db0;c
<-- T0505:68efffbf;04:30efffbf;08:51840408;thread:p2db0.2db0;core:1;

As we can see, GDB tells GDBserver a range of stepping, and GDBserver
only sends stop reply back when programs goes out of the range.
The number of packets is reduced dramatically, and as a result,
the performance of stepping a source line is improved.  We call it
"range stepping".

In this patch series, we add range stepping support in GDBserver on
x86/linux and in GDB.  GDB is able to send "vCont;r" packet for other
remote stubs if "vCont;r" is supported.  We tested GDB with GDBserver
on x86_64-linux, and also test mips-sde-elf GDB with a stub understands
range stepping.  No regressions.

Is it OK after the release branch is created?

-- 
1.7.7.6

[PATCH 1/7] New macro THREAD_WITHIN_SINGLE_STEP_RANGE

[Yao Qi]

This patch moves code into a macro, which will be used in my
following patches.

gdb:

2013-03-11  Yao Qi  <yao@codesourcery.com>

	* gdbthread.h (THREAD_WITHIN_SINGLE_STEP_RANGE): New macro.
	* infrun.c (handle_inferior_event): Use it.
---
 gdb/gdbthread.h |    6 ++++++
 gdb/infrun.c    |    6 ++----
 2 files changed, 8 insertions(+), 4 deletions(-)

diff --git a/gdb/gdbthread.h b/gdb/gdbthread.h
index 824e4d0..6f47006 100644
--- a/gdb/gdbthread.h
+++ b/gdb/gdbthread.h
@@ -37,6 +37,12 @@ enum thread_state
   THREAD_EXITED,
 };
 
+/* PC is within the range of single stepping of thread THR.  */
+
+#define THREAD_WITHIN_SINGLE_STEP_RANGE(THR, PC)	\
+  ((PC) >= (THR)->control.step_range_start		\
+   && (PC) < (THR)->control.step_range_end)
+
 /* Inferior thread specific part of `struct infcall_control_state'.
 
    Inferior process counterpart is `struct inferior_control_state'.  */
diff --git a/gdb/infrun.c b/gdb/infrun.c
index 1cf30fe..5bc276c 100644
--- a/gdb/infrun.c
+++ b/gdb/infrun.c
@@ -4340,8 +4340,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)
+	  && THREAD_WITHIN_SINGLE_STEP_RANGE (ecs->event_thread, stop_pc)
 	  && frame_id_eq (get_stack_frame_id (frame),
 			  ecs->event_thread->control.step_stack_frame_id)
 	  && ecs->event_thread->control.step_resume_breakpoint == NULL)
@@ -4710,8 +4709,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 (THREAD_WITHIN_SINGLE_STEP_RANGE (ecs->event_thread, stop_pc)
       && (execution_direction != EXEC_REVERSE
 	  || frame_id_eq (get_frame_id (frame),
 			  ecs->event_thread->control.step_frame_id)))
-- 
1.7.7.6

Re: [PATCH 1/7] New macro THREAD_WITHIN_SINGLE_STEP_RANGE

[Pedro Alves]

Hi Yao,

Finally I'm replying to this series.  As I said before, I ended up
working on the code, and I'll post a v3 next.  I'm still replying to
the individual patches, to point out some of the issues I found.

On 03/11/2013 12:51 PM, Yao Qi wrote:
> This patch moves code into a macro, which will be used in my
> following patches.

I agree with the idea.

> 2013-03-11  Yao Qi  <yao@codesourcery.com>
> 
> 	* gdbthread.h (THREAD_WITHIN_SINGLE_STEP_RANGE): New macro.
> 	* infrun.c (handle_inferior_event): Use it.

However, the gdbserver patch does:

+/* Return true if THREAD is in range stepping.  PC is the value of pc
+   of THREAD.  */
+
+int
+thread_in_range_stepping_p (struct thread_info *thread, CORE_ADDR pc)
+{
+  return (pc >= thread->start && pc < thread->end);
 }

If we have code doing the exact same at the same conceptual level
in both gdb and gdbserver, I'd rather it is similar.

-- 
Pedro Alves

[PATCH 6/7] range stepping: test case

[Yao Qi]

This test case is used to verify range stepping is used by means of
checking the rsp packets.

gdb/testsuite:

2013-03-11  Yao Qi  <yao@codesourcery.com>

	* lib/range-stepping-support.exp: New.
	* gdb.base/range-stepping.c: New.
	* gdb.base/range-stepping.exp: New.
	* gdb.trace/range-stepping.c: New.
	* gdb.trace/range-stepping.exp: New.
---
 gdb/testsuite/gdb.base/range-stepping.c      |   69 ++++++++
 gdb/testsuite/gdb.base/range-stepping.exp    |  220 ++++++++++++++++++++++++++
 gdb/testsuite/gdb.trace/range-stepping.c     |   61 +++++++
 gdb/testsuite/gdb.trace/range-stepping.exp   |   88 ++++++++++
 gdb/testsuite/lib/range-stepping-support.exp |   55 +++++++
 5 files changed, 493 insertions(+), 0 deletions(-)
 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..930cb8d
--- /dev/null
+++ b/gdb/testsuite/gdb.base/range-stepping.c
@@ -0,0 +1,69 @@
+/* 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/>.  */
+
+static int
+func1 (int a, int b)
+{
+  int r = a * b;
+
+  r =+ (a | b);
+  r =+ (a - b);
+
+  return r;
+}
+
+int
+main(void)
+{
+  int a = 0;
+  int b = 1;
+  int c = 2;
+  int d = 3;
+  int e = 4;
+  double d1 = 1.0;
+  double d2 = 2.0;
+
+  /* A line of source will be generated to a number of
+     instructions by compiler.  */
+  a = b + c + d * e - a; /* location 1 */
+
+  /* To compose multiple instructions before and after
+     'function call' or 'branch' instruction.  This line
+     of source will be generated to the following 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.  */
+  for (a = 0, e = 0; a < 15; a++) { e += a;}
+  /* Generate a range that includes a loop in it.  */
+  for (a = 0, e = 0; a < 15; a++) { e += a;}
+  /* 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 * a;}}
+  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..90083d6
--- /dev/null
+++ b/gdb/testsuite/gdb.base/range-stepping.exp
@@ -0,0 +1,220 @@
+# 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 .c
+set executable $testfile
+
+if  { [gdb_compile "${srcdir}/${subdir}/${srcfile}" "${binfile}" \
+	   executable {debug nowarnings}] != "" } {
+     untested ${testfile}.exp
+     return -1
+}
+
+clean_restart $executable
+
+if ![runto_main] {
+    fail "Can't run to main"
+    return -1
+}
+
+# Check range stepping is supported by target or not.
+set test "range-stepping is supported"
+gdb_test_multiple "maintenance show range-stepping" "show range-stepping" {
+    -re "Debugger's willingness to do range-stepping is off.*\r\n$gdb_prompt $" {
+	unsupported $test
+	return -1
+    }
+    -re "Debugger's willingness to do range-stepping is on.*\r\n$gdb_prompt $" {
+	pass $test
+    }
+}
+
+# Check range stepping can step a range of multiple instructions.
+
+with_test_prefix "multi insns" {
+    global executable
+    global hex decimal
+    global gdb_prompt
+
+    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 execute command "next", GDB should send one vCont;s and
+    # vCont;r and receive two stop replies.
+    # --> vCont;s  (step over breakpoint)
+    # <-- T0505:XXX
+    # --> vCont;rSTART,END  (do range stepping)
+    # <-- T0505:XXX
+    exec_cmd_and_check_rsp_packets "next" 1 1
+
+    set pc_after_stepping ""
+    set msg "pc before 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
+	}
+    }
+
+    # At least, there are 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 range stepping can step over a function.
+
+with_test_prefix "step over func" {
+    global srcfile
+
+    set line_num [gdb_get_line_number "location 2"]
+    gdb_test "where" "main \\(\\) at .*${srcfile}:${line_num}.*"
+
+    # It is expected to get three stops and two 'vCont;r'.  In the C
+    # code, the line of C source produces the following instructions,
+    #
+    # addr1:
+    #  insn1;
+    #  insn2;
+    #  ...
+    #  call func1;
+    # addr2:
+    #  ...
+    #  insn3;
+    # addr3:
+    #  insn4;
+    #
+    # Something like this will happen:
+    # --> vCont;rADDR1,ADDR3 (do range stepping from ADDR1 to ADDR3)
+    # <-- T0505:XXXX
+    # (inferior is single-stepping to func, which is out of the range)
+    # --> $Z0,ADDR2
+    # --> vCont;c (set step-resume breakpoint on ADDR2, and resume)
+    # <-- T0505:XXXX  (inferior stops at ADD2)
+    # --> vCont;rADDR1,ADDR3  (continues to do range stepping)
+    # <-- T0505:XXXX
+    exec_cmd_and_check_rsp_packets "next" 0 2
+}
+
+# Check range stepping works well with breakpoint.
+
+with_test_prefix "breakpoint" {
+    gdb_breakpoint "func1"
+    # Something like this will happen:
+    # --> vCont;rADDR1,ADDR3
+    # <-- T0505:XXXX
+    # (inferior is single-stepping to func1, which is out of the range)
+    # --> $Z0,ADDR2
+    # --> vCont;c (set step-resume breakpoint on ADDR2, and resume)
+    # <-- T0505:XXXX  (inferior hits the breakpoint on fun1)
+    exec_cmd_and_check_rsp_packets "next" 0 1
+
+    gdb_test "backtrace" "#0 .* func1 .*#1 .* main .*" \
+	"backtrace from func1"
+    # The range stepping was interrupted by a breakpoint, but the
+    # state related to range stepping should be cleared.
+    exec_cmd_and_check_rsp_packets "stepi" 1 0
+    gdb_test "finish" ".*"
+
+    gdb_test "next" ".*"
+    delete_breakpoints
+}
+
+# Check range stepping works well with a loop in the range.
+
+with_test_prefix "loop" {
+
+    # GDB should send one vCont;r and receive one stop reply.
+    # --> vCont;rSTART,END  (do range stepping)
+    # <-- T0505:XXX
+    exec_cmd_and_check_rsp_packets "next" 0 1
+
+    # Check loop is done.
+    gdb_test "print a" "\\$${decimal} = 15\[\r\n\].*"
+    gdb_test "print e" "\\$${decimal} = 105\[\r\n\].*"
+}
+
+# Check range stepping works well when PC has already within the loop
+# body..
+
+with_test_prefix "loop" {
+    # Stepi into the loop body.  15 is large enough to make sure
+    # program goes to loop body.
+    gdb_test "stepi 15" ".*"
+    # GDB should send one vCont;r and receive one stop reply.
+    # --> vCont;rSTART,END  (do range stepping)
+    # <-- T0505:XXX
+    exec_cmd_and_check_rsp_packets "next" 0 1
+
+    # Check loop is done.
+    gdb_test "print a" "\\$${decimal} = 15\[\r\n\].*"
+    gdb_test "print e" "\\$${decimal} = 105\[\r\n\].*"
+}
+
+# Check range stepping works well 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 about
+    # SIGINT.
+    # --> vCont;rSTART,END  (do range stepping)
+    # <-- T0205:XXX
+
+    set vcont_r_counter 0
+
+    set test "send gdb ctrl-c"
+    gdb_expect {
+	-re "vCont;r\[^\r\n\]*\.\.\." {
+	    incr vcont_r_counter
+	    exp_continue
+	}
+	-re "Program received signal SIGINT.*$gdb_prompt $" {
+	    pass $test
+	}
+	timeout { fail "${test} (timeout)" }
+	eof { fail "${test} (eof)" }
+    }
+    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"
+    }
+
+    # Terminate the loop earlier and continue to do range stepping.
+    gdb_test "set variable c = 0"
+    exec_cmd_and_check_rsp_packets "next" 0 1
+}
+
+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..50db5a0
--- /dev/null
+++ b/gdb/testsuite/gdb.trace/range-stepping.c
@@ -0,0 +1,61 @@
+/* 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
+
+/* Called from asm.  */
+static void __attribute__((used))
+func2 (void)
+{}
+
+static int
+func1 (int a, int b)
+{
+  int r = a * b;
+
+  /* `set_point' is the label where we'll set tracepoint at.  The insn
+     at the label must the large enough to fit a fast tracepoint
+     jump.  */
+  asm ("    .global " SYMBOL(set_point) "\n"
+       SYMBOL(set_point) ":\n"
+#if (defined __x86_64__ || defined __i386__)
+       "    call " SYMBOL(func2) "\n"
+#endif
+       );
+
+  r =+ (a | b);
+  r =+ (a - b);
+
+  return r;
+}
+
+int
+main(void)
+{
+  int a = 0;
+  int b = 1;
+  int c = 2;
+  int d = 3;
+  int e = 4;
+
+  e = 10 + func1 (a + b, c * d); /* 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..6ff3ffa
--- /dev/null
+++ b/gdb/testsuite/gdb.trace/range-stepping.exp
@@ -0,0 +1,88 @@
+# 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
+set expfile $testfile.exp
+
+if [prepare_for_testing $expfile $executable $srcfile \
+	{debug nowarnings}] {
+    untested "failed to prepare for trace tests"
+    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 range stepping works well with tracepoint.
+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"
+
+	# GDB will step over function func1, in which a tracepoint is
+	# set.  GDB will send two vCont;r packets because calling to
+	# func1 is out of the range.  However, tracepoint itself
+	# shouldn't have any effect on range stepping.
+	exec_cmd_and_check_rsp_packets "next" 0 2
+	gdb_test_no_output "tstop"
+	gdb_test "tfind" "Found trace frame 0.*" "tfind 0"
+	gdb_test "tfind" \
+	    "Target failed to find requested trace frame.*" \
+	    "tfind 1"
+
+	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..c46e92d
--- /dev/null
+++ b/gdb/testsuite/lib/range-stepping-support.exp
@@ -0,0 +1,55 @@
+# 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 the expected RSP packets
+# (vCont;s and vCont;r) during range stepping.
+
+proc exec_cmd_and_check_rsp_packets { cmd vcont_s vcont_r} {
+    global gdb_prompt
+
+    gdb_test_no_output "set debug remote 1" ""
+
+    set test "range stepping"
+    set vcont_r_counter 0
+    set vcont_s_counter 0
+    gdb_test_multiple $cmd $test {
+	-re "vCont;s\[^\r\n\]*Packet received: T\[\[:xdigit:\]\]\[\[:xdigit:\]\]" {
+	    incr vcont_s_counter
+	    exp_continue
+	}
+	-re "vCont;r\[^\r\n\]*Packet received: T\[\[:xdigit:\]\]\[\[:xdigit:\]\]" {
+	    incr vcont_r_counter
+	    exp_continue
+	}
+	-re "$gdb_prompt $" {
+	    pass $test
+	}
+    }
+    gdb_test_no_output "set debug remote 0" ""
+
+    # Check the number of 'vCont;r' packets.
+    if { $vcont_r_counter == ${vcont_r} } {
+	pass "${test}: ${vcont_r} vCont;r"
+    } else {
+	fail "${test}: ${vcont_r} vCont;r"
+    }
+
+    # Check the number of 'vCont;s' packets.
+    if { $vcont_s_counter == ${vcont_s} } {
+	pass "${test}: ${vcont_s} vCont;s"
+    } else {
+	fail "${test}: ${vcont_s} vCont;s"
+    }
+}
-- 
1.7.7.6

Re: [PATCH 6/7] range stepping: test case

[Pedro Alves]

Hi Yao,

Thanks a lot for writing this.

On 03/11/2013 12:51 PM, Yao Qi wrote:
> This test case is used to verify range stepping is used by means of
> checking the rsp packets.

I agree with the idea.  There's not much black box style testing
we can do, as GDB is supposed to work the same from the user's
perspective, with or without range stepping.

I prefer this over the convenience variable hack.  If we ever
make range stepping work on a target !remote, we can switch to
look at "set debug infrun 1" output or some new "set debug range-step"
output.  I didn't understand your stated reason in v2 for
switching away from this.  If it was that expect buffer
overflows, v3 has a tweak that likely fixes it.

> +
> +static int
> +func1 (int a, int b)
> +{
> +  int r = a * b;
> +
> +  r =+ (a | b);
> +  r =+ (a - b);

"=+" very much looks like a typo for "+=" (and appears elsewhere too).

> +
> +  return r;
> +}
> +
> +int
> +main(void)
> +{
> +  int a = 0;
> +  int b = 1;
> +  int c = 2;
> +  int d = 3;
> +  int e = 4;
> +  double d1 = 1.0;
> +  double d2 = 2.0;

It's probably a good idea to make these volatile,

> +
> +  /* A line of source will be generated to a number of
> +     instructions by compiler.  */
> +  a = b + c + d * e - a; /* location 1 */

in case the compiler folds these, even at -O0.  We really want
to make sure these are multiple instructions.  I think that
for softfp targets, the compiler will end up emitting calls (which break
the tests) for these uses of floats, though I have left those in
v3 too.

> +
> +  /* To compose multiple instructions before and after
> +     'function call' or 'branch' instruction.  This line
> +     of source will be generated to the following 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.  */
> +  for (a = 0, e = 0; a < 15; a++) { e += a;}
> +  /* Generate a range that includes a loop in it.  */
> +  for (a = 0, e = 0; a < 15; a++) { e += a;}
> +  /* 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 * a;}}

We can put these multi-statement lines behind macros, so we can still
write them in multiple lines, making it easier to read.

> +
> +standard_testfile .c

.c is the default.

> +    # When execute command "next", GDB should send one vCont;s and
> +    # vCont;r and receive two stop replies.
> +    # --> vCont;s  (step over breakpoint)
> +    # <-- T0505:XXX

The second 05 in "T0505" is a register number.  Best leave it out.


> +    # GDB should send one vCont;r and receive one stop reply about
> +    # SIGINT.
> +    # --> vCont;rSTART,END  (do range stepping)
> +    # <-- T0205:XXX
> +
> +    set vcont_r_counter 0
> +
> +    set test "send gdb ctrl-c"
> +    gdb_expect {

I didn't see why this needed to be gdb_expect instead of
gdb_test_multiple.

> +	-re "vCont;r\[^\r\n\]*\.\.\." {
> +	    incr vcont_r_counter
> +	    exp_continue
> +	}
> +	-re "Program received signal SIGINT.*$gdb_prompt $" {
> +	    pass $test
> +	}
> +	timeout { fail "${test} (timeout)" }
> +	eof { fail "${test} (eof)" }
> +    }
> +    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"
> +    }
> +
> +    # Terminate the loop earlier and continue to do range stepping.
> +    gdb_test "set variable c = 0"
> +    exec_cmd_and_check_rsp_packets "next" 0 1
> +}
> +




> +static int
> +func1 (int a, int b)
> +{
> +  int r = a * b;
> +
> +  /* `set_point' is the label where we'll set tracepoint at.  The insn
> +     at the label must the large enough to fit a fast tracepoint
> +     jump.  */
> +  asm ("    .global " SYMBOL(set_point) "\n"
> +       SYMBOL(set_point) ":\n"
> +#if (defined __x86_64__ || defined __i386__)
> +       "    call " SYMBOL(func2) "\n"
> +#endif
> +       );
> +
> +  r =+ (a | b);
> +  r =+ (a - b);
> +
> +  return r;
> +}
> +
> +int
> +main(void)
> +{
> +  int a = 0;
> +  int b = 1;
> +  int c = 2;
> +  int d = 3;
> +  int e = 4;
> +
> +  e = 10 + func1 (a + b, c * d); /* location 1 */
> +  return 0;
> +}

> +# Check range stepping works well with tracepoint.
> +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"
> +
> +	# GDB will step over function func1, in which a tracepoint is
> +	# set.  GDB will send two vCont;r packets because calling to
> +	# func1 is out of the range.  However, tracepoint itself
> +	# shouldn't have any effect on range stepping.

I think we'd be more certain the tracepoint doesn't affect range
stepping if it is set in the middle of a line that is a single
range.  I changed the test's source to do a 5-byte nop instead of
a "call" in the asm part (and removed func1).

> +	exec_cmd_and_check_rsp_packets "next" 0 2
> +	gdb_test_no_output "tstop"
> +	gdb_test "tfind" "Found trace frame 0.*" "tfind 0"
> +	gdb_test "tfind" \
> +	    "Target failed to find requested trace frame.*" \
> +	    "tfind 1"
> +
> +	delete_breakpoints
> +    }
> +}
> +


> +proc exec_cmd_and_check_rsp_packets { cmd vcont_s vcont_r} {
> +    global gdb_prompt
> +
> +    gdb_test_no_output "set debug remote 1" ""
> +
> +    set test "range stepping"
> +    set vcont_r_counter 0
> +    set vcont_s_counter 0
> +    gdb_test_multiple $cmd $test {
> +	-re "vCont;s\[^\r\n\]*Packet received: T\[\[:xdigit:\]\]\[\[:xdigit:\]\]" {
> +	    incr vcont_s_counter
> +	    exp_continue
> +	}
> +	-re "vCont;r\[^\r\n\]*Packet received: T\[\[:xdigit:\]\]\[\[:xdigit:\]\]" {
> +	    incr vcont_r_counter
> +	    exp_continue
> +	}
> +	-re "$gdb_prompt $" {
> +	    pass $test
> +	}
> +    }
> +    gdb_test_no_output "set debug remote 0" ""
> +
> +    # Check the number of 'vCont;r' packets.
> +    if { $vcont_r_counter == ${vcont_r} } {
> +	pass "${test}: ${vcont_r} vCont;r"
> +    } else {
> +	fail "${test}: ${vcont_r} vCont;r"
> +    }
> +
> +    # Check the number of 'vCont;s' packets.
> +    if { $vcont_s_counter == ${vcont_s} } {
> +	pass "${test}: ${vcont_s} vCont;s"
> +    } else {
> +	fail "${test}: ${vcont_s} vCont;s"
> +    }

This generates three PASS/FAILs per test.  I'd prefer merging them
into one.

In v3, I tweaked a lot many little details in formatting
and wording, but the essence is mostly the same.

-- 
Pedro Alves

Re: [PATCH 6/7] range stepping: test case

[Yao Qi]

On 05/15/2013 02:32 AM, Pedro Alves wrote:
> I prefer this over the convenience variable hack.  If we ever
> make range stepping work on a target !remote, we can switch to
> look at "set debug infrun 1" output or some new "set debug range-step"
> output.  I didn't understand your stated reason in v2 for
> switching away from this.  If it was that expect buffer
> overflows, v3 has a tweak that likely fixes it.

In our tests, when running tests with a problematic stub supporting 
range stepping, the range stepping of the stub falls back to the single 
stepping.  This will generate a huge number of rsp packets, and the 
gdb.log will blow up to several giga-byte.  We use convince variable to 
avoid this problem.  On the other hand, I don't think it is a good idea 
to peek the GDB internal states by checking some rsp packets and 
personally I prefer the way that GDB is able to expose some internal 
states by some means (command "maint" and convince variables, for example).

-- 
Yao (齐尧)

Re: [PATCH 6/7] range stepping: test case

[Pedro Alves]

On 05/15/2013 09:27 AM, Yao Qi wrote:
> On 05/15/2013 02:32 AM, Pedro Alves wrote:
>> I prefer this over the convenience variable hack.  If we ever
>> make range stepping work on a target !remote, we can switch to
>> look at "set debug infrun 1" output or some new "set debug range-step"
>> output.  I didn't understand your stated reason in v2 for
>> switching away from this.  If it was that expect buffer
>> overflows, v3 has a tweak that likely fixes it.
> 
> In our tests, when running tests with a problematic stub supporting range
> stepping, the range stepping of the stub falls back to the single stepping.
> This will generate a huge number of rsp packets, and the gdb.log will blow up
> to several giga-byte.

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?

> We use convince variable to avoid this problem.  On the other
> hand, I don't think it is a good idea to peek the GDB internal states by
> checking some rsp packets and personally I prefer the way that GDB is able to expose
> some internal states by some means (command "maint" and convince variables, for example).

I don't like using a convenience variable for this, as those are visible
to the user (show convenience).

"maint" seems better.  But I'm not certain of it.  An issue I have with it
is that from the log you just see the condensed report of what happened
(sent 4 vCont;r, should have been 3), while to diagnose the issue you'll
most likely need to get more info than that ("Okay, what was really sent?
What were the ranges?  Were there signals interrupting the range?  Etc.").  I
have a suspicion we'll end up needing to end up with "set debug infrun 1"
on, and look at that too for some of the trickier cases, and end up with
different number of expected ranges depending on target on some cases.

The actual difference between v1 -> v2 wrt to the RSP packets vs
convenience var was surprisingly smaller than I anticipated.  So how
about we go with RSP first, and see how things go from there?

-- 
Pedro Alves

Re: [PATCH 6/7] range stepping: test case

[Yao Qi]

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.

>
>> >We use convince variable to avoid this problem.  On the other
>> >hand, I don't think it is a good idea to peek the GDB internal states by
>> >checking some rsp packets and personally I prefer the way that GDB is able to expose
>> >some internal states by some means (command "maint" and convince variables, for example).
> I don't like using a convenience variable for this, as those are visible
> to the user (show convenience).
>
> "maint" seems better.  But I'm not certain of it.  An issue I have with it
> is that from the log you just see the condensed report of what happened
> (sent 4 vCont;r, should have been 3), while to diagnose the issue you'll
> most likely need to get more info than that ("Okay, what was really sent?
> What were the ranges?  Were there signals interrupting the range?  Etc.").  I
> have a suspicion we'll end up needing to end up with "set debug infrun 1"
> on, and look at that too for some of the trickier cases, and end up with
> different number of expected ranges depending on target on some cases.
>
> The actual difference between v1 -> v2 wrt to the RSP packets vs
> convenience var was surprisingly smaller than I anticipated.  So how
> about we go with RSP first, and see how things go from there?

I agree.

-- 
Yao (齐尧)

[PATCH 5/7] range stepping: New command 'maint set range stepping'

[Yao Qi]

gdb:

2013-03-11  Yao Qi  <yao@codesourcery.com>

	* remote.c (use_range_stepping): New.
	(remote_vcont_probe): Set 'use_range_stepping' if the remote
	target supports range stepping.
	(append_resumption): Use 'vCont;r' packet if 'use_range_stepping'
	is true.
	(maint_show_range_stepping): New.
	(maint_set_range_stepping): New.
	(_initialize_remote): Call add_setshow_boolean_cmd to register
	commands 'maint set range-stepping' and
	'maint show range-stepping'.

gdb/doc:

2013-03-11  Yao Qi  <yao@codesourcery.com>

	* gdb.texinfo (Maintenance Commands): Document commands
	'maint set range-stepping' and 'maint show range-stepping'.
---
 gdb/doc/gdb.texinfo |    7 +++++
 gdb/remote.c        |   64 ++++++++++++++++++++++++++++++++++++++++++++++++++-
 2 files changed, 70 insertions(+), 1 deletions(-)

diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index a607166..7c06120 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -35547,6 +35547,13 @@ data in a @file{gmon.out} file, be sure to move it to a safe location.
 Configuring with @samp{--enable-profiling} arranges for @value{GDBN} to be
 compiled with the @samp{-pg} compiler option.
 
+@kindex maint set range-stepping
+@kindex maint show range-stepping
+@cindex range-stepping
+@item maint set range-stepping
+@itemx maint show range-stepping
+Control whether to do stepping in an address range.
+
 @kindex maint set show-debug-regs
 @kindex maint show show-debug-regs
 @cindex hardware debug registers
diff --git a/gdb/remote.c b/gdb/remote.c
index 84e09b0..de7391f 100644
--- a/gdb/remote.c
+++ b/gdb/remote.c
@@ -257,6 +257,10 @@ struct support_v_cont
   int r;
 };
 
+/* A flag on GDB is willing to use range-stepping or not.  */
+
+static int use_range_stepping = 0;
+
 /* Description of the remote protocol state for the currently
    connected target.  This is per-target state, and independent of the
    selected architecture.  */
@@ -4647,6 +4651,7 @@ remote_vcont_probe (struct remote_state *rs)
       support_C = 0;
       rs->support_vCont.t = 0;
       rs->support_vCont.r = 0;
+      use_range_stepping = 0;
       while (p && *p == ';')
 	{
 	  p++;
@@ -4661,7 +4666,10 @@ remote_vcont_probe (struct remote_state *rs)
 	  else if (*p == 't' && (*(p + 1) == ';' || *(p + 1) == 0))
 	    rs->support_vCont.t = 1;
 	  else if (*p == 'r' && (*(p + 1) == ';' || *(p + 1) == 0))
-	    rs->support_vCont.r = 1;
+	    {
+	      rs->support_vCont.r = 1;
+	      use_range_stepping = 1;
+	    }
 
 	  p = strchr (p, ';');
 	}
@@ -4708,6 +4716,8 @@ append_resumption (char *p, char *endp,
 
       pc = regcache_read_pc (get_thread_regcache (ptid));
       if (rs->support_vCont.r /* Target supports step range.  */
+	  /* GDB is willing to do range stepping.  */
+	  && use_range_stepping
 	  /* Can't do range stepping for all threads of a process
 	     'pPID.-1'.  */
 	  && !(remote_multi_process_p (rs) && ptid_is_pid (ptid))
@@ -11488,6 +11498,45 @@ remote_upload_trace_state_variables (struct uploaded_tsv **utsvp)
   return 0;
 }
 
+static void
+maint_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 do range-stepping "
+		      "is %s.\n"), value);
+}
+
+static void
+maint_set_range_stepping (char *ignore_args, int from_tty,
+			  struct cmd_list_element *c)
+{
+  /* Check range stepping is supported when turns it on.  */
+  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_DISABLE
+	      || !rs->support_vCont.r)
+	    {
+	      use_range_stepping = 0;
+	      error (_("Range stepping is not supported"));
+	    }
+	}
+      else
+	{
+	  use_range_stepping = 0;
+	  error (_("Range stepping is not supported"));
+	}
+    }
+}
+
 void
 _initialize_remote (void)
 {
@@ -11873,6 +11922,19 @@ 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_maintenance,
+			   &use_range_stepping, _("\
+Enable or disable range-stepping."), _("\
+Show whether range-stepping is enabled."), _("\
+If On, GDB will tell the target to do stepping a range of address.\n\
+This will speed up stepping a line of source file.\n\
+If off, GDB will not use it, even if such is supported by the \n\
+target"),
+			   maint_set_range_stepping,
+			   maint_show_range_stepping,
+			   &maintenance_set_cmdlist,
+			   &maintenance_show_cmdlist);
+
   /* Eventually initialize fileio.  See fileio.c */
   initialize_remote_fileio (remote_set_cmdlist, remote_show_cmdlist);
 
-- 
1.7.7.6

Re: [PATCH 5/7] range stepping: New command 'maint set range stepping'

[Eli Zaretskii]

> From: Yao Qi <yao@codesourcery.com>
> Date: Mon, 11 Mar 2013 20:51:29 +0800
> 
> +@kindex maint set range-stepping
> +@kindex maint show range-stepping
> +@cindex range-stepping
> +@item maint set range-stepping
> +@itemx maint show range-stepping
> +Control whether to do stepping in an address range.

Why is this a maintenance command?

Anyway, the documentation part is OK.

> +  add_setshow_boolean_cmd ("range-stepping", class_maintenance,
> +			   &use_range_stepping, _("\
> +Enable or disable range-stepping."), _("\
> +Show whether range-stepping is enabled."), _("\
> +If On, GDB will tell the target to do stepping a range of address.\n\

I suggest

 If ON, GDB will send a single packet to a target telling it to keep
 single-stepping as long as PC is inside a certain range of addresses.

> +This will speed up stepping a line of source file.\n\
> +If off, GDB will not use it, even if such is supported by the \n\

"will not use this feature".  "It" is ambiguous here.

Thanks.

Re: [PATCH 5/7] range stepping: New command 'maint set range stepping'

[Yao Qi]

On 03/12/2013 01:04 AM, Eli Zaretskii wrote:
>> +@kindex maint set range-stepping
>> >+@kindex maint show range-stepping
>> >+@cindex range-stepping
>> >+@item maint set range-stepping
>> >+@itemx maint show range-stepping
>> >+Control whether to do stepping in an address range.
> Why is this a maintenance command?

Don't have any special reasons to do this.  The range stepping should be 
always on if target supports, and it can be turned off for some 
"maintenance purpose", so I set this command as a maintenance command.

-- 
Yao (齐尧)

Re: [PATCH 5/7] range stepping: New command 'maint set range stepping'

[Eli Zaretskii]

> Date: Mon, 18 Mar 2013 10:08:37 +0800
> From: Yao Qi <yao@codesourcery.com>
> CC: <gdb-patches@sourceware.org>
> 
> On 03/12/2013 01:04 AM, Eli Zaretskii wrote:
> >> +@kindex maint set range-stepping
> >> >+@kindex maint show range-stepping
> >> >+@cindex range-stepping
> >> >+@item maint set range-stepping
> >> >+@itemx maint show range-stepping
> >> >+Control whether to do stepping in an address range.
> > Why is this a maintenance command?
> 
> Don't have any special reasons to do this.  The range stepping should be 
> always on if target supports, and it can be turned off for some 
> "maintenance purpose", so I set this command as a maintenance command.

What would be the possible reasons to turn this feature off?

Once we have the reasons, we could try thinking whether or not they
are limited to GDB maintenance.

Re: [PATCH 5/7] range stepping: New command 'maint set range stepping'

[Pedro Alves]

On 03/11/2013 12:51 PM, Yao Qi wrote:

> @@ -4647,6 +4651,7 @@ remote_vcont_probe (struct remote_state *rs)
>        support_C = 0;
>        rs->support_vCont.t = 0;
>        rs->support_vCont.r = 0;
> +      use_range_stepping = 0;
>        while (p && *p == ';')
>  	{
>  	  p++;
> @@ -4661,7 +4666,10 @@ remote_vcont_probe (struct remote_state *rs)
>  	  else if (*p == 't' && (*(p + 1) == ';' || *(p + 1) == 0))
>  	    rs->support_vCont.t = 1;
>  	  else if (*p == 'r' && (*(p + 1) == ';' || *(p + 1) == 0))
> -	    rs->support_vCont.r = 1;
> +	    {
> +	      rs->support_vCont.r = 1;
> +	      use_range_stepping = 1;
> +	    }

I don't think this is a good approach.  :-(  If the user does
"maint set range-stepping", and then connects, she'll reasonably
assume range stepping will be disabled.  She could have a stub that
has known broken range stepping, for example.  However, with this
approach, if the target supports it, range stepping will always end
up forced enabled as soon as GDB sends the first vCont packet, even
if the user set range stepping off.

So I think the "maint set range-stepping" toggle should be enabled
by default, and it should only represent GDB's willingness to use
the feature.

>  
>  	  p = strchr (p, ';');
>  	}
> @@ -4708,6 +4716,8 @@ append_resumption (char *p, char *endp,
>  
>        pc = regcache_read_pc (get_thread_regcache (ptid));
>        if (rs->support_vCont.r /* Target supports step range.  */
> +	  /* GDB is willing to do range stepping.  */
> +	  && use_range_stepping
>  	  /* Can't do range stepping for all threads of a process
>  	     'pPID.-1'.  */
>  	  && !(remote_multi_process_p (rs) && ptid_is_pid (ptid))
> @@ -11488,6 +11498,45 @@ remote_upload_trace_state_variables (struct uploaded_tsv **utsvp)
>    return 0;
>  }
>  
> +static void
> +maint_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 do range-stepping "
> +		      "is %s.\n"), value);
> +}
> +
> +static void
> +maint_set_range_stepping (char *ignore_args, int from_tty,
> +			  struct cmd_list_element *c)
> +{
> +  /* Check range stepping is supported when turns it on.  */
> +  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_DISABLE
> +	      || !rs->support_vCont.r)

If we check instead for PACKET_ENABLE && rs->support_vCont.r here,


> +	    {
> +	      use_range_stepping = 0;
> +	      error (_("Range stepping is not supported"));
> +	    }
> +	}
> +      else
> +	{
> +	  use_range_stepping = 0;
> +	  error (_("Range stepping is not supported"));
> +	}

Then we can merge these two errors.  Following up on the "willingness-only"
idea, I'd rather make this a warning though.

In v3, I've make moved the setting out of "maint".

-- 
Pedro Alves

[PATCH 4/7] range stepping: gdb

[Yao Qi]

This patch teaches GDB to send 'vCont;r' packet when appropriate.
GDB has to check whether the target understands 'r' action of 'vCont'
packet.  We also make use of fields 'step_range_start' and
'step_range_end' of 'struct thread_control_state' to compose the range
of the stepping.

gdb:

2013-03-11  Yao Qi  <yao@codesourcery.com>

	* remote.c (struct support_v_cont) <r>: New field.
	(remote_vcont_probe): Handle 'r' action of 'vCont' packet.
	(append_resumption): Send 'vCont;r' for range stepping.
---
 gdb/remote.c |   36 +++++++++++++++++++++++++++++++++++-
 1 files changed, 35 insertions(+), 1 deletions(-)

diff --git a/gdb/remote.c b/gdb/remote.c
index ae0ae7e..84e09b0 100644
--- a/gdb/remote.c
+++ b/gdb/remote.c
@@ -253,6 +253,8 @@ struct support_v_cont
 {
   /* True if the stub reports support for vCont;t.  */
   int t;
+  /* True if the stub reports support for vCont;r.  */
+  int r;
 };
 
 /* Description of the remote protocol state for the currently
@@ -4644,6 +4646,7 @@ remote_vcont_probe (struct remote_state *rs)
       support_c = 0;
       support_C = 0;
       rs->support_vCont.t = 0;
+      rs->support_vCont.r = 0;
       while (p && *p == ';')
 	{
 	  p++;
@@ -4657,6 +4660,8 @@ remote_vcont_probe (struct remote_state *rs)
 	    support_C = 1;
 	  else if (*p == 't' && (*(p + 1) == ';' || *(p + 1) == 0))
 	    rs->support_vCont.t = 1;
+	  else if (*p == 'r' && (*(p + 1) == ';' || *(p + 1) == 0))
+	    rs->support_vCont.r = 1;
 
 	  p = strchr (p, ';');
 	}
@@ -4689,7 +4694,36 @@ append_resumption (char *p, char *endp,
   if (step && siggnal != GDB_SIGNAL_0)
     p += xsnprintf (p, endp - p, ";S%02x", siggnal);
   else if (step)
-    p += xsnprintf (p, endp - p, ";s");
+    {
+      struct thread_info *tp = NULL;
+      CORE_ADDR pc;
+
+      gdb_assert (!ptid_equal (ptid, minus_one_ptid));
+
+      if (ptid_is_pid (ptid))
+	tp = find_thread_ptid (inferior_ptid);
+      else
+	tp = find_thread_ptid (ptid);
+      gdb_assert (tp != NULL);
+
+      pc = regcache_read_pc (get_thread_regcache (ptid));
+      if (rs->support_vCont.r /* Target supports step range.  */
+	  /* Can't do range stepping for all threads of a process
+	     'pPID.-1'.  */
+	  && !(remote_multi_process_p (rs) && ptid_is_pid (ptid))
+	  /* Not step over a breakpoint.  */
+	  && !tp->control.trap_expected
+	  /* We have a range to single step.  */
+	  && THREAD_WITHIN_SINGLE_STEP_RANGE (tp, pc))
+	{
+	  p += xsnprintf (p, endp - p, ";r");
+	  p += hexnumstr (p, tp->control.step_range_start);
+	  p += xsnprintf (p, endp - p, ",");
+	  p += hexnumstr (p, tp->control.step_range_end);
+	}
+      else
+	p += xsnprintf (p, endp - p, ";s");
+    }
   else if (siggnal != GDB_SIGNAL_0)
     p += xsnprintf (p, endp - p, ";C%02x", siggnal);
   else
-- 
1.7.7.6

Re: [PATCH 4/7] range stepping: gdb

[Pedro Alves]

On 03/11/2013 12:51 PM, Yao Qi wrote:

> @@ -4689,7 +4694,36 @@ append_resumption (char *p, char *endp,
>    if (step && siggnal != GDB_SIGNAL_0)
>      p += xsnprintf (p, endp - p, ";S%02x", siggnal);
>    else if (step)
> -    p += xsnprintf (p, endp - p, ";s");
> +    {
> +      struct thread_info *tp = NULL;
> +      CORE_ADDR pc;
> +
> +      gdb_assert (!ptid_equal (ptid, minus_one_ptid));

I think you'll trip on this in case the target supports
vCont but not actually listing thread ids (magic_null_ptid).


> +
> +      if (ptid_is_pid (ptid))
> +	tp = find_thread_ptid (inferior_ptid);
> +      else
> +	tp = find_thread_ptid (ptid);
> +      gdb_assert (tp != NULL);
> +
> +      pc = regcache_read_pc (get_thread_regcache (ptid));
> +      if (rs->support_vCont.r /* Target supports step range.  */
> +	  /* Can't do range stepping for all threads of a process
> +	     'pPID.-1'.  */
> +	  && !(remote_multi_process_p (rs) && ptid_is_pid (ptid))
> +	  /* Not step over a breakpoint.  */
> +	  && !tp->control.trap_expected
> +	  /* We have a range to single step.  */
> +	  && THREAD_WITHIN_SINGLE_STEP_RANGE (tp, pc))

This is problematic.  It's better to _not_ have target itself decide
when to range step or to single step, and peeking at "infrun-owned"
variables.  For example, with software watchpoints, GDB needs to have control
of single-steps, in order to evaluate the watchpoints at after each
instruction is executed, so trap_expected isn't enough.  (My v3 adds a test for
that, that v2 fails.)
I dislike the design of using PC checks here too :-/.  That
seems fragile, and potentially inefficient (considering GDB ever
sending more than one range action per packet, that might end up
fetching registers for threads unnecessarily).  IMO, it's better to have
the core run control (infrun.c/infcmd.c) decide when to range step or
single-step.

> +	{
> +	  p += xsnprintf (p, endp - p, ";r");
> +	  p += hexnumstr (p, tp->control.step_range_start);
> +	  p += xsnprintf (p, endp - p, ",");
> +	  p += hexnumstr (p, tp->control.step_range_end);
> +	}
> +      else
> +	p += xsnprintf (p, endp - p, ";s");
> +    }
>    else if (siggnal != GDB_SIGNAL_0)
>      p += xsnprintf (p, endp - p, ";C%02x", siggnal);
>    else
> 

Thanks,
-- 
Pedro Alves

Re: [PATCH 4/7] range stepping: gdb

[Yao Qi]

On 05/15/2013 02:31 AM, Pedro Alves wrote:
> This is problematic.  It's better to_not_  have target itself decide
> when to range step or to single step, and peeking at "infrun-owned"
> variables.  For example, with software watchpoints, GDB needs to have control
> of single-steps, in order to evaluate the watchpoints at after each
> instruction is executed, so trap_expected isn't enough.  (My v3 adds a test for
> that, that v2 fails.)

I didn't take software watchpoint into account in V2, so probably some 
problems there.

> I dislike the design of using PC checks here too :-/.  That
> seems fragile, and potentially inefficient (considering GDB ever
> sending more than one range action per packet, that might end up
> fetching registers for threads unnecessarily).  IMO, it's better to have

Sorry, I don't understand why it is inefficient.

-- 
Yao (齐尧)

Re: [PATCH 4/7] range stepping: gdb

[Pedro Alves]

On 05/15/2013 09:07 AM, Yao Qi wrote:

>> I dislike the design of using PC checks here too :-/.  That
>> seems fragile, and potentially inefficient (considering GDB ever
>> sending more than one range action per packet, that might end up
>> fetching registers for threads unnecessarily).  IMO, it's better to have
> 
> Sorry, I don't understand why it is inefficient.

I meant, thinking forward, if GDB ever sends 100 vCont;r's in the same packet,
checking the PC potentially forces fetching the current registers for
100 threads, if GDB doesn't have them (or no longer has them) cached at that
point, with the associated extra RSP rountrips/traffic that causes.  It seems
better not to have a design that forces this refetch.

The fragility aspect was a much stronger concern though.
I ran the testsuite with an assertion in remote.c to catch the cases
of when the PC would be out of the step range.  It caught things like
displaced stepping, stepping through the dynamic linker, stepping through
inlines, and probably others I don't recall right now.  Along with software
watchpoints, this made wary of other cases (not controlled by trap_expected)
where complex run control might end up requiring the target reporting
a stop after a single instruction step even if the thread was in the
step range to begin with.

-- 
Pedro Alves

[PATCH 3/7] range stepping: gdbserver on x86/linux

[Yao Qi]

This patch adds the support of range stepping in GDBserver for
x86/linux.

gdb/gdbserver:

2013-03-11  Yao Qi  <yao@codesourcery.com>

	* gdbthread.h (struct thread_info) <start, end>: New fields.
	(thread_in_range_stepping_p): Declare.
	(thread_clear_range_stepping): Declare.
	* inferiors.c (thread_clear_range_stepping): New.
	(add_thread): Call thread_clear_range_stepping.
	(thread_in_range_stepping_p): New.
	* linux-low.c (linux_wait_1): Call thread_in_range_stepping_p
	to check if report stop to GDB.
	Call thread_clear_range_stepping.
	(linux_supports_range_stepping): New.
	(linux_target_ops): Initialize 'supports_range_stepping' to
	linux_supports_range_stepping.
	* linux-low.h (struct linux_target_ops)
	<supports_range_stepping>: New field.
	* linux-x86-low.c (x86_supports_range_stepping): New.
	(the_low_target): Initialize 'supports_range_stepping' to
	* server.c (handle_v_cont): Handle action 'r'.
	(handle_v_requests): Append ";r" if target supports range
	stepping.
	* target.h (struct target_ops) <supports_range_stepping>>:
	New field.
	(target_supports_range_stepping): New macro.
---
 gdb/gdbserver/gdbthread.h     |    6 ++++++
 gdb/gdbserver/inferiors.c     |   19 +++++++++++++++++++
 gdb/gdbserver/linux-low.c     |   38 +++++++++++++++++++++++++++++++++++---
 gdb/gdbserver/linux-low.h     |    2 ++
 gdb/gdbserver/linux-x86-low.c |    7 +++++++
 gdb/gdbserver/server.c        |   40 ++++++++++++++++++++++++++++++++++++++++
 gdb/gdbserver/target.h        |    7 +++++++
 7 files changed, 116 insertions(+), 3 deletions(-)

diff --git a/gdb/gdbserver/gdbthread.h b/gdb/gdbserver/gdbthread.h
index 85951d2..86542dd 100644
--- a/gdb/gdbserver/gdbthread.h
+++ b/gdb/gdbserver/gdbthread.h
@@ -57,12 +57,18 @@ struct thread_info
    Each item in the list holds the current step of the while-stepping
    action.  */
   struct wstep_state *while_stepping;
+
+  /* The start and end address of range stepping.  */
+  CORE_ADDR start, end;
 };
 
 extern struct inferior_list all_threads;
 
 void remove_thread (struct thread_info *thread);
 void add_thread (ptid_t ptid, void *target_data);
+int thread_in_range_stepping_p (struct thread_info *thread,
+				CORE_ADDR pc);
+void thread_clear_range_stepping (struct thread_info *thread);
 
 struct thread_info *find_thread_ptid (ptid_t ptid);
 struct thread_info *gdb_id_to_thread (unsigned int);
diff --git a/gdb/gdbserver/inferiors.c b/gdb/gdbserver/inferiors.c
index ba3c6cd..8d85af4 100644
--- a/gdb/gdbserver/inferiors.c
+++ b/gdb/gdbserver/inferiors.c
@@ -85,6 +85,15 @@ remove_inferior (struct inferior_list *list,
     list->tail = *cur;
 }
 
+/* Clear the state of range stepping of THREAD.  */
+
+void
+thread_clear_range_stepping (struct thread_info *thread)
+{
+  thread->start = 0;
+  thread->end = 0;
+}
+
 void
 add_thread (ptid_t thread_id, void *target_data)
 {
@@ -103,6 +112,16 @@ add_thread (ptid_t thread_id, void *target_data)
 
   new_thread->target_data = target_data;
   set_inferior_regcache_data (new_thread, new_register_cache ());
+  thread_clear_range_stepping (new_thread);
+}
+
+/* Return true if THREAD is in range stepping.  PC is the value of pc
+   of THREAD.  */
+
+int
+thread_in_range_stepping_p (struct thread_info *thread, CORE_ADDR pc)
+{
+  return (pc >= thread->start && pc < thread->end);
 }
 
 ptid_t
diff --git a/gdb/gdbserver/linux-low.c b/gdb/gdbserver/linux-low.c
index 5f03628..813a018 100644
--- a/gdb/gdbserver/linux-low.c
+++ b/gdb/gdbserver/linux-low.c
@@ -2311,6 +2311,7 @@ linux_wait_1 (ptid_t ptid,
   int maybe_internal_trap;
   int report_to_gdb;
   int trace_event;
+  int in_range_stepping;
 
   /* Translate generic target options into linux options.  */
   options = __WALL;
@@ -2320,6 +2321,7 @@ linux_wait_1 (ptid_t ptid,
 retry:
   bp_explains_trap = 0;
   trace_event = 0;
+  in_range_stepping = 0;
   ourstatus->kind = TARGET_WAITKIND_IGNORE;
 
   /* If we were only supposed to resume one thread, only wait for
@@ -2613,6 +2615,9 @@ Check if we're already there.\n",
       goto retry;
     }
 
+  in_range_stepping
+    = thread_in_range_stepping_p (current_inferior,
+				  event_child->stop_pc);
   /* 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
@@ -2622,9 +2627,12 @@ Check if we're already there.\n",
      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
+		   /* Report the event back to GDB if the inferior is
+		      stepping and not in range stepping.  */
+		   || (current_inferior->last_resume_kind == resume_step
+		       && !in_range_stepping)
 		   || event_child->stopped_by_watchpoint
-		   || (!step_over_finished
+		   || (!step_over_finished && !in_range_stepping
 		       && !bp_explains_trap && !trace_event)
 		   || (gdb_breakpoint_here (event_child->stop_pc)
 		       && gdb_condition_true_at_breakpoint (event_child->stop_pc)
@@ -2645,6 +2653,12 @@ Check if we're already there.\n",
 	    fprintf (stderr, "Step-over finished.\n");
 	  if (trace_event)
 	    fprintf (stderr, "Tracepoint event.\n");
+	  if (thread_in_range_stepping_p (current_inferior,
+					  event_child->stop_pc))
+	    fprintf (stderr, "Range stepping pc 0x%s [0x%s, 0x%s).\n",
+		     paddress (event_child->stop_pc),
+		     paddress (current_inferior->start),
+		     paddress (current_inferior->end));
 	}
 
       /* We're not reporting this breakpoint to GDB, so apply the
@@ -2676,7 +2690,13 @@ 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 (!thread_in_range_stepping_p (current_inferior,
+					   event_child->stop_pc))
+	    fprintf (stderr, "Range stepping is done, reporting event.\n");
+	  else
+	    fprintf (stderr, "GDB wanted to single-step, reporting event.\n");
+	}
       if (event_child->stopped_by_watchpoint)
 	fprintf (stderr, "Stopped by watchpoint.\n");
       if (gdb_breakpoint_here (event_child->stop_pc))
@@ -2685,6 +2705,8 @@ Check if we're already there.\n",
 	fprintf (stderr, "Hit a non-gdbserver trap event.\n");
     }
 
+  thread_clear_range_stepping (current_inferior);
+
   /* Alright, we're going to report a stop.  */
 
   if (!non_stop && !stabilizing_threads)
@@ -5081,6 +5103,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)
@@ -5885,6 +5916,7 @@ static struct target_ops linux_target_ops = {
   linux_get_min_fast_tracepoint_insn_len,
   linux_qxfer_libraries_svr4,
   linux_supports_agent,
+  linux_supports_range_stepping,
 };
 
 static void
diff --git a/gdb/gdbserver/linux-low.h b/gdb/gdbserver/linux-low.h
index 27dd3b5..5d3e8cf 100644
--- a/gdb/gdbserver/linux-low.h
+++ b/gdb/gdbserver/linux-low.h
@@ -168,6 +168,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;
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 922a5bf..0b330d1 100644
--- a/gdb/gdbserver/server.c
+++ b/gdb/gdbserver/server.c
@@ -1871,10 +1871,15 @@ handle_v_cont (char *own_buf)
   p = &own_buf[5];
   while (*p)
     {
+      CORE_ADDR start = 0;
+      CORE_ADDR end = 0;
+
       p++;
 
       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')
@@ -1894,6 +1899,21 @@ 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 (&start, p, p1 - p);
+
+	  p = p1 + 1;
+	  p1 = strchr (p, ':');
+	  decode_address (&end, p, p1 - p);
+
+	  resume_info[i].sig = 0;
+	  p = p1;
+	}
       else
 	{
 	  resume_info[i].sig = 0;
@@ -1919,6 +1939,21 @@ handle_v_cont (char *own_buf)
 	    goto err;
 
 	  resume_info[i].thread = ptid;
+	  if (end > 0)
+	    {
+	      struct thread_info *tp = find_thread_ptid (ptid);
+
+	      /* GDB should not send range stepping for all threads of
+		 a process, like 'vCont;rSTART,END:pPID.-1', TP can't
+		 be NULL.  */
+	      gdb_assert (tp != NULL);
+
+	      tp->start = start;
+	      tp->end = end;
+
+	      start = 0;
+	      end = 0;
+	    }
 
 	  i++;
 	}
@@ -2142,6 +2177,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 cc9a910..b1dd3b1 100644
--- a/gdb/gdbserver/target.h
+++ b/gdb/gdbserver/target.h
@@ -397,6 +397,9 @@ struct target_ops
 
   /* Return true if target supports debugging agent.  */
   int (*supports_agent) (void);
+
+  /* Return true if target supports range stepping.  */
+  int (*supports_range_stepping) (void);
 };
 
 extern struct target_ops *the_target;
@@ -520,6 +523,10 @@ int kill_inferior (int);
   (the_target->supports_agent ? \
    (*the_target->supports_agent) () : 0)
 
+#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);
-- 
1.7.7.6

Re: [PATCH 3/7] range stepping: gdbserver on x86/linux

[Pedro Alves]

On 03/11/2013 12:51 PM, Yao Qi wrote:


> diff --git a/gdb/gdbserver/server.c b/gdb/gdbserver/server.c
> index 922a5bf..0b330d1 100644
> --- a/gdb/gdbserver/server.c
> +++ b/gdb/gdbserver/server.c
> @@ -1871,10 +1871,15 @@ handle_v_cont (char *own_buf)
>    p = &own_buf[5];
>    while (*p)
>      {
> +      CORE_ADDR start = 0;
> +      CORE_ADDR end = 0;
> +
>        p++;
>  
>        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')
> @@ -1894,6 +1899,21 @@ 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 (&start, p, p1 - p);
> +
> +	  p = p1 + 1;
> +	  p1 = strchr (p, ':');
> +	  decode_address (&end, p, p1 - p);
> +
> +	  resume_info[i].sig = 0;
> +	  p = p1;
> +	}
>        else
>  	{
>  	  resume_info[i].sig = 0;
> @@ -1919,6 +1939,21 @@ handle_v_cont (char *own_buf)
>  	    goto err;
>  
>  	  resume_info[i].thread = ptid;
> +	  if (end > 0)
> +	    {
> +	      struct thread_info *tp = find_thread_ptid (ptid);
> +
> +	      /* GDB should not send range stepping for all threads of
> +		 a process, like 'vCont;rSTART,END:pPID.-1', TP can't
> +		 be NULL.  */
> +	      gdb_assert (tp != NULL);
> +
> +	      tp->start = start;
> +	      tp->end = end;
> +
> +	      start = 0;
> +	      end = 0;
> +	    }

My main issues with this are:

 - it assumes GDB will ever only send one r action per vCont.  I'd much
   rather we don't bake in that assumption.

 - GDBserver should not gdb_assert for unexpected input it can't control.

 - It seems like this leaves threads with stale step ranges.  Because
   linux-low.c only clears the stepping range of the thread that is reporting
   the event (here):

> @@ -2685,6 +2705,8 @@ Check if we're already there.\n",
>  	fprintf (stderr, "Hit a non-gdbserver trap event.\n");
>      }
>
> +  thread_clear_range_stepping (current_inferior);
> +
>    /* Alright, we're going to report a stop.  */
>
>    if (!non_stop && !stabilizing_threads)
> @@ -5081,6 +5103,15 @@ linux_supports_agent (void)
>    return 1;
>  }

That is, e.g.,:

    - user "step"s thread 1 (vCont;r)
    - thread 2 hits breakpoint (step range of thread 2 is cleared)
    - user goes back to thread 1, and does "si".
    - GDB sends vCont;s, but thread 1 still has the stale step range
      from the previous vCont;r.

We should probably have an explicit test for this, though I
haven't added one myself (at least yet).

-- 
Pedro Alves

Re: [PATCH 3/7] range stepping: gdbserver on x86/linux

[Yao Qi]

On 05/15/2013 02:30 AM, Pedro Alves wrote:
>   - it assumes GDB will ever only send one r action per vCont.  I'd much
>     rather we don't bake in that assumption.
>

I am afraid not.  It works for multiple r actions in one vCont.

>   - It seems like this leaves threads with stale step ranges.  Because
>     linux-low.c only clears the stepping range of the thread that is reporting
>     the event (here):

OK.  Looks we need to call thread_clear_range_stepping for the thread we 
parsed in handle_v_cont first.

-- 
Yao (齐尧)

Re: [PATCH 3/7] range stepping: gdbserver on x86/linux

[Pedro Alves]

On 05/15/2013 08:40 AM, Yao Qi wrote:
> On 05/15/2013 02:30 AM, Pedro Alves wrote:
>>   - it assumes GDB will ever only send one r action per vCont.  I'd much
>>     rather we don't bake in that assumption.
>>
> 
> I am afraid not.  It works for multiple r actions in one vCont.

Oh, sorry about that.  :-/  That wasn't actually the issue I had noticed
initially, and then when I went back to compose the email, I had
already forgotten and got it mixed up.  The issue was actually with:

	  if (end > 0)
	    {
	      struct thread_info *tp = find_thread_ptid (ptid);

	      /* GDB should not send range stepping for all threads of
		 a process, like 'vCont;rSTART,END:pPID.-1', TP can't
		 be NULL.  */
	      gdb_assert (tp != NULL);

I think it's best not to error on this as nothing in the protocol actually
prohibits it, and we might take advantage of it at some point.  The way
gdbserver handles vCont requests currently is by letting the target
match the ptid to whatever thread/lwp.  That means leaving server.c
only knowing about how to parse the rsp and construct a thread_resume,
and then pass that down to the target.  Conceivably, a target might
not need to keep the step range anywhere, if it has something like
a PTRACE_STEP_RANGE at the kernel level.  (There's PTRACE_BLOCKSTEP,
but it's not the same).

-- 
Pedro Alves

Re: [PATCH 3/7] range stepping: gdbserver on x86/linux

[Yao Qi]

On 05/21/2013 02:00 AM, Pedro Alves wrote:
> The issue was actually with:
>
> 	  if (end > 0)
> 	    {
> 	      struct thread_info *tp = find_thread_ptid (ptid);
>
> 	      /* GDB should not send range stepping for all threads of
> 		 a process, like 'vCont;rSTART,END:pPID.-1', TP can't
> 		 be NULL.  */
> 	      gdb_assert (tp != NULL);
>
> I think it's best not to error on this as nothing in the protocol actually
> prohibits it, and we might take advantage of it at some point.  The way

OK, looks I abused gdb_assert some times, :).

> gdbserver handles vCont requests currently is by letting the target
> match the ptid to whatever thread/lwp.  That means leaving server.c
> only knowing about how to parse the rsp and construct a thread_resume,
> and then pass that down to the target.  Conceivably, a target might
> not need to keep the step range anywhere, if it has something like
> a PTRACE_STEP_RANGE at the kernel level.  (There's PTRACE_BLOCKSTEP,
> but it's not the same).

Yeah, that makes sense to me.  I didn't realize such abstraction when 
reading the code.  Thanks for your explanation.

-- 
Yao (齐尧)

[PATCH 7/7] range stepping: doc and NEWS

[Yao Qi]

gdb/doc:

2013-03-11  Yao Qi  <yao@codesourcery.com>

	* gdb.texinfo (Packets): Document about 'vCont;r'.

gdb:

2013-03-11  Yao Qi  <yao@codesourcery.com>

	* NEWS: Mention range stepping, new packet and new
	commands.
---
 gdb/NEWS            |   12 ++++++++++++
 gdb/doc/gdb.texinfo |    5 +++++
 2 files changed, 17 insertions(+), 0 deletions(-)

diff --git a/gdb/NEWS b/gdb/NEWS
index 99b8add..836638c 100644
--- a/gdb/NEWS
+++ b/gdb/NEWS
@@ -89,6 +89,10 @@ catch signal
 maint info bfds
   List the BFDs known to GDB.
 
+maint set range-stepping
+maint show range-stepping
+  Control and show whether do range stepping.
+
 python-interactive [command]
 pi [command]
   Start a Python interactive prompt, or evaluate the optional command
@@ -164,11 +168,19 @@ show filename-display
   feature to be enabled.  For more information, see:
       http://fedoraproject.org/wiki/Features/MiniDebugInfo
 
+* GDB now supports range stepping, which improves the performance of
+  single stepping over a source line by reducing the number of control
+  packets from GDB.
+
 * New remote packets
 
 QTBuffer:size
    Set the size of trace buffer.  The remote stub reports support for this
    packet to gdb's qSupported query.
+vCont;r
+  Tell the remote stub to do range stepping in an address range.  The remote
+  stub reports a stop reply when the program goes out of the range or is
+  stopped due to other reasons, such as hitting a breakpoint.
 
 *** Changes in GDB 7.5
 
diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index 7c06120..b505cda 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -36261,6 +36261,11 @@ 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 PC is within the range [@var{start},
+@var{end}).  Note that a stop reply may be sent at any point even if
+the PC is within the stepping range; for example, it is permissible to
+implement this packet in a degenerate way as a single step operation.
 @end table
 
 The optional argument @var{addr} normally associated with the 
-- 
1.7.7.6

Re: [PATCH 7/7] range stepping: doc and NEWS

[Abid, Hafiz]

On 11/03/13 12:51:31, Yao Qi wrote:
> gdb/doc:
> 
> 2013-03-11  Yao Qi  <yao@codesourcery.com>
> 
> 	* gdb.texinfo (Packets): Document about 'vCont;r'.
> 
> gdb:
> 
> 2013-03-11  Yao Qi  <yao@codesourcery.com>
> 
> 	* NEWS: Mention range stepping, new packet and new
> 	commands.
> ---
>  gdb/NEWS            |   12 ++++++++++++
>  gdb/doc/gdb.texinfo |    5 +++++
>  2 files changed, 17 insertions(+), 0 deletions(-)
> 
> diff --git a/gdb/NEWS b/gdb/NEWS
> index 99b8add..836638c 100644
> --- a/gdb/NEWS
> +++ b/gdb/NEWS
> @@ -89,6 +89,10 @@ catch signal
>  maint info bfds
>    List the BFDs known to GDB.
> 
> +maint set range-stepping
> +maint show range-stepping
> +  Control and show whether do range stepping.
What about having a to here. ^^

> +
>  python-interactive [command]
>  pi [command]
>    Start a Python interactive prompt, or evaluate the optional command
> @@ -164,11 +168,19 @@ show filename-display
>    feature to be enabled.  For more information, see:
>        http://fedoraproject.org/wiki/Features/MiniDebugInfo
> 
> +* GDB now supports range stepping, which improves the performance of
> +  single stepping over a source line by reducing the number of  
> control
> +  packets from GDB.
> +
>  * New remote packets
> 
>  QTBuffer:size
>     Set the size of trace buffer.  The remote stub reports support  
> for this
>     packet to gdb's qSupported query.
> +vCont;r
> +  Tell the remote stub to do range stepping in an address range.   
> The remote
> +  stub reports a stop reply when the program goes out of the range  
> or is
> +  stopped due to other reasons, such as hitting a breakpoint.
> 
>  *** Changes in GDB 7.5
> 
> diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
> index 7c06120..b505cda 100644
> --- a/gdb/doc/gdb.texinfo
> +++ b/gdb/doc/gdb.texinfo
> @@ -36261,6 +36261,11 @@ 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 PC is within the range [@var{start},
> +@var{end}).  Note that a stop reply may be sent at any point even if
> +the PC is within the stepping range; for example, it is permissible  
> to
> +implement this packet in a degenerate way as a single step operation.
>  @end table
> 
>  The optional argument @var{addr} normally associated with the
> --
> 1.7.7.6
> 
> 

Re: [PATCH 7/7] range stepping: doc and NEWS

[Eli Zaretskii]

> From: Yao Qi <yao@codesourcery.com>
> Date: Mon, 11 Mar 2013 20:51:31 +0800
> 
> +maint set range-stepping
> +maint show range-stepping
> +  Control and show whether do range stepping.
                             ^^^^
"to do"

OK with that change.

Thanks.

Re: [PATCH 7/7] range stepping: doc and NEWS

[Pedro Alves]

On 03/11/2013 12:51 PM, Yao Qi wrote:

> diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
> index 7c06120..b505cda 100644
> --- a/gdb/doc/gdb.texinfo
> +++ b/gdb/doc/gdb.texinfo
> @@ -36261,6 +36261,11 @@ 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 PC is within the range [@var{start},
> +@var{end}).  Note that a stop reply may be sent at any point even if
> +the PC is within the stepping range; for example, it is permissible to
> +implement this packet in a degenerate way as a single step operation.
>  @end table

I think we should be explicit on what it means when start==end.

-- 
Pedro Alves

[PATCH 2/7] Move rs->support_vCont_t to a separate struct

[Yao Qi]

This is a refactor patch to move field 'support_vCont_t' to a separate
struct 'support_v_cont', to prepare for adding more fields in it in
the next patch.

gdb:

2013-03-11  Yao Qi  <yao@codesourcery.com>

	* remote.c (struct support_v_cont): New.
	(struct remote_state) <support_vCont_t>: Remove.
	<support_vCont>: New field.
	(remote_vcont_probe, remote_stop_ns): Update.
---
 gdb/remote.c |   18 +++++++++++++-----
 1 files changed, 13 insertions(+), 5 deletions(-)

diff --git a/gdb/remote.c b/gdb/remote.c
index 4978fca..ae0ae7e 100644
--- a/gdb/remote.c
+++ b/gdb/remote.c
@@ -247,6 +247,14 @@ static struct cmd_list_element *remote_cmdlist;
 static struct cmd_list_element *remote_set_cmdlist;
 static struct cmd_list_element *remote_show_cmdlist;
 
+/* The status of the stub supports for various vCont; packets.  */
+
+struct support_v_cont
+{
+  /* True if the stub reports support for 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.  */
@@ -304,8 +312,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 supports for various vCont; packets.  */
+  struct support_v_cont support_vCont;
 
   /* True if the stub reports support for conditional tracepoints.  */
   int cond_tracepoints;
@@ -4635,7 +4643,7 @@ remote_vcont_probe (struct remote_state *rs)
       support_S = 0;
       support_c = 0;
       support_C = 0;
-      rs->support_vCont_t = 0;
+      rs->support_vCont.t = 0;
       while (p && *p == ';')
 	{
 	  p++;
@@ -4648,7 +4656,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->support_vCont.t = 1;
 
 	  p = strchr (p, ';');
 	}
@@ -4997,7 +5005,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->support_vCont.t)
     error (_("Remote server does not support stopping threads"));
 
   if (ptid_equal (ptid, minus_one_ptid)
-- 
1.7.7.6

Re: [PATCH 0/7] Range stepping

[Pedro Alves]

Just a FYI, so it doesn't seem this is stalled too,
I'm looking at/playing with this series.

Thanks much for doing this.  It's a long overdue feature.

I'll reply back with a review shortly (today/tomorrow).

-- 
Pedro Alves

Re: [PATCH 0/7] Range stepping

[Pedro Alves]

On 03/14/2013 08:12 PM, Pedro Alves wrote:
> Just a FYI, so it doesn't seem this is stalled too,
> I'm looking at/playing with this series.
> 
> Thanks much for doing this.  It's a long overdue feature.
> 
> I'll reply back with a review shortly (today/tomorrow).

I ended up spending the afternoon around it, tweaking
a few things.  More early next week.

Thanks again!

-- 
Pedro Alves

Re: [PATCH 0/7] Range stepping

[Yao Qi]

On 03/16/2013 03:44 AM, Pedro Alves wrote:
> I ended up spending the afternoon around it, tweaking
> a few things.  More early next week.

Pedro,
Are your comments ready?

-- 
Yao (齐尧)

Re: [PATCH 0/7] Range stepping

[Pedro Alves]

On 03/22/2013 02:16 AM, Yao Qi wrote:
> On 03/16/2013 03:44 AM, Pedro Alves wrote:
>> I ended up spending the afternoon around it, tweaking
>> a few things.  More early next week.
> 
> Pedro,
> Are your comments ready?

Sorry, not yet.  It's at the top of my TODO just below 7.6
issues.  I did hack on this more a couple days early this
week, but then spent the rest of the week mostly fixing
the integer command regressions for 7.6, and reviewing
patches that block the release.

-- 
Pedro Alves

[PATCH 0/7 V2] Range stepping

[Yao Qi]

Hello,
This is the V2 of range stepping series.  V1 can be found here
<http://sourceware.org/ml/gdb-patches/2013-03/msg00450.html>.

There are some updates in V2:

 - Rebase patches on GDB trunk, and resolve conflicts.
 - Use phex_nz instead of hexnumstr to get the address.  It fixes a
bug on mips target when sending address to the remote target.
 - Define an internalvar range_stepping_counter and increment it each
time GDB does range-stepping, so that in the test, we don't have to
turn 'remote debug' on and parse the RSP packets.  During the test,
we find that it is unsafe to turn the debugging output on, because it
may blow up the gdb.log.

Patch 1,2,3,7 are unchanged.

The whole series is tested on x86_64-linux, native and gdbserver.


 gdb/NEWS                                   |   12 ++
 gdb/doc/gdb.texinfo                        |   15 ++
 gdb/gdbserver/gdbthread.h                  |    6 +
 gdb/gdbserver/inferiors.c                  |   19 +++
 gdb/gdbserver/linux-low.c                  |   38 +++++-
 gdb/gdbserver/linux-low.h                  |    2 +
 gdb/gdbserver/linux-x86-low.c              |    7 +
 gdb/gdbserver/server.c                     |   40 +++++
 gdb/gdbserver/target.h                     |    6 +
 gdb/gdbthread.h                            |    6 +
 gdb/infrun.c                               |    6 +-
 gdb/remote.c                               |  127 +++++++++++++++-
 gdb/testsuite/gdb.base/range-stepping.c    |   69 +++++++++
 gdb/testsuite/gdb.base/range-stepping.exp  |  222 ++++++++++++++++++++++++++++
 gdb/testsuite/gdb.trace/range-stepping.c   |   61 ++++++++
 gdb/testsuite/gdb.trace/range-stepping.exp |   89 +++++++++++
 16 files changed, 712 insertions(+), 13 deletions(-)
 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

-- 
1.7.7.6

[PATCH 2/7] Move rs->support_vCont_t to a separate struct

[Yao Qi]

This is a refactor patch to move field 'support_vCont_t' to a separate
struct 'support_v_cont', to prepare for adding more fields in it in
the next patch.

gdb:

2013-03-11  Yao Qi  <yao@codesourcery.com>

	* remote.c (struct support_v_cont): New.
	(struct remote_state) <support_vCont_t>: Remove.
	<support_vCont>: New field.
	(remote_vcont_probe, remote_stop_ns): Update.
---
 gdb/remote.c |   18 +++++++++++++-----
 1 files changed, 13 insertions(+), 5 deletions(-)

diff --git a/gdb/remote.c b/gdb/remote.c
index de075c8..f6a3f4c 100644
--- a/gdb/remote.c
+++ b/gdb/remote.c
@@ -250,6 +250,14 @@ static struct cmd_list_element *remote_cmdlist;
 static struct cmd_list_element *remote_set_cmdlist;
 static struct cmd_list_element *remote_show_cmdlist;
 
+/* The status of the stub supports for various vCont; packets.  */
+
+struct support_v_cont
+{
+  /* True if the stub reports support for 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.  */
@@ -307,8 +315,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 supports for various vCont; packets.  */
+  struct support_v_cont support_vCont;
 
   /* True if the stub reports support for conditional tracepoints.  */
   int cond_tracepoints;
@@ -4640,7 +4648,7 @@ remote_vcont_probe (struct remote_state *rs)
       support_S = 0;
       support_c = 0;
       support_C = 0;
-      rs->support_vCont_t = 0;
+      rs->support_vCont.t = 0;
       while (p && *p == ';')
 	{
 	  p++;
@@ -4653,7 +4661,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->support_vCont.t = 1;
 
 	  p = strchr (p, ';');
 	}
@@ -5002,7 +5010,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->support_vCont.t)
     error (_("Remote server does not support stopping threads"));
 
   if (ptid_equal (ptid, minus_one_ptid)
-- 
1.7.7.6

[PATCH 1/7] New macro THREAD_WITHIN_SINGLE_STEP_RANGE

[Yao Qi]

This patch moves code into a macro, which will be used in my
following patches.

gdb:

2013-03-11  Yao Qi  <yao@codesourcery.com>

	* gdbthread.h (THREAD_WITHIN_SINGLE_STEP_RANGE): New macro.
	* infrun.c (handle_inferior_event): Use it.
---
 gdb/gdbthread.h |    6 ++++++
 gdb/infrun.c    |    6 ++----
 2 files changed, 8 insertions(+), 4 deletions(-)

diff --git a/gdb/gdbthread.h b/gdb/gdbthread.h
index 0846322..3525de6 100644
--- a/gdb/gdbthread.h
+++ b/gdb/gdbthread.h
@@ -38,6 +38,12 @@ enum thread_state
   THREAD_EXITED,
 };
 
+/* PC is within the range of single stepping of thread THR.  */
+
+#define THREAD_WITHIN_SINGLE_STEP_RANGE(THR, PC)	\
+  ((PC) >= (THR)->control.step_range_start		\
+   && (PC) < (THR)->control.step_range_end)
+
 /* Inferior thread specific part of `struct infcall_control_state'.
 
    Inferior process counterpart is `struct inferior_control_state'.  */
diff --git a/gdb/infrun.c b/gdb/infrun.c
index 7031ecc..6876dd6 100644
--- a/gdb/infrun.c
+++ b/gdb/infrun.c
@@ -4341,8 +4341,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)
+	  && THREAD_WITHIN_SINGLE_STEP_RANGE (ecs->event_thread, stop_pc)
 	  && frame_id_eq (get_stack_frame_id (frame),
 			  ecs->event_thread->control.step_stack_frame_id)
 	  && ecs->event_thread->control.step_resume_breakpoint == NULL)
@@ -4711,8 +4710,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 (THREAD_WITHIN_SINGLE_STEP_RANGE (ecs->event_thread, stop_pc)
       && (execution_direction != EXEC_REVERSE
 	  || frame_id_eq (get_frame_id (frame),
 			  ecs->event_thread->control.step_frame_id)))
-- 
1.7.7.6

[PATCH 3/7] range stepping: gdbserver on x86/linux

[Yao Qi]

This patch adds the support of range stepping in GDBserver for
x86/linux.

gdb/gdbserver:

2013-03-11  Yao Qi  <yao@codesourcery.com>

	* gdbthread.h (struct thread_info) <start, end>: New fields.
	(thread_in_range_stepping_p): Declare.
	(thread_clear_range_stepping): Declare.
	* inferiors.c (thread_clear_range_stepping): New.
	(add_thread): Call thread_clear_range_stepping.
	(thread_in_range_stepping_p): New.
	* linux-low.c (linux_wait_1): Call thread_in_range_stepping_p
	to check if report stop to GDB.
	Call thread_clear_range_stepping.
	(linux_supports_range_stepping): New.
	(linux_target_ops): Initialize 'supports_range_stepping' to
	linux_supports_range_stepping.
	* linux-low.h (struct linux_target_ops)
	<supports_range_stepping>: New field.
	* linux-x86-low.c (x86_supports_range_stepping): New.
	(the_low_target): Initialize 'supports_range_stepping' to
	* server.c (handle_v_cont): Handle action 'r'.
	(handle_v_requests): Append ";r" if target supports range
	stepping.
	* target.h (struct target_ops) <supports_range_stepping>>:
	New field.
	(target_supports_range_stepping): New macro.
---
 gdb/gdbserver/gdbthread.h     |    6 ++++++
 gdb/gdbserver/inferiors.c     |   19 +++++++++++++++++++
 gdb/gdbserver/linux-low.c     |   38 +++++++++++++++++++++++++++++++++++---
 gdb/gdbserver/linux-low.h     |    2 ++
 gdb/gdbserver/linux-x86-low.c |    7 +++++++
 gdb/gdbserver/server.c        |   40 ++++++++++++++++++++++++++++++++++++++++
 gdb/gdbserver/target.h        |    6 ++++++
 7 files changed, 115 insertions(+), 3 deletions(-)

diff --git a/gdb/gdbserver/gdbthread.h b/gdb/gdbserver/gdbthread.h
index 5d4955b..fbd768b 100644
--- a/gdb/gdbserver/gdbthread.h
+++ b/gdb/gdbserver/gdbthread.h
@@ -62,12 +62,18 @@ struct thread_info
 
   /* Branch trace target information for this thread.  */
   struct btrace_target_info *btrace;
+
+  /* The start and end address of range stepping.  */
+  CORE_ADDR start, end;
 };
 
 extern struct inferior_list all_threads;
 
 void remove_thread (struct thread_info *thread);
 void add_thread (ptid_t ptid, void *target_data);
+int thread_in_range_stepping_p (struct thread_info *thread,
+				CORE_ADDR pc);
+void thread_clear_range_stepping (struct thread_info *thread);
 
 struct thread_info *find_thread_ptid (ptid_t ptid);
 struct thread_info *gdb_id_to_thread (unsigned int);
diff --git a/gdb/gdbserver/inferiors.c b/gdb/gdbserver/inferiors.c
index 6953d0e..2184a7b 100644
--- a/gdb/gdbserver/inferiors.c
+++ b/gdb/gdbserver/inferiors.c
@@ -85,6 +85,15 @@ remove_inferior (struct inferior_list *list,
     list->tail = *cur;
 }
 
+/* Clear the state of range stepping of THREAD.  */
+
+void
+thread_clear_range_stepping (struct thread_info *thread)
+{
+  thread->start = 0;
+  thread->end = 0;
+}
+
 void
 add_thread (ptid_t thread_id, void *target_data)
 {
@@ -103,6 +112,16 @@ add_thread (ptid_t thread_id, void *target_data)
 
   new_thread->target_data = target_data;
   set_inferior_regcache_data (new_thread, new_register_cache ());
+  thread_clear_range_stepping (new_thread);
+}
+
+/* Return true if THREAD is in range stepping.  PC is the value of pc
+   of THREAD.  */
+
+int
+thread_in_range_stepping_p (struct thread_info *thread, CORE_ADDR pc)
+{
+  return (pc >= thread->start && pc < thread->end);
 }
 
 ptid_t
diff --git a/gdb/gdbserver/linux-low.c b/gdb/gdbserver/linux-low.c
index 72c51e0..e9879cc 100644
--- a/gdb/gdbserver/linux-low.c
+++ b/gdb/gdbserver/linux-low.c
@@ -2313,6 +2313,7 @@ linux_wait_1 (ptid_t ptid,
   int maybe_internal_trap;
   int report_to_gdb;
   int trace_event;
+  int in_range_stepping;
 
   /* Translate generic target options into linux options.  */
   options = __WALL;
@@ -2322,6 +2323,7 @@ linux_wait_1 (ptid_t ptid,
 retry:
   bp_explains_trap = 0;
   trace_event = 0;
+  in_range_stepping = 0;
   ourstatus->kind = TARGET_WAITKIND_IGNORE;
 
   /* If we were only supposed to resume one thread, only wait for
@@ -2615,6 +2617,9 @@ Check if we're already there.\n",
       goto retry;
     }
 
+  in_range_stepping
+    = thread_in_range_stepping_p (current_inferior,
+				  event_child->stop_pc);
   /* 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
@@ -2624,9 +2629,12 @@ Check if we're already there.\n",
      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
+		   /* Report the event back to GDB if the inferior is
+		      stepping and not in range stepping.  */
+		   || (current_inferior->last_resume_kind == resume_step
+		       && !in_range_stepping)
 		   || event_child->stopped_by_watchpoint
-		   || (!step_over_finished
+		   || (!step_over_finished && !in_range_stepping
 		       && !bp_explains_trap && !trace_event)
 		   || (gdb_breakpoint_here (event_child->stop_pc)
 		       && gdb_condition_true_at_breakpoint (event_child->stop_pc)
@@ -2647,6 +2655,12 @@ Check if we're already there.\n",
 	    fprintf (stderr, "Step-over finished.\n");
 	  if (trace_event)
 	    fprintf (stderr, "Tracepoint event.\n");
+	  if (thread_in_range_stepping_p (current_inferior,
+					  event_child->stop_pc))
+	    fprintf (stderr, "Range stepping pc 0x%s [0x%s, 0x%s).\n",
+		     paddress (event_child->stop_pc),
+		     paddress (current_inferior->start),
+		     paddress (current_inferior->end));
 	}
 
       /* We're not reporting this breakpoint to GDB, so apply the
@@ -2678,7 +2692,13 @@ 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 (!thread_in_range_stepping_p (current_inferior,
+					   event_child->stop_pc))
+	    fprintf (stderr, "Range stepping is done, reporting event.\n");
+	  else
+	    fprintf (stderr, "GDB wanted to single-step, reporting event.\n");
+	}
       if (event_child->stopped_by_watchpoint)
 	fprintf (stderr, "Stopped by watchpoint.\n");
       if (gdb_breakpoint_here (event_child->stop_pc))
@@ -2687,6 +2707,8 @@ Check if we're already there.\n",
 	fprintf (stderr, "Hit a non-gdbserver trap event.\n");
     }
 
+  thread_clear_range_stepping (current_inferior);
+
   /* Alright, we're going to report a stop.  */
 
   if (!non_stop && !stabilizing_threads)
@@ -5083,6 +5105,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 +5970,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 27dd3b5..5d3e8cf 100644
--- a/gdb/gdbserver/linux-low.h
+++ b/gdb/gdbserver/linux-low.h
@@ -168,6 +168,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;
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..aa4ddf6 100644
--- a/gdb/gdbserver/server.c
+++ b/gdb/gdbserver/server.c
@@ -2040,10 +2040,15 @@ handle_v_cont (char *own_buf)
   p = &own_buf[5];
   while (*p)
     {
+      CORE_ADDR start = 0;
+      CORE_ADDR end = 0;
+
       p++;
 
       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,6 +2068,21 @@ 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 (&start, p, p1 - p);
+
+	  p = p1 + 1;
+	  p1 = strchr (p, ':');
+	  decode_address (&end, p, p1 - p);
+
+	  resume_info[i].sig = 0;
+	  p = p1;
+	}
       else
 	{
 	  resume_info[i].sig = 0;
@@ -2088,6 +2108,21 @@ handle_v_cont (char *own_buf)
 	    goto err;
 
 	  resume_info[i].thread = ptid;
+	  if (end > 0)
+	    {
+	      struct thread_info *tp = find_thread_ptid (ptid);
+
+	      /* GDB should not send range stepping for all threads of
+		 a process, like 'vCont;rSTART,END:pPID.-1', TP can't
+		 be NULL.  */
+	      gdb_assert (tp != NULL);
+
+	      tp->start = start;
+	      tp->end = end;
+
+	      start = 0;
+	      end = 0;
+	    }
 
 	  i++;
 	}
@@ -2311,6 +2346,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..8c0790e 100644
--- a/gdb/gdbserver/target.h
+++ b/gdb/gdbserver/target.h
@@ -414,6 +414,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 +551,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);
-- 
1.7.7.6

[PATCH 4/7] range stepping: gdb

[Yao Qi]

This patch teaches GDB to send 'vCont;r' packet when appropriate.
GDB has to check whether the target understand 'r' action of 'vCont'
packet.  We also make use of fields 'step_range_start' and
'step_range_end' of 'struct thread_control_state' to compose the range
of the stepping.

In V2, there are several changes:

 - Use phex_nz instead of hexnumstr  to get the address.
 - Define an internalvar range_stepping_counter and increment it each
time GDB does range-stepping.

gdb:

2013-04-10  Yao Qi  <yao@codesourcery.com>

	* remote.c (struct support_v_cont) <r>: New field.
	(remote_vcont_probe): Handle 'r' action of 'vCont' packet.
	(append_resumption): Send 'vCont;r' for range stepping and
	increment internalvar range_stepping_counter.
---
 gdb/remote.c |   47 ++++++++++++++++++++++++++++++++++++++++++++++-
 1 files changed, 46 insertions(+), 1 deletions(-)

diff --git a/gdb/remote.c b/gdb/remote.c
index f6a3f4c..a063698 100644
--- a/gdb/remote.c
+++ b/gdb/remote.c
@@ -256,6 +256,8 @@ struct support_v_cont
 {
   /* True if the stub reports support for vCont;t.  */
   int t;
+  /* True if the stub reports support for vCont;r.  */
+  int r;
 };
 
 /* Description of the remote protocol state for the currently
@@ -4649,6 +4651,7 @@ remote_vcont_probe (struct remote_state *rs)
       support_c = 0;
       support_C = 0;
       rs->support_vCont.t = 0;
+      rs->support_vCont.r = 0;
       while (p && *p == ';')
 	{
 	  p++;
@@ -4662,6 +4665,8 @@ remote_vcont_probe (struct remote_state *rs)
 	    support_C = 1;
 	  else if (*p == 't' && (*(p + 1) == ';' || *(p + 1) == 0))
 	    rs->support_vCont.t = 1;
+	  else if (*p == 'r' && (*(p + 1) == ';' || *(p + 1) == 0))
+	    rs->support_vCont.r = 1;
 
 	  p = strchr (p, ';');
 	}
@@ -4694,7 +4699,47 @@ append_resumption (char *p, char *endp,
   if (step && siggnal != GDB_SIGNAL_0)
     p += xsnprintf (p, endp - p, ";S%02x", siggnal);
   else if (step)
-    p += xsnprintf (p, endp - p, ";s");
+    {
+      struct thread_info *tp = NULL;
+      CORE_ADDR pc;
+
+      gdb_assert (!ptid_equal (ptid, minus_one_ptid));
+
+      if (ptid_is_pid (ptid))
+	tp = find_thread_ptid (inferior_ptid);
+      else
+	tp = find_thread_ptid (ptid);
+      gdb_assert (tp != NULL);
+
+      pc = regcache_read_pc (get_thread_regcache (ptid));
+      if (rs->support_vCont.r /* Target supports step range.  */
+	  /* Can't do range stepping for all threads of a process
+	     'pPID.-1'.  */
+	  && !(remote_multi_process_p (rs) && ptid_is_pid (ptid))
+	  /* Not step over a breakpoint.  */
+	  && !tp->control.trap_expected
+	  /* We have a range to single step.  */
+	  && THREAD_WITHIN_SINGLE_STEP_RANGE (tp, pc))
+	{
+	  int addr_size = gdbarch_addr_bit (target_gdbarch ()) / 8;
+	  struct internalvar *range_stepping_counter
+	    = lookup_internalvar ("range_stepping_counter");
+	  LONGEST result = 0;
+
+	  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));
+
+	  get_internalvar_integer (range_stepping_counter, &result);
+	  set_internalvar_integer (range_stepping_counter,
+				   result + 1);
+
+	}
+      else
+	p += xsnprintf (p, endp - p, ";s");
+    }
   else if (siggnal != GDB_SIGNAL_0)
     p += xsnprintf (p, endp - p, ";C%02x", siggnal);
   else
-- 
1.7.7.6

Re: [PATCH 4/7] range stepping: gdb

[Yao Qi]

On 04/11/2013 10:43 AM, Yao Qi wrote:
>   - Use phex_nz instead of hexnumstr to get the address.
.....

> +
> +	  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));

It is incorrect to copy address to buffer via phex_nz here and
somewhere else in remote.c.  We should use remote_address_masked first
to mask the CORE_ADDR and then call hexnumstr to copy address to
buffer.  Here is an updated one.

This patch teaches GDB to send 'vCont;r' packet when appropriate.
GDB has to check whether the target understand 'r' action of 'vCont'
packet.  We also make use of fields 'step_range_start' and
'step_range_end' of 'struct thread_control_state' to compose the range
of the stepping.

In V2, there are several changes:

 - Call remote_address_masked first before call hexnumstr to get the
address.
 - Define an internalvar range_stepping_counter and increment it each
time GDB does range-stepping.

-- 
Yao (齐尧)

gdb:

2013-04-10  Yao Qi  <yao@codesourcery.com>

	* remote.c (struct support_v_cont) <r>: New field.
	(remote_vcont_probe): Handle 'r' action of 'vCont' packet.
	(append_resumption): Send 'vCont;r' for range stepping and
	increment internalvar range_stepping_counter.
---
 gdb/remote.c |   49 ++++++++++++++++++++++++++++++++++++++++++++++++-
 1 files changed, 48 insertions(+), 1 deletions(-)

diff --git a/gdb/remote.c b/gdb/remote.c
index f6a3f4c..4158a09 100644
--- a/gdb/remote.c
+++ b/gdb/remote.c
@@ -256,6 +256,8 @@ struct support_v_cont
 {
   /* True if the stub reports support for vCont;t.  */
   int t;
+  /* True if the stub reports support for vCont;r.  */
+  int r;
 };
 
 /* Description of the remote protocol state for the currently
@@ -4649,6 +4651,7 @@ remote_vcont_probe (struct remote_state *rs)
       support_c = 0;
       support_C = 0;
       rs->support_vCont.t = 0;
+      rs->support_vCont.r = 0;
       while (p && *p == ';')
 	{
 	  p++;
@@ -4662,6 +4665,8 @@ remote_vcont_probe (struct remote_state *rs)
 	    support_C = 1;
 	  else if (*p == 't' && (*(p + 1) == ';' || *(p + 1) == 0))
 	    rs->support_vCont.t = 1;
+	  else if (*p == 'r' && (*(p + 1) == ';' || *(p + 1) == 0))
+	    rs->support_vCont.r = 1;
 
 	  p = strchr (p, ';');
 	}
@@ -4694,7 +4699,49 @@ append_resumption (char *p, char *endp,
   if (step && siggnal != GDB_SIGNAL_0)
     p += xsnprintf (p, endp - p, ";S%02x", siggnal);
   else if (step)
-    p += xsnprintf (p, endp - p, ";s");
+    {
+      struct thread_info *tp = NULL;
+      CORE_ADDR pc;
+
+      gdb_assert (!ptid_equal (ptid, minus_one_ptid));
+
+      if (ptid_is_pid (ptid))
+	tp = find_thread_ptid (inferior_ptid);
+      else
+	tp = find_thread_ptid (ptid);
+      gdb_assert (tp != NULL);
+
+      pc = regcache_read_pc (get_thread_regcache (ptid));
+      if (rs->support_vCont.r /* Target supports step range.  */
+	  /* Can't do range stepping for all threads of a process
+	     'pPID.-1'.  */
+	  && !(remote_multi_process_p (rs) && ptid_is_pid (ptid))
+	  /* Not step over a breakpoint.  */
+	  && !tp->control.trap_expected
+	  /* We have a range to single step.  */
+	  && THREAD_WITHIN_SINGLE_STEP_RANGE (tp, pc))
+	{
+	  struct internalvar *range_stepping_counter
+	    = lookup_internalvar ("range_stepping_counter");
+	  LONGEST result = 0;
+	  CORE_ADDR addr;
+
+	  p += xsnprintf (p, endp - p, ";r");
+
+	  addr = tp->control.step_range_start;
+	  p += hexnumstr (p, (ULONGEST) remote_address_masked (addr));
+	  p += xsnprintf (p, endp - p, ",");
+	  addr = tp->control.step_range_end;
+	  p += hexnumstr (p, (ULONGEST) remote_address_masked (addr));
+
+	  get_internalvar_integer (range_stepping_counter, &result);
+	  set_internalvar_integer (range_stepping_counter,
+				   result + 1);
+
+	}
+      else
+	p += xsnprintf (p, endp - p, ";s");
+    }
   else if (siggnal != GDB_SIGNAL_0)
     p += xsnprintf (p, endp - p, ";C%02x", siggnal);
   else
-- 
1.7.7.6

Re: [PATCH 4/7] range stepping: gdb

[Yao Qi]

On 04/11/2013 03:36 PM, Yao Qi wrote:
> It is incorrect to copy address to buffer via phex_nz here and
> somewhere else in remote.c.  We should use remote_address_masked first
> to mask the CORE_ADDR and then call hexnumstr to copy address to
> buffer.  Here is an updated one.

After a further thought, I decide to withdraw the updated patch 4/7, and 
keep using the original V2 patch 4/7.  The comments of variable 
'remote_address_size' say:

/* This variable sets the number of bits in an address that are to be
    sent in a memory ("M" or "m") packet.  Normally, after stripping
    leading zeros, the entire address would be sent.  This variable
    restricts the address to REMOTE_ADDRESS_SIZE bits.  HISTORY: The
    initial implementation of remote.c restricted the address sent in
    memory packets to ``host::sizeof long'' bytes - (typically 32
    bits).  Consequently, for 64 bit targets, the upper 32 bits of an
    address was never sent.  Since fixing this bug may cause a break in
    some remote targets this variable is principly provided to
    facilitate backward compatibility.  */

static unsigned int remote_address_size;

variable 'remote_address_size' has only effect on memory packet and 
breakpoint packet (it is not documented, but actually used).  Address in 
these packets should be masked by remote_address_masked, and address in 
new packets should not be, because the stub supports these new packets 
should be able to parse address correctly.

-- 
Yao (齐尧)

[PATCH 5/7] range stepping: New command 'maint set range stepping'

[Yao Qi]

On 03/18/2013 11:45 AM, Eli Zaretskii wrote:
>> Don't have any special reasons to do this.  The range stepping should be
>> >always on if target supports, and it can be turned off for some
>> >"maintenance purpose", so I set this command as a maintenance
>> >command.
> What would be the possible reasons to turn this feature off?
>
> Once we have the reasons, we could try thinking whether or not they
> are limited to GDB maintenance.

Range-stepping is useful to speed up some operations, so users don't
have to turn it off.  The reason turning it off could be that the
remote stub has a bug in supporting range stepping, so we have to
disable it in GDB side.

I considered to name the command to "set remote range-stepping", but
range-stepping is not specific to remote target, and it can be
implemented on native target as well.  So "set remote range-stepping"
is not a good choice.  I also considered "set range-stepping", but
unable to find a reasonable place for documentation on it.  In V2, I
don't update the command name.

gdb:

2013-04-11  Yao Qi  <yao@codesourcery.com>

	* remote.c (use_range_stepping): New.
	(remote_vcont_probe): Set 'use_range_stepping' if the remote
	target supports range stepping.
	(append_resumption): Use 'vCont;r' packet if 'use_range_stepping'
	is true.
	(maint_show_range_stepping): New.
	(maint_set_range_stepping): New.
	(_initialize_remote): Call add_setshow_boolean_cmd to register
	commands 'maint set range-stepping' and
	'maint show range-stepping'.

gdb/doc:

2013-04-11  Yao Qi  <yao@codesourcery.com>

	* gdb.texinfo (Maintenance Commands): Document commands
	'maint set range-stepping' and 'maint show range-stepping' and
	convenience variable '$range_stepping_counter'.
---
 gdb/doc/gdb.texinfo |   10 ++++++++
 gdb/remote.c        |   64 ++++++++++++++++++++++++++++++++++++++++++++++++++-
 2 files changed, 73 insertions(+), 1 deletions(-)

diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index bf7e25e..9292b94 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -35620,6 +35620,16 @@ data in a @file{gmon.out} file, be sure to move it to a safe location.
 Configuring with @samp{--enable-profiling} arranges for @value{GDBN} to be
 compiled with the @samp{-pg} compiler option.
 
+@kindex maint set range-stepping
+@kindex maint show range-stepping
+@cindex range-stepping
+@item maint set range-stepping
+@itemx maint show range-stepping
+@vindex $range_stepping_counter
+Control whether to do stepping in an address range.  The debugger convenience
+variable @samp{$range_stepping_counter} contains the number of range stepping
+@value{GDBN} has performed.
+
 @kindex maint set show-debug-regs
 @kindex maint show show-debug-regs
 @cindex hardware debug registers
diff --git a/gdb/remote.c b/gdb/remote.c
index a063698..7022275 100644
--- a/gdb/remote.c
+++ b/gdb/remote.c
@@ -260,6 +260,10 @@ struct support_v_cont
   int r;
 };
 
+/* A flag on GDB is willing to use range-stepping or not.  */
+
+static int use_range_stepping = 0;
+
 /* Description of the remote protocol state for the currently
    connected target.  This is per-target state, and independent of the
    selected architecture.  */
@@ -4652,6 +4656,7 @@ remote_vcont_probe (struct remote_state *rs)
       support_C = 0;
       rs->support_vCont.t = 0;
       rs->support_vCont.r = 0;
+      use_range_stepping = 0;
       while (p && *p == ';')
 	{
 	  p++;
@@ -4666,7 +4671,10 @@ remote_vcont_probe (struct remote_state *rs)
 	  else if (*p == 't' && (*(p + 1) == ';' || *(p + 1) == 0))
 	    rs->support_vCont.t = 1;
 	  else if (*p == 'r' && (*(p + 1) == ';' || *(p + 1) == 0))
-	    rs->support_vCont.r = 1;
+	    {
+	      rs->support_vCont.r = 1;
+	      use_range_stepping = 1;
+	    }
 
 	  p = strchr (p, ';');
 	}
@@ -4713,6 +4721,8 @@ append_resumption (char *p, char *endp,
 
       pc = regcache_read_pc (get_thread_regcache (ptid));
       if (rs->support_vCont.r /* Target supports step range.  */
+	  /* GDB is willing to do range stepping.  */
+	  && use_range_stepping
 	  /* Can't do range stepping for all threads of a process
 	     'pPID.-1'.  */
 	  && !(remote_multi_process_p (rs) && ptid_is_pid (ptid))
@@ -11697,6 +11707,45 @@ remote_upload_trace_state_variables (struct uploaded_tsv **utsvp)
   return 0;
 }
 
+static void
+maint_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 do range-stepping "
+		      "is %s.\n"), value);
+}
+
+static void
+maint_set_range_stepping (char *ignore_args, int from_tty,
+			  struct cmd_list_element *c)
+{
+  /* Check range stepping is supported when turns it on.  */
+  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_DISABLE
+	      || !rs->support_vCont.r)
+	    {
+	      use_range_stepping = 0;
+	      error (_("Range stepping is not supported"));
+	    }
+	}
+      else
+	{
+	  use_range_stepping = 0;
+	  error (_("Range stepping is not supported"));
+	}
+    }
+}
+
 void
 _initialize_remote (void)
 {
@@ -12094,6 +12143,19 @@ 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_maintenance,
+			   &use_range_stepping, _("\
+Enable or disable range-stepping."), _("\
+Show whether range-stepping is enabled."), _("\
+If On, GDB will tell the target to do stepping a range of address.\n\
+This will speed up stepping a line of source file.\n\
+If off, GDB will not use it, even if such is supported by the \n\
+target"),
+			   maint_set_range_stepping,
+			   maint_show_range_stepping,
+			   &maintenance_set_cmdlist,
+			   &maintenance_show_cmdlist);
+
   /* Eventually initialize fileio.  See fileio.c */
   initialize_remote_fileio (remote_set_cmdlist, remote_show_cmdlist);
 
-- 
1.7.7.6

Re: [PATCH 5/7] range stepping: New command 'maint set range stepping'

[Eli Zaretskii]

> From: Yao Qi <yao@codesourcery.com>
> Date: Thu, 11 Apr 2013 10:43:40 +0800
> 
> On 03/18/2013 11:45 AM, Eli Zaretskii wrote:
> >> Don't have any special reasons to do this.  The range stepping should be
> >> >always on if target supports, and it can be turned off for some
> >> >"maintenance purpose", so I set this command as a maintenance
> >> >command.
> > What would be the possible reasons to turn this feature off?
> >
> > Once we have the reasons, we could try thinking whether or not they
> > are limited to GDB maintenance.
> 
> Range-stepping is useful to speed up some operations, so users don't
> have to turn it off.  The reason turning it off could be that the
> remote stub has a bug in supporting range stepping, so we have to
> disable it in GDB side.

To me, this means that the command is not really a maintenance
command.  It is a user option, albeit one that is not expected to be
used frequently.

> I considered to name the command to "set remote range-stepping", but
> range-stepping is not specific to remote target, and it can be
> implemented on native target as well.  So "set remote range-stepping"
> is not a good choice.  I also considered "set range-stepping", but
> unable to find a reasonable place for documentation on it.

How about "Continuing and Stepping"?

> In V2, I don't update the command name.

> +@kindex maint set range-stepping
> +@kindex maint show range-stepping
> +@cindex range-stepping
> +@item maint set range-stepping
> +@itemx maint show range-stepping
> +@vindex $range_stepping_counter
> +Control whether to do stepping in an address range.  The debugger convenience
> +variable @samp{$range_stepping_counter} contains the number of range stepping
> +@value{GDBN} has performed.

This is OK, but it doesn't really explain what the setting does.  The
doc string of the command is much better:

> +  add_setshow_boolean_cmd ("range-stepping", class_maintenance,
> +			   &use_range_stepping, _("\
> +Enable or disable range-stepping."), _("\
> +Show whether range-stepping is enabled."), _("\
> +If On, GDB will tell the target to do stepping a range of address.\n\
> +This will speed up stepping a line of source file.\n\
> +If off, GDB will not use it, even if such is supported by the \n\
> +target"),

Except that "do stepping a range of address" is not really correct.
How about this rewording:

  If On, GDB will instruct the target to single-step through all the
  instructions in a range of addresses, and report back only once at
  the end.  This reduces the amount of communication during stepping.

[PATCH 6/7] range stepping: test case

[Yao Qi]

This test case is used to verify range stepping is used by means of
checking the internalvar '$range_stepping_counter'.

In V2, the test cases check the value of '$range_stepping_counter'
instead of parsing RSP packets.

gdb/testsuite:

2013-04-10  Yao Qi  <yao@codesourcery.com>

	* gdb.base/range-stepping.c: New.
	* gdb.base/range-stepping.exp: New.
	* gdb.trace/range-stepping.c: New.
	* gdb.trace/range-stepping.exp: New.
---
 gdb/testsuite/gdb.base/range-stepping.c    |   69 +++++++++
 gdb/testsuite/gdb.base/range-stepping.exp  |  222 ++++++++++++++++++++++++++++
 gdb/testsuite/gdb.trace/range-stepping.c   |   61 ++++++++
 gdb/testsuite/gdb.trace/range-stepping.exp |   89 +++++++++++
 4 files changed, 441 insertions(+), 0 deletions(-)
 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

diff --git a/gdb/testsuite/gdb.base/range-stepping.c b/gdb/testsuite/gdb.base/range-stepping.c
new file mode 100644
index 0000000..930cb8d
--- /dev/null
+++ b/gdb/testsuite/gdb.base/range-stepping.c
@@ -0,0 +1,69 @@
+/* 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/>.  */
+
+static int
+func1 (int a, int b)
+{
+  int r = a * b;
+
+  r =+ (a | b);
+  r =+ (a - b);
+
+  return r;
+}
+
+int
+main(void)
+{
+  int a = 0;
+  int b = 1;
+  int c = 2;
+  int d = 3;
+  int e = 4;
+  double d1 = 1.0;
+  double d2 = 2.0;
+
+  /* A line of source will be generated to a number of
+     instructions by compiler.  */
+  a = b + c + d * e - a; /* location 1 */
+
+  /* To compose multiple instructions before and after
+     'function call' or 'branch' instruction.  This line
+     of source will be generated to the following 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.  */
+  for (a = 0, e = 0; a < 15; a++) { e += a;}
+  /* Generate a range that includes a loop in it.  */
+  for (a = 0, e = 0; a < 15; a++) { e += a;}
+  /* 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 * a;}}
+  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..0f2a2dd
--- /dev/null
+++ b/gdb/testsuite/gdb.base/range-stepping.exp
@@ -0,0 +1,222 @@
+# 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/>.
+
+standard_testfile .c
+set executable $testfile
+
+if  { [gdb_compile "${srcdir}/${subdir}/${srcfile}" "${binfile}" \
+	   executable {debug nowarnings}] != "" } {
+     untested ${testfile}.exp
+     return -1
+}
+
+clean_restart $executable
+
+if ![runto_main] {
+    fail "Can't run to main"
+    return -1
+}
+
+# Check range stepping is supported by target or not.
+set test "range-stepping is supported"
+gdb_test_multiple "maintenance show range-stepping" "show range-stepping" {
+    -re "Debugger's willingness to do range-stepping is off.*\r\n$gdb_prompt $" {
+	unsupported $test
+	return -1
+    }
+    -re "Debugger's willingness to do range-stepping is on.*\r\n$gdb_prompt $" {
+	pass $test
+    }
+}
+
+# Check range stepping can step a range of multiple instructions.
+
+with_test_prefix "multi insns" {
+    global executable
+    global hex decimal
+    global gdb_prompt
+
+    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 execute command "next", GDB should send one vCont;s and
+    # vCont;r and receive two stop replies.
+    # --> vCont;s  (step over breakpoint)
+    # <-- T0505:XXX
+    # --> vCont;rSTART,END  (do range stepping)
+    # <-- T0505:XXX
+    gdb_test_no_output "set \$range_stepping_counter = 0" ""
+    gdb_test "next"
+    gdb_test "p \$range_stepping_counter" " = 1"
+
+    set pc_after_stepping ""
+    set msg "pc before 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
+	}
+    }
+
+    # At least, there are 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 range stepping can step over a function.
+
+with_test_prefix "step over func" {
+    global srcfile
+
+    set line_num [gdb_get_line_number "location 2"]
+    gdb_test "where" "main \\(\\) at .*${srcfile}:${line_num}.*"
+
+    # It is expected to get three stops and two 'vCont;r'.  In the C
+    # code, the line of C source produces the following instructions,
+    #
+    # addr1:
+    #  insn1;
+    #  insn2;
+    #  ...
+    #  call func1;
+    # addr2:
+    #  ...
+    #  insn3;
+    # addr3:
+    #  insn4;
+    #
+    # Something like this will happen:
+    # --> vCont;rADDR1,ADDR3 (do range stepping from ADDR1 to ADDR3)
+    # <-- T0505:XXXX
+    # (inferior is single-stepping to func, which is out of the range)
+    # --> $Z0,ADDR2
+    # --> vCont;c (set step-resume breakpoint on ADDR2, and resume)
+    # <-- T0505:XXXX  (inferior stops at ADD2)
+    # --> vCont;rADDR1,ADDR3  (continues to do range stepping)
+    # <-- T0505:XXXX
+    gdb_test_no_output "set \$range_stepping_counter = 0" ""
+    gdb_test "next"
+    gdb_test "p \$range_stepping_counter" " = 2"
+}
+
+# Check range stepping works well with breakpoint.
+
+with_test_prefix "breakpoint" {
+    gdb_breakpoint "func1"
+    # Something like this will happen:
+    # --> vCont;rADDR1,ADDR3
+    # <-- T0505:XXXX
+    # (inferior is single-stepping to func1, which is out of the range)
+    # --> $Z0,ADDR2
+    # --> vCont;c (set step-resume breakpoint on ADDR2, and resume)
+    # <-- T0505:XXXX  (inferior hits the breakpoint on fun1)
+    gdb_test_no_output "set \$range_stepping_counter = 0" ""
+    gdb_test "next" "." ""
+    gdb_test "p \$range_stepping_counter" " = 1" \
+       "range-stepping-counter is 1"
+
+    gdb_test "backtrace" "#0 .* func1 .*#1 .* main .*" \
+	"backtrace from func1"
+    # The range stepping was interrupted by a breakpoint, but the
+    # state related to range stepping should be cleared.
+    gdb_test_no_output "set \$range_stepping_counter = 0" ""
+    gdb_test "stepi"
+    gdb_test "p \$range_stepping_counter" " = 0" \
+       "range-stepping-counter is 0"
+    gdb_test "finish" ".*"
+
+    gdb_test "next" ".*"
+    delete_breakpoints
+}
+
+# Check range stepping works well with a loop in the range.
+
+with_test_prefix "loop" {
+
+    # GDB should send one vCont;r and receive one stop reply.
+    # --> vCont;rSTART,END  (do range stepping)
+    # <-- T0505:XXX
+    gdb_test_no_output "set \$range_stepping_counter = 0" ""
+    gdb_test "next"
+    gdb_test "p \$range_stepping_counter" " = 1"
+
+    # Check loop is done.
+    gdb_test "print a" "\\$${decimal} = 15\[\r\n\].*"
+    gdb_test "print e" "\\$${decimal} = 105\[\r\n\].*"
+}
+
+# Check range stepping works well when PC has already within the loop
+# body.
+
+with_test_prefix "loop" {
+    # Stepi into the loop body.  15 is large enough to make sure
+    # program goes to loop body.
+    gdb_test "stepi 15" ".*"
+    # GDB should send one vCont;r and receive one stop reply.
+    # --> vCont;rSTART,END  (do range stepping)
+    # <-- T0505:XXX
+    gdb_test_no_output "set \$range_stepping_counter = 0" ""
+    gdb_test "next"
+    gdb_test "p \$range_stepping_counter" " = 1"
+
+    # Check loop is done.
+    gdb_test "print a" "\\$${decimal} = 15\[\r\n\].*"
+    gdb_test "print e" "\\$${decimal} = 105\[\r\n\].*"
+}
+
+# Check range stepping works well when it is interrupted by ctrl-c.
+
+with_test_prefix "interrupt" {
+    gdb_test_no_output "set \$range_stepping_counter = 0" ""
+
+    send_gdb "next\n"
+    sleep 1
+    send_gdb "\003"
+
+    # GDB should send one vCont;r and receive one stop reply about
+    # SIGINT.
+    # --> vCont;rSTART,END  (do range stepping)
+    # <-- T0205:XXX
+
+    set test "send gdb ctrl-c"
+    gdb_expect {
+	-re "Program received signal SIGINT.*$gdb_prompt $" {
+	    pass $test
+	}
+	timeout { fail "${test} (timeout)" }
+	eof { fail "${test} (eof)" }
+    }
+    gdb_test "p \$range_stepping_counter" " = 1" \
+	"range-stepping-counter is 1"
+
+    # Terminate the loop earlier and continue to do range stepping.
+    gdb_test "set variable c = 0"
+    gdb_test "next"
+    gdb_test "p \$range_stepping_counter" " = 2" \
+	"range-stepping-counter is 2"
+}
+
+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..50db5a0
--- /dev/null
+++ b/gdb/testsuite/gdb.trace/range-stepping.c
@@ -0,0 +1,61 @@
+/* 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
+
+/* Called from asm.  */
+static void __attribute__((used))
+func2 (void)
+{}
+
+static int
+func1 (int a, int b)
+{
+  int r = a * b;
+
+  /* `set_point' is the label where we'll set tracepoint at.  The insn
+     at the label must the large enough to fit a fast tracepoint
+     jump.  */
+  asm ("    .global " SYMBOL(set_point) "\n"
+       SYMBOL(set_point) ":\n"
+#if (defined __x86_64__ || defined __i386__)
+       "    call " SYMBOL(func2) "\n"
+#endif
+       );
+
+  r =+ (a | b);
+  r =+ (a - b);
+
+  return r;
+}
+
+int
+main(void)
+{
+  int a = 0;
+  int b = 1;
+  int c = 2;
+  int d = 3;
+  int e = 4;
+
+  e = 10 + func1 (a + b, c * d); /* 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..22c705d
--- /dev/null
+++ b/gdb/testsuite/gdb.trace/range-stepping.exp
@@ -0,0 +1,89 @@
+# 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"
+
+standard_testfile
+set executable $testfile
+set expfile $testfile.exp
+
+if [prepare_for_testing $expfile $executable $srcfile \
+	{debug nowarnings}] {
+    untested "failed to prepare for trace tests"
+    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 range stepping works well with tracepoint.
+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"
+
+	# GDB will step over function func1, in which a tracepoint is
+	# set.  GDB will send two vCont;r packets because calling to
+	# func1 is out of the range.  However, tracepoint itself
+	# shouldn't have any effect on range stepping.
+	gdb_test_no_output "set \$range_stepping_counter = 0" ""
+	gdb_test "next"
+	gdb_test "p \$range_stepping_counter" " = 2"
+	gdb_test_no_output "tstop"
+	gdb_test "tfind" "Found trace frame 0.*" "tfind 0"
+	gdb_test "tfind" \
+	    "Target failed to find requested trace frame.*" \
+	    "tfind 1"
+
+	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"
+}
-- 
1.7.7.6

[PATCH 7/7] range stepping: doc and NEWS

[Yao Qi]

It was reviewed and approved by Eli here
<http://sourceware.org/ml/gdb-patches/2013-03/msg00475.html>

gdb/doc:

2013-04-10  Yao Qi  <yao@codesourcery.com>

	* gdb.texinfo (Packets): Document about 'vCont;r'.

gdb:

2013-04-10  Yao Qi  <yao@codesourcery.com>

	* NEWS: Mention range stepping, new packet and new
	commands.
---
 gdb/NEWS            |   12 ++++++++++++
 gdb/doc/gdb.texinfo |    5 +++++
 2 files changed, 17 insertions(+), 0 deletions(-)

diff --git a/gdb/NEWS b/gdb/NEWS
index 6f202e2..0368c10 100644
--- a/gdb/NEWS
+++ b/gdb/NEWS
@@ -10,6 +10,10 @@ maint set|show per-command time
 maint set|show per-command symtab
   Enable display of per-command gdb resource usage.
 
+maint set range-stepping
+maint show range-stepping
+  Control and show whether to do range stepping.
+
 * New options
 
 set remote trace-status-packet
@@ -240,11 +244,19 @@ show debug notification
   feature to be enabled.  For more information, see:
       http://fedoraproject.org/wiki/Features/MiniDebugInfo
 
+* GDB now supports range stepping, which improves the performance of
+  single stepping over a source line by reducing the number of control
+  packets from GDB.
+
 * New remote packets
 
 QTBuffer:size
    Set the size of trace buffer.  The remote stub reports support for this
    packet to gdb's qSupported query.
+vCont;r
+  Tell the remote stub to do range stepping in an address range.  The remote
+  stub reports a stop reply when the program goes out of the range or is
+  stopped due to other reasons, such as hitting a breakpoint.
 
 Qbtrace:bts
   Enable Branch Trace Store (BTS)-based branch tracing for the current
diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index 9292b94..16059dc 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -36375,6 +36375,11 @@ 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 PC is within the range [@var{start},
+@var{end}).  Note that a stop reply may be sent at any point even if
+the PC is within the stepping range; for example, it is permissible to
+implement this packet in a degenerate way as a single step operation.
 @end table
 
 The optional argument @var{addr} normally associated with the 
-- 
1.7.7.6

Re: [PATCH 7/7] range stepping: doc and NEWS

[Eli Zaretskii]

> From: Yao Qi <yao@codesourcery.com>
> Date: Thu, 11 Apr 2013 10:43:42 +0800
> 
> 2013-04-10  Yao Qi  <yao@codesourcery.com>
> 
> 	* NEWS: Mention range stepping, new packet and new
> 	commands.

OK for this part.

> diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
> index 9292b94..16059dc 100644
> --- a/gdb/doc/gdb.texinfo
> +++ b/gdb/doc/gdb.texinfo
> @@ -36375,6 +36375,11 @@ 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 PC is within the range [@var{start},
> +@var{end}).  Note that a stop reply may be sent at any point even if
> +the PC is within the stepping range; for example, it is permissible to
> +implement this packet in a degenerate way as a single step operation.
>  @end table

This is also OK.

Thanks.

Re: [PATCH 0/7 V2] Range stepping

[Pedro Alves]

On 04/11/2013 03:43 AM, Yao Qi wrote:

> This is the V2 of range stepping series.  V1 can be found here
> <http://sourceware.org/ml/gdb-patches/2013-03/msg00450.html>.
> 
> There are some updates in V2:
> 
>  - Rebase patches on GDB trunk, and resolve conflicts.
>  - Use phex_nz instead of hexnumstr to get the address.  It fixes a
> bug on mips target when sending address to the remote target.
>  - Define an internalvar range_stepping_counter and increment it each
> time GDB does range-stepping, so that in the test, we don't have to
> turn 'remote debug' on and parse the RSP packets.  During the test,
> we find that it is unsafe to turn the debugging output on, because it
> may blow up the gdb.log.
> 
> Patch 1,2,3,7 are unchanged.
> 
> The whole series is tested on x86_64-linux, native and gdbserver.

Thanks!  As I'm no longer much distracted with 7.6 things or that
dreaded -Wpointer-sign thing, I've resumed review/hacking on this.
I really really want to get this over with, but please bear with
me.  :-)

-- 
Pedro Alves