[PATCH] Reset tracepoint step_count to 0 before set it action

[PATCH] Reset tracepoint step_count to 0 before set it action

[Hui Zhu]

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

Hi,

If a tracepoint's action include a while-stepping, when it set to actions without while-stepping.  The step_count will keep to its old 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".

Post a patch to fix it and there also a test for this issue.

Please help me review it.  And I suggest this change can be checked to 7.6 branch.

Thanks,
Hui

2013-03-24  Hui Zhu  <hui@codesourcery.com>

	* breakpoint.c (do_map_commands_command): Reset step_count to 0
	if this is a tracepoint.
	* tracepoint.c (trace_actions_command): Ditto.

2013-03-24  Stan Shebs  <stan@codesourcery.com>

	* gdb.trace/Makefile.in (PROGS): Add actions-changed.
	* gdb.trace/actions-changed.c: New.
	* gdb.trace/actions-changed.exp: New.

[-- Attachment #2: tp-actions-clear-step_count.txt --]
[-- Type: text/plain, Size: 1066 bytes --]

--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -1296,6 +1296,16 @@ do_map_commands_command (struct breakpoi
 {
   struct commands_info *info = data;
 
+  if (b->type == bp_tracepoint)
+    {
+      struct tracepoint *t = (struct tracepoint *) b;
+
+      /* Reset the step count to 0 because if this tracepoint has step
+         action in before, it will not reset it to 0 if new actions
+         doesn't have while-stepping.  */
+      t->step_count = 0;
+    }
+
   if (info->cmd == NULL)
     {
       struct command_line *l;
--- a/gdb/tracepoint.c
+++ b/gdb/tracepoint.c
@@ -666,6 +666,11 @@ trace_actions_command (char *args, int f
 		    t->base.number);
       struct cleanup *cleanups = make_cleanup (xfree, tmpbuf);
 
+      /* Reset the step count to 0 because if this tracepoint has step
+         action in before, it will not reset it to 0 if new actions
+         doesn't have while-stepping.  */
+      t->step_count = 0;
+
       l = read_command_lines (tmpbuf, from_tty, 1,
 			      check_tracepoint_command, t);
       do_cleanups (cleanups);

[-- Attachment #3: tp-actions-clear-step_count-test.txt --]
[-- Type: text/plain, Size: 5155 bytes --]

--- 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 $@..."
--- /dev/null
+++ b/gdb/testsuite/gdb.trace/actions-changed.c
@@ -0,0 +1,65 @@
+/* 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/>.  */
+
+int
+begin ()
+{
+}
+
+int
+middle ()
+{
+}
+
+int
+later ()
+{
+}
+
+int
+endish ()
+{
+}
+
+int
+end ()
+{
+}
+
+int
+subr (int parm)
+{
+  int keeping, busy;
+
+  keeping = parm + parm;
+  busy = keeping * keeping;
+
+  return busy;
+}
+
+main()
+{
+  begin ();
+  subr (1);
+  middle ();
+  subr (2);
+  later ();
+  subr (3);
+  endish ();
+  subr (4);
+  end ();
+}
--- /dev/null
+++ b/gdb/testsuite/gdb.trace/actions-changed.exp
@@ -0,0 +1,121 @@
+# 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 actions-changed.c
+if { [gdb_compile "$srcdir/$subdir/$srcfile" $binfile \
+	  executable {debug nowarnings}] != "" } {
+    untested actions-changed.exp
+    return -1
+}
+
+proc test_actions_changed { } \
+{
+    gdb_breakpoint "begin"
+    gdb_breakpoint "middle"
+    gdb_breakpoint "later"
+    gdb_breakpoint "endish"
+    gdb_breakpoint "end"
+
+    gdb_test "continue" ".*Breakpoint \[0-9\]+, begin .*" \
+	"advance to tracing"
+
+    gdb_test "trace subr" "Tracepoint .*" \
+	"tracepoint at subr"
+
+    # First pass, define simple action
+
+    gdb_trace_setactions "define simple action" \
+	"" \
+	"collect parm" "^$"
+
+    gdb_test_no_output "tstart"
+
+    gdb_test "continue" ".*Breakpoint \[0-9\]+, middle .*" \
+	"advance through tracing, 1st"
+
+    gdb_test "tstatus" ".*Collected 1 trace frame.*" \
+	"check on first trace status"
+
+    gdb_test_no_output "tstop"
+
+    # Redefine action, run second trace
+
+    gdb_trace_setactions "redefine simple action" \
+	"" \
+	"collect keeping, busy" "^$"
+
+    gdb_test_no_output "tstart"
+
+    gdb_test "continue" ".*Breakpoint \[0-9\]+, later .*" \
+	"advance through tracing, 2nd"
+
+    gdb_test "tstatus" ".*Collected 1 trace frame.*" \
+	"check on redefined trace status"
+
+    gdb_test_no_output "tstop"
+
+    # Redefine to stepping action, run third trace
+
+    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\]+, endish .*" \
+	"advance through tracing, 3rd"
+
+    gdb_test "tstatus" ".*Collected 6 trace frame.*" \
+	"check on stepping trace status"
+
+    gdb_test_no_output "tstop"
+
+    # Redefine to non-stepping, run fourth trace.
+
+    gdb_trace_setactions "redefine to non-stepping action" \
+	"" \
+	"collect parm" "^$"
+
+    gdb_test_no_output "tstart"
+
+    gdb_test "continue" ".*Breakpoint \[0-9\]+, end .*" \
+	"advance to tracing, 4th"
+
+    gdb_test "tstatus" ".*Collected 1 trace frame.*" \
+	"check on redefined non-stepping trace status"
+}
+
+# Test if target supports tracepoints or not.
+
+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

Re: [PATCH] Reset tracepoint step_count to 0 before set it action

[Yao Qi]

On 03/24/2013 07:32 PM, Hui Zhu wrote:
> 2013-03-24  Hui Zhu<hui@codesourcery.com>
>
> 	* breakpoint.c (do_map_commands_command): Reset step_count to 0
> 	if this is a tracepoint.
> 	* tracepoint.c (trace_actions_command): Ditto.

Looks both do_map_commands_command and trace_actions_command call 
check_tracepoint_command, so why don't set step_count in 
check_tracepoint_command?

-- 
Yao (齐尧)

Re: [PATCH] Reset tracepoint step_count to 0 before set it action

[Hui Zhu]

On 03/24/13 22:09, Yao Qi wrote:
> On 03/24/2013 07:32 PM, Hui Zhu wrote:
>> 2013-03-24  Hui Zhu<hui@codesourcery.com>
>>
>>     * breakpoint.c (do_map_commands_command): Reset step_count to 0
>>     if this is a tracepoint.
>>     * tracepoint.c (trace_actions_command): Ditto.
>
> Looks both do_map_commands_command and trace_actions_command call check_tracepoint_command, so why don't set step_count in check_tracepoint_command?
>

I don't think this is a good place for put this reset code because this function will be call each time when user input a new line for actions.

Thanks,
Hui

Re: [PATCH] Reset tracepoint step_count to 0 before set it action

[Yao Qi]

On 03/25/2013 11:30 AM, Hui Zhu wrote:
> I don't think this is a good place for put this reset code because this function will be call each time when user input a new line for actions.

Oh, yeah.  You are right.  Sorry for noise.

-- 
Yao (齐尧)

Re: [PATCH] Reset tracepoint step_count to 0 before set it action

[Hui Zhu]

Ping.
http://old.nabble.com/-PATCH--Reset-tracepoint-step_count-to-0-before-set-it-action-p35211711.html

Wish this bug fix get review before new release.

Thanks,
Hui

On Sun, Mar 24, 2013 at 7:32 PM, Hui Zhu <hui_zhu@mentor.com> wrote:
> Hi,
>
> If a tracepoint's action include a while-stepping, when it set to actions
> without while-stepping.  The step_count will keep to its old 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".
>
> Post a patch to fix it and there also a test for this issue.
>
> Please help me review it.  And I suggest this change can be checked to 7.6
> branch.
>
> Thanks,
> Hui
>
> 2013-03-24  Hui Zhu  <hui@codesourcery.com>
>
>         * breakpoint.c (do_map_commands_command): Reset step_count to 0
>         if this is a tracepoint.
>         * tracepoint.c (trace_actions_command): Ditto.
>
> 2013-03-24  Stan Shebs  <stan@codesourcery.com>
>
>         * gdb.trace/Makefile.in (PROGS): Add actions-changed.
>         * gdb.trace/actions-changed.c: New.
>         * gdb.trace/actions-changed.exp: New.

Re: [PATCH] Reset tracepoint step_count to 0 before set it action

[Pedro Alves]

Hi,

Thanks for the patch.

> --- a/gdb/breakpoint.c
> +++ b/gdb/breakpoint.c
> @@ -1296,6 +1296,16 @@ do_map_commands_command (struct breakpoi
>  {
>    struct commands_info *info = data;
>  
> +  if (b->type == bp_tracepoint)
> +    {
> +      struct tracepoint *t = (struct tracepoint *) b;
> +
> +      /* Reset the step count to 0 because if this tracepoint has step
> +         action in before, it will not reset it to 0 if new actions
> +         doesn't have while-stepping.  */
> +      t->step_count = 0;
> +    }
> +
>    if (info->cmd == NULL)
>      {
>        struct command_line *l;
> --- a/gdb/tracepoint.c
> +++ b/gdb/tracepoint.c
> @@ -666,6 +666,11 @@ trace_actions_command (char *args, int f
>  		    t->base.number);
>        struct cleanup *cleanups = make_cleanup (xfree, tmpbuf);
>  
> +      /* Reset the step count to 0 because if this tracepoint has step
> +         action in before, it will not reset it to 0 if new actions
> +         doesn't have while-stepping.  */
> +      t->step_count = 0;
> +
>        l = read_command_lines (tmpbuf, from_tty, 1,
>  			      check_tracepoint_command, t);
>        do_cleanups (cleanups);
> 
> 

I was about to okay this, but then I recalled that the "commands"
command actually supports setting commands to a range of
breakpoints/tracepoints at once.  Looking at the code, it didn't look
like it was doing the right thing.  So I hacked "maint info breakpoints"
to print t->step_count, and noticed (with or without your
patch applied):

(top-gdb) trace main
Tracepoint 5 at 0x45a2ab: file ../../src/gdb/gdb.c, line 29.
(top-gdb) trace main
Note: breakpoint 5 also set at pc 0x45a2ab.
Tracepoint 6 at 0x45a2ab: file ../../src/gdb/gdb.c, line 29.
(top-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
(top-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
(top-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
(top-gdb) 

Tracepoint 6 didn't end up with the correct step_count.

I couldn't give a suggestion right away, and I had a hunch fixing that
would end up fixing your related issue differently, and before you know it,
I ended up hacking on it myself.  The resulting patch is at the bottom.

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 your 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 IMO that is harmless.

> --- /dev/null
> +++ b/gdb/testsuite/gdb.trace/actions-changed.exp
> @@ -0,0 +1,121 @@

> +load_lib trace-support.exp
> +
> +standard_testfile actions-changed.c

No need to pass "actions-changed.c", it's the default.

> +if { [gdb_compile "$srcdir/$subdir/$srcfile" $binfile \
> +	  executable {debug nowarnings}] != "" } {
> +    untested actions-changed.exp
> +    return -1
> +}

Should be using prepare_for_testing.  "nowarning" is not necessary.

> +
> +proc test_actions_changed { } \
> +{

It's more standard to leave the { in the previous line.

> +    gdb_breakpoint "begin"
> +    gdb_breakpoint "middle"
> +    gdb_breakpoint "later"
> +    gdb_breakpoint "endish"
> +    gdb_breakpoint "end"
> +
> +    gdb_test "continue" ".*Breakpoint \[0-9\]+, begin .*" \
> +	"advance to tracing"
> +
> +    gdb_test "trace subr" "Tracepoint .*" \
> +	"tracepoint at subr"
> +
> +    # First pass, define simple action
> +
> +    gdb_trace_setactions "define simple action" \
> +	"" \
> +	"collect parm" "^$"
> +
> +    gdb_test_no_output "tstart"
> +
> +    gdb_test "continue" ".*Breakpoint \[0-9\]+, middle .*" \
> +	"advance through tracing, 1st"
> +
> +    gdb_test "tstatus" ".*Collected 1 trace frame.*" \
> +	"check on first trace status"

I changed the test messages a bit.

> +
> +    gdb_test_no_output "tstop"
> +
> +    # Redefine action, run second trace
> +
> +    gdb_trace_setactions "redefine simple action" \
> +	"" \
> +	"collect keeping, busy" "^$"
> +
> +    gdb_test_no_output "tstart"
> +
> +    gdb_test "continue" ".*Breakpoint \[0-9\]+, later .*" \
> +	"advance through tracing, 2nd"
> +
> +    gdb_test "tstatus" ".*Collected 1 trace frame.*" \
> +	"check on redefined trace status"
> +
> +    gdb_test_no_output "tstop"
> +
> +    # Redefine to stepping action, run third trace

These "tstart" "tstop" make it so there are non unique messages
in the test results:

$ cat testsuite/gdb.sum | grep PASS| sort| uniq -c | sort -n | tail -n 4
      1 PASS: gdb.trace/actions-changed.exp: redefine to stepping action
      1 PASS: gdb.trace/actions-changed.exp: tracepoint at subr
      3 PASS: gdb.trace/actions-changed.exp: tstop
      4 PASS: gdb.trace/actions-changed.exp: tstart

Instead of those "1st", "2nd" hardcoded bits, we can use
with_test_prefix.

> +    gdb_breakpoint "begin"
> +    gdb_breakpoint "middle"
> +    gdb_breakpoint "later"
> +    gdb_breakpoint "endish"
> +    gdb_breakpoint "end"

I was adding more tests to cover the "commands 5-6" issue,
and I found this to not be very flexible. :-)  I instead made
the "end" take an argument that matches the current pass, and
got rid of begin/middle/etc.

This refactors gdb_trace_setactions a little in order to
add a variant uses the "commands" command instead of
the "actions" command.

Here's what I'm checking in.

2013-04-04  Pedro Alves  <palves@redhat.com>
	    Hui Zhu  <hui@codesourcery.com>

	* breakpoint.c (validate_commands_for_breakpoint): If validating a
	tracepoint, reset its STEP_COUNT and call validate_actionline.

2013-04-04  Stan Shebs  <stan@codesourcery.com>
	    Pedro Alves  <palves@redhat.com>

	* 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.

---

 gdb/testsuite/gdb.trace/Makefile.in         |    6 -
 gdb/testsuite/gdb.trace/actions-changed.c   |   66 ++++++++++
 gdb/testsuite/gdb.trace/actions-changed.exp |  174 +++++++++++++++++++++++++++
 3 files changed, 243 insertions(+), 3 deletions(-)
 create mode 100644 gdb/testsuite/gdb.trace/actions-changed.c
 create mode 100644 gdb/testsuite/gdb.trace/actions-changed.exp

diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
index ff161a0..5ba1f2f 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/gdb.trace/Makefile.in b/gdb/testsuite/gdb.trace/Makefile.in
index 8a7d523..2e23223 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 0000000..5443e93
--- /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 <http://www.gnu.org/licenses/>.  */
+
+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 (6);
+  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 0000000..e850da2
--- /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 <http://www.gnu.org/licenses/>.
+
+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 10482b8..2601ad8 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.

Re: [PATCH] Reset tracepoint step_count to 0 before set it action

[Pedro Alves]

On 04/04/2013 07:30 PM, Pedro Alves wrote:

> Here's what I'm checking in.

On 03/24/2013 11:32 AM, Hui Zhu wrote:
> And I suggest this change can be checked to 7.6 branch.

I think it's safe enough, so I went ahead and put it in the branch as well.

Here it is again, now with full commit log.  The 7.6 version
had a minor adjustment as validate_actionline's prototype
is slightly different there.

--------------
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  <palves@redhat.com>
	    Hui Zhu  <hui@codesourcery.com>

	* breakpoint.c (validate_commands_for_breakpoint): If validating a
	tracepoint, reset its STEP_COUNT and call validate_actionline.

2013-04-04  Stan Shebs  <stan@codesourcery.com>
	    Pedro Alves  <palves@redhat.com>

	* 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.
---

 gdb/breakpoint.c                            |   23 +++-
 gdb/testsuite/gdb.trace/Makefile.in         |    6 -
 gdb/testsuite/gdb.trace/actions-changed.c   |   66 ++++++++++
 gdb/testsuite/gdb.trace/actions-changed.exp |  174 +++++++++++++++++++++++++++
 gdb/testsuite/lib/trace-support.exp         |   24 +++-
 5 files changed, 281 insertions(+), 12 deletions(-)
 create mode 100644 gdb/testsuite/gdb.trace/actions-changed.c
 create mode 100644 gdb/testsuite/gdb.trace/actions-changed.exp

diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
index ff161a0..5ba1f2f 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/gdb.trace/Makefile.in b/gdb/testsuite/gdb.trace/Makefile.in
index 8a7d523..2e23223 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 0000000..bac24a7
--- /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 <http://www.gnu.org/licenses/>.  */
+
+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 0000000..e850da2
--- /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 <http://www.gnu.org/licenses/>.
+
+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 10482b8..2601ad8 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.

Re: [PATCH] Reset tracepoint step_count to 0 before set it action

[Hui Zhu]

Hi Pedro,

Thanks for your help.

Best.
Hui

On Fri, Apr 5, 2013 at 3:27 AM, Pedro Alves <palves@redhat.com> wrote:
> On 04/04/2013 07:30 PM, Pedro Alves wrote:
>
>> Here's what I'm checking in.
>
> On 03/24/2013 11:32 AM, Hui Zhu wrote:
>> And I suggest this change can be checked to 7.6 branch.
>
> I think it's safe enough, so I went ahead and put it in the branch as well.
>
> Here it is again, now with full commit log.  The 7.6 version
> had a minor adjustment as validate_actionline's prototype
> is slightly different there.
>
> --------------
> 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  <palves@redhat.com>
>             Hui Zhu  <hui@codesourcery.com>
>
>         * breakpoint.c (validate_commands_for_breakpoint): If validating a
>         tracepoint, reset its STEP_COUNT and call validate_actionline.
>
> 2013-04-04  Stan Shebs  <stan@codesourcery.com>
>             Pedro Alves  <palves@redhat.com>
>
>         * 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.
> ---
>
>  gdb/breakpoint.c                            |   23 +++-
>  gdb/testsuite/gdb.trace/Makefile.in         |    6 -
>  gdb/testsuite/gdb.trace/actions-changed.c   |   66 ++++++++++
>  gdb/testsuite/gdb.trace/actions-changed.exp |  174 +++++++++++++++++++++++++++
>  gdb/testsuite/lib/trace-support.exp         |   24 +++-
>  5 files changed, 281 insertions(+), 12 deletions(-)
>  create mode 100644 gdb/testsuite/gdb.trace/actions-changed.c
>  create mode 100644 gdb/testsuite/gdb.trace/actions-changed.exp
>
> diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
> index ff161a0..5ba1f2f 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/gdb.trace/Makefile.in b/gdb/testsuite/gdb.trace/Makefile.in
> index 8a7d523..2e23223 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 0000000..bac24a7
> --- /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 <http://www.gnu.org/licenses/>.  */
> +
> +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 0000000..e850da2
> --- /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 <http://www.gnu.org/licenses/>.
> +
> +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 10482b8..2601ad8 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.
>