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

[Pedro Alves <palves@redhat.com>]

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