From: Pedro Alves Date: Thu, 4 Apr 2013 19:22:38 +0000 (+0000) Subject: tracepoint->step_count fixes X-Git-Url: https://git.libre-soc.org/?a=commitdiff_plain;h=c9a6ce02b5c29a24f555af97362bff2198b36ef0;p=binutils-gdb.git tracepoint->step_count fixes If a tracepoint's actions list includes a while-stepping action, and then the actions are changed to a list without any while-stepping action, the tracepoint's step_count will be left with a stale value. For example: (gdb) trace subr Tracepoint 1 at 0x4004d9: file ../../../src/gdb/testsuite//actions-changed.c, line 31. (gdb) actions Enter actions for tracepoint 1, one per line. End with a line saying just "end". >collect $reg >end (gdb) set debug remote 1 (gdb) tstart Sending packet: $QTinit#59...Packet received: OK Sending packet: $QTDP:1:00000000004004d9:E:0:0-#a3...Packet received: OK Sending packet: $QTDP:-1:00000000004004d9:R03FFFFFFFFFFFFFFFFFF#2b...Packet received: OK (gdb) tstop Sending packet: $QTStop#4b...Packet received: OK Sending packet: $QTNotes:#e8...Packet received: OK (gdb) actions Enter actions for tracepoint 1, one per line. End with a line saying just "end". >collect $reg >while-stepping 1 >collect $reg >end >end (gdb) tstart Sending packet: $QTinit#59...Packet received: OK Sending packet: $QTDP:1:00000000004004d9:E:1:0-#a4...Packet received: OK Sending packet: $QTDP:-1:00000000004004d9:R03FFFFFFFFFFFFFFFFFF-#58...Packet received: OK Sending packet: $QTDP:-1:00000000004004d9:SR03FFFFFFFFFFFFFFFFFF#7e...Packet received: OK (gdb) tstop Sending packet: $QTStop#4b...Packet received: OK Sending packet: $QTNotes:#e8...Packet received: OK (gdb) actions Enter actions for tracepoint 1, one per line. End with a line saying just "end". >collect $regs >end (gdb) tstart Sending packet: $QTinit#59...Packet received: OK Sending packet: $QTDP:1:00000000004004d9:E:1:0-#a4...Packet received: OK Sending packet: $QTDP:-1:00000000004004d9:R03FFFFFFFFFFFFFFFFFF#2b...Packet received: OK The last "$QTDP:1:00000000004004d9:E:1:0-#a4" should be "$QTDP:1:00000000004004d9:E:0:0-#a3". In pseudo-diff: -$QTDP:1:00000000004004d9:E:1:0-#a4 +$QTDP:1:00000000004004d9:E:0:0-#a3 A related issue is that the "commands" command actually supports setting commands to a range of breakpoints/tracepoints at once. But, hacking "maint info breakpoints" to print t->step_count, reveals: (gdb) trace main Tracepoint 5 at 0x45a2ab: file ../../src/gdb/gdb.c, line 29. (gdb) trace main Note: breakpoint 5 also set at pc 0x45a2ab. Tracepoint 6 at 0x45a2ab: file ../../src/gdb/gdb.c, line 29. (gdb) commands 5-6 Type commands for breakpoint(s) 5-6, one per line. End with a line saying just "end". > while-stepping 5 >end > end (gdb) maint info breakpoints 5 Num Type Disp Enb Address What 5 tracepoint keep y 0x000000000045a2ab in main at ../../src/gdb/gdb.c:29 inf 1 step_count=5 ^^^^^^^^^^^^ while-stepping 5 end not installed on target (gdb) maint info breakpoints 6 Num Type Disp Enb Address What 6 tracepoint keep y 0x000000000045a2ab in main at ../../src/gdb/gdb.c:29 inf 1 step_count=0 ^^^^^^^^^^^^ while-stepping 5 end not installed on target (gdb) that tracepoint 6 doesn't end up with the correct step_count. The issue is that here: static void do_map_commands_command (struct breakpoint *b, void *data) { struct commands_info *info = data; if (info->cmd == NULL) { struct command_line *l; if (info->control != NULL) l = copy_command_lines (info->control->body_list[0]); else { struct cleanup *old_chain; char *str; str = xstrprintf (_("Type commands for breakpoint(s) " "%s, one per line."), info->arg); old_chain = make_cleanup (xfree, str); l = read_command_lines (str, info->from_tty, 1, (is_tracepoint (b) ? check_tracepoint_command : 0), b); do_cleanups (old_chain); } info->cmd = alloc_counted_command_line (l); } validate_actionline is never called for tracepoints other than the first (the copy_command_lines path). Right below, we have: /* If a breakpoint was on the list more than once, we don't need to do anything. */ if (b->commands != info->cmd) { validate_commands_for_breakpoint (b, info->cmd->commands); incref_counted_command_line (info->cmd); decref_counted_command_line (&b->commands); b->commands = info->cmd; observer_notify_breakpoint_modified (b); } And validate_commands_for_breakpoint looks like the right place to put a call; if we reset step_count there too, we have a nice central fix for the first issue as well, because trace_actions_command calls breakpoint_set_commands that also calls validate_commands_for_breakpoint. We end up calling validate_actionline twice for the first tracepoint, since read_command_lines calls it too, through check_tracepoint_command, but that should be harmless. 2013-04-04 Pedro Alves Hui Zhu * breakpoint.c (validate_commands_for_breakpoint): If validating a tracepoint, reset its STEP_COUNT and call validate_actionline. 2013-04-04 Stan Shebs Pedro Alves * gdb.trace/Makefile.in (PROGS): Add actions-changed. * gdb.trace/actions-changed.c: New file. * gdb.trace/actions-changed.exp: New file. * lib/trace-support.exp (gdb_trace_setactions): Rename to ... (gdb_trace_setactions_command): ... this. Add "actions_command" parameter, and handle it. (gdb_trace_setactions, gdb_trace_setcommands): New procedures. --- diff --git a/gdb/ChangeLog b/gdb/ChangeLog index 024cd9ddf53..30315af4860 100644 --- a/gdb/ChangeLog +++ b/gdb/ChangeLog @@ -1,3 +1,9 @@ +2013-04-04 Pedro Alves + Hui Zhu + + * breakpoint.c (validate_commands_for_breakpoint): If validating a + tracepoint, reset its STEP_COUNT and call validate_actionline. + 2013-04-03 Doug Evans * dwarf2read.c (read_die_and_siblings_1): Renamed from diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c index ff161a0d551..5ba1f2f65bc 100644 --- a/gdb/breakpoint.c +++ b/gdb/breakpoint.c @@ -1142,12 +1142,25 @@ validate_commands_for_breakpoint (struct breakpoint *b, { if (is_tracepoint (b)) { - /* We need to verify that each top-level element of commands is - valid for tracepoints, that there's at most one - while-stepping element, and that while-stepping's body has - valid tracing commands excluding nested while-stepping. */ + struct tracepoint *t = (struct tracepoint *) b; struct command_line *c; struct command_line *while_stepping = 0; + + /* Reset the while-stepping step count. The previous commands + might have included a while-stepping action, while the new + ones might not. */ + t->step_count = 0; + + /* We need to verify that each top-level element of commands is + valid for tracepoints, that there's at most one + while-stepping element, and that the while-stepping's body + has valid tracing commands excluding nested while-stepping. + We also need to validate the tracepoint action line in the + context of the tracepoint --- validate_actionline actually + has side effects, like setting the tracepoint's + while-stepping STEP_COUNT, in addition to checking if the + collect/teval actions parse and make sense in the + tracepoint's context. */ for (c = commands; c; c = c->next) { if (c->control_type == while_stepping_control) @@ -1165,6 +1178,8 @@ validate_commands_for_breakpoint (struct breakpoint *b, else while_stepping = c; } + + validate_actionline (c->line, b); } if (while_stepping) { diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog index fc2e96a601c..b2b700922ce 100644 --- a/gdb/testsuite/ChangeLog +++ b/gdb/testsuite/ChangeLog @@ -1,3 +1,14 @@ +2013-04-04 Stan Shebs + Pedro Alves + + * gdb.trace/Makefile.in (PROGS): Add actions-changed. + * gdb.trace/actions-changed.c: New file. + * gdb.trace/actions-changed.exp: New file. + * lib/trace-support.exp (gdb_trace_setactions): Rename to ... + (gdb_trace_setactions_command): ... this. Add "actions_command" + parameter, and handle it. + (gdb_trace_setactions, gdb_trace_setcommands): New procedures. + 2013-04-04 Yao Qi * gdb.server/server-kill.exp: Use command 'tstatus' instead of diff --git a/gdb/testsuite/gdb.trace/Makefile.in b/gdb/testsuite/gdb.trace/Makefile.in index 8a7d523da2b..2e23223dd18 100644 --- a/gdb/testsuite/gdb.trace/Makefile.in +++ b/gdb/testsuite/gdb.trace/Makefile.in @@ -3,9 +3,9 @@ srcdir = @srcdir@ .PHONY: all clean mostlyclean distclean realclean -PROGS = ax backtrace deltrace disconnected-tracing infotrace packetlen \ - passc-dyn passcount report save-trace tfile tfind tracecmd tsv \ - unavailable while-dyn while-stepping +PROGS = actions-changed ax backtrace deltrace disconnected-tracing \ + infotrace packetlen passc-dyn passcount report save-trace tfile \ + tfind tracecmd tsv unavailable while-dyn while-stepping all info install-info dvi install uninstall installcheck check: @echo "Nothing to be done for $@..." diff --git a/gdb/testsuite/gdb.trace/actions-changed.c b/gdb/testsuite/gdb.trace/actions-changed.c new file mode 100644 index 00000000000..bac24a7c194 --- /dev/null +++ b/gdb/testsuite/gdb.trace/actions-changed.c @@ -0,0 +1,66 @@ +/* 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 . */ + +int +end (int i) +{ +} + +int +subr2 (int parm) +{ + int keeping, busy; + + keeping = parm + parm; + busy = keeping * keeping; + + return busy; +} + +int +subr (int parm) +{ + int keeping, busy; + + keeping = parm + parm; + busy = keeping * keeping; + + return busy; +} + +main() +{ + subr (1); + end (1); + + subr (2); + end (2); + + subr (3); + end (3); + + subr (4); + end (4); + + subr (5); + subr2 (5); + end (5); + + subr (6); + subr2 (6); + end (6); +} diff --git a/gdb/testsuite/gdb.trace/actions-changed.exp b/gdb/testsuite/gdb.trace/actions-changed.exp new file mode 100644 index 00000000000..e850da2afcb --- /dev/null +++ b/gdb/testsuite/gdb.trace/actions-changed.exp @@ -0,0 +1,174 @@ +# 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 . + +load_lib trace-support.exp + +standard_testfile + +if {[prepare_for_testing $testfile.exp $testfile $srcfile debug]} { + return -1 +} + +proc test_actions_changed { } { + gdb_breakpoint "end" + + gdb_test "trace subr" "Tracepoint .*" \ + "tracepoint at subr" + + # The first set of tests are regression tests for a GDB bug where + # the while-stepping count of a tracepoint would be left stale if + # the tracepoint's actions were redefined, and the new action list + # had no while-stepping action. + + # First pass, define simple action. + with_test_prefix "1" { + gdb_trace_setactions "define simple action" \ + "" \ + "collect parm" "^$" + + gdb_test_no_output "tstart" + + gdb_test "continue" ".*Breakpoint \[0-9\]+, end \\(i=1\\) .*" \ + "advance through tracing" + + gdb_test "tstatus" ".*Collected 1 trace frame.*" \ + "collected 1 trace frame" + + gdb_test_no_output "tstop" + } + + # Redefine action, run second trace. + with_test_prefix "2" { + gdb_trace_setactions "redefine simple action" \ + "" \ + "collect keeping, busy" "^$" + + gdb_test_no_output "tstart" + + gdb_test "continue" ".*Breakpoint \[0-9\]+, end \\(i=2\\) .*" \ + "advance through tracing" + + gdb_test "tstatus" ".*Collected 1 trace frame.*" \ + "collected 1 trace frame" + + gdb_test_no_output "tstop" + } + + # Redefine to stepping action, run third trace. + with_test_prefix "3" { + gdb_trace_setactions "redefine to stepping action" \ + "" \ + "collect parm" "^$" \ + "while-stepping 5" "^$" \ + "collect parm" "^$" \ + "end" "^$" + + gdb_test_no_output "tstart" + + gdb_test "continue" ".*Breakpoint \[0-9\]+, end \\(i=3\\) .*" \ + "advance through tracing" + + gdb_test "tstatus" ".*Collected 6 trace frames.*" \ + "collected 6 trace frames" + + gdb_test_no_output "tstop" + } + + # Redefine to non-stepping, run fourth trace. + with_test_prefix "4" { + gdb_trace_setactions "redefine to non-stepping action" \ + "" \ + "collect parm" "^$" + + gdb_test_no_output "tstart" + + gdb_test "continue" ".*Breakpoint \[0-9\]+, end \\(i=4\\) .*" \ + "advance through tracing" + + gdb_test "tstatus" ".*Collected 1 trace frame.*" \ + "collected 1 trace frame" + + gdb_test_no_output "tstop" + } + + # The following tests are related to the above, but use two + # tracepoints. They are regression tests for a GDB bug where only + # the first tracepoint would end up with the step count set. + + # Store the first tracepoint's number. + gdb_test_no_output "set \$prev_tpnum=\$tpnum" "store previous \$tpnum" + + # And here's the second tracepoint. + gdb_test "trace subr2" "Tracepoint .*" "tracepoint at subr2" + + # Set a stepping action in both tracepoints, with the "commands" + # command. + with_test_prefix "5" { + gdb_trace_setcommands \ + "redefine 2 tracepoints to stepping action, using commands" \ + "\$prev_tpnum-\$tpnum" \ + "collect parm" "^$" \ + "while-stepping 5" "^$" \ + "collect parm" "^$" \ + "end" "^$" + + gdb_test_no_output "tstart" + + gdb_test "continue" ".*Breakpoint \[0-9\]+, end \\(i=5\\) .*" \ + "advance through tracing" + + gdb_test "tstatus" ".*Collected 12 trace frames.*" \ + "collected 12 trace frames" + + gdb_test_no_output "tstop" + } + + # Redefine the actions of both tracepoints to non-stepping, also + # using the "commands" command. + with_test_prefix "6" { + gdb_trace_setcommands \ + "redefine 2 tracepoints to non-stepping action, using commands" \ + "\$prev_tpnum-\$tpnum" \ + "collect parm" "^$" + + gdb_test_no_output "tstart" + + gdb_test "continue" ".*Breakpoint \[0-9\]+, end \\(i=6\\) .*" \ + "advance through tracing" + + gdb_test "tstatus" ".*Collected 2 trace frame.*" \ + "collected 2 trace frames" + + gdb_test_no_output "tstop" + } +} + +# Check whether the target supports tracepoints. + +clean_restart $testfile + +if ![runto_main] { + fail "Can't run to main to check for trace support" + return -1 +} + +if ![gdb_target_supports_trace] { + unsupported "Current target does not support trace" + return -1; +} + +test_actions_changed + +return 0 diff --git a/gdb/testsuite/lib/trace-support.exp b/gdb/testsuite/lib/trace-support.exp index 10482b84611..2601ad837d9 100644 --- a/gdb/testsuite/lib/trace-support.exp +++ b/gdb/testsuite/lib/trace-support.exp @@ -86,23 +86,23 @@ proc gdb_delete_tracepoints {} { } } -# -# Procedure: gdb_trace_setactions # Define actions for a tracepoint. # Arguments: +# actions_command -- the command used to create the actions. +# either "actions" or "commands". # testname -- identifying string for pass/fail output -# tracepoint -- to which tracepoint do these actions apply? (optional) +# tracepoint -- to which tracepoint(s) do these actions apply? (optional) # args -- list of actions to be defined. # Returns: # zero -- success # non-zero -- failure -proc gdb_trace_setactions { testname tracepoint args } { +proc gdb_trace_setactions_command { actions_command testname tracepoint args } { global gdb_prompt; set state 0; set passfail "pass"; - send_gdb "actions $tracepoint\n"; + send_gdb "$actions_command $tracepoint\n"; set expected_result ""; gdb_expect 5 { -re "No tracepoint number .*$gdb_prompt $" { @@ -165,6 +165,20 @@ proc gdb_trace_setactions { testname tracepoint args } { } } +# Define actions for a tracepoint, using the "actions" command. See +# gdb_trace_setactions_command. +# +proc gdb_trace_setactions { testname tracepoint args } { + eval gdb_trace_setactions_command "actions" {$testname} {$tracepoint} $args +} + +# Define actions for a tracepoint, using the "commands" command. See +# gdb_trace_setactions_command. +# +proc gdb_trace_setcommands { testname tracepoint args } { + eval gdb_trace_setactions_command "commands" {$testname} {$tracepoint} $args +} + # # Procedure: gdb_tfind_test # Find a specified trace frame.