From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 28394 invoked by alias); 8 May 2013 16:18:52 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Received: (qmail 28366 invoked by uid 89); 8 May 2013 16:18:48 -0000 X-Spam-SWARE-Status: No, score=-4.7 required=5.0 tests=AWL,BAYES_00,KHOP_RCVD_UNTRUST,KHOP_THREADED,RCVD_IN_HOSTKARMA_W,RCVD_IN_HOSTKARMA_WL autolearn=ham version=3.3.1 Received: from relay1.mentorg.com (HELO relay1.mentorg.com) (192.94.38.131) by sourceware.org (qpsmtpd/0.84/v0.84-167-ge50287c) with ESMTP; Wed, 08 May 2013 16:18:46 +0000 Received: from svr-orw-exc-10.mgc.mentorg.com ([147.34.98.58]) by relay1.mentorg.com with esmtp id 1Ua74l-0007Rz-EB from Hafiz_Abid@mentor.com ; Wed, 08 May 2013 09:18:43 -0700 Received: from SVR-IES-FEM-02.mgc.mentorg.com ([137.202.0.106]) by SVR-ORW-EXC-10.mgc.mentorg.com with Microsoft SMTPSVC(6.0.3790.4675); Wed, 8 May 2013 09:18:43 -0700 Received: from abidh-ubunto1104 (137.202.0.76) by SVR-IES-FEM-02.mgc.mentorg.com (137.202.0.106) with Microsoft SMTP Server (TLS) id 14.2.247.3; Wed, 8 May 2013 17:18:41 +0100 Date: Wed, 08 May 2013 16:18:00 -0000 From: "Abid, Hafiz" Subject: Re: [patch] circ.exp To: Pedro Alves CC: References: <1368007885.2238.4@abidh-ubunto1104> <518A6B97.6070402@redhat.com> In-Reply-To: <518A6B97.6070402@redhat.com> (from palves@redhat.com on Wed May 8 16:13:27 2013) Message-ID: <1368029920.2238.9@abidh-ubunto1104> MIME-Version: 1.0 Content-Type: multipart/mixed; boundary="=-5xKW635/UoWMN1nJcqnn" X-Virus-Found: No X-SW-Source: 2013-05/txt/msg00283.txt.bz2 --=-5xKW635/UoWMN1nJcqnn Content-Type: text/plain; charset="us-ascii"; delsp=Yes; format=Flowed Content-Disposition: inline Content-Transfer-Encoding: quoted-printable Content-length: 3749 > 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.=20=20 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: >=20 > > I have used similar code sequence as you described above. But I was=20= =20 > wondering if we can > > issue an unsupported call in the 2nd '-re' and then return. That=20=20 > should eliminate the need > > for 'circular_supported'. It also generates an extra pass while we=20=20 > then call unsupported below. >=20 > It's really not a biggie either way. This, >=20 > PASS: check whether circular buffer is supported > FAIL: check whether circular buffer is supported > UNSUPPORTED: check whether circular buffer is supported >=20 > 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: >=20 > PASS: target buffer is set to circular > FAIL: target buffer is set to circular > UNSUPPORTED: target buffer is set to circular >=20 > Here FAIL would be an unexpected outcome (caught by gdb_test_multiple > internally, so adorned with "(timeout)" or something else). >=20 > Note a separate a variable is still somewhat useful to bail in the=20=20 > case > gdb_test_multiple catches a FAILs internally (timeout, internal=20=20 > error). > We can also check those in the return of gdb_test_multiple, but a > separate variable usually reads nicer, IMO. >=20 > > -# 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. >=20 > I'd find extending the comment with something like: >=20 > ", because we use 'tfind pc func' to find the corresponding trace=20=20 > frame > afterwards, and that looks for entry address." >=20 > to be useful. >=20 > > +# Check if changing the trace buffer size is supported. This step=20= =20 > is > > +# repeated twice. This helps in case if trace buffer size is 100. >=20 > s/This helps in case if/The helps in case the/ >=20 > > + # Check that we get the total_size and free_size >=20 > Missing period. >=20 > > + 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 >=20 > Missing period. Might as well just give it a quick audit > over all strings and add the missing periods. :-) >=20 > > +# 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. >=20 > s/that frame for/that the frame for/ > s/but frame for/but the the frame for/ >=20 > > +# 1) the first frame will be overwritten and therefore unavailable >=20 > Missing period here too. >=20 > > + # Check that teh first frame is actually at func0. >=20 > typo: "teh". >=20 > The patch is okay with these fixed. Go ahead and apply it > (but still post the final version). >=20 > Thanks, > -- > Pedro Alves >=20 >=20 --=-5xKW635/UoWMN1nJcqnn Content-Type: text/x-patch; charset="us-ascii"; name="circ_v5.patch" Content-Disposition: attachment; filename="circ_v5.patch" Content-Transfer-Encoding: quoted-printable Content-length: 15622 ? src/gdb/.new.ChangeLog Index: gdb/testsuite/ChangeLog =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D 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 + + * 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 =20 * lib/selftest-support.exp: New file. Index: gdb/testsuite/gdb.trace/circ.exp =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D 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 @@ =20 load_lib "trace-support.exp" =20 - standard_testfile =20 if {[prepare_for_testing $testfile.exp $testfile $srcfile {debug nowarning= s}]} { @@ -23,194 +22,266 @@ } =20 # 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=20 -# several frames are collected, but the first several are not. +# 3) Artificially limit the trace buffer to 4x + a bytes. Here x is the s= ize +# of single trace frame and a is a small constant. Rerun the +# experiment. Confirm that the frame for the first tracepoint is colle= cted, +# 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. # =20 -# return 0 for success, 1 for failure -proc run_trace_experiment { pass } { - gdb_run_cmd=20 - - 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=20 +# '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" "^$" } =20 -# 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 } =20 -# 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 } +} =20 - 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 + } +} =20 - 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.*$gd= b_prompt $" { + set total_size $expect_out(2,string) + if { $test_size !=3D $total_size } { + unsupported "target does not support changing trace buffer size" + return 1 + } + pass $test + } } =20 -# return 0 for success, 1 for failure -proc gdb_trace_circular_tests { } { - if { ![gdb_target_supports_trace] } then {=20 - 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.*$gd= b_prompt $" { + set total_size $expect_out(2,string) + if { $test_size !=3D $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)" =20 - if [trace_buffer_normal] then { return 1 } +set total_size -1 +set free_size -1 +set frame_size -1 =20 - 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 =20 - if [setup_tracepoints] then { return 1 } + gdb_test "break end" "Breakpoint $decimal.*" "breakpoint at end" =20 - # 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" =20 - 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" =20 - if [gdb_test "tfind none" \ - "#0 end .*" \ - "quit trace debugging, pass 1"] then { return 1 } + gdb_test "tstop" "\[\r\n\]*" "stop trace" =20 - # 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 {=20 + set test "get buffer size" + + gdb_test_multiple "tstatus" $test { + -re ".*Trace buffer has ($decimal) bytes of ($decimal) bytes free.*$gdb_p= rompt $" { + 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 } =20 - if [gdb_test "tfind 9" \ - ".* failed to find .*" \ - "fail to find frame nine, pass 2"] then { return 1 } + if { $free_size < 0 } { + return 1 + } +} =20 - 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)" =20 - # 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 {=20 +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 } =20 - if [gdb_test "tfind none" \ - "#0 end .*" \ - "quit trace debugging, pass 3"] then { return 1 } + run_trace_experiment =20 - 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" } =20 -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 + } =20 -gdb_test "show circular-trace-buffer" "Target's use of circular trace buff= er is on." "show circular-trace-buffer (on)" + gdb_test_no_output "set trace-buffer-size $buffer_size" \ + "shrink the target trace buffer" =20 -gdb_test_no_output "set circular-trace-buffer off" \ - "set circular-trace-buffer off" + run_trace_experiment =20 -gdb_test "show circular-trace-buffer" "Target's use of circular trace buff= er is off." "show circular-trace-buffer (off)" + gdb_test "tfind start" ".*#0 func0 .*" \ + "first frame is at func0" =20 -# 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" } =20 -# 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" +} --=-5xKW635/UoWMN1nJcqnn--