Re: [patch] circ.exp

Abid, Hafiz
2013-05-08T16:18:52Z

[-- Attachment #1: Type: text/plain, Size: 3774 bytes --]

> The patch is okay with these fixed. Go ahead and apply it
> (but still post the final version).
Thanks Pedro. I have made the suggested changes and applied the patch.
I am attacing the version that I committed.

Regards,
Abid

On 08/05/13 16:13:27, Pedro Alves wrote:
> On 05/08/2013 11:11 AM, Abid, Hafiz wrote:
>
> > I have used similar code sequence as you described above. But I was
> wondering if we can
> > issue an unsupported call in the 2nd '-re' and then return. That
> should eliminate the need
> > for 'circular_supported'. It also generates an extra pass while we
> then call unsupported below.
>
> It's really not a biggie either way. This,
>
> PASS: check whether circular buffer is supported
> FAIL: check whether circular buffer is supported
> UNSUPPORTED: check whether circular buffer is supported
>
> reads to me as if it's the checking itself that is not supported.
> (Very) Pedantically, that 2nd -re means the "check whether circular
> buffer is supported" test succeeded. And from that successful
> test, we can infer that the target in fact does not support a
> circular buffer. Contrast with an alternative message
> that focuses on what we want to outcome to be, and it'd
> make a little more sense to me to issue the unsupported
> in the 2nd -re:
>
> PASS: target buffer is set to circular
> FAIL: target buffer is set to circular
> UNSUPPORTED: target buffer is set to circular
>
> Here FAIL would be an unexpected outcome (caught by gdb_test_multiple
> internally, so adorned with "(timeout)" or something else).
>
> Note a separate a variable is still somewhat useful to bail in the
> case
> gdb_test_multiple catches a FAILs internally (timeout, internal
> error).
> We can also check those in the return of gdb_test_multiple, but a
> separate variable usually reads nicer, IMO.
>
> > -# return 0 for success, 1 for failure
> > -proc run_trace_experiment { pass } {
> > - gdb_run_cmd
> > +# Set a tracepoint on given func. The tracepoint is set at entry
> > +# address and not 'after prologue' address.
>
> I'd find extending the comment with something like:
>
> ", because we use 'tfind pc func' to find the corresponding trace
> frame
> afterwards, and that looks for entry address."
>
> to be useful.
>
> > +# Check if changing the trace buffer size is supported. This step
> is
> > +# repeated twice. This helps in case if trace buffer size is 100.
>
> s/This helps in case if/The helps in case the/
>
> > + # Check that we get the total_size and free_size
>
> Missing period.
>
> > + if { $total_size < 0 } {
> > return 1
> > }
> >
> > - return 0
> > + if { $free_size < 0 } {
> > + return 1
> > + }
> > }
> >
> > -# return 0 for success, 1 for failure
> > -proc gdb_trace_circular_tests { } {
> > - if { ![gdb_target_supports_trace] } then {
> > - unsupported "Current target does not support trace"
> > +# Calculate the size of a single frame
>
> Missing period. Might as well just give it a quick audit
> over all strings and add the missing periods. :-)
>
> > +# Shrink the trace buffer so that it will not hold
> > +# all ten trace frames. Verify that frame for func0 is still
> > +# collected, but frame for func9 is not.
>
> s/that frame for/that the frame for/
> s/but frame for/but the the frame for/
>
> > +# 1) the first frame will be overwritten and therefore unavailable
>
> Missing period here too.
>
> > + # Check that teh first frame is actually at func0.
>
> typo: "teh".
>
> The patch is okay with these fixed. Go ahead and apply it
> (but still post the final version).
>
> Thanks,
> --
> Pedro Alves
>
>

[-- Attachment #2: circ_v5.patch --]
[-- Type: text/x-patch, Size: 15707 bytes --]

? src/gdb/.new.ChangeLog
Index: gdb/testsuite/ChangeLog
===================================================================
RCS file: /cvs/src/src/gdb/testsuite/ChangeLog,v
retrieving revision 1.3651
diff -u -r1.3651 ChangeLog
--- gdb/testsuite/ChangeLog 7 May 2013 18:06:16 -0000 1.3651
+++ gdb/testsuite/ChangeLog 8 May 2013 16:12:04 -0000
@@ -1,3 +1,18 @@
+2013-05-08 Hafiz Abid Qadeer <abidh@codesourcery.com>
+
+ * gdb.trace/circ.exp: Remove unnecessary 'if then' checks.
+ (set_a_tracepoint): Set tracepoint before prologue.
+ (run_trace_experiment): Test setup_tracepoints and 'break end'
+ in it.
+ (trace_buffer_normal): Remove.
+ (gdb_trace_circular_tests): Remove. Move tests to...
+ (top level): ...here. Call 'runto_main' before checking for
+ trace support. Use commands to check the support for circular
+ trace buffer and changing of trace buffer size. Add test
+ to calculate size of single frame. Use this size to
+ calculate the size of trace buffer. Use 'tfind pc func9'
+ instead of 'tfind 9'. Use 'with_test_prefix'.
+
2013-05-07 Tom Tromey <tromey@redhat.com>

* lib/selftest-support.exp: New file.
Index: gdb/testsuite/gdb.trace/circ.exp
===================================================================
RCS file: /cvs/src/src/gdb/testsuite/gdb.trace/circ.exp,v
retrieving revision 1.25
diff -u -r1.25 circ.exp
--- gdb/testsuite/gdb.trace/circ.exp 14 Mar 2013 13:34:06 -0000 1.25
+++ gdb/testsuite/gdb.trace/circ.exp 8 May 2013 16:12:05 -0000
@@ -15,7 +15,6 @@

load_lib "trace-support.exp"

-
standard_testfile

if {[prepare_for_testing $testfile.exp $testfile $srcfile {debug nowarnings}]} {
@@ -23,194 +22,266 @@
}

# Tests:
-# 1) Set up a trace experiment that will collect approximately 10 frames,
+# 1) Calculate the size taken by one trace frame.
+# 2) Set up a trace experiment that will collect approximately 10 frames,
# requiring more than 512 but less than 1024 bytes of cache buffer.
# (most targets should have at least 1024 bytes of cache buffer!)
# Run and confirm that it collects all 10 frames.
-# 2) Artificially limit the trace buffer to 512 bytes, and rerun the
-# experiment. Confirm that the first several frames are collected,
-# but that the last several are not.
-# 3) Set trace buffer to circular mode, still with the artificial limit
-# of 512 bytes, and rerun the experiment. Confirm that the last
-# several frames are collected, but the first several are not.
+# 3) Artificially limit the trace buffer to 4x + a bytes. Here x is the size
+# of single trace frame and a is a small constant. Rerun the
+# experiment. Confirm that the frame for the first tracepoint is collected,
+# but frames for the last several tracepoints are not.
+# 4) Set trace buffer to circular mode, with the buffer size as in
+# step 3 above. Rerun the experiment. Confirm that the frame for the last
+# tracepoint is collected but not for the first one.
#

-# return 0 for success, 1 for failure
-proc run_trace_experiment { pass } {
- gdb_run_cmd
-
- if [gdb_test "tstart" \
- "\[\r\n\]*" \
- "start trace experiment, pass $pass"] then { return 1 }
- if [gdb_test "continue" \
- "Continuing.*Breakpoint \[0-9\]+, end.*" \
- "run to end, pass $pass"] then { return 1 }
- if [gdb_test "tstop" \
- "\[\r\n\]*" \
- "stop trace experiment, pass $pass"] then { return 1 }
- return 0
+# Set a tracepoint on given func. The tracepoint is set at entry
+# address and not 'after prologue' address because we use
+# 'tfind pc func' to find the corresponding trace frame afterwards,
+# and that looks for entry address.
+proc set_a_tracepoint { func } {
+ gdb_test "trace \*$func" "Tracepoint \[0-9\]+ at .*" \
+ "set tracepoint at $func"
+ gdb_trace_setactions "set actions for $func" "" "collect testload" "^$"
}

-# return 0 for success, 1 for failure
-proc set_a_tracepoint { func } {
- if [gdb_test "trace $func" \
- "Tracepoint \[0-9\]+ at .*" \
- "set tracepoint at $func"] then {
+# Sets the tracepoints from func0 to func9 using set_a_tracepoint.
+proc setup_tracepoints { } {
+ gdb_delete_tracepoints
+ set_a_tracepoint func0
+ set_a_tracepoint func1
+ set_a_tracepoint func2
+ set_a_tracepoint func3
+ set_a_tracepoint func4
+ set_a_tracepoint func5
+ set_a_tracepoint func6
+ set_a_tracepoint func7
+ set_a_tracepoint func8
+ set_a_tracepoint func9
+}
+
+# Start the trace, run to end and then stop the trace.
+proc run_trace_experiment { } {
+ global decimal
+
+ setup_tracepoints
+ gdb_test "break end" "Breakpoint $decimal.*" "breakpoint at end"
+ gdb_test "tstart" "\[\r\n\]*" "start trace experiment"
+ gdb_test "continue" "Continuing.*Breakpoint \[0-9\]+, end.*" \
+ "run to end"
+ gdb_test "tstop" "\[\r\n\]*" "stop trace experiment"
+}
+
+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
+}
+
+set test "set circular-trace-buffer on"
+gdb_test_multiple "set circular-trace-buffer on" $test {
+ -re ".*Target does not support this command.*$gdb_prompt $" {
+ unsupported "target does not support circular trace buffer"
return 1
}
- if [gdb_trace_setactions "set actions for $func" \
- "" \
- "collect testload" "^$"] then {
- return 1
+ -re "$gdb_prompt $" {
+ pass $test
}
- return 0
}

-# return 0 for success, 1 for failure
-proc setup_tracepoints { } {
- gdb_delete_tracepoints
- if [set_a_tracepoint func0] then { return 1 }
- if [set_a_tracepoint func1] then { return 1 }
- if [set_a_tracepoint func2] then { return 1 }
- if [set_a_tracepoint func3] then { return 1 }
- if [set_a_tracepoint func4] then { return 1 }
- if [set_a_tracepoint func5] then { return 1 }
- if [set_a_tracepoint func6] then { return 1 }
- if [set_a_tracepoint func7] then { return 1 }
- if [set_a_tracepoint func8] then { return 1 }
- if [set_a_tracepoint func9] then { return 1 }
- return 0
-}
-
-# return 0 for success, 1 for failure
-proc trace_buffer_normal { } {
- global gdb_prompt
-
- set ok 0
- set test "maint packet QTBuffer:size:ffffffff"
- gdb_test_multiple $test $test {
- -re "received: .OK.\r\n$gdb_prompt $" {
- set ok 1
- pass $test
- }
- -re "\r\n$gdb_prompt $" {
- }
+set circular_supported -1
+set test "check whether circular buffer is supported"
+
+gdb_test_multiple "tstatus" $test {
+ -re ".*Trace buffer is circular.*$gdb_prompt $" {
+ set circular_supported 1
+ pass $test
}
- if { !$ok } {
- unsupported $test
- return 1
+ -re "$gdb_prompt $" {
+ pass $test
}
+}

- set ok 0
- set test "maint packet QTBuffer:circular:0"
- gdb_test_multiple $test $test {
- -re "received: .OK.\r\n$gdb_prompt $" {
- set ok 1
- pass $test
- }
- -re "\r\n$gdb_prompt $" {
- }
- }
- if { !$ok } {
- unsupported $test
+if { $circular_supported < 0 } {
+ unsupported "target does not support circular trace buffer"
+ return 1
+}
+
+gdb_test "show circular-trace-buffer" \
+ "Target's use of circular trace buffer is on." \
+ "show circular-trace-buffer (on)"
+
+# Check if changing the trace buffer size is supported. This step is
+# repeated twice. This helps in case the trace buffer size is 100.
+set test_size 100
+set test "change buffer size to $test_size"
+gdb_test_multiple "set trace-buffer-size $test_size" $test {
+ -re ".*Target does not support this command.*$gdb_prompt $" {
+ unsupported "target does not support changing trace buffer size"
return 1
}
+ -re "$gdb_prompt $" {
+ pass $test
+ }
+}

- return 0
+set test "check whether setting trace buffer size is supported"
+gdb_test_multiple "tstatus" $test {
+ -re ".*Trace buffer has ($decimal) bytes of ($decimal) bytes free.*$gdb_prompt $" {
+ set total_size $expect_out(2,string)
+ if { $test_size != $total_size } {
+ unsupported "target does not support changing trace buffer size"
+ return 1
+ }
+ pass $test
+ }
}

-# return 0 for success, 1 for failure
-proc gdb_trace_circular_tests { } {
- if { ![gdb_target_supports_trace] } then {
- unsupported "Current target does not support trace"
- return 1
+set test_size 400
+gdb_test_no_output "set trace-buffer-size $test_size" \
+ "change buffer size to $test_size"
+
+gdb_test_multiple "tstatus" $test {
+ -re ".*Trace buffer has ($decimal) bytes of ($decimal) bytes free.*$gdb_prompt $" {
+ set total_size $expect_out(2,string)
+ if { $test_size != $total_size } {
+ unsupported "target does not support changing trace buffer size"
+ return 1
+ }
+ pass $test
}
+}
+
+gdb_test_no_output "set circular-trace-buffer off" \
+ "set circular-trace-buffer off"
+
+gdb_test "show circular-trace-buffer" \
+ "Target's use of circular trace buffer is off." \
+ "show circular-trace-buffer (off)"

- if [trace_buffer_normal] then { return 1 }
+set total_size -1
+set free_size -1
+set frame_size -1

- gdb_test "break begin" ".*" ""
- gdb_test "break end" ".*" ""
- gdb_test "tstop" ".*" ""
- gdb_test "tfind none" ".*" ""
+# Determine the size used by a single frame. Set a single tracepoint,
+# run and then check the total and free size using the tstatus command.
+# Then subtracting free from total gives us the size of a frame.
+with_test_prefix "frame size" {
+ set_a_tracepoint func0

- if [setup_tracepoints] then { return 1 }
+ gdb_test "break end" "Breakpoint $decimal.*" "breakpoint at end"

- # First, run the trace experiment with default attributes:
- # Make sure it behaves as expected.
- if [run_trace_experiment 1] then { return 1 }
- if [gdb_test "tfind start" \
- "#0 func0 .*" \
- "find frame zero, pass 1"] then { return 1 }
+ gdb_test "tstart" "\[\r\n\]*" "start trace"

- if [gdb_test "tfind 9" \
- "#0 func9 .*" \
- "find frame nine, pass 1"] then { return 1 }
+ gdb_test "continue" "Continuing.*Breakpoint \[0-9\]+, end.*" \
+ "run to end"

- if [gdb_test "tfind none" \
- "#0 end .*" \
- "quit trace debugging, pass 1"] then { return 1 }
+ gdb_test "tstop" "\[\r\n\]*" "stop trace"

- # Then, shrink the trace buffer so that it will not hold
- # all ten trace frames. Verify that frame zero is still
- # collected, but frame nine is not.
- if [gdb_test "maint packet QTBuffer:size:200" \
- "received: .OK." "shrink the target trace buffer"] then {
+ set test "get buffer size"
+
+ gdb_test_multiple "tstatus" $test {
+ -re ".*Trace buffer has ($decimal) bytes of ($decimal) bytes free.*$gdb_prompt $" {
+ set free_size $expect_out(1,string)
+ set total_size $expect_out(2,string)
+ pass $test
+ }
+ }
+
+ # Check that we get the total_size and free_size.
+ if { $total_size < 0 } {
return 1
}
- if [run_trace_experiment 2] then { return 1 }
- if [gdb_test "tfind start" \
- "#0 func0 .*" \
- "find frame zero, pass 2"] then { return 1 }

- if [gdb_test "tfind 9" \
- ".* failed to find .*" \
- "fail to find frame nine, pass 2"] then { return 1 }
+ if { $free_size < 0 } {
+ return 1
+ }
+}

- if [gdb_test "tfind none" \
- "#0 end .*" \
- "quit trace debugging, pass 2"] then { return 1 }
+# Calculate the size of a single frame.
+set frame_size "($total_size - $free_size)"

- # Finally, make the buffer circular. Now when it runs out of
- # space, it should wrap around and overwrite the earliest frames.
- # This means that:
- # 1) frame zero will be overwritten and therefore unavailable
- # 2) the earliest frame in the buffer will be other-than-zero
- # 3) frame nine will be available (unlike on pass 2).
- if [gdb_test "maint packet QTBuffer:circular:1" \
- "received: .OK." "make the target trace buffer circular"] then {
+with_test_prefix "normal buffer" {
+ clean_restart $testfile
+
+ if { ![runto_main] } {
+ fail "can't run to main"
return 1
}
- if [run_trace_experiment 3] then { return 1 }
- if [gdb_test "tfind start" \
- "#0 func\[1-9\] .*" \
- "first frame is NOT frame zero, pass 3"] then { return 1 }
-
- if [gdb_test "tfind 9" \
- "#0 func9 .*" \
- "find frame nine, pass 3"] then { return 1 }

- if [gdb_test "tfind none" \
- "#0 end .*" \
- "quit trace debugging, pass 3"] then { return 1 }
+ run_trace_experiment

- return 0
+ # Check that the first frame is actually at func0.
+ gdb_test "tfind start" ".*#0 func0 .*" \
+ "first frame is at func0"
+
+ gdb_test "tfind pc func9" \
+ ".*Found trace frame $decimal, tracepoint $decimal.*" \
+ "find frame for func9"
}

-gdb_test_no_output "set circular-trace-buffer on" \
- "set circular-trace-buffer on"
+# Shrink the trace buffer so that it will not hold
+# all ten trace frames. Verify that the frame for func0 is still
+# collected, but the frame for func9 is not.
+
+set buffer_size "((4 * $frame_size) + 10)"
+with_test_prefix "small buffer" {
+ clean_restart $testfile
+
+ if { ![runto_main] } {
+ fail "can't run to main"
+ return 1
+ }

-gdb_test "show circular-trace-buffer" "Target's use of circular trace buffer is on." "show circular-trace-buffer (on)"
+ gdb_test_no_output "set trace-buffer-size $buffer_size" \
+ "shrink the target trace buffer"

-gdb_test_no_output "set circular-trace-buffer off" \
- "set circular-trace-buffer off"
+ run_trace_experiment

-gdb_test "show circular-trace-buffer" "Target's use of circular trace buffer is off." "show circular-trace-buffer (off)"
+ gdb_test "tfind start" ".*#0 func0 .*" \
+ "first frame is at func0"

-# Body of test encased in a proc so we can return prematurely.
-if { ![gdb_trace_circular_tests] } then {
- # Set trace buffer attributes back to normal
- trace_buffer_normal;
+ gdb_test "tfind pc func9" ".* failed to find .*" \
+ "find frame for func9"
}

-# Finished!
-gdb_test "tfind none" ".*" ""
+# Finally, make the buffer circular. Now when it runs out of
+# space, it should wrap around and overwrite the earliest frames.
+# This means that:
+# 1) the first frame will be overwritten and therefore unavailable.
+# 2) the earliest frame in the buffer will not be for func0.
+# 3) the frame for func9 will be available (unlike "small buffer" case).
+with_test_prefix "circular buffer" {
+ clean_restart $testfile
+
+ if { ![runto_main] } {
+ fail "can't run to main"
+ return 1
+ }
+
+ gdb_test_no_output "set trace-buffer-size $buffer_size" \
+ "shrink the target trace buffer"
+
+ gdb_test_no_output "set circular-trace-buffer on" \
+ "make the target trace buffer circular"
+
+ run_trace_experiment
+
+ gdb_test "tstatus" \
+ ".*Buffer contains $decimal trace frames \\(of $decimal created total\\).*Trace buffer is circular.*" \
+ "trace buffer is circular"
+
+ # The first frame should not be at func0.
+ gdb_test "tfind start" ".*#0 func\[1-9\] .*" \
+ "first frame is NOT at func0"
+
+ gdb_test \
+ "tfind pc func9" \
+ ".*Found trace frame $decimal, tracepoint $decimal.*" \
+ "find frame for func9"
+}